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

Reenable unmanagedcallersonly related tests #35926

Merged
merged 3 commits into from
May 7, 2020
Merged

Reenable unmanagedcallersonly related tests #35926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
02a22b6
Reenable UnmanagedCallersOnlyAttribute tests
AaronRobinsonMSFT May 6, 2020
cc78abe
Reenable Vector2_3_4 tests.
AaronRobinsonMSFT May 6, 2020
2e1be1d
Reenable MarshalSeqStruct/ReversePInvoke tests.
AaronRobinsonMSFT May 6, 2020
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
8 changes: 7 additions & 1 deletion src/coreclr/src/vm/jitinterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12425,7 +12425,13 @@ CorJitResult CallCompileMethodWithSEHWrapper(EEJitManager *jitMgr,
#if !defined(TARGET_X86)
if (ftn->HasUnmanagedCallersOnlyAttribute())
{
COMDelegate::ThrowIfInvalidUnmanagedCallersOnlyUsage(ftn);
// If the stub was generated by the runtime, don't validate
// it for UnmanagedCallersOnlyAttribute usage. There are cases
// where the validation doesn't handle all of the cases we can
// permit during stub generation (e.g. Vector2 returns).
if (!ftn->IsILStub())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate why this is a problem today? I'm not quite connecting how or why we have special handling for Vector2/3/4?

Copy link
Member Author

@AaronRobinsonMSFT AaronRobinsonMSFT May 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was always a problem given our existing checking mechanism. We recently moved this check to catch all cases instead of just cases that result from a query by the JIT to getCallInfo(). Now we don't miss any and this deficiency in our validation was discovered. The Vector* types were just the thing that popped,

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MarshalingRequired rejects non-primitive value types here:

runtime/src/coreclr/src/vm/dllimport.cpp

Line 3506 in ddd458e

// return value is fine as long as it can be normalized to an integer

The problem is that the JIT did not always get the return buffers right for non-primitive value types and so the interop IL stub generation compensated for it via a very complex hack.

This should go away once #12375 is fixed.

Copy link
Member

@jkotas jkotas May 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have opened #35928 so that we make the unmanaged function pointers returning non-primitive value types work for .NET 5 at least.

COMDelegate::ThrowIfInvalidUnmanagedCallersOnlyUsage(ftn);

flags.Set(CORJIT_FLAGS::CORJIT_FLAG_REVERSE_PINVOKE);
}
#endif // !TARGET_X86
Expand Down
9 changes: 0 additions & 9 deletions src/coreclr/tests/issues.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,6 @@
<ExcludeList Include="$(XunitTestBinBase)/JIT/SIMD/Vector3Interop_ro/*">
<Issue>https://github.com/dotnet/runtime/issues/35798</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/Interop/StructMarshalling/ReversePInvoke/MarshalSeqStruct/ReversePInvoke/ReversePInvokeTest/*">
<Issue>https://github.com/dotnet/runtime/issues/35798</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/Interop/PInvoke/Vector2_3_4/Vector2_3_4/*">
<Issue>https://github.com/dotnet/runtime/issues/35798</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/Interop/UnmanagedCallersOnly/UnmanagedCallersOnlyTest/*">
<Issue>https://github.com/dotnet/runtime/issues/35798</Issue>
</ExcludeList>
</ItemGroup>

<!-- All OS/Arch CoreCLR excludes -->
Expand Down
7 changes: 5 additions & 2 deletions src/coreclr/tests/src/Interop/PInvoke/Vector2_3_4/Vector2_3_4.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@ public static int Main()
}
catch (System.Exception ex)
{
Console.WriteLine(ex.ToString());
Console.WriteLine(ex);
return 101;
}
return 100;
}

private static void RunVector2Tests()
{
Console.WriteLine($"Running {nameof(RunVector2Tests)}... ");
float X = StartingIntValue;
float Y = StartingIntValue + 1;
float Z = StartingIntValue + 232;
Expand Down Expand Up @@ -68,9 +69,10 @@ private static void RunVector2Tests()
return newVector;
}));
}

private static void RunVector3Tests()
{
Console.WriteLine($"Running {nameof(RunVector3Tests)}... ");
float X = StartingIntValue;
float Y = StartingIntValue + 1;
float Z = StartingIntValue + 232;
Expand Down Expand Up @@ -112,6 +114,7 @@ private static void RunVector3Tests()

private static void RunVector4Tests()
{
Console.WriteLine($"Running {nameof(RunVector4Tests)}... ");
float X = StartingIntValue;
float Y = StartingIntValue + 1;
float Z = StartingIntValue + 232;
Expand Down
63 changes: 53 additions & 10 deletions src/coreclr/tests/src/Interop/UnmanagedCallersOnly/UnmanagedCallersOnlyTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,10 @@ void CallAsDelegate()
}

[UnmanagedCallersOnly]
public void CallbackNonStatic()
public int CallbackNonStatic(int val)
{
Assert.Fail($"Instance functions with attribute {nameof(UnmanagedCallersOnlyAttribute)} are invalid");
return -1;
}

public static void NegativeTest_NonStaticMethod()
Expand All @@ -284,9 +285,14 @@ void TestUnmanagedCallersOnlyNonStatic()
{
.locals init ([0] native int ptr)
IL_0000: nop
IL_0001: ldftn void CallbackNonStatic()
IL_0001: ldftn int CallbackNonStatic(int)
IL_0007: stloc.0
IL_0008: ret

IL_0008: ldloc.0
IL_0009: ldc.i4 <n> local
IL_000e: call bool UnmanagedCallersOnlyDll::CallManagedProc(native int, int)

IL_0013: ret
}
*/
DynamicMethod testUnmanagedCallersOnly = new DynamicMethod("TestUnmanagedCallersOnlyNonStatic", null, null, typeof(Program).Module);
Expand All @@ -297,18 +303,24 @@ .locals init ([0] native int ptr)
// Get native function pointer of the callback
il.Emit(OpCodes.Ldftn, typeof(Program).GetMethod(nameof(CallbackNonStatic)));
il.Emit(OpCodes.Stloc_0);
il.Emit(OpCodes.Ldloc_0);

int n = 12345;
il.Emit(OpCodes.Ldc_I4, n);
il.Emit(OpCodes.Call, typeof(UnmanagedCallersOnlyDll).GetMethod("CallManagedProc"));
il.Emit(OpCodes.Ret);

var testNativeMethod = (NativeMethodInvoker)testUnmanagedCallersOnly.CreateDelegate(typeof(NativeMethodInvoker));

// Try invoking method
Assert.Throws<InvalidProgramException>(() => { testNativeMethod(); });
}

[UnmanagedCallersOnly]
public static void CallbackMethodNonBlittable(bool x1)
public static int CallbackMethodNonBlittable(bool x1)
{
Assert.Fail($"Functions with attribute {nameof(UnmanagedCallersOnlyAttribute)} cannot have non-blittable arguments");
return -1;
}

public static void NegativeTest_NonBlittable()
Expand All @@ -320,9 +332,14 @@ void TestUnmanagedCallersOnlyNonBlittable()
{
.locals init ([0] native int ptr)
IL_0000: nop
IL_0001: ldftn void CallbackMethodNonBlittable(bool)
IL_0001: ldftn int CallbackMethodNonBlittable(bool)
IL_0007: stloc.0
IL_0008: ret

IL_0008: ldloc.0
IL_0009: ldc.i4 <n> local
IL_000e: call bool UnmanagedCallersOnlyDll::CallManagedProc(native int, int)

IL_0013: ret
}
*/
DynamicMethod testUnmanagedCallersOnly = new DynamicMethod("TestUnmanagedCallersOnlyNonBlittable", null, null, typeof(Program).Module);
Expand All @@ -333,18 +350,24 @@ .locals init ([0] native int ptr)
// Get native function pointer of the callback
il.Emit(OpCodes.Ldftn, typeof(Program).GetMethod(nameof(CallbackMethodNonBlittable)));
il.Emit(OpCodes.Stloc_0);
il.Emit(OpCodes.Ldloc_0);

int n = 12345;
il.Emit(OpCodes.Ldc_I4, n);
il.Emit(OpCodes.Call, typeof(UnmanagedCallersOnlyDll).GetMethod("CallManagedProc"));
il.Emit(OpCodes.Ret);

var testNativeMethod = (NativeMethodInvoker)testUnmanagedCallersOnly.CreateDelegate(typeof(NativeMethodInvoker));

// Try invoking method
Assert.Throws<InvalidProgramException>(() => { testNativeMethod(); });
}

[UnmanagedCallersOnly]
public static void CallbackMethodGeneric<T>(T arg)
public static int CallbackMethodGeneric<T>(T arg)
{
Assert.Fail($"Functions with attribute {nameof(UnmanagedCallersOnlyAttribute)} cannot have generic arguments");
return -1;
}

public static void NegativeTest_NonInstantiatedGenericArguments()
Expand Down Expand Up @@ -388,7 +411,12 @@ .locals init ([0] native int ptr)
IL_0000: nop
IL_0001: ldftn void CallbackMethodGeneric(int)
IL_0007: stloc.0
IL_0008: ret

IL_0008: ldloc.0
IL_0009: ldc.i4 <n> local
IL_000e: call bool UnmanagedCallersOnlyDll::CallManagedProc(native int, int)

IL_0013: ret
}
*/
DynamicMethod testUnmanagedCallersOnly = new DynamicMethod("TestUnmanagedCallersOnlyInstGenericArguments", null, null, typeof(Program).Module);
Expand All @@ -399,8 +427,13 @@ .locals init ([0] native int ptr)
// Get native function pointer of the instantiated generic callback
il.Emit(OpCodes.Ldftn, typeof(Program).GetMethod(nameof(CallbackMethodGeneric)).MakeGenericMethod(new [] { typeof(int) }));
il.Emit(OpCodes.Stloc_0);
il.Emit(OpCodes.Ldloc_0);

int n = 12345;
il.Emit(OpCodes.Ldc_I4, n);
il.Emit(OpCodes.Call, typeof(UnmanagedCallersOnlyDll).GetMethod("CallManagedProc"));
il.Emit(OpCodes.Ret);

var testNativeMethod = (NativeMethodInvoker)testUnmanagedCallersOnly.CreateDelegate(typeof(NativeMethodInvoker));

// Try invoking method
Expand All @@ -425,9 +458,14 @@ void TestUnmanagedCallersOnlyInstGenericType()
{
.locals init ([0] native int ptr)
IL_0000: nop
IL_0001: ldftn void GenericClass<int>::CallbackMethod(int)
IL_0001: ldftn int GenericClass<int>::CallbackMethod(int)
IL_0007: stloc.0
IL_0008: ret

IL_0008: ldloc.0
IL_0009: ldc.i4 <n> local
IL_000e: call bool UnmanagedCallersOnlyDll::CallManagedProc(native int, int)

IL_0013: ret
}
*/
DynamicMethod testUnmanagedCallersOnly = new DynamicMethod("TestUnmanagedCallersOnlyInstGenericClass", null, null, typeof(Program).Module);
Expand All @@ -438,8 +476,13 @@ .locals init ([0] native int ptr)
// Get native function pointer of the callback from the instantiated generic class.
il.Emit(OpCodes.Ldftn, typeof(GenericClass<int>).GetMethod(nameof(GenericClass<int>.CallbackMethod)));
il.Emit(OpCodes.Stloc_0);
il.Emit(OpCodes.Ldloc_0);

int n = 12345;
il.Emit(OpCodes.Ldc_I4, n);
il.Emit(OpCodes.Call, typeof(UnmanagedCallersOnlyDll).GetMethod("CallManagedProc"));
il.Emit(OpCodes.Ret);

var testNativeMethod = (NativeMethodInvoker)testUnmanagedCallersOnly.CreateDelegate(typeof(NativeMethodInvoker));

// Try invoking method
Expand Down