From cdcc4b5425db3f5f2be1c5efcf1559540fcdf445 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 5 Apr 2021 10:02:00 -0700 Subject: [PATCH 1/4] Move type check to after the null ref branch. --- src/coreclr/vm/ilmarshalers.cpp | 6 +++--- src/tests/Interop/LayoutClass/LayoutClassNative.cpp | 6 ++++++ src/tests/Interop/LayoutClass/LayoutClassTest.cs | 11 +++++++++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/ilmarshalers.cpp b/src/coreclr/vm/ilmarshalers.cpp index 0c753e6929b1cd..3b45dc673c2332 100644 --- a/src/coreclr/vm/ilmarshalers.cpp +++ b/src/coreclr/vm/ilmarshalers.cpp @@ -2466,12 +2466,12 @@ void ILBlittablePtrMarshaler::EmitConvertContentsNativeToCLR(ILCodeStream* pslIL UINT uNativeSize = m_pargs->m_pMT->GetNativeSize(); int fieldDef = pslILEmit->GetToken(CoreLibBinder::GetField(FIELD__RAW_DATA__DATA)); - ILCodeLabel* isNotMatchingTypeLabel = pslILEmit->NewCodeLabel(); - bool emittedTypeCheck = EmitExactTypeCheck(pslILEmit, isNotMatchingTypeLabel); - EmitLoadManagedValue(pslILEmit); pslILEmit->EmitBRFALSE(pNullRefLabel); + ILCodeLabel* isNotMatchingTypeLabel = pslILEmit->NewCodeLabel(); + bool emittedTypeCheck = EmitExactTypeCheck(pslILEmit, isNotMatchingTypeLabel); + EmitLoadManagedValue(pslILEmit); pslILEmit->EmitLDFLDA(fieldDef); // dest diff --git a/src/tests/Interop/LayoutClass/LayoutClassNative.cpp b/src/tests/Interop/LayoutClass/LayoutClassNative.cpp index 59c4645afba321..4336bba6ac76e4 100644 --- a/src/tests/Interop/LayoutClass/LayoutClassNative.cpp +++ b/src/tests/Interop/LayoutClass/LayoutClassNative.cpp @@ -90,6 +90,12 @@ DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleBlittableSeqLayoutClass_UpdateField(Blit return TRUE; } +extern "C" +DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleBlittableSeqLayoutClass_Null(BlittableClass* p) +{ + return p == NULL ? TRUE : FALSE; +} + extern "C" DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleNestedLayoutClassByValue(NestedLayoutClass v) { diff --git a/src/tests/Interop/LayoutClass/LayoutClassTest.cs b/src/tests/Interop/LayoutClass/LayoutClassTest.cs index 81048a22b332d8..6fd72ef57c48d8 100644 --- a/src/tests/Interop/LayoutClass/LayoutClassTest.cs +++ b/src/tests/Interop/LayoutClass/LayoutClassTest.cs @@ -134,6 +134,9 @@ class StructureTests [DllImport("LayoutClassNative")] private static extern bool SimpleExpLayoutClassByRef(ExpClass p); + [DllImport("LayoutClassNative")] + private static extern bool SimpleBlittableSeqLayoutClass_Null(Blittable p); + [DllImport("LayoutClassNative", EntryPoint = SimpleBlittableSeqLayoutClass_UpdateField)] private static extern bool SimpleBlittableSeqLayoutClassByRef(Blittable p); @@ -200,6 +203,13 @@ public static void BlittableClass() ValidateBlittableClassInOut(SimpleBlittableSeqLayoutClassByRef); } + public static void BlittableClassNull() + { + // [Compat] Marshalled with [In, Out] behaviour by default + Console.WriteLine($"Running {nameof(BlittableClassNull)}..."); + Assert.IsTrue(SimpleBlittableSeqLayoutClass_Null(null))); + } + public static void BlittableClassByInAttr() { // [Compat] Marshalled with [In, Out] behaviour even when only [In] is specified @@ -272,6 +282,7 @@ public static int Main(string[] argv) DerivedClassWithEmptyBase(); ExplicitClass(); BlittableClass(); + BlittableClassNull(); SealedBlittableClass(); BlittableClassByInAttr(); SealedBlittableClassByInAttr(); From 49349180d8b72de80b4f4fe74b35494f8151ce30 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 5 Apr 2021 11:39:59 -0700 Subject: [PATCH 2/4] Add non-blittable null test. --- src/tests/Interop/LayoutClass/LayoutClassNative.cpp | 6 ++++++ src/tests/Interop/LayoutClass/LayoutClassTest.cs | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/src/tests/Interop/LayoutClass/LayoutClassNative.cpp b/src/tests/Interop/LayoutClass/LayoutClassNative.cpp index 4336bba6ac76e4..7798ffb5cf0a21 100644 --- a/src/tests/Interop/LayoutClass/LayoutClassNative.cpp +++ b/src/tests/Interop/LayoutClass/LayoutClassNative.cpp @@ -55,6 +55,12 @@ DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleSeqLayoutClassByRef(SeqClass* p) return TRUE; } +extern "C" +DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleSeqLayoutClassByRefNull(SeqClass* p) +{ + return p == NULL ? TRUE : FALSE; +} + extern "C" DLL_EXPORT BOOL STDMETHODCALLTYPE DerivedSeqLayoutClassByRef(EmptyBase* p, int expected) { diff --git a/src/tests/Interop/LayoutClass/LayoutClassTest.cs b/src/tests/Interop/LayoutClass/LayoutClassTest.cs index 6fd72ef57c48d8..5f44576c1973f2 100644 --- a/src/tests/Interop/LayoutClass/LayoutClassTest.cs +++ b/src/tests/Interop/LayoutClass/LayoutClassTest.cs @@ -128,6 +128,9 @@ class StructureTests [DllImport("LayoutClassNative")] private static extern bool SimpleSeqLayoutClassByRef(SeqClass p); + [DllImport("LayoutClassNative")] + private static extern bool SimpleSeqLayoutClassByRefNull([In, Out] Blittable p); + [DllImport("LayoutClassNative")] private static extern bool DerivedSeqLayoutClassByRef(EmptyBase p, int expected); @@ -170,6 +173,13 @@ public static void SequentialClass() Assert.IsTrue(SimpleSeqLayoutClassByRef(p)); } + public static void SequentialClassNull() + { + Console.WriteLine($"Running {nameof(SequentialClassNull)}..."); + + Assert.IsTrue(SimpleSeqLayoutClassByRefNull(null)); + } + public static void DerivedClassWithEmptyBase() { Console.WriteLine($"Running {nameof(DerivedClassWithEmptyBase)}..."); @@ -279,6 +289,7 @@ public static int Main(string[] argv) try { SequentialClass(); + SequentialClassNull(); DerivedClassWithEmptyBase(); ExplicitClass(); BlittableClass(); From bdfca2d40f8b9826174d903ea772dff233e77dbc Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 5 Apr 2021 14:03:09 -0700 Subject: [PATCH 3/4] Remove extraneous paren. --- src/tests/Interop/LayoutClass/LayoutClassTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/Interop/LayoutClass/LayoutClassTest.cs b/src/tests/Interop/LayoutClass/LayoutClassTest.cs index 5f44576c1973f2..2d27f0c6953660 100644 --- a/src/tests/Interop/LayoutClass/LayoutClassTest.cs +++ b/src/tests/Interop/LayoutClass/LayoutClassTest.cs @@ -217,7 +217,7 @@ public static void BlittableClassNull() { // [Compat] Marshalled with [In, Out] behaviour by default Console.WriteLine($"Running {nameof(BlittableClassNull)}..."); - Assert.IsTrue(SimpleBlittableSeqLayoutClass_Null(null))); + Assert.IsTrue(SimpleBlittableSeqLayoutClass_Null(null)); } public static void BlittableClassByInAttr() From 3d20e067fecf8c55f8e918e55ed8253f3a499796 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 5 Apr 2021 17:05:12 -0700 Subject: [PATCH 4/4] Update src/tests/Interop/LayoutClass/LayoutClassTest.cs --- src/tests/Interop/LayoutClass/LayoutClassTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/Interop/LayoutClass/LayoutClassTest.cs b/src/tests/Interop/LayoutClass/LayoutClassTest.cs index 2d27f0c6953660..d87be9e81c3955 100644 --- a/src/tests/Interop/LayoutClass/LayoutClassTest.cs +++ b/src/tests/Interop/LayoutClass/LayoutClassTest.cs @@ -129,7 +129,7 @@ class StructureTests private static extern bool SimpleSeqLayoutClassByRef(SeqClass p); [DllImport("LayoutClassNative")] - private static extern bool SimpleSeqLayoutClassByRefNull([In, Out] Blittable p); + private static extern bool SimpleSeqLayoutClassByRefNull([In, Out] SeqClass p); [DllImport("LayoutClassNative")] private static extern bool DerivedSeqLayoutClassByRef(EmptyBase p, int expected);