diff --git a/src/coreclr/vm/classlayoutinfo.cpp b/src/coreclr/vm/classlayoutinfo.cpp index 612c119be79f0..07667804110c1 100644 --- a/src/coreclr/vm/classlayoutinfo.cpp +++ b/src/coreclr/vm/classlayoutinfo.cpp @@ -653,12 +653,8 @@ VOID EEClassLayoutInfo::CollectLayoutFieldMetadataThrowing( DEBUGARG(szName) ); - // Type is blittable only if parent is also blittable and is not empty. - if (isBlittable && fHasNonTrivialParent) - { - isBlittable = pParentMT->IsBlittable() // Check parent - && (!pParentLayoutInfo || !pParentLayoutInfo->IsZeroSized()); // Ensure non-zero size - } + // Type is blittable only if parent is also blittable + isBlittable = isBlittable && (fHasNonTrivialParent ? pParentMT->IsBlittable() : TRUE); pEEClassLayoutInfoOut->SetIsBlittable(isBlittable); S_UINT32 cbSortArraySize = S_UINT32(cTotalFields) * S_UINT32(sizeof(LayoutRawFieldInfo*)); diff --git a/src/coreclr/vm/ilmarshalers.cpp b/src/coreclr/vm/ilmarshalers.cpp index 5c7517b28847c..163222d423d5a 100644 --- a/src/coreclr/vm/ilmarshalers.cpp +++ b/src/coreclr/vm/ilmarshalers.cpp @@ -2496,10 +2496,14 @@ void ILBlittablePtrMarshaler::EmitConvertContentsNativeToCLR(ILCodeStream* pslIL bool ILBlittablePtrMarshaler::CanMarshalViaPinning() { + // [COMPAT] For correctness, we can't marshal via pinning if we might need to marshal differently at runtime. + // See calls to EmitExactTypeCheck where we check the runtime type of the object being marshalled. + // However, we previously supported pinning non-sealed blittable classes, even though that could result + // in some data still being unmarshalled if a subclass is provided. This optimization is incorrect, + // but libraries like NAudio have taken a hard dependency on this incorrect behavior, so we need to preserve it. return IsCLRToNative(m_dwMarshalFlags) && !IsByref(m_dwMarshalFlags) && - !IsFieldMarshal(m_dwMarshalFlags) && - m_pargs->m_pMT->IsSealed(); // We can't marshal via pinning if we might need to marshal differently at runtime. See calls to EmitExactTypeCheck where we check the runtime type of the object being marshalled. + !IsFieldMarshal(m_dwMarshalFlags); } void ILBlittablePtrMarshaler::EmitMarshalViaPinning(ILCodeStream* pslILEmit) diff --git a/src/coreclr/vm/mlinfo.cpp b/src/coreclr/vm/mlinfo.cpp index 57ee8a4825bcc..487dd236767f0 100644 --- a/src/coreclr/vm/mlinfo.cpp +++ b/src/coreclr/vm/mlinfo.cpp @@ -1138,8 +1138,6 @@ MarshalInfo::MarshalInfo(Module* pModule, CorElementType corElemType = ELEMENT_TYPE_END; m_pMT = NULL; m_pMD = pMD; - // [Compat] For backward compatibility reasons, some marshalers imply [In, Out] behavior when marked as [In], [Out], or not marked with either. - BOOL byValAlwaysInOut = FALSE; #ifdef FEATURE_COMINTEROP m_fDispItf = FALSE; @@ -1884,7 +1882,6 @@ MarshalInfo::MarshalInfo(Module* pModule, } m_type = IsFieldScenario() ? MARSHAL_TYPE_BLITTABLE_LAYOUTCLASS : MARSHAL_TYPE_BLITTABLEPTR; m_args.m_pMT = m_pMT; - byValAlwaysInOut = TRUE; } else if (m_pMT->HasLayout()) { @@ -2377,13 +2374,7 @@ MarshalInfo::MarshalInfo(Module* pModule, } } - if (!m_byref && byValAlwaysInOut) - { - // Some marshalers expect [In, Out] behavior with [In], [Out], or no directional attributes. - m_in = TRUE; - m_out = TRUE; - } - else if (!m_in && !m_out) + if (!m_in && !m_out) { // If neither IN nor OUT are true, this signals the URT to use the default // rules. diff --git a/src/tests/Interop/LayoutClass/LayoutClassNative.cpp b/src/tests/Interop/LayoutClass/LayoutClassNative.cpp index 7798ffb5cf0a2..ff0ff5b904f8e 100644 --- a/src/tests/Interop/LayoutClass/LayoutClassNative.cpp +++ b/src/tests/Interop/LayoutClass/LayoutClassNative.cpp @@ -108,6 +108,12 @@ DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleNestedLayoutClassByValue(NestedLayoutCla return SimpleSeqLayoutClassByRef(&v.str); } +extern "C" +DLL_EXPORT BOOL STDMETHODCALLTYPE PointersEqual(void* ptr, void* ptr2) +{ + return ptr == ptr2 ? TRUE : FALSE; +} + extern "C" DLL_EXPORT void __cdecl Invalid(...) { diff --git a/src/tests/Interop/LayoutClass/LayoutClassTest.cs b/src/tests/Interop/LayoutClass/LayoutClassTest.cs index 2181bb30528d4..b333c7ddd9851 100644 --- a/src/tests/Interop/LayoutClass/LayoutClassTest.cs +++ b/src/tests/Interop/LayoutClass/LayoutClassTest.cs @@ -174,6 +174,12 @@ class StructureTests [DllImport("LayoutClassNative", EntryPoint = SimpleBlittableSeqLayoutClass_UpdateField)] private static extern bool SealedBlittableSeqLayoutClassByOutAttr([Out] SealedBlittable p); + [DllImport("LayoutClassNative")] + private static extern bool PointersEqual(SealedBlittable obj, ref int field); + + [DllImport("LayoutClassNative")] + private static extern bool PointersEqual(Blittable obj, ref int field); + [DllImport("LayoutClassNative")] private static extern bool SimpleNestedLayoutClassByValue(NestedLayout p); @@ -280,6 +286,20 @@ public static void SealedBlittableClassByOutAttr() ValidateSealedBlittableClassInOut(SealedBlittableSeqLayoutClassByOutAttr); } + public static void SealedBlittablePinned() + { + Console.WriteLine($"Running {nameof(SealedBlittablePinned)}..."); + var blittable = new SealedBlittable(1); + Assert.IsTrue(PointersEqual(blittable, ref blittable.a)); + } + + public static void BlittablePinned() + { + Console.WriteLine($"Running {nameof(BlittablePinned)}..."); + var blittable = new Blittable(1); + Assert.IsTrue(PointersEqual(blittable, ref blittable.a)); + } + public static void NestedLayoutClass() { Console.WriteLine($"Running {nameof(NestedLayoutClass)}..."); @@ -317,6 +337,8 @@ public static int Main(string[] argv) SealedBlittableClassByOutAttr(); NestedLayoutClass(); RecursiveNativeLayout(); + SealedBlittablePinned(); + BlittablePinned(); } catch (Exception e) {