diff options
author | Jan Kotas <jkotas@microsoft.com> | 2016-08-28 23:18:11 -0700 |
---|---|---|
committer | Jan Kotas <jkotas@microsoft.com> | 2016-10-30 17:27:31 -0700 |
commit | e7dfda575898129eb77a3a8541990c43c4e29149 (patch) | |
tree | 885188360a88c9c7ed43472a6b6e994928fddc7d | |
parent | 9c7c1b741b132cd78bfc9db2654227bc2aec2e45 (diff) | |
download | coreclr-e7dfda575898129eb77a3a8541990c43c4e29149.tar.gz coreclr-e7dfda575898129eb77a3a8541990c43c4e29149.tar.bz2 coreclr-e7dfda575898129eb77a3a8541990c43c4e29149.zip |
Fix reference types (#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.
-rw-r--r-- | src/mscorlib/src/System/Runtime/CompilerServices/jithelpers.cs | 31 | ||||
-rw-r--r-- | src/mscorlib/src/System/Span.cs | 12 | ||||
-rw-r--r-- | src/mscorlib/src/System/ThrowHelper.cs | 4 | ||||
-rw-r--r-- | src/vm/jitinterface.cpp | 68 | ||||
-rw-r--r-- | src/vm/mscorlib.h | 4 | ||||
-rw-r--r-- | tests/src/CoreMangLib/system/span/BasicSpanTest.cs | 78 |
6 files changed, 165 insertions, 32 deletions
diff --git a/src/mscorlib/src/System/Runtime/CompilerServices/jithelpers.cs b/src/mscorlib/src/System/Runtime/CompilerServices/jithelpers.cs index a355834324..cac5d2aac7 100644 --- a/src/mscorlib/src/System/Runtime/CompilerServices/jithelpers.cs +++ b/src/mscorlib/src/System/Runtime/CompilerServices/jithelpers.cs @@ -60,10 +60,10 @@ namespace System.Runtime.CompilerServices { 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 @@ namespace System.Runtime.CompilerServices { #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 } } diff --git a/src/mscorlib/src/System/Span.cs b/src/mscorlib/src/System/Span.cs index c3e2f635d6..b56cb9cccc 100644 --- a/src/mscorlib/src/System/Span.cs +++ b/src/mscorlib/src/System/Span.cs @@ -27,6 +27,12 @@ namespace System /// <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 @@ namespace System 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; } diff --git a/src/mscorlib/src/System/ThrowHelper.cs b/src/mscorlib/src/System/ThrowHelper.cs index 77028dce40..98df462772 100644 --- a/src/mscorlib/src/System/ThrowHelper.cs +++ b/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(); } diff --git a/src/vm/jitinterface.cpp b/src/vm/jitinterface.cpp index 03dc3c4de4..a6ba9b9770 100644 --- a/src/vm/jitinterface.cpp +++ b/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; diff --git a/src/vm/mscorlib.h b/src/vm/mscorlib.h index a98095b2ca..7c2c244c40 100644 --- a/src/vm/mscorlib.h +++ b/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) diff --git a/tests/src/CoreMangLib/system/span/BasicSpanTest.cs b/tests/src/CoreMangLib/system/span/BasicSpanTest.cs index c7c9618b9a..496234f8a6 100644 --- a/tests/src/CoreMangLib/system/span/BasicSpanTest.cs +++ b/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 @@ class My 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"); } } |