Skip to content

Keep blittable layout class In/Out by default #50655

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

Merged
elinor-fung merged 2 commits into from
Apr 2, 2021
Merged

Keep blittable layout class In/Out by default #50655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
17eb82f
Keep blittable layout class In/Out by default
elinor-fung Apr 2, 2021
27dec87
By-val only
elinor-fung Apr 2, 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
24 changes: 12 additions & 12 deletions src/coreclr/vm/mlinfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1133,8 +1133,8 @@ 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 [Out].
BOOL outImpliesInOut = FALSE;
// [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 @@ -1879,7 +1879,7 @@ MarshalInfo::MarshalInfo(Module* pModule,
}
m_type = IsFieldScenario() ? MARSHAL_TYPE_BLITTABLE_LAYOUTCLASS : MARSHAL_TYPE_BLITTABLEPTR;
m_args.m_pMT = m_pMT;
outImpliesInOut = TRUE;
byValAlwaysInOut = TRUE;
}
else if (m_pMT->HasLayout())
{
Expand Down Expand Up @@ -2372,10 +2372,16 @@ MarshalInfo::MarshalInfo(Module* pModule,
}
}

// If neither IN nor OUT are true, this signals the URT to use the default
// rules.
if (!m_in && !m_out)
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 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 All @@ -2390,12 +2396,6 @@ MarshalInfo::MarshalInfo(Module* pModule,
m_out = FALSE;
}
}

// For marshalers that expect [Out] behavior to match [In, Out] behavior, update that here.
if (m_out && outImpliesInOut)
{
m_in = TRUE;
}
}

lReallyExit:
Expand Down
10 changes: 1 addition & 9 deletions src/tests/Interop/LayoutClass/LayoutClassNative.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,21 +78,13 @@ DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleExpLayoutClassByRef(ExpClass* p)
}

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

extern "C"
DLL_EXPORT BOOL STDMETHODCALLTYPE SimpleBlittableSeqLayoutClassByOutAttr(BlittableClass* p)
{
if(!SimpleBlittableSeqLayoutClassByRef(p))
return FALSE;

p->a++;
return TRUE;
Expand Down
68 changes: 50 additions & 18 deletions src/tests/Interop/LayoutClass/LayoutClassTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ public struct RecursiveTestStruct

class StructureTests
{
private const string SimpleBlittableSeqLayoutClass_UpdateField = nameof(SimpleBlittableSeqLayoutClass_UpdateField);

[DllImport("LayoutClassNative")]
private static extern bool SimpleSeqLayoutClassByRef(SeqClass p);

Expand All @@ -132,17 +134,23 @@ class StructureTests
[DllImport("LayoutClassNative")]
private static extern bool SimpleExpLayoutClassByRef(ExpClass p);

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

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

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

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

[DllImport("LayoutClassNative")]
private static extern bool SimpleBlittableSeqLayoutClassByOutAttr([Out] SealedBlittable p);
[DllImport("LayoutClassNative", EntryPoint = SimpleBlittableSeqLayoutClass_UpdateField)]
private static extern bool SealedBlittableSeqLayoutClassByInAttr([In] SealedBlittable p);

[DllImport("LayoutClassNative", EntryPoint = SimpleBlittableSeqLayoutClass_UpdateField)]
private static extern bool SealedBlittableSeqLayoutClassByOutAttr([Out] SealedBlittable p);

[DllImport("LayoutClassNative")]
private static extern bool SimpleNestedLayoutClassByValue(NestedLayout p);
Expand Down Expand Up @@ -176,42 +184,64 @@ public static void ExplicitClass()
Assert.IsTrue(SimpleExpLayoutClassByRef(p));
}

private static void ValidateBlittableClassInOut(Func<Blittable, bool> pinvoke)
{
int a = 10;
int expected = a + 1;
Blittable p = new Blittable(a);
Assert.IsTrue(pinvoke(p));
Assert.AreEqual(expected, p.a);
}

public static void BlittableClass()
{
// [Compat] Marshalled with [In, Out] behaviour by default
Console.WriteLine($"Running {nameof(BlittableClass)}...");
ValidateBlittableClassInOut(SimpleBlittableSeqLayoutClassByRef);
}

Blittable p = new Blittable(10);
Assert.IsTrue(SimpleBlittableSeqLayoutClassByRef(p));
public static void BlittableClassByInAttr()
{
// [Compat] Marshalled with [In, Out] behaviour even when only [In] is specified
Console.WriteLine($"Running {nameof(BlittableClassByInAttr)}...");
ValidateBlittableClassInOut(SimpleBlittableSeqLayoutClassByInAttr);
}

public static void BlittableClassByOutAttr()
{
// [Compat] Marshalled with [In, Out] behaviour even when only [Out] is specified
Console.WriteLine($"Running {nameof(BlittableClassByOutAttr)}...");
ValidateBlittableClassInOut(SimpleBlittableSeqLayoutClassByOutAttr);
}

private static void ValidateSealedBlittableClassInOut(Func<SealedBlittable, bool> pinvoke)
{
int a = 10;
int expected = a + 1;
Blittable p = new Blittable(a);
Assert.IsTrue(SimpleBlittableSeqLayoutClassByOutAttr(p));
SealedBlittable p = new SealedBlittable(a);
Assert.IsTrue(pinvoke(p));
Assert.AreEqual(expected, p.a);
}

public static void SealedBlittableClass()
{
// [Compat] Marshalled with [In, Out] behaviour by default
Console.WriteLine($"Running {nameof(SealedBlittableClass)}...");
ValidateSealedBlittableClassInOut(SealedBlittableSeqLayoutClassByRef);
}

SealedBlittable p = new SealedBlittable(10);
Assert.IsTrue(SimpleBlittableSeqLayoutClassByRef(p));
public static void SealedBlittableClassByInAttr()
{
// [Compat] Marshalled with [In, Out] behaviour even when only [In] is specified
Console.WriteLine($"Running {nameof(SealedBlittableClassByOutAttr)}...");
ValidateSealedBlittableClassInOut(SealedBlittableSeqLayoutClassByInAttr);
}

public static void SealedBlittableClassByOutAttr()
{
// [Compat] Marshalled with [In, Out] behaviour even when only [Out] is specified
Console.WriteLine($"Running {nameof(SealedBlittableClassByOutAttr)}...");

int a = 10;
int expected = a + 1;
SealedBlittable p = new SealedBlittable(a);
Assert.IsTrue(SimpleBlittableSeqLayoutClassByOutAttr(p));
Assert.AreEqual(expected, p.a);
ValidateSealedBlittableClassInOut(SealedBlittableSeqLayoutClassByOutAttr);
}

public static void NestedLayoutClass()
Expand Down Expand Up @@ -243,6 +273,8 @@ public static int Main(string[] argv)
ExplicitClass();
BlittableClass();
SealedBlittableClass();
BlittableClassByInAttr();
SealedBlittableClassByInAttr();
BlittableClassByOutAttr();
SealedBlittableClassByOutAttr();
NestedLayoutClass();
Expand Down