From f43185b07889fabb2e07c8fe4c1dfcd74f17747c Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 27 Aug 2019 15:13:03 -0700 Subject: [PATCH] =?UTF-8?q?Fix=20marshalling=20a=20pinnable=20multi-dimens?= =?UTF-8?q?ional=20array=20via=20a=20P/Inv=E2=80=A6=20(#26279)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Use the exact methodtable of the parameter to calculate the offset into a pinned array. * Fix logic check. * Update precondition. * Fix accidental union overlap. --- src/vm/ilmarshalers.cpp | 8 +++-- src/vm/mlinfo.cpp | 2 ++ src/vm/mlinfo.h | 1 + .../AsDefault/AsDefaultTest.cs | 35 +++++++++++++++++++ .../MarshalArrayLPArrayNative.cpp | 13 +++++++ 5 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/vm/ilmarshalers.cpp b/src/vm/ilmarshalers.cpp index 0fb2f712affe..a384f4af964c 100644 --- a/src/vm/ilmarshalers.cpp +++ b/src/vm/ilmarshalers.cpp @@ -3701,7 +3701,9 @@ void ILNativeArrayMarshaler::EmitCreateMngdMarshaler(ILCodeStream* pslILEmit) bool ILNativeArrayMarshaler::CanMarshalViaPinning() { - return IsCLRToNative(m_dwMarshalFlags) && !IsByref(m_dwMarshalFlags) && (NULL == OleVariant::GetMarshalerForVarType(m_pargs->na.m_vt, TRUE)); + // We can't pin an array if we have a marshaler for the var type + // or if we can't get a method-table representing the array (how we determine the offset to pin). + return IsCLRToNative(m_dwMarshalFlags) && !IsByref(m_dwMarshalFlags) && (NULL != m_pargs->na.m_pArrayMT) && (NULL == OleVariant::GetMarshalerForVarType(m_pargs->na.m_vt, TRUE)); } void ILNativeArrayMarshaler::EmitMarshalViaPinning(ILCodeStream* pslILEmit) @@ -3709,7 +3711,7 @@ void ILNativeArrayMarshaler::EmitMarshalViaPinning(ILCodeStream* pslILEmit) CONTRACTL { STANDARD_VM_CHECK; - PRECONDITION(IsCLRToNative(m_dwMarshalFlags) && !IsByref(m_dwMarshalFlags)); + PRECONDITION(CanMarshalViaPinning()); } CONTRACTL_END; @@ -3744,7 +3746,7 @@ void ILNativeArrayMarshaler::EmitMarshalViaPinning(ILCodeStream* pslILEmit) pslILEmit->EmitCONV_I(); // Optimize marshalling by emitting the data ptr offset directly into the IL stream // instead of doing an FCall to recalulate it each time when possible. - pslILEmit->EmitLDC(ArrayBase::GetDataPtrOffset(m_pargs->m_pMarshalInfo->GetArrayElementTypeHandle().MakeSZArray().GetMethodTable())); + pslILEmit->EmitLDC(ArrayBase::GetDataPtrOffset(m_pargs->na.m_pArrayMT)); pslILEmit->EmitADD(); EmitStoreNativeValue(pslILEmit); diff --git a/src/vm/mlinfo.cpp b/src/vm/mlinfo.cpp index 607463ac9adb..db6f01abbf80 100644 --- a/src/vm/mlinfo.cpp +++ b/src/vm/mlinfo.cpp @@ -2802,6 +2802,8 @@ MarshalInfo::MarshalInfo(Module* pModule, } } + m_args.na.m_pArrayMT = arrayTypeHnd.GetMethodTable(); + // Handle retrieving the information for the array type. IfFailGoto(HandleArrayElemType(&ParamInfo, thElement, asArray->GetRank(), mtype == ELEMENT_TYPE_SZARRAY, isParam, pAssembly), lFail); break; diff --git a/src/vm/mlinfo.h b/src/vm/mlinfo.h index 46057d58755b..46dd79cfc7dc 100644 --- a/src/vm/mlinfo.h +++ b/src/vm/mlinfo.h @@ -82,6 +82,7 @@ struct OverrideProcArgs struct { + MethodTable* m_pArrayMT; VARTYPE m_vt; #ifdef FEATURE_COMINTEROP SIZE_T m_cbElementSize; diff --git a/tests/src/Interop/PInvoke/Array/MarshalArrayAsParam/AsDefault/AsDefaultTest.cs b/tests/src/Interop/PInvoke/Array/MarshalArrayAsParam/AsDefault/AsDefaultTest.cs index 5cdeea58e088..3fc4e5475114 100644 --- a/tests/src/Interop/PInvoke/Array/MarshalArrayAsParam/AsDefault/AsDefaultTest.cs +++ b/tests/src/Interop/PInvoke/Array/MarshalArrayAsParam/AsDefault/AsDefaultTest.cs @@ -244,12 +244,19 @@ private static extern bool CStyle_Array_Struct_Out( [DllImport("MarshalArrayLPArrayNative")] private static extern bool CStyle_Array_Bool_Out( [Out] bool[] actual, int cActual); + + [DllImport("MarshalArrayLPArrayNative")] + private static extern int Get_Multidimensional_Array_Sum(int[,] array, int rows, int columns); #endregion #region Marshal ByVal private const int ARRAY_SIZE = 100; + private const int ROWS = 3; + + private const int COLUMNS = 2; + private static T[] InitArray(int size) { T[] array = new T[size]; @@ -290,6 +297,20 @@ private static TestStruct[] InitStructArray(int size) return array; } + private static int[,] InitMultidimensionalBlittableArray(int rows, int columns) + { + int[,] array = new int[rows, columns]; + + for (int i = 0; i < array.GetLength(0); i++) + { + for (int j = 0; j < array.GetLength(1); j++) + { + array[i, j] = i * j; + } + } + return array; + } + private static void TestMarshalByVal_NoAttributes() { Console.WriteLine("ByVal marshaling CLR array as c-style-array no attributes"); @@ -619,6 +640,19 @@ private static void TestMarshalOut_ByVal() #endregion + private static void TestMultidimensional() + { + Console.WriteLine("================== [Get_Multidimensional_Array_Sum] ============"); + int[,] array = InitMultidimensionalBlittableArray(ROWS, COLUMNS); + int sum = 0; + foreach (int item in array) + { + sum += item; + } + + Assert.AreEqual(sum, Get_Multidimensional_Array_Sum(array, ROWS, COLUMNS)); + } + public static int Main() { try @@ -627,6 +661,7 @@ public static int Main() TestMarshalByVal_In(); TestMarshalInOut_ByVal(); TestMarshalOut_ByVal(); + TestMultidimensional(); Console.WriteLine("\nTest PASS."); return 100; diff --git a/tests/src/Interop/PInvoke/Array/MarshalArrayAsParam/LPArrayNative/MarshalArrayLPArrayNative.cpp b/tests/src/Interop/PInvoke/Array/MarshalArrayAsParam/LPArrayNative/MarshalArrayLPArrayNative.cpp index fea447c69bff..5f29b239be45 100644 --- a/tests/src/Interop/PInvoke/Array/MarshalArrayAsParam/LPArrayNative/MarshalArrayLPArrayNative.cpp +++ b/tests/src/Interop/PInvoke/Array/MarshalArrayAsParam/LPArrayNative/MarshalArrayLPArrayNative.cpp @@ -1111,3 +1111,16 @@ extern "C" DLL_EXPORT BOOL MarshalArrayOfStructAsLPArrayByRefOut(S2 **pActual, i } return breturn; } + +extern "C" DLL_EXPORT int Get_Multidimensional_Array_Sum(int* array, int rows, int columns) +{ + int sum = 0; + for(int i = 0; i < rows; ++i) + { + for (int j = 0; j < columns; ++j) + { + sum += array[i * columns + j]; + } + } + return sum; +}