Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move type check to after the null ref branch in out marshalling of blittable classes. #50735

Merged
merged 4 commits into from
Apr 7, 2021
Merged

Move type check to after the null ref branch in out marshalling of blittable classes. #50735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cdcc4b5
Move type check to after the null ref branch.
jkoritzinsky Apr 5, 2021
4934918
Add non-blittable null test.
jkoritzinsky Apr 5, 2021
bdfca2d
Remove extraneous paren.
jkoritzinsky Apr 5, 2021
3d20e06
Update src/tests/Interop/LayoutClass/LayoutClassTest.cs
jkoritzinsky Apr 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/coreclr/vm/ilmarshalers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
AaronRobinsonMSFT marked this conversation as resolved.
Show resolved Hide resolved

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

Expand Down
12 changes: 12 additions & 0 deletions src/tests/Interop/LayoutClass/LayoutClassNative.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
{
Expand Down
22 changes: 22 additions & 0 deletions src/tests/Interop/LayoutClass/LayoutClassTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved

[DllImport("LayoutClassNative", EntryPoint = SimpleBlittableSeqLayoutClass_UpdateField)]
private static extern bool SimpleBlittableSeqLayoutClassByRef(Blittable p);

Expand Down Expand Up @@ -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)}...");
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -269,9 +289,11 @@ public static int Main(string[] argv)
try
{
SequentialClass();
SequentialClassNull();
DerivedClassWithEmptyBase();
ExplicitClass();
BlittableClass();
BlittableClassNull();
SealedBlittableClass();
BlittableClassByInAttr();
SealedBlittableClassByInAttr();
Expand Down