From 48883606b4b2c34fbed7dc5555b3f89c11ce23a0 Mon Sep 17 00:00:00 2001
From: Jeremy Koritzinsky <jekoritz@microsoft.com>
Date: Wed, 7 Apr 2021 09:29:20 -0700
Subject: [PATCH] Move type check to after the null ref branch in out
 marshalling of blittable classes. (#50735)

---
 src/coreclr/vm/ilmarshalers.cpp               |  6 ++---
 .../Interop/LayoutClass/LayoutClassNative.cpp | 12 ++++++++++
 .../Interop/LayoutClass/LayoutClassTest.cs    | 22 +++++++++++++++++++
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/src/coreclr/vm/ilmarshalers.cpp b/src/coreclr/vm/ilmarshalers.cpp
index 0c753e6929b1c..3b45dc673c233 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 59c4645afba32..7798ffb5cf0a2 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)
 {
@@ -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
index 81048a22b332d..d87be9e81c395 100644
--- 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();