Skip to content

Commit

Permalink
Revert "[release/5.0] When marshalling a layout class, fall-back to d…
Browse files Browse the repository at this point in the history 
…ynamically marshalling the type if it doesn't match the static type in the signature. (#50138)"

This reverts commit 718c0ed.
  • Loading branch information
@jkoritzinsky
jkoritzinsky authored Apr 7, 2021
1 parent 718c0ed commit 1329252
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 276 deletions.
5 changes: 0 additions & 5 deletions src/coreclr/src/vm/corelib.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,11 +488,6 @@ DEFINE_METHOD(MARSHAL, ALLOC_CO_TASK_MEM, AllocCoTa
DEFINE_METHOD(MARSHAL, FREE_CO_TASK_MEM, FreeCoTaskMem, SM_IntPtr_RetVoid)
DEFINE_FIELD(MARSHAL, SYSTEM_MAX_DBCS_CHAR_SIZE, SystemMaxDBCSCharSize)

DEFINE_METHOD(MARSHAL, STRUCTURE_TO_PTR, StructureToPtr, SM_Obj_IntPtr_Bool_RetVoid)
DEFINE_METHOD(MARSHAL, PTR_TO_STRUCTURE, PtrToStructure, SM_IntPtr_Obj_RetVoid)
DEFINE_METHOD(MARSHAL, DESTROY_STRUCTURE, DestroyStructure, SM_IntPtr_Type_RetVoid)
DEFINE_METHOD(MARSHAL, SIZEOF_TYPE, SizeOf, SM_Type_RetInt)

DEFINE_CLASS(NATIVELIBRARY, Interop, NativeLibrary)
DEFINE_METHOD(NATIVELIBRARY, LOADLIBRARYCALLBACKSTUB, LoadLibraryCallbackStub, SM_Str_AssemblyBase_Bool_UInt_RetIntPtr)

Expand Down
129 changes: 3 additions & 126 deletions src/coreclr/src/vm/ilmarshalers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2149,30 +2149,14 @@ void ILLayoutClassPtrMarshalerBase::EmitConvertSpaceCLRToNative(ILCodeStream* ps

EmitLoadManagedValue(pslILEmit);
pslILEmit->EmitBRFALSE(pNullRefLabel);
ILCodeLabel* pTypeMismatchedLabel = pslILEmit->NewCodeLabel();
bool emittedTypeCheck = EmitExactTypeCheck(pslILEmit, pTypeMismatchedLabel);
DWORD sizeLocal = pslILEmit->NewLocal(LocalDesc(ELEMENT_TYPE_I4));

pslILEmit->EmitLDC(uNativeSize);
if (emittedTypeCheck)
{
ILCodeLabel* pHaveSizeLabel = pslILEmit->NewCodeLabel();
pslILEmit->EmitBR(pHaveSizeLabel);
pslILEmit->EmitLabel(pTypeMismatchedLabel);
EmitLoadManagedValue(pslILEmit);
pslILEmit->EmitCALL(METHOD__OBJECT__GET_TYPE, 1, 1);
pslILEmit->EmitCALL(METHOD__MARSHAL__SIZEOF_TYPE, 1, 1);
pslILEmit->EmitLabel(pHaveSizeLabel);
}
pslILEmit->EmitSTLOC(sizeLocal);
pslILEmit->EmitLDLOC(sizeLocal);
pslILEmit->EmitCALL(METHOD__MARSHAL__ALLOC_CO_TASK_MEM, 1, 1);
pslILEmit->EmitDUP(); // for INITBLK
EmitStoreNativeValue(pslILEmit);

// initialize local block we just allocated
pslILEmit->EmitLDC(0);
pslILEmit->EmitLDLOC(sizeLocal);
pslILEmit->EmitLDC(uNativeSize);
pslILEmit->EmitINITBLK();

pslILEmit->EmitLabel(pNullRefLabel);
Expand All @@ -2196,30 +2180,15 @@ void ILLayoutClassPtrMarshalerBase::EmitConvertSpaceCLRToNativeTemp(ILCodeStream

EmitLoadManagedValue(pslILEmit);
pslILEmit->EmitBRFALSE(pNullRefLabel);
ILCodeLabel* pTypeMismatchedLabel = pslILEmit->NewCodeLabel();
bool emittedTypeCheck = EmitExactTypeCheck(pslILEmit, pTypeMismatchedLabel);
DWORD sizeLocal = pslILEmit->NewLocal(LocalDesc(ELEMENT_TYPE_I4));

pslILEmit->EmitLDC(uNativeSize);
if (emittedTypeCheck)
{
ILCodeLabel* pHaveSizeLabel = pslILEmit->NewCodeLabel();
pslILEmit->EmitBR(pHaveSizeLabel);
pslILEmit->EmitLabel(pTypeMismatchedLabel);
EmitLoadManagedValue(pslILEmit);
pslILEmit->EmitCALL(METHOD__OBJECT__GET_TYPE, 1, 1);
pslILEmit->EmitCALL(METHOD__MARSHAL__SIZEOF_TYPE, 1, 1);
pslILEmit->EmitLabel(pHaveSizeLabel);
}
pslILEmit->EmitSTLOC(sizeLocal);
pslILEmit->EmitLDLOC(sizeLocal);
pslILEmit->EmitLOCALLOC();
pslILEmit->EmitDUP(); // for INITBLK
EmitStoreNativeValue(pslILEmit);

// initialize local block we just allocated
pslILEmit->EmitLDC(0);
pslILEmit->EmitLDLOC(sizeLocal);
pslILEmit->EmitLDC(uNativeSize);
pslILEmit->EmitINITBLK();

pslILEmit->EmitLabel(pNullRefLabel);
Expand Down Expand Up @@ -2295,24 +2264,7 @@ void ILLayoutClassPtrMarshalerBase::EmitClearNativeTemp(ILCodeStream* pslILEmit)
}
}

bool ILLayoutClassPtrMarshalerBase::EmitExactTypeCheck(ILCodeStream* pslILEmit, ILCodeLabel* isNotMatchingTypeLabel)
{
STANDARD_VM_CONTRACT;

if (m_pargs->m_pMT->IsSealed())
{
// If the provided type cannot be derived from, then we don't need to emit the type check.
return false;
}
EmitLoadManagedValue(pslILEmit);
pslILEmit->EmitCALL(METHOD__OBJECT__GET_TYPE, 1, 1);
pslILEmit->EmitLDTOKEN(pslILEmit->GetToken(m_pargs->m_pMT));
pslILEmit->EmitCALL(METHOD__TYPE__GET_TYPE_FROM_HANDLE, 1, 1);
pslILEmit->EmitCALLVIRT(pslILEmit->GetToken(CoreLibBinder::GetMethod(METHOD__OBJECT__EQUALS)), 1, 1);
pslILEmit->EmitBRFALSE(isNotMatchingTypeLabel);

return true;
}

void ILLayoutClassPtrMarshaler::EmitConvertContentsCLRToNative(ILCodeStream* pslILEmit)
{
Expand All @@ -2329,9 +2281,6 @@ void ILLayoutClassPtrMarshaler::EmitConvertContentsCLRToNative(ILCodeStream* psl
pslILEmit->EmitLDC(uNativeSize);
pslILEmit->EmitINITBLK();

ILCodeLabel* isNotMatchingTypeLabel = pslILEmit->NewCodeLabel();
bool emittedTypeCheck = EmitExactTypeCheck(pslILEmit, isNotMatchingTypeLabel);

MethodDesc* pStructMarshalStub = NDirect::CreateStructMarshalILStub(m_pargs->m_pMT);

EmitLoadManagedValue(pslILEmit);
Expand All @@ -2341,18 +2290,6 @@ void ILLayoutClassPtrMarshaler::EmitConvertContentsCLRToNative(ILCodeStream* psl
EmitLoadCleanupWorkList(pslILEmit);

pslILEmit->EmitCALL(pslILEmit->GetToken(pStructMarshalStub), 4, 0);

if (emittedTypeCheck)
{
pslILEmit->EmitBR(pNullRefLabel);

pslILEmit->EmitLabel(isNotMatchingTypeLabel);
EmitLoadManagedValue(pslILEmit);
EmitLoadNativeValue(pslILEmit);
pslILEmit->EmitLDC(0);
pslILEmit->EmitCALL(METHOD__MARSHAL__STRUCTURE_TO_PTR, 3, 0);
}

pslILEmit->EmitLabel(pNullRefLabel);
}

Expand All @@ -2365,9 +2302,6 @@ void ILLayoutClassPtrMarshaler::EmitConvertContentsNativeToCLR(ILCodeStream* psl
EmitLoadManagedValue(pslILEmit);
pslILEmit->EmitBRFALSE(pNullRefLabel);

ILCodeLabel* isNotMatchingTypeLabel = pslILEmit->NewCodeLabel();
bool emittedTypeCheck = EmitExactTypeCheck(pslILEmit, isNotMatchingTypeLabel);

MethodDesc* pStructMarshalStub = NDirect::CreateStructMarshalILStub(m_pargs->m_pMT);

EmitLoadManagedValue(pslILEmit);
Expand All @@ -2377,26 +2311,13 @@ void ILLayoutClassPtrMarshaler::EmitConvertContentsNativeToCLR(ILCodeStream* psl
EmitLoadCleanupWorkList(pslILEmit);

pslILEmit->EmitCALL(pslILEmit->GetToken(pStructMarshalStub), 4, 0);
if (emittedTypeCheck)
{
pslILEmit->EmitBR(pNullRefLabel);

pslILEmit->EmitLabel(isNotMatchingTypeLabel);
EmitLoadNativeValue(pslILEmit);
EmitLoadManagedValue(pslILEmit);
pslILEmit->EmitCALL(METHOD__MARSHAL__PTR_TO_STRUCTURE, 2, 0);
}
pslILEmit->EmitLabel(pNullRefLabel);
}

void ILLayoutClassPtrMarshaler::EmitClearNativeContents(ILCodeStream * pslILEmit)
{
STANDARD_VM_CONTRACT;

ILCodeLabel* isNotMatchingTypeLabel = pslILEmit->NewCodeLabel();
ILCodeLabel* cleanedUpLabel = pslILEmit->NewCodeLabel();
bool emittedTypeCheck = EmitExactTypeCheck(pslILEmit, isNotMatchingTypeLabel);

MethodDesc* pStructMarshalStub = NDirect::CreateStructMarshalILStub(m_pargs->m_pMT);

EmitLoadManagedValue(pslILEmit);
Expand All @@ -2406,19 +2327,6 @@ void ILLayoutClassPtrMarshaler::EmitClearNativeContents(ILCodeStream * pslILEmit
EmitLoadCleanupWorkList(pslILEmit);

pslILEmit->EmitCALL(pslILEmit->GetToken(pStructMarshalStub), 4, 0);

if (emittedTypeCheck)
{
pslILEmit->EmitBR(cleanedUpLabel);

pslILEmit->EmitLabel(isNotMatchingTypeLabel);
EmitLoadNativeValue(pslILEmit);
EmitLoadManagedValue(pslILEmit);
pslILEmit->EmitCALL(METHOD__OBJECT__GET_TYPE, 1, 1);
pslILEmit->EmitCALL(METHOD__MARSHAL__DESTROY_STRUCTURE, 2, 0);
}

pslILEmit->EmitLabel(cleanedUpLabel);
}


Expand All @@ -2433,9 +2341,6 @@ void ILBlittablePtrMarshaler::EmitConvertContentsCLRToNative(ILCodeStream* pslIL
EmitLoadNativeValue(pslILEmit);
pslILEmit->EmitBRFALSE(pNullRefLabel);

ILCodeLabel* isNotMatchingTypeLabel = pslILEmit->NewCodeLabel();
bool emittedTypeCheck = EmitExactTypeCheck(pslILEmit, isNotMatchingTypeLabel);

EmitLoadNativeValue(pslILEmit); // dest

EmitLoadManagedValue(pslILEmit);
Expand All @@ -2444,17 +2349,6 @@ void ILBlittablePtrMarshaler::EmitConvertContentsCLRToNative(ILCodeStream* pslIL
pslILEmit->EmitLDC(uNativeSize); // size

pslILEmit->EmitCPBLK();

if (emittedTypeCheck)
{
pslILEmit->EmitBR(pNullRefLabel);

pslILEmit->EmitLabel(isNotMatchingTypeLabel);
EmitLoadManagedValue(pslILEmit);
EmitLoadNativeValue(pslILEmit);
pslILEmit->EmitLDC(0);
pslILEmit->EmitCALL(METHOD__MARSHAL__STRUCTURE_TO_PTR, 3, 0);
}
pslILEmit->EmitLabel(pNullRefLabel);
}

Expand All @@ -2469,9 +2363,6 @@ void ILBlittablePtrMarshaler::EmitConvertContentsNativeToCLR(ILCodeStream* pslIL
EmitLoadManagedValue(pslILEmit);
pslILEmit->EmitBRFALSE(pNullRefLabel);

ILCodeLabel* isNotMatchingTypeLabel = pslILEmit->NewCodeLabel();
bool emittedTypeCheck = EmitExactTypeCheck(pslILEmit, isNotMatchingTypeLabel);

EmitLoadManagedValue(pslILEmit);
pslILEmit->EmitLDFLDA(fieldDef); // dest

Expand All @@ -2480,26 +2371,12 @@ void ILBlittablePtrMarshaler::EmitConvertContentsNativeToCLR(ILCodeStream* pslIL
pslILEmit->EmitLDC(uNativeSize); // size

pslILEmit->EmitCPBLK();

if (emittedTypeCheck)
{
pslILEmit->EmitBR(pNullRefLabel);

pslILEmit->EmitLabel(isNotMatchingTypeLabel);
EmitLoadNativeValue(pslILEmit);
EmitLoadManagedValue(pslILEmit);
pslILEmit->EmitCALL(METHOD__MARSHAL__PTR_TO_STRUCTURE, 2, 0);
}

pslILEmit->EmitLabel(pNullRefLabel);
}

bool ILBlittablePtrMarshaler::CanMarshalViaPinning()
{
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.
return IsCLRToNative(m_dwMarshalFlags) && !IsByref(m_dwMarshalFlags) && !IsFieldMarshal(m_dwMarshalFlags);
}

void ILBlittablePtrMarshaler::EmitMarshalViaPinning(ILCodeStream* pslILEmit)
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/src/vm/ilmarshalers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2915,7 +2915,6 @@ class ILLayoutClassPtrMarshalerBase : public ILMarshaler
bool NeedsClearNative() override;
void EmitClearNative(ILCodeStream* pslILEmit) override;
void EmitClearNativeTemp(ILCodeStream* pslILEmit) override;
bool EmitExactTypeCheck(ILCodeStream* pslILEmit, ILCodeLabel* isNotMatchingTypeLabel);
};

class ILLayoutClassPtrMarshaler : public ILLayoutClassPtrMarshalerBase
Expand Down
4 changes: 0 additions & 4 deletions src/coreclr/src/vm/metasig.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -604,10 +604,6 @@ DEFINE_METASIG_T(SM(Array_Int_Array_Int_Int_RetVoid, C(ARRAY) i C(ARRAY) i i, v)
DEFINE_METASIG_T(SM(Array_Int_Obj_RetVoid, C(ARRAY) i j, v))
DEFINE_METASIG_T(SM(Array_Int_PtrVoid_RetRefObj, C(ARRAY) i P(v), r(j)))

DEFINE_METASIG(SM(Obj_IntPtr_Bool_RetVoid, j I F, v))
DEFINE_METASIG(SM(IntPtr_Obj_RetVoid, I j, v))
DEFINE_METASIG_T(SM(IntPtr_Type_RetVoid, I C(TYPE), v))

// Undefine macros in case we include the file again in the compilation unit

#undef DEFINE_METASIG
Expand Down
15 changes: 3 additions & 12 deletions src/coreclr/src/vm/mlinfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1231,8 +1231,6 @@ MarshalInfo::MarshalInfo(Module* pModule,
m_pMT = NULL;
m_pMD = pMD;
m_onInstanceMethod = onInstanceMethod;
// [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;
Expand Down Expand Up @@ -2009,7 +2007,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())
{
Expand Down Expand Up @@ -2517,16 +2514,10 @@ MarshalInfo::MarshalInfo(Module* pModule,
}
}

if (!m_byref && byValAlwaysInOut)
// If neither IN nor OUT are true, this signals the URT to use the default
// rules.
if (!m_in && !m_out)
{
// 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 neither IN nor OUT are true, this signals the URT to use the default
// rules.
if (m_byref ||
(mtype == ELEMENT_TYPE_CLASS
&& !(sig.IsStringType(pModule, pTypeContext))
Expand Down
32 changes: 10 additions & 22 deletions src/tests/Interop/LayoutClass/LayoutClassNative.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,7 @@
#include <xplatform.h>

typedef void *voidPtr;

struct EmptyBase
{
};

struct DerivedSeqClass : public EmptyBase
{
int a;
};


struct SeqClass
{
int a;
Expand All @@ -23,7 +14,7 @@ struct SeqClass
};

struct ExpClass
{
{
int a;
int padding; //padding needs to be added here as we have added 8 byte offset.
union
Expand Down Expand Up @@ -56,35 +47,32 @@ DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleSeqLayoutClassByRef(SeqClass* p)
}

extern "C"
DLL_EXPORT BOOL STDMETHODCALLTYPE DerivedSeqLayoutClassByRef(EmptyBase* p, int expected)
DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleExpLayoutClassByRef(ExpClass* p)
{
if(((DerivedSeqClass*)p)->a != expected)
if((p->a != 0) || (p->udata.i != 10))
{
printf("FAIL: p->a=%d, expected %d\n", ((DerivedSeqClass*)p)->a, expected);
printf("FAIL: p->a=%d, p->udata.i=%d\n",p->a,p->udata.i);
return FALSE;
}
return TRUE;
}

extern "C"
DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleExpLayoutClassByRef(ExpClass* p)
DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleBlittableSeqLayoutClassByRef(BlittableClass* p)
{
if((p->a != 0) || (p->udata.i != 10))
if(p->a != 10)
{
printf("FAIL: p->a=%d, p->udata.i=%d\n",p->a,p->udata.i);
printf("FAIL: p->a=%d\n", p->a);
return FALSE;
}
return TRUE;
}

extern "C"
DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleBlittableSeqLayoutClass_UpdateField(BlittableClass* p)
DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleBlittableSeqLayoutClassByOutAttr(BlittableClass* p)
{
if(p->a != 10)
{
printf("FAIL: p->a=%d\n", p->a);
if(!SimpleBlittableSeqLayoutClassByRef(p))
return FALSE;
}

p->a++;
return TRUE;
Expand Down
Loading

0 comments on commit 1329252

Please sign in to comment.