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.

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

Extensible Calling Conventions for native callee functions #51156

Closed
AaronRobinsonMSFT opened this issue Apr 13, 2021 · 15 comments · Fixed by #52869
Closed

Extensible Calling Conventions for native callee functions #51156

AaronRobinsonMSFT opened this issue Apr 13, 2021 · 15 comments · Fixed by #52869
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime.InteropServices
Milestone

Comments

@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Apr 13, 2021

Background and Motivation

The UnmanagedCallersOnlyAttribute provides an extensible mechanism to define native calling conventions on a .NET method. This enables users to author functions that are callable in a native environment. A symmetrical need arises for functions that are defined in native code but declared to be called in a .NET environment. These native function declarations are done using DllImportAttribute.

This approach complements UnmanagedCallersOnlyAttribute and aligns with the reuse of the CallConv* types under the System.Runtime.CompilerServices namespace.

Proposed API

If the user specifies the CallingConvention.Winapi (the default) on the DllImportAttribute the following attribute will be queried.

namespace System.Runtime.InteropServices
{
+    /// <summary>
+    /// Provides an equivalent to <see cref="UnmanagedCallersOnlyAttribute"/> for native
+    /// functions declared in .NET.
+    /// </summary>
+    [AttributeUsage(AttributeTargets.Method)]
+    public class UnmanagedCalleeAttribute : Attribute
+    {
+        public UnmanagedCalleeAttribute() { }
+
+        /// <summary>
+        /// Types indicating calling conventions for the unmanaged callee.
+        /// </summary>
+        /// <remarks>
+        /// If <c>null</c>, the semantics are identical to <c>CallingConvention.Winapi</c>.
+        /// </remarks>
+        public Type[]? CallConvs;
+    }
}

Usage Examples

P/Invoke

Using the defaults, all 3 of the following declarations are the same.

[DllImport("nativelib")]
static extern int Func(IntPtr a, nint val);

[DllImport("nativelib", CallingConvention = CallingConvention.Winapi)]
static extern int Func(IntPtr a, nint val);

[UnmanagedCallee]
[DllImport("nativelib")]
static extern int Func(IntPtr a, nint val);

Some calling conventions can be declared in multiple ways.

[DllImport("nativelib", CallingConvention = CallingConvention.Cdecl)]
static extern int Func(IntPtr a, nint val);

[UnmanagedCallee(CallConvs = new[] { typeof(CallConvCdecl) })]
[DllImport("nativelib")]
static extern int Func(IntPtr a, nint val);

[UnmanagedCallee(CallConvs = new[] { typeof(CallConvCdecl) })]
[DllImport("nativelib", CallingConvention = CallingConvention.Winapi)]
static extern int Func(IntPtr a, nint val);

Future calling conventions would be enabled without updating the CallingConvention enum. Examples in C# and VB.NET.

[UnmanagedCallee(CallConvs = new[] { typeof(CallConvVectorCall) })]
[DllImport("nativelib")]
static extern int Func(IntPtr a, nint val);
<UnmanagedCallee(CallConvs := New Type() { GetType(CallConvVectorCall) })>
<DllImport("nativelib")>
Public Shared Function Func(a As IntPtr, val As IntPtr) As Integer
End Function

This proposal provides an alternative mechanism to support Native Varargs in #48796.

COM style interface - Not proposed but example of how the attribute could evolve.

To enable this scenario the attribute would be updated with the following: [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)]

Using the defaults, all 3 of the following declarations are the same on Windows.

[ComVisible(true)]
[Guid("C509A1C1-A6C9-4229-89CE-10B53CF8271B")]
interface IFoo
{
    int Call(ref SomeType valuetype);
}

[ComVisible(true)]
[Guid("C509A1C1-A6C9-4229-89CE-10B53CF8271B")]
[UnmanagedCallee]
interface IFoo
{
    int Call(ref SomeType valuetype);
}

[ComVisible(true)]
[Guid("C509A1C1-A6C9-4229-89CE-10B53CF8271B")]
[UnmanagedCallee(CallConvs = new[] {  typeof(CallConvStdCall), typeof(CallConvMemberInstance) })]
interface IFoo
{
    int Call(ref SomeType valuetype);
}

Alternative Designs

Provide an enum to trigger off for reading the new attribute.

namespace System.Runtime.InteropServices
{
    public enum CallingConvention
    {
+        /// <summary>
+        /// Instructs the runtime to consult the <see cref="UnmanagedCalleeAttribute"/>.
+        /// </summary>
+        UnmanagedCallee = 7
    }
}

Alternative name proposed could be CallingConvention.UnmanagedCallConv.

@AaronRobinsonMSFT AaronRobinsonMSFT added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime.InteropServices labels Apr 13, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Apr 13, 2021
@AaronRobinsonMSFT
Copy link
Member Author

@tannergooding
Copy link
Member

It took me a second to put this together😄
So this is just how we plan on evolving calling conventions for DllImport and similar scenarios?

If I understood right, for future calling conventions, we basically annotate as CallingConvention.UnmanagedCallee and that will cause the runtime to resolve the UnmanagedCallee attribute (looking on method first, then directly containing type). The CallConv* types specified determine what the correct convention is?

What is the behavior if the attribute doesn't exist or contains invalid types?
How far up the type hierarchy will the runtime look?
Have any alternative names to UnmanagedCallee been suggested (such as just UnmanagedCallConv)?

@AaronRobinsonMSFT
Copy link
Member Author

AaronRobinsonMSFT commented Apr 13, 2021

So this is just how we plan on evolving calling conventions for DllImport and similar scenarios?

Yep. This is handling the case where someone wants to consume a native function. We have it already for C# function pointers so we have NaitveLibrary users covered. The only other existing ways are traditional COM interop and P/Invoke.

The CallConv* types specified determine what the correct convention is?

Yes. Now we can enable all existing interop mechanisms to consume the CallConv* approach.

What is the behavior if the attribute doesn't exist or contains invalid types?

My preference would be a hard fail. The attribute is the contract and the CallingConvention is stating - look here.

How far up the type hierarchy will the runtime look?

Great question. I would say on the closest enclosing type. From the runtime data structure view it would be what is on the associated MethodTable. More than willing to drop the class support but I think it might be useful longer term with source generators involving C++ interop. COM interop was simply to use an existing concrete example.

Have any alternative names to UnmanagedCallee been suggested (such as just UnmanagedCallConv)?

You're the first. Will add.

@jkotas
Copy link
Member

jkotas commented Apr 13, 2021

UnmanagedCallee = 7

A alternative design would be allow this attribute with the default calling convention, ie:

[UnmanagedCallee(CallConvs = new[] { typeof(CallConvVectorCall) })]
[DllImport("nativelib")]
static extern int Func(IntPtr a, nint val);

It looks better, but it is more expensive at runtime because we have to lookup an extra attribute most of the time. I am not sure which one I like better.

FWIW, we have introduced an extra attribute lookup for SuppressGCTransitionAttribute recently and it has not showed up on the radar, so adding one more would be probably fine.

More than willing to drop the class support but I think it might be useful longer term with source generators involving C++ interop

I think we should keep this simple and only support this on DllImport methods. We can always extend it later if needed. None of the existing CallConv constructs (function pointers, UnmanagedCallersOnlyAttribute) have a type default like this.

@AaronRobinsonMSFT
Copy link
Member Author

A alternative design would be allow this attribute with the default calling convention

Agreed. I had considered this approach because of the potential tooling impact of adding a new enum value. Would you then get rid of the new enum entirely?

we have introduced an extra attribute lookup for SuppressGCTransitionAttribute recently and it has not showed up on the radar, so adding one more would be probably fine.

That is true, but this one will likely have data content as opposed to SuppressGCTransitionAttribute which acts as an indicator with no data content. Either way, I think it warrants a quick perf investigation to see exactly what we could expect.

I think we should keep this simple and only support this on DllImport methods. We can always extend it later if needed.

That is reasonable.

@jkotas
Copy link
Member

jkotas commented Apr 13, 2021

Would you then get rid of the new enum entirely?

Yep.

@AaronRobinsonMSFT
Copy link
Member Author

AaronRobinsonMSFT commented Apr 13, 2021

Another aspect of the default proposal is that regardless of the user setting CallingConvention = CallingConvention.Winapi or not, we would need to check for the attribute since DllImport is a pseudo attribute and embedded in metadata - I don't think there is a way to disambiguate between explicitly setting CallingConvention.Winapi or the user not setting anything.

I'm a little leery of that approach because I would prefer this sort of thing to be explicit and not a magic contract. However, a rebuttal to that is it follows our desire to make the default declaration the canonical approach and move away from mechanisms that expose .NET's legacy x86 focus – see C#'s function pointer design.

I am also moving this to .NET 6 since I would like to fold support here into the DllImport source generator and be able to avoid the CallingConvention on any new attribute.

@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 6.0.0 milestone Apr 13, 2021
@AaronRobinsonMSFT AaronRobinsonMSFT removed the untriaged New issue has not been triaged by the area owner label Apr 13, 2021
@jkoritzinsky
Copy link
Member

In IL there's a way to disambiguate between Winapi and no setting, but Roslyn makes the default Winapi, so we'd have to check for the attribute whenever Winapi is set.

@AaronRobinsonMSFT
Copy link
Member Author

AaronRobinsonMSFT commented Apr 14, 2021

In IL there's a way to disambiguate between Winapi and no setting, but Roslyn makes the default Winapi, so we'd have to check for the attribute whenever Winapi is set.

No, I don't think there is - #51156 (comment).

Really? How do we do that in IL?

@jkoritzinsky
Copy link
Member

jkoritzinsky commented Apr 14, 2021

In IL, in the pinvokeimpl() declaration, you can just not specify any calling convention flag. That'll set none of the calling convention flags. Since the winapi flag is non-zero, it needs to be set explicitly to show up.

@AaronRobinsonMSFT
Copy link
Member Author

In IL, in the pinvokeimpl() declaration, you can just not specify any calling convention flag.

Gotcha. I completely misunderstood your statement but see what you are saying. You mean actually writing in IL it can be elided and an IL parser recognizes that - sure. The issue here is from a metadata perspective - which is where we are in the runtime - there is no difference so we can't make that determination, unless there is something I am missing.

@jkoritzinsky
Copy link
Member

When we're in the runtime, we can distinguish between someone specifying winapi or not if they use IL, but not if they use C# (since Roslyn sets the flag even when no calling convention is specified).

Since we can't disambiguate when developers are using C#, we still need to check for the attribute when they use Winapi.

@AaronRobinsonMSFT
Copy link
Member Author

When we're in the runtime, we can distinguish between someone specifying winapi or not if they use IL, but not if they use C# (since Roslyn sets the flag even when no calling convention is specified).

Sigh. Apologies. I genuinely thought Winapi was 0 in this case. Confused between all our callconv flags in the runtime. Understand your point now.

@AaronRobinsonMSFT AaronRobinsonMSFT added blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Apr 14, 2021
@bartonjs
Copy link
Member

bartonjs commented Apr 15, 2021

Video

  • Should it also work on Delegates? (We feel no is OK)
  • Sealed the attribute, since its for tooling
  • AllowMultiple should be false
  • We like CallConv more than Callee in this context
  • The CallConvs member should be a property, but since UnmanagedCallersOnly has a field we went for symmetry.
  • An analyzer to notice that DllImport had a convention specified and UnmanagedCallConv was also specified would be valuable.
namespace System.Runtime.InteropServices
{
     /// <summary>
     /// Provides an equivalent to <see cref="UnmanagedCallersOnlyAttribute"/> for native
     /// functions declared in .NET.
     /// </summary>
     [AttributeUsage(AttributeTargets.Method, AllowMultiple = false)]
     public sealed class UnmanagedCallConvAttribute : Attribute
     {
         public UnmanagedCallConvAttribute() { }
 
         /// <summary>
         /// Types indicating calling conventions for the unmanaged target.
         /// </summary>
         /// <remarks>
         /// If <c>null</c>, the semantics are identical to <c>CallingConvention.Winapi</c>.
         /// </remarks>
         public Type[]? CallConvs;
     }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Apr 15, 2021
@stephentoub stephentoub removed the blocking Marks issues that we want to fast track in order to unblock other important work label Apr 15, 2021
@AaronRobinsonMSFT AaronRobinsonMSFT self-assigned this Apr 17, 2021
@AaronRobinsonMSFT AaronRobinsonMSFT removed their assignment Apr 29, 2021
@AaronRobinsonMSFT
Copy link
Member Author

Addressed with #52869

@ghost ghost locked as resolved and limited conversation to collaborators Jul 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime.InteropServices
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants