The DestroyStructure API passes null to the IL marshaller for managed valuetypes...

Co-authored-by: Aaron Robinson <arobins@microsoft.com>

diff --git a/src/coreclr/vm/ilmarshalers.h b/src/coreclr/vm/ilmarshalers.h
index ba07cd5..8d24d07 100644 (file)
--- a/src/coreclr/vm/ilmarshalers.h
+++ b/src/coreclr/vm/ilmarshalers.h
@@ -988,11 +988,19 @@ protected:
 
         m_nativeHome.InitHome(ILStubMarshalHome::HomeType_ILByrefLocal, pcsSetup->NewLocal(nativeFieldTypeByRef), &nativeType, /* unalignedIndirectStore */ true);
 
+        // During the cleanup pass, the managed typed is explicitly passed as null - see DestroyStructure(). This works
+        // except if the type has nested non-blittable fields. Not checking for null will compute invalid offsets that
+        // can cause an access violation during the field clean-up.
+        ILCodeLabel *pSkipComputeOffsetLabel = pcsSetup->NewCodeLabel();
+
         pcsSetup->EmitNOP("// field setup {");
         pcsSetup->EmitNOP("// managed field setup {");
         pcsSetup->EmitLDARG(StructMarshalStubs::MANAGED_STRUCT_ARGIDX);
+        pcsSetup->EmitDUP();
+        pcsSetup->EmitBRFALSE(pSkipComputeOffsetLabel);
         pcsSetup->EmitLDC(managedOffset);
         pcsSetup->EmitADD();
+        pcsSetup->EmitLabel(pSkipComputeOffsetLabel);
         EmitStoreManagedHomeAddr(pcsSetup);
         pcsSetup->EmitNOP("// } managed field setup");
         pcsSetup->EmitNOP("// native field setup {");

diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/StructureToPtrTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/StructureToPtrTests.cs
index f174ab1..ea55d5c 100644 (file)
--- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/StructureToPtrTests.cs
+++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/StructureToPtrTests.cs
@@ -256,6 +256,30 @@ namespace System.Runtime.InteropServices.Tests
             Assert.Equal(*opaqueData, *marshaledOpaqueData);
         }
 
+        [Fact]
+        public void StructureToPtr_Flat_And_Nested_NonBlittableStructure_Success()
+        {
+            MarshalAndDestroy(new NonBlittableStruct_Flat
+            {
+                del = null,
+                b = 0x55,
+            });
+
+            MarshalAndDestroy(new NonBlittableStruct_Nested
+            {
+                s = { del = null },
+                b = 0x55,
+            });
+
+            static unsafe void MarshalAndDestroy<T>(T value) where T : struct
+            {
+                int sizeof_T = Marshal.SizeOf<T>();
+                void* ptr = stackalloc byte[sizeof_T];
+                Marshal.StructureToPtr(value, (IntPtr)ptr, false);
+                Marshal.DestroyStructure<T>((IntPtr)ptr);
+            }
+        }
+
         public struct StructWithIntField
         {
             public int value;
@@ -318,5 +342,22 @@ namespace System.Runtime.InteropServices.Tests
             public OpaqueStruct opaque;
             public string str;
         }
+
+        public struct NonBlittableStruct_Flat
+        {
+            public Delegate del;
+            public byte b;
+        }
+
+        public struct NonBlittableStruct_Nested
+        {
+            public struct InnerStruct
+            {
+                public Delegate del;
+            }
+
+            public InnerStruct s;
+            public byte b;
+        }
     }
 }