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

Convert invalid C# uses of UnmanagedCallersOnly to IL. #42146

Merged
merged 3 commits into from
Sep 13, 2020
Merged

Convert invalid C# uses of UnmanagedCallersOnly to IL. #42146

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
3036986
Convert invalid C# uses of UnmanagedCallersOnly to IL for negative te…
AaronRobinsonMSFT Sep 11, 2020
23e6b2f
PR feedback
AaronRobinsonMSFT Sep 11, 2020
fa02c18
Nit issue.
AaronRobinsonMSFT Sep 12, 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: 8 additions & 0 deletions src/tests/Interop/UnmanagedCallersOnly/InvalidCSharp.ilproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk.IL">
<PropertyGroup>
<OutputType>library</OutputType>
</PropertyGroup>
<ItemGroup>
<Compile Include="InvalidCallbacks.il" />
</ItemGroup>
</Project>
75 changes: 75 additions & 0 deletions src/tests/Interop/UnmanagedCallersOnly/InvalidCallbacks.il
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

.assembly extern System.Runtime { }
.assembly extern System.Runtime.InteropServices { }

.assembly InvalidCSharp { }

.class public auto ansi beforefieldinit InvalidCSharp.GenericClass`1<T>
extends System.Object
{
.method public hidebysig static
void CallbackMethod (
int32 n
) cil managed preservesig
{
.custom instance void [System.Runtime.InteropServices]System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute::.ctor() = (
01 00 00 00
)
.maxstack 8
IL_0000: ldstr "Functions with attribute UnmanagedCallersOnlyAttribute within a generic type are invalid"
IL_0005: newobj instance void [System.Runtime]System.Exception::.ctor(string)
IL_000a: throw
}

.method public hidebysig specialname rtspecialname
instance void .ctor () cil managed
{
.maxstack 8
IL_0000: ldarg.0
IL_0001: call instance void [System.Runtime]System.Object::.ctor()
IL_0006: ret
}
}

.class public auto ansi beforefieldinit InvalidCSharp.Callbacks
extends [System.Runtime]System.Object
{
.method public hidebysig static
int32 CallbackMethodGeneric<T> (
!!T arg
) cil managed preservesig
{
.custom instance void [System.Runtime.InteropServices]System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute::.ctor() = (
01 00 00 00
)
.maxstack 8
IL_0000: ldstr "Functions with attribute UnmanagedCallersOnlyAttribute cannot have generic arguments"
IL_0005: newobj instance void [System.Runtime]System.Exception::.ctor(string)
IL_000a: throw
}

.method public hidebysig
instance int32 CallbackNonStatic (
int32 val
) cil managed preservesig
{
.custom instance void [System.Runtime.InteropServices]System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute::.ctor() = (
01 00 00 00
)
.maxstack 8
IL_0000: ldstr "Instance functions with attribute UnmanagedCallersOnlyAttribute are invalid"
IL_0005: newobj instance void [System.Runtime]System.Exception::.ctor(string)
IL_000a: throw
}

.method public hidebysig specialname rtspecialname
instance void .ctor () cil managed
{
.maxstack 8
IL_0000: ldarg.0
IL_0001: call instance void [System.Runtime]System.Object::.ctor()
IL_0006: ret
}
}
54 changes: 23 additions & 31 deletions src/tests/Interop/UnmanagedCallersOnly/UnmanagedCallersOnlyTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Reflection;
using System.Reflection.Emit;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Threading;
using TestLibrary;
Expand Down Expand Up @@ -35,6 +35,20 @@ public static class UnmanagedCallersOnlyDll
public static extern int PInvokeMarkedWithUnmanagedCallersOnly(int n);
}

private const string InvalidCSharpAssemblyName = "InvalidCSharp";

public static Type GetCallbacksType()
{
var asm = Assembly.Load(InvalidCSharpAssemblyName);
return asm.GetType("InvalidCSharp.Callbacks");
Copy link
Member

Choose a reason for hiding this comment

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

The ProjectReferences seems to be working fine for test projects. For example: https://github.com/dotnet/runtime/blob/master/src/tests/baseservices/callconvs/TestCallingConventions.csproj#L11

Project reference would be even better than Assembly.Load.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, ProjectReferences do work for build but not to refer to the assembly. The typical issue is the assembly that contains System.Object isn't the same when compiled with ilasm as the one for csc and during assembly reference an error occurs. The only way I have found to circumvent this is to load during runtime.

The reason the referenced test works is because of the following:

runtime/src/tests/baseservices/callconvs/CallFunctionPointers.il

Lines 4 to 8 in c867337

.assembly extern System.Runtime
{
.publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....:
.ver 5:0:0:0
}

This is fine for now, but when we version to .NET 6, this will break. The current approach is the only way I have found that continue to work through time.

Copy link
Member

@jkotas jkotas Sep 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I do not think this should be a problem. What is the csc error that you are seeing? I know that csc was picky about situations like these in the past, but these checks were removed a while ago. They did not make sense for .NET Core nor for .NET Framework with binding redirects.

It is fairly normal to have multiple different System.Runtime references in .NET Core world. For example, it is fully supported for project targeting net6 (that has System.Runtime 6.0) to reference project targeting net5 (that has System.Runtime 5.0.).

If you do not like hardcoding versions, you can just omit them. For example: https://github.com/jkotas/runtime/blob/5095f1f75f86c2aedaedf9e35457cb6635a3c989/src/tests/Regressions/coreclr/16355/boring.il#L4

Copy link
Member Author

Choose a reason for hiding this comment

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

I was hitting the error about System.Object from assembly is not known. However, when I try it now I don't see that... this is unsettling because I did try that first. I'm not a fan of not understanding why I hit that error for 20 minutes changed to the runtime load but now it doesn't repro.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, that works just fine throughout. Any idea how I could have made that error occur? That is frustrating beyond belief. I will push up a new PR with the IL updates as well.

Copy link
Member

Choose a reason for hiding this comment

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

error about System.Object from assembly is not known.

This error is typically caused by missing System.Runtime reference, not by having incompatible System.Runtime references.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here is what I am seeing now.

Failure during build of C# test application:

error CS0012: The type 'Object' is defined in an assembly that is not referenced. You must add a reference to assembly 'System.Runtime, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.

Call site in C#:

InvalidCSharp.Test.Invoke(null, n);

Snippet of IL code being called in C#:

.assembly extern System.Runtime { }
.assembly extern System.Runtime.InteropServices { }

.assembly InvalidCSharp { }

.class public auto ansi beforefieldinit InvalidCSharp.Test
    extends System.Object
{
    .method public hidebysig static 
        void Invoke (
            void* fptr,
            int32 n
        ) cil managed 
    {
        .maxstack 2
        .locals init ([0] native int ptr)

        nop
        ldarg.0
        stloc.0

        ldloc.0
        ldarg.1
        calli int32 *(int32)
        
        ret
    }
}

Copy link
Member

Choose a reason for hiding this comment

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

Maybe C# compiler needs the public key tokens to match? You can try adding .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ).

Copy link
Member

Choose a reason for hiding this comment

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

Does the C# compiler command line have /r:....\System.Runtime.dll ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe C# compiler needs the public key tokens to match? You can try adding .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ).

That did it. This is a very confusing issue. But it works now so yay.

Does the C# compiler command line have /r:....\System.Runtime.dll ?

It does.

 /reference:D:\runtime\artifacts\bin\ref\net5.0\System.Runtime.dll

}

public static Type GetGenericClassOfIntType()
{
var asm = Assembly.Load(InvalidCSharpAssemblyName);
return asm.GetType("InvalidCSharp.GenericClass`1").MakeGenericType(typeof(int));
}

private delegate int IntNativeMethodInvoker();
private delegate void NativeMethodInvoker();

Expand Down Expand Up @@ -338,12 +352,6 @@ void CallAsDelegate()
}
}

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

public static void NegativeTest_NonStaticMethod()
{
Expand All @@ -354,7 +362,7 @@ void TestUnmanagedCallersOnlyNonStatic()
{
.locals init ([0] native int ptr)
nop
ldftn int CallbackNonStatic(int)
ldftn int GetCallbacksType().CallbackNonStatic(int)
stloc.0
ldloc.0
Expand All @@ -371,7 +379,7 @@ ldftn int CallbackNonStatic(int)
il.Emit(OpCodes.Nop);
Copy link
Member

Choose a reason for hiding this comment

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

The dynamic IL generation can be moved to the IL project too. It would improve the test coverage for AOT.

It can be done as a separate PR.


// Get native function pointer of the callback
il.Emit(OpCodes.Ldftn, typeof(Program).GetMethod(nameof(CallbackNonStatic)));
il.Emit(OpCodes.Ldftn, GetCallbacksType().GetMethod("CallbackNonStatic"));
il.Emit(OpCodes.Stloc_0);
il.Emit(OpCodes.Ldloc_0);

Expand Down Expand Up @@ -436,13 +444,6 @@ ldftn int CallbackMethodNonBlittable(bool)
Assert.Throws<InvalidProgramException>(() => { testNativeMethod(); });
}

[UnmanagedCallersOnly]
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()
{
Console.WriteLine($"Running {nameof(NegativeTest_NonInstantiatedGenericArguments)}...");
Expand All @@ -452,7 +453,7 @@ void TestUnmanagedCallersOnlyNonInstGenericArguments()
{
.locals init ([0] native int ptr)
IL_0000: nop
IL_0001: ldftn void CallbackMethodGeneric(T)
IL_0001: ldftn void InvalidCSharp.Callbacks.CallbackMethodGeneric(T)
IL_0007: stloc.0
IL_0008: ret
}
Expand All @@ -463,7 +464,7 @@ .locals init ([0] native int ptr)
il.Emit(OpCodes.Nop);

// Get native function pointer of the callback
il.Emit(OpCodes.Ldftn, typeof(Program).GetMethod(nameof(CallbackMethodGeneric)));
il.Emit(OpCodes.Ldftn, GetCallbacksType().GetMethod("CallbackMethodGeneric"));
il.Emit(OpCodes.Stloc_0);

il.Emit(OpCodes.Ret);
Expand All @@ -482,7 +483,7 @@ void TestUnmanagedCallersOnlyInstGenericArguments()
{
.locals init ([0] native int ptr)
nop
ldftn void CallbackMethodGeneric(int)
ldftn void InvalidCSharp.Callbacks.CallbackMethodGeneric(int)
stloc.0
ldloc.0
Expand All @@ -499,7 +500,7 @@ ldftn void CallbackMethodGeneric(int)
il.Emit(OpCodes.Nop);

// 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.Ldftn, GetCallbacksType().GetMethod("CallbackMethodGeneric").MakeGenericMethod(new [] { typeof(int) }));
il.Emit(OpCodes.Stloc_0);
il.Emit(OpCodes.Ldloc_0);

Expand All @@ -515,15 +516,6 @@ ldftn void CallbackMethodGeneric(int)
Assert.Throws<InvalidProgramException>(() => { testNativeMethod(); });
}

public class GenericClass<T>
{
[UnmanagedCallersOnly]
public static void CallbackMethod(int n)
{
Assert.Fail($"Functions with attribute {nameof(UnmanagedCallersOnlyAttribute)} within a generic type are invalid");
}
}

public static void NegativeTest_FromInstantiatedGenericClass()
{
Console.WriteLine($"Running {nameof(NegativeTest_FromInstantiatedGenericClass)}...");
Expand All @@ -533,7 +525,7 @@ void TestUnmanagedCallersOnlyInstGenericType()
{
.locals init ([0] native int ptr)
nop
ldftn int GenericClass<int>::CallbackMethod(int)
ldftn int InvalidCSharp.GenericClass<int>::CallbackMethod(int)
stloc.0
ldloc.0
Expand All @@ -550,7 +542,7 @@ .locals init ([0] native int ptr)
il.Emit(OpCodes.Nop);

// 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.Ldftn, GetGenericClassOfIntType().GetMethod("CallbackMethod"));
il.Emit(OpCodes.Stloc_0);
il.Emit(OpCodes.Ldloc_0);

Expand Down
2 changes: 1 addition & 1 deletion src/tests/Interop/UnmanagedCallersOnly/UnmanagedCallersOnlyTest.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<CLRTestPriority>1</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="UnmanagedCallersOnlyTest.cs" />
Expand All @@ -10,5 +9,6 @@
<!-- This is needed to make sure native binary gets installed in the right location -->
<ProjectReference Include="CMakeLists.txt" />
<ProjectReference Include="$(TestSourceDir)Common/CoreCLRTestLibrary/CoreCLRTestLibrary.csproj" />
<ProjectReference Include="InvalidCSharp.ilproj" />
</ItemGroup>
</Project>