Fix reference types (dotnet#6954)

- Generic ArrayPinningHelper resulted into incorrect array data offset for reference types. Use non-generic one instead.
- Add array covariance checks to guarantee type safety.

src/mscorlib/src/System/Runtime/CompilerServices/jithelpers.cs

@@ -60,10 +60,10 @@ internal class PinningHelper
         public byte m_data;
     }
 
-    internal class ArrayPinningHelper<T>
+    internal class ArrayPinningHelper
     {
         public IntPtr m_lengthAndPadding;
-        public T m_arrayData;
+        public byte m_arrayData;
     }
 
     [FriendAccessAllowed]
@@ -230,41 +230,40 @@ static internal PinningHelper GetPinningHelper(Object o)
 #if FEATURE_SPAN_OF_T
         static internal ref T GetByRef<T>(ref IntPtr byref)
         {
-            // The body of this function will be replaced by the EE with unsafe code that just returns o!!!
-            // See getILIntrinsicImplementation for how this happens.  
+            // The body of this function will be replaced by the EE with unsafe code!!!
+            // See getILIntrinsicImplementation for how this happens.
             throw new InvalidOperationException();
         }
 
         static internal void SetByRef<T>(out IntPtr byref, ref T value)
         {
-            // The body of this function will be replaced by the EE with unsafe code that just returns o!!!
-            // See getILIntrinsicImplementation for how this happens.  
+            // The body of this function will be replaced by the EE with unsafe code!!!
+            // See getILIntrinsicImplementation for how this happens.
             throw new InvalidOperationException();
         }
 
         static internal ref T AddByRef<T>(ref T pointer, int count)
         {
-            // The body of this function will be replaced by the EE with unsafe code that just returns o!!!
-            // See getILIntrinsicImplementation for how this happens.  
+            // The body of this function will be replaced by the EE with unsafe code!!!
+            // See getILIntrinsicImplementation for how this happens.
+            typeof(T).ToString(); // Type used by the actual method body
             throw new InvalidOperationException();
         }
 
         static internal bool ByRefEquals<T>(ref T refA, ref T refB)
         {
-            // The body of this function will be replaced by the EE with unsafe code that just returns o!!!
+            // The body of this function will be replaced by the EE with unsafe code!!!
             // See getILIntrinsicImplementation for how this happens.  
             throw new InvalidOperationException();
         }
-#endif
 
         static internal ref T GetArrayData<T>(T[] array)
         {
-            // This cast is really unsafe - call the private version that does not assert in debug
-#if _DEBUG
-            return ref UnsafeCastInternal<ArrayPinningHelper<T>>(array).m_arrayData;
-#else
-            return ref UnsafeCast<ArrayPinningHelper<T>>(array).m_arrayData;
-#endif
+            // The body of this function will be replaced by the EE with unsafe code!!!
+            // See getILIntrinsicImplementation for how this happens.
+            typeof(ArrayPinningHelper).ToString(); // Type used by the actual method body
+            throw new InvalidOperationException();
         }
+#endif // FEATURE_SPAN_OF_T
     }
 }

src/mscorlib/src/System/Span.cs

@@ -27,6 +27,12 @@ public struct Span<T>
         /// <param name="array">The target array.</param>
         public Span(T[] array)
         {
+            if (default(T) == null) // Arrays of valuetypes are never covariant
+            {
+                if (array.GetType() != typeof(T[]))
+                    ThrowHelper.ThrowArrayTypeMismatchException();
+            }
+
             JitHelpers.SetByRef(out _rawPointer, ref JitHelpers.GetArrayData(array));
             _length = array.Length;
         }
@@ -43,6 +49,12 @@ public Span(T[] array, int start, int length)
             if ((uint)start >= (uint)array.Length || (uint)length > (uint)(array.Length - start))
                 ThrowHelper.ThrowArgumentOutOfRangeException();
 
+            if (default(T) == null) // Arrays of valuetypes are never covariant
+            {
+                if (array.GetType() != typeof(T[]))
+                    ThrowHelper.ThrowArrayTypeMismatchException();
+            }
+
             JitHelpers.SetByRef(out _rawPointer, ref JitHelpers.AddByRef(ref JitHelpers.GetArrayData(array), start));
             _length = length;
         }

src/mscorlib/src/System/ThrowHelper.cs

@@ -44,6 +44,10 @@ namespace System {
     [Pure]
     internal static class ThrowHelper {    
 #if FEATURE_SPAN_OF_T
+        internal static void ThrowArrayTypeMismatchException() {
+            throw new ArrayTypeMismatchException();
+        }
+
         internal static void ThrowArgumentOutOfRangeException() {
             throw new ArgumentOutOfRangeException();
         }

src/vm/jitinterface.cpp

@@ -6778,6 +6778,28 @@ void getMethodInfoILMethodHeaderHelper(
         (CorInfoOptions)((header->GetFlags() & CorILMethod_InitLocals) ? CORINFO_OPT_INIT_LOCALS : 0) ;
 }
 
+mdToken FindGenericMethodArgTypeSpec(IMDInternalImport* pInternalImport)
+{
+    STANDARD_VM_CONTRACT;
+
+    HENUMInternalHolder hEnumTypeSpecs(pInternalImport);
+    mdToken token;
+
+    static const BYTE signature[] = { ELEMENT_TYPE_MVAR, 0 };
+
+    hEnumTypeSpecs.EnumAllInit(mdtTypeSpec);
+    while (hEnumTypeSpecs.EnumNext(&token))
+    {
+        PCCOR_SIGNATURE pSig;
+        ULONG cbSig;
+        IfFailThrow(pInternalImport->GetTypeSpecFromToken(token, &pSig, &cbSig));
+        if (cbSig == sizeof(signature) && memcmp(pSig, signature, cbSig) == 0)
+            return token;
+    }
+
+    COMPlusThrowHR(COR_E_BADIMAGEFORMAT);
+}
+
 /*********************************************************************
 
 IL is the most efficient and portable way to implement certain low level methods 
@@ -6914,19 +6936,21 @@ bool getILIntrinsicImplementation(MethodDesc * ftn,
     }
     else if (tk == MscorlibBinder::GetMethod(METHOD__JIT_HELPERS__ADD_BYREF)->GetMemberDef())
     {
-        _ASSERTE(ftn->HasMethodInstantiation());
-        Instantiation inst = ftn->GetMethodInstantiation();
+        mdToken tokGenericArg = FindGenericMethodArgTypeSpec(MscorlibBinder::GetModule()->GetMDImport());
+
+        static BYTE ilcode[] = { CEE_LDARG_1,
+                                 CEE_PREFIX1,(BYTE)CEE_SIZEOF,0,0,0,0,
+                                 CEE_CONV_I,
+                                 CEE_MUL,
+                                 CEE_LDARG_0,
+                                 CEE_ADD,
+                                 CEE_RET };
+
+        ilcode[3] = (BYTE)(tokGenericArg);
+        ilcode[4] = (BYTE)(tokGenericArg >> 8);
+        ilcode[5] = (BYTE)(tokGenericArg >> 16);
+        ilcode[6] = (BYTE)(tokGenericArg >> 24);
 
-        _ASSERTE(inst.GetNumArgs() == 1);
-        unsigned size = inst[0].GetSize();
-
-        static const BYTE ilcode[] = { CEE_LDARG_1, 
-                                       CEE_LDC_I4, (BYTE)(size), (BYTE)(size >> 8), (BYTE)(size >> 16), (BYTE)(size >> 24), 
-                                       CEE_CONV_I, 
-                                       CEE_MUL, 
-                                       CEE_LDARG_0, 
-                                       CEE_ADD, 
-                                       CEE_RET };
         methInfo->ILCode = const_cast<BYTE*>(ilcode);
         methInfo->ILCodeSize = sizeof(ilcode);
         methInfo->maxStack = 2;
@@ -6945,6 +6969,26 @@ bool getILIntrinsicImplementation(MethodDesc * ftn,
         methInfo->options = (CorInfoOptions)0;
         return true;
     }
+    else if (tk == MscorlibBinder::GetMethod(METHOD__JIT_HELPERS__GET_ARRAY_DATA)->GetMemberDef())
+    {
+        mdToken tokArrayPinningHelper = MscorlibBinder::GetField(FIELD__ARRAY_PINNING_HELPER__M_ARRAY_DATA)->GetMemberDef();
+
+        static BYTE ilcode[] = { CEE_LDARG_0,
+                                 CEE_LDFLDA,0,0,0,0,
+                                 CEE_RET };
+
+        ilcode[2] = (BYTE)(tokArrayPinningHelper);
+        ilcode[3] = (BYTE)(tokArrayPinningHelper >> 8);
+        ilcode[4] = (BYTE)(tokArrayPinningHelper >> 16);
+        ilcode[5] = (BYTE)(tokArrayPinningHelper >> 24);
+
+        methInfo->ILCode = const_cast<BYTE*>(ilcode);
+        methInfo->ILCodeSize = sizeof(ilcode);
+        methInfo->maxStack = 1;
+        methInfo->EHcount = 0;
+        methInfo->options = (CorInfoOptions)0;
+        return true;
+    }
 #endif
 
     return false;

src/vm/mscorlib.h

@@ -1349,6 +1349,7 @@ DEFINE_METHOD(JIT_HELPERS,          GET_BYREF,              GetByRef, NoSig)
 DEFINE_METHOD(JIT_HELPERS,          SET_BYREF,              SetByRef, NoSig)
 DEFINE_METHOD(JIT_HELPERS,          ADD_BYREF,              AddByRef, NoSig)
 DEFINE_METHOD(JIT_HELPERS,          BYREF_EQUALS,           ByRefEquals, NoSig)
+DEFINE_METHOD(JIT_HELPERS,          GET_ARRAY_DATA,         GetArrayData, NoSig)
 #endif
 
 DEFINE_CLASS(INTERLOCKED,           Threading,              Interlocked)
@@ -1358,6 +1359,9 @@ DEFINE_METHOD(INTERLOCKED,          COMPARE_EXCHANGE_OBJECT,CompareExchange, SM_
 DEFINE_CLASS(PINNING_HELPER,        CompilerServices,       PinningHelper)
 DEFINE_FIELD(PINNING_HELPER,        M_DATA,                 m_data)
 
+DEFINE_CLASS(ARRAY_PINNING_HELPER,  CompilerServices,       ArrayPinningHelper)
+DEFINE_FIELD(ARRAY_PINNING_HELPER,  M_ARRAY_DATA,           m_arrayData)
+
 DEFINE_CLASS(RUNTIME_WRAPPED_EXCEPTION, CompilerServices,   RuntimeWrappedException)
 DEFINE_METHOD(RUNTIME_WRAPPED_EXCEPTION, OBJ_CTOR,          .ctor,                      IM_Obj_RetVoid)
 DEFINE_FIELD(RUNTIME_WRAPPED_EXCEPTION, WRAPPED_EXCEPTION,  m_wrappedException)

tests/src/CoreMangLib/system/span/BasicSpanTest.cs

@@ -1,8 +1,23 @@
 using System;
 using System.Collections.Generic;
 
+class ReferenceType
+{
+    internal byte Value;
+    public ReferenceType(byte value) { Value = value; }
+}
+
 class My
 {
+    static void AssertTrue(bool condition, string message)
+    {
+        if (!condition)
+        {
+            Console.WriteLine(message);
+            Environment.Exit(1);
+        }
+    }
+
     static int Sum(Span<int> span)
     {
         int sum = 0;
@@ -11,12 +26,67 @@ static int Sum(Span<int> span)
         return sum;
     }
 
-    static void Main()
+    static void TestSum()
     {
-        int[] a = new int[] { 1, 2, 3 };
+        int[] a = new int[] { 1, 2, 3, 4 };
         Span<int> span = new Span<int>(a);
-        Console.WriteLine(Sum(span).ToString());
+        AssertTrue(Sum(span) == 10, "Unexpected sum of array");
         Span<int> slice = span.Slice(1, 2);
-        Console.WriteLine(Sum(slice).ToString());
+        AssertTrue(Sum(slice) == 5, "Unexpected sum of slice");
+    }
+
+    static void TestReferenceTypes()
+    {
+        var underlyingArray = new ReferenceType[] { new ReferenceType(0), new ReferenceType(1), new ReferenceType(2) };
+        var slice = new Span<ReferenceType>(underlyingArray);
+
+        for (int i = 0; i < underlyingArray.Length; i++)
+        {
+            AssertTrue(underlyingArray[i].Value == slice[i].Value, "Values are different");
+            AssertTrue(object.ReferenceEquals(underlyingArray[i], slice[i]), "References are broken");
+        }
+    }
+
+    static void TestArrayCoVariance()
+    {
+        var array = new ReferenceType[1];
+        var objArray = (object[])array;
+        try
+        {
+            new Span<object>(objArray);
+            AssertTrue(false, "Expected exception not thrown");
+        }
+        catch (ArrayTypeMismatchException)
+        {
+        }
+
+        var objEmptyArray = Array.Empty<ReferenceType>();
+        try
+        {
+            new Span<object>(objEmptyArray);
+            AssertTrue(false, "Expected exception not thrown");
+        }
+        catch (ArrayTypeMismatchException)
+        {
+        }
+    }
+
+    static void TestArrayCoVarianceReadOnly()
+    {
+        var array = new ReferenceType[1];
+        var objArray = (object[])array;
+        AssertTrue(new ReadOnlySpan<object>(objArray).Length == 1, "Unexpected length");
+
+        var objEmptyArray = Array.Empty<ReferenceType>();
+        AssertTrue(new ReadOnlySpan<object>(objEmptyArray).Length == 0, "Unexpected length");
+   }
+
+    static void Main()
+    {
+        TestSum();
+        TestReferenceTypes();
+        TestArrayCoVariance();
+        TestArrayCoVarianceReadOnly();
+        Console.WriteLine("All tests passed");
     }
 }