Skip to content

Commit 718c0ed

Browse files
jkoritzinskyelinor-fung
jkoritzinsky
and
elinor-fung
authored
[release/5.0] When marshalling a layout class, fall-back to dynamically marshalling the type if it doesn't match the static type in the signature. (#50138)
* When marshalling a layout class, fall-back to dynamically marshalling the type if it doesn't match the static type in the signature. * Allocate the correct amount of space for the native data based on the runtime type. * Apply suggestions from code review * Keep blittable layout class In/Out by default (#50655) * Do the exact type check after the null check. Co-authored-by: Elinor Fung <[email protected]>
1 parent 7eb058b commit 718c0ed

File tree

7 files changed

+276
-23
lines changed

7 files changed

+276
-23
lines changed

‎src/coreclr/src/vm/corelib.h

+5
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,11 @@ DEFINE_METHOD(MARSHAL, ALLOC_CO_TASK_MEM, AllocCoTa
488488
DEFINE_METHOD(MARSHAL, FREE_CO_TASK_MEM, FreeCoTaskMem, SM_IntPtr_RetVoid)
489489
DEFINE_FIELD(MARSHAL, SYSTEM_MAX_DBCS_CHAR_SIZE, SystemMaxDBCSCharSize)
490490

491+
DEFINE_METHOD(MARSHAL, STRUCTURE_TO_PTR, StructureToPtr, SM_Obj_IntPtr_Bool_RetVoid)
492+
DEFINE_METHOD(MARSHAL, PTR_TO_STRUCTURE, PtrToStructure, SM_IntPtr_Obj_RetVoid)
493+
DEFINE_METHOD(MARSHAL, DESTROY_STRUCTURE, DestroyStructure, SM_IntPtr_Type_RetVoid)
494+
DEFINE_METHOD(MARSHAL, SIZEOF_TYPE, SizeOf, SM_Type_RetInt)
495+
491496
DEFINE_CLASS(NATIVELIBRARY, Interop, NativeLibrary)
492497
DEFINE_METHOD(NATIVELIBRARY, LOADLIBRARYCALLBACKSTUB, LoadLibraryCallbackStub, SM_Str_AssemblyBase_Bool_UInt_RetIntPtr)
493498

‎src/coreclr/src/vm/ilmarshalers.cpp

+126-3
Original file line numberDiff line numberDiff line change
@@ -2149,14 +2149,30 @@ void ILLayoutClassPtrMarshalerBase::EmitConvertSpaceCLRToNative(ILCodeStream* ps
21492149

21502150
EmitLoadManagedValue(pslILEmit);
21512151
pslILEmit->EmitBRFALSE(pNullRefLabel);
2152+
ILCodeLabel* pTypeMismatchedLabel = pslILEmit->NewCodeLabel();
2153+
bool emittedTypeCheck = EmitExactTypeCheck(pslILEmit, pTypeMismatchedLabel);
2154+
DWORD sizeLocal = pslILEmit->NewLocal(LocalDesc(ELEMENT_TYPE_I4));
2155+
21522156
pslILEmit->EmitLDC(uNativeSize);
2157+
if (emittedTypeCheck)
2158+
{
2159+
ILCodeLabel* pHaveSizeLabel = pslILEmit->NewCodeLabel();
2160+
pslILEmit->EmitBR(pHaveSizeLabel);
2161+
pslILEmit->EmitLabel(pTypeMismatchedLabel);
2162+
EmitLoadManagedValue(pslILEmit);
2163+
pslILEmit->EmitCALL(METHOD__OBJECT__GET_TYPE, 1, 1);
2164+
pslILEmit->EmitCALL(METHOD__MARSHAL__SIZEOF_TYPE, 1, 1);
2165+
pslILEmit->EmitLabel(pHaveSizeLabel);
2166+
}
2167+
pslILEmit->EmitSTLOC(sizeLocal);
2168+
pslILEmit->EmitLDLOC(sizeLocal);
21532169
pslILEmit->EmitCALL(METHOD__MARSHAL__ALLOC_CO_TASK_MEM, 1, 1);
21542170
pslILEmit->EmitDUP(); // for INITBLK
21552171
EmitStoreNativeValue(pslILEmit);
21562172

21572173
// initialize local block we just allocated
21582174
pslILEmit->EmitLDC(0);
2159-
pslILEmit->EmitLDC(uNativeSize);
2175+
pslILEmit->EmitLDLOC(sizeLocal);
21602176
pslILEmit->EmitINITBLK();
21612177

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

21812197
EmitLoadManagedValue(pslILEmit);
21822198
pslILEmit->EmitBRFALSE(pNullRefLabel);
2199+
ILCodeLabel* pTypeMismatchedLabel = pslILEmit->NewCodeLabel();
2200+
bool emittedTypeCheck = EmitExactTypeCheck(pslILEmit, pTypeMismatchedLabel);
2201+
DWORD sizeLocal = pslILEmit->NewLocal(LocalDesc(ELEMENT_TYPE_I4));
21832202

21842203
pslILEmit->EmitLDC(uNativeSize);
2204+
if (emittedTypeCheck)
2205+
{
2206+
ILCodeLabel* pHaveSizeLabel = pslILEmit->NewCodeLabel();
2207+
pslILEmit->EmitBR(pHaveSizeLabel);
2208+
pslILEmit->EmitLabel(pTypeMismatchedLabel);
2209+
EmitLoadManagedValue(pslILEmit);
2210+
pslILEmit->EmitCALL(METHOD__OBJECT__GET_TYPE, 1, 1);
2211+
pslILEmit->EmitCALL(METHOD__MARSHAL__SIZEOF_TYPE, 1, 1);
2212+
pslILEmit->EmitLabel(pHaveSizeLabel);
2213+
}
2214+
pslILEmit->EmitSTLOC(sizeLocal);
2215+
pslILEmit->EmitLDLOC(sizeLocal);
21852216
pslILEmit->EmitLOCALLOC();
21862217
pslILEmit->EmitDUP(); // for INITBLK
21872218
EmitStoreNativeValue(pslILEmit);
21882219

21892220
// initialize local block we just allocated
21902221
pslILEmit->EmitLDC(0);
2191-
pslILEmit->EmitLDC(uNativeSize);
2222+
pslILEmit->EmitLDLOC(sizeLocal);
21922223
pslILEmit->EmitINITBLK();
21932224

21942225
pslILEmit->EmitLabel(pNullRefLabel);
@@ -2264,7 +2295,24 @@ void ILLayoutClassPtrMarshalerBase::EmitClearNativeTemp(ILCodeStream* pslILEmit)
22642295
}
22652296
}
22662297

2298+
bool ILLayoutClassPtrMarshalerBase::EmitExactTypeCheck(ILCodeStream* pslILEmit, ILCodeLabel* isNotMatchingTypeLabel)
2299+
{
2300+
STANDARD_VM_CONTRACT;
22672301

2302+
if (m_pargs->m_pMT->IsSealed())
2303+
{
2304+
// If the provided type cannot be derived from, then we don't need to emit the type check.
2305+
return false;
2306+
}
2307+
EmitLoadManagedValue(pslILEmit);
2308+
pslILEmit->EmitCALL(METHOD__OBJECT__GET_TYPE, 1, 1);
2309+
pslILEmit->EmitLDTOKEN(pslILEmit->GetToken(m_pargs->m_pMT));
2310+
pslILEmit->EmitCALL(METHOD__TYPE__GET_TYPE_FROM_HANDLE, 1, 1);
2311+
pslILEmit->EmitCALLVIRT(pslILEmit->GetToken(CoreLibBinder::GetMethod(METHOD__OBJECT__EQUALS)), 1, 1);
2312+
pslILEmit->EmitBRFALSE(isNotMatchingTypeLabel);
2313+
2314+
return true;
2315+
}
22682316

22692317
void ILLayoutClassPtrMarshaler::EmitConvertContentsCLRToNative(ILCodeStream* pslILEmit)
22702318
{
@@ -2281,6 +2329,9 @@ void ILLayoutClassPtrMarshaler::EmitConvertContentsCLRToNative(ILCodeStream* psl
22812329
pslILEmit->EmitLDC(uNativeSize);
22822330
pslILEmit->EmitINITBLK();
22832331

2332+
ILCodeLabel* isNotMatchingTypeLabel = pslILEmit->NewCodeLabel();
2333+
bool emittedTypeCheck = EmitExactTypeCheck(pslILEmit, isNotMatchingTypeLabel);
2334+
22842335
MethodDesc* pStructMarshalStub = NDirect::CreateStructMarshalILStub(m_pargs->m_pMT);
22852336

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

22922343
pslILEmit->EmitCALL(pslILEmit->GetToken(pStructMarshalStub), 4, 0);
2344+
2345+
if (emittedTypeCheck)
2346+
{
2347+
pslILEmit->EmitBR(pNullRefLabel);
2348+
2349+
pslILEmit->EmitLabel(isNotMatchingTypeLabel);
2350+
EmitLoadManagedValue(pslILEmit);
2351+
EmitLoadNativeValue(pslILEmit);
2352+
pslILEmit->EmitLDC(0);
2353+
pslILEmit->EmitCALL(METHOD__MARSHAL__STRUCTURE_TO_PTR, 3, 0);
2354+
}
2355+
22932356
pslILEmit->EmitLabel(pNullRefLabel);
22942357
}
22952358

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

2368+
ILCodeLabel* isNotMatchingTypeLabel = pslILEmit->NewCodeLabel();
2369+
bool emittedTypeCheck = EmitExactTypeCheck(pslILEmit, isNotMatchingTypeLabel);
2370+
23052371
MethodDesc* pStructMarshalStub = NDirect::CreateStructMarshalILStub(m_pargs->m_pMT);
23062372

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

23132379
pslILEmit->EmitCALL(pslILEmit->GetToken(pStructMarshalStub), 4, 0);
2380+
if (emittedTypeCheck)
2381+
{
2382+
pslILEmit->EmitBR(pNullRefLabel);
2383+
2384+
pslILEmit->EmitLabel(isNotMatchingTypeLabel);
2385+
EmitLoadNativeValue(pslILEmit);
2386+
EmitLoadManagedValue(pslILEmit);
2387+
pslILEmit->EmitCALL(METHOD__MARSHAL__PTR_TO_STRUCTURE, 2, 0);
2388+
}
23142389
pslILEmit->EmitLabel(pNullRefLabel);
23152390
}
23162391

23172392
void ILLayoutClassPtrMarshaler::EmitClearNativeContents(ILCodeStream * pslILEmit)
23182393
{
23192394
STANDARD_VM_CONTRACT;
23202395

2396+
ILCodeLabel* isNotMatchingTypeLabel = pslILEmit->NewCodeLabel();
2397+
ILCodeLabel* cleanedUpLabel = pslILEmit->NewCodeLabel();
2398+
bool emittedTypeCheck = EmitExactTypeCheck(pslILEmit, isNotMatchingTypeLabel);
2399+
23212400
MethodDesc* pStructMarshalStub = NDirect::CreateStructMarshalILStub(m_pargs->m_pMT);
23222401

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

23292408
pslILEmit->EmitCALL(pslILEmit->GetToken(pStructMarshalStub), 4, 0);
2409+
2410+
if (emittedTypeCheck)
2411+
{
2412+
pslILEmit->EmitBR(cleanedUpLabel);
2413+
2414+
pslILEmit->EmitLabel(isNotMatchingTypeLabel);
2415+
EmitLoadNativeValue(pslILEmit);
2416+
EmitLoadManagedValue(pslILEmit);
2417+
pslILEmit->EmitCALL(METHOD__OBJECT__GET_TYPE, 1, 1);
2418+
pslILEmit->EmitCALL(METHOD__MARSHAL__DESTROY_STRUCTURE, 2, 0);
2419+
}
2420+
2421+
pslILEmit->EmitLabel(cleanedUpLabel);
23302422
}
23312423

23322424

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

2436+
ILCodeLabel* isNotMatchingTypeLabel = pslILEmit->NewCodeLabel();
2437+
bool emittedTypeCheck = EmitExactTypeCheck(pslILEmit, isNotMatchingTypeLabel);
2438+
23442439
EmitLoadNativeValue(pslILEmit); // dest
23452440

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

23512446
pslILEmit->EmitCPBLK();
2447+
2448+
if (emittedTypeCheck)
2449+
{
2450+
pslILEmit->EmitBR(pNullRefLabel);
2451+
2452+
pslILEmit->EmitLabel(isNotMatchingTypeLabel);
2453+
EmitLoadManagedValue(pslILEmit);
2454+
EmitLoadNativeValue(pslILEmit);
2455+
pslILEmit->EmitLDC(0);
2456+
pslILEmit->EmitCALL(METHOD__MARSHAL__STRUCTURE_TO_PTR, 3, 0);
2457+
}
23522458
pslILEmit->EmitLabel(pNullRefLabel);
23532459
}
23542460

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

2472+
ILCodeLabel* isNotMatchingTypeLabel = pslILEmit->NewCodeLabel();
2473+
bool emittedTypeCheck = EmitExactTypeCheck(pslILEmit, isNotMatchingTypeLabel);
2474+
23662475
EmitLoadManagedValue(pslILEmit);
23672476
pslILEmit->EmitLDFLDA(fieldDef); // dest
23682477

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

23732482
pslILEmit->EmitCPBLK();
2483+
2484+
if (emittedTypeCheck)
2485+
{
2486+
pslILEmit->EmitBR(pNullRefLabel);
2487+
2488+
pslILEmit->EmitLabel(isNotMatchingTypeLabel);
2489+
EmitLoadNativeValue(pslILEmit);
2490+
EmitLoadManagedValue(pslILEmit);
2491+
pslILEmit->EmitCALL(METHOD__MARSHAL__PTR_TO_STRUCTURE, 2, 0);
2492+
}
2493+
23742494
pslILEmit->EmitLabel(pNullRefLabel);
23752495
}
23762496

23772497
bool ILBlittablePtrMarshaler::CanMarshalViaPinning()
23782498
{
2379-
return IsCLRToNative(m_dwMarshalFlags) && !IsByref(m_dwMarshalFlags) && !IsFieldMarshal(m_dwMarshalFlags);
2499+
return IsCLRToNative(m_dwMarshalFlags) &&
2500+
!IsByref(m_dwMarshalFlags) &&
2501+
!IsFieldMarshal(m_dwMarshalFlags) &&
2502+
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.
23802503
}
23812504

23822505
void ILBlittablePtrMarshaler::EmitMarshalViaPinning(ILCodeStream* pslILEmit)

‎src/coreclr/src/vm/ilmarshalers.h

+1
Original file line numberDiff line numberDiff line change
@@ -2915,6 +2915,7 @@ class ILLayoutClassPtrMarshalerBase : public ILMarshaler
29152915
bool NeedsClearNative() override;
29162916
void EmitClearNative(ILCodeStream* pslILEmit) override;
29172917
void EmitClearNativeTemp(ILCodeStream* pslILEmit) override;
2918+
bool EmitExactTypeCheck(ILCodeStream* pslILEmit, ILCodeLabel* isNotMatchingTypeLabel);
29182919
};
29192920

29202921
class ILLayoutClassPtrMarshaler : public ILLayoutClassPtrMarshalerBase

‎src/coreclr/src/vm/metasig.h

+4
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,10 @@ DEFINE_METASIG_T(SM(Array_Int_Array_Int_Int_RetVoid, C(ARRAY) i C(ARRAY) i i, v)
604604
DEFINE_METASIG_T(SM(Array_Int_Obj_RetVoid, C(ARRAY) i j, v))
605605
DEFINE_METASIG_T(SM(Array_Int_PtrVoid_RetRefObj, C(ARRAY) i P(v), r(j)))
606606

607+
DEFINE_METASIG(SM(Obj_IntPtr_Bool_RetVoid, j I F, v))
608+
DEFINE_METASIG(SM(IntPtr_Obj_RetVoid, I j, v))
609+
DEFINE_METASIG_T(SM(IntPtr_Type_RetVoid, I C(TYPE), v))
610+
607611
// Undefine macros in case we include the file again in the compilation unit
608612

609613
#undef DEFINE_METASIG

‎src/coreclr/src/vm/mlinfo.cpp

+12-3
Original file line numberDiff line numberDiff line change
@@ -1231,6 +1231,8 @@ MarshalInfo::MarshalInfo(Module* pModule,
12311231
m_pMT = NULL;
12321232
m_pMD = pMD;
12331233
m_onInstanceMethod = onInstanceMethod;
1234+
// [Compat] For backward compatibility reasons, some marshalers imply [In, Out] behavior when marked as [In], [Out], or not marked with either.
1235+
BOOL byValAlwaysInOut = FALSE;
12341236

12351237
#ifdef FEATURE_COMINTEROP
12361238
m_fDispItf = FALSE;
@@ -2007,6 +2009,7 @@ MarshalInfo::MarshalInfo(Module* pModule,
20072009
}
20082010
m_type = IsFieldScenario() ? MARSHAL_TYPE_BLITTABLE_LAYOUTCLASS : MARSHAL_TYPE_BLITTABLEPTR;
20092011
m_args.m_pMT = m_pMT;
2012+
byValAlwaysInOut = TRUE;
20102013
}
20112014
else if (m_pMT->HasLayout())
20122015
{
@@ -2514,10 +2517,16 @@ MarshalInfo::MarshalInfo(Module* pModule,
25142517
}
25152518
}
25162519

2517-
// If neither IN nor OUT are true, this signals the URT to use the default
2518-
// rules.
2519-
if (!m_in && !m_out)
2520+
if (!m_byref && byValAlwaysInOut)
25202521
{
2522+
// Some marshalers expect [In, Out] behavior with [In], [Out], or no directional attributes.
2523+
m_in = TRUE;
2524+
m_out = TRUE;
2525+
}
2526+
else if (!m_in && !m_out)
2527+
{
2528+
// If neither IN nor OUT are true, this signals the URT to use the default
2529+
// rules.
25212530
if (m_byref ||
25222531
(mtype == ELEMENT_TYPE_CLASS
25232532
&& !(sig.IsStringType(pModule, pTypeContext))

‎src/tests/Interop/LayoutClass/LayoutClassNative.cpp

+22-10
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,16 @@
55
#include <xplatform.h>
66

77
typedef void *voidPtr;
8-
8+
9+
struct EmptyBase
10+
{
11+
};
12+
13+
struct DerivedSeqClass : public EmptyBase
14+
{
15+
int a;
16+
};
17+
918
struct SeqClass
1019
{
1120
int a;
@@ -14,7 +23,7 @@ struct SeqClass
1423
};
1524

1625
struct ExpClass
17-
{
26+
{
1827
int a;
1928
int padding; //padding needs to be added here as we have added 8 byte offset.
2029
union
@@ -47,32 +56,35 @@ DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleSeqLayoutClassByRef(SeqClass* p)
4756
}
4857

4958
extern "C"
50-
DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleExpLayoutClassByRef(ExpClass* p)
59+
DLL_EXPORT BOOL STDMETHODCALLTYPE DerivedSeqLayoutClassByRef(EmptyBase* p, int expected)
5160
{
52-
if((p->a != 0) || (p->udata.i != 10))
61+
if(((DerivedSeqClass*)p)->a != expected)
5362
{
54-
printf("FAIL: p->a=%d, p->udata.i=%d\n",p->a,p->udata.i);
63+
printf("FAIL: p->a=%d, expected %d\n", ((DerivedSeqClass*)p)->a, expected);
5564
return FALSE;
5665
}
5766
return TRUE;
5867
}
5968

6069
extern "C"
61-
DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleBlittableSeqLayoutClassByRef(BlittableClass* p)
70+
DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleExpLayoutClassByRef(ExpClass* p)
6271
{
63-
if(p->a != 10)
72+
if((p->a != 0) || (p->udata.i != 10))
6473
{
65-
printf("FAIL: p->a=%d\n", p->a);
74+
printf("FAIL: p->a=%d, p->udata.i=%d\n",p->a,p->udata.i);
6675
return FALSE;
6776
}
6877
return TRUE;
6978
}
7079

7180
extern "C"
72-
DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleBlittableSeqLayoutClassByOutAttr(BlittableClass* p)
81+
DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleBlittableSeqLayoutClass_UpdateField(BlittableClass* p)
7382
{
74-
if(!SimpleBlittableSeqLayoutClassByRef(p))
83+
if(p->a != 10)
84+
{
85+
printf("FAIL: p->a=%d\n", p->a);
7586
return FALSE;
87+
}
7688

7789
p->a++;
7890
return TRUE;

0 commit comments

Comments
 (0)