Roslyn: System.AccessViolationException when invoking extension method on ValueTuple

[anpaz]

Issue Title

With .NET Core 3.1.100 we're getting a:
Fatal error. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
when invoking an extension method on a field of a ValueTuple.

This was fine with .NET Core 3.0.100

General

We're trying to update the Quantum Development Kit runtime from 3.0.100 to 3.1.100 and one of our tests is failing and crashing execution with the exception mentioned above.

I have pinpoint the problem, it happens when we're trying to call an extension method (an extension for object) of a field of a ValueTuple if the field is a generic Type Parameter, for example:

```C#
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;

namespace bug
{
public static class Extension
{
public static String GetValue(this object value) => value?.ToString();
}

public partial class BGen<__T__>
{
    public (Int64, __T__) Data { get; }

    public BGen((Int64, __T__) data)
    {
        this.Data = data;
    }

    internal String Value
    {
        get
        {
            /// This works fine:
            //var i2 = Data.Item2;
            //var q2 = i2?.GetValue();

            // This causes the AccessViolation:
            var q2 = Data.Item2?.GetValue();
            return q2;
        }
    }
}

class Program
{
    static void Main(string[] args)
    {
        var i = new BGen<string>((0, "foo"));
        var r = i.Value;

        Console.WriteLine($"Hello World: {string.Join(',', r)}");
    }
}

}
```

This happens on windows with version 3.1.100. Works fine if downgrading to 3.0.100.

[jkotas]

cc @dotnet/jit-contrib

[AndyAyersMS]

With relatively current master ...

Assert failure(PID 22924 [0x0000598c], Thread: 44364 [0xad4c]): Assertion failed '!"Bad type in gtNewZeroConNode"' in 'bug.BGen`1[__Canon][System.__Canon]:get_Value():System.String:this' (IL size 41)

    File: C:\repos\runtime1\src\coreclr\src\jit\gentree.cpp Line: 5935
    Image: c:\repos\runtime1\artifacts\tests\coreclr\Windows_NT.x64.Checked\Tests\Core_Root\CoreRun.exe

Wonder if this is bad IL...

  .locals init ([0] valuetype [System.Private.CoreLib]System.ValueTuple`2<int64,!__T__> V_0)
  IL_0000:  ldarg.0
  IL_0001:  call       instance valuetype [System.Private.CoreLib]System.ValueTuple`2<int64,!0> class bug.BGen`1<!__T__>::get_Data()
  IL_0006:  stloc.0
  IL_0007:  ldloca.s   V_0
  IL_0009:  ldflda     !1 valuetype [System.Private.CoreLib]System.ValueTuple`2<int64,!__T__>::Item2
  IL_000e:  ldloc.0
  IL_000f:  box        !__T__
  IL_0014:  brtrue.s   IL_0019
  IL_0016:  pop
  IL_0017:  ldnull
  IL_0018:  ret
  IL_0019:  ldobj      !__T__
  IL_001e:  box        !__T__
  IL_0023:  call       string bug.Extension::GetValue(object)
  IL_0028:  ret
} // end of method BGen`1::get_Value

Here _T_ is __Canon and so a ref type, thus the box becomes a nop, leaving a struct on the stack for brtrue, which causes the assert. Per Ecma-355 the top of stack type needs to be verifier-assignable to the box type, that doesn't seem to be the case here.

[jkotas]

@jaredpar @agocke This is invalid IL generated by Roslyn. Could you please transfer this issue to the Roslyn repo? (I do not have the rights to do it.)

[RikkiGibson]

ref struct may not be used as type arguments or in tuple fields. Could you show us where __Canon is being provided as a type argument? Unless "ref type" means something else here? @anpaz-msft @AndyAyersMS

[AndyAyersMS]

Here T is actually string.

We informally use "ref type" to mean IL type O or a reference to a heap object (ref class or boxed value class).

The coreclr runtime shares generic instantiations across ref class types, since the values of that type all have the same size and ABI behavior. So in the runtime, string gets mapped to a special shared placeholder type __Canon and that's what the jit sees. But it is really string.

[RikkiGibson]

Thanks, that makes a lot of things click. Will investigate.

[RikkiGibson]

This bug appears to be introduced by #35979

[cston]

@RikkiGibson, thanks for investigating.

With this repro it looks like the value of receiverTemp at https://github.com/dotnet/roslyn/blob/master/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs#L406 is the tuple (Int64, __T__) rather than __T__ Item2, so the optimization in #35979 is using the wrong value.

We should consider reverting the optimization and the corresponding change to VB.

[YairHalberstadt]

Fix (as opposed to revert at https://github.com/dotnet/roslyn/pull/40878)