Move type check to after the null ref branch in out marshalling of bl…

…ittable classes. (#50735)
dotnet · Apr 7, 2021 · 4888360 · 4888360
1 parent 27591f5
commit 4888360
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 3 deletions.
diff --git 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
@@ -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)
 {
@@ -90,6 +96,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
@@ -128,12 +128,18 @@ class StructureTests
         [DllImport("LayoutClassNative")]
         private static extern bool SimpleSeqLayoutClassByRef(SeqClass p);
 
+        [DllImport("LayoutClassNative")]
+        private static extern bool SimpleSeqLayoutClassByRefNull([In, Out] SeqClass p);
+
         [DllImport("LayoutClassNative")]
         private static extern bool DerivedSeqLayoutClassByRef(EmptyBase p, int expected);
 
         [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);
 
@@ -167,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)}...");
@@ -200,6 +213,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
@@ -269,9 +289,11 @@ public static int Main(string[] argv)
             try
             {
                 SequentialClass();
+                SequentialClassNull();
                 DerivedClassWithEmptyBase();
                 ExplicitClass();
                 BlittableClass();
+                BlittableClassNull();
                 SealedBlittableClass();
                 BlittableClassByInAttr();
                 SealedBlittableClassByInAttr();