-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Introduce mechanism to indicate arguments are to be marshalled as native varargs #48796
Comments
The _arglist keyword is supported for interop with P/Invoke on Windows platforms and the JIT already knows how to understand the encoding. I think we should just use the _arglist keyword instead of adding a new attribute. The attribute read is much more expensive. |
Maybe the trick is |
Not sure the attribute read should be a concern here to be honest. The generation of the stub is likely to be more costly than that. Also that read would be a single time. Also, this is not about supporting varargs as it looks on Windows. This is about supporting the calling of varargs with a fully typed P/Invoke. The keyword would have no where to go and is not a placeholder we are likely to want. |
What is the difficulty here? Is it not largely just the same as it does today, which is to push the args onto the execution stack and so (other than it being not an official keyword) it would largely just be the runtime handling it correctly for other platforms? |
This can be fixed by combining this with adding a new member to
Alternatively, this can be just a new
The We should also think how we would enable this with function pointers.
Note that varargs interop on Windows supports both forward and reverse interop, and both |
ABIs vary across platforms. Some platforms can treat vararg arguments differently than normal arguments. On Apple Silicon, they are definitely treated differently. So for interop we need to be explicit to support al platforms correctly. |
I was avoiding that since it would be encoded directly in metadata through |
Sure, but I don't see why A method in native is written as So I'm not seeing what is blocking the same from working on say Unix or Apple Silicon, other than the JIT not correctly handling these on non-windows. I would expect that if we just simply implemented the ABI defined (https://gitlab.com/x86-psABIs/x86-64-ABI for System V) then both forward and reverse P/Invoke would work (and that this is necessary anyways for whatever new concept is exposed) |
I don't think there is any reason other than this proposal is for a non-language impacting update :-) We can also start a conversation with Roslyn about how one would express this in C#. |
Yes, it has an impact on tooling. My actual concern is that we need a scalable pattern to add new calling conventions for DllImport. Let's say that we add 10 new calling conventions. What is the pattern we want to follow? Is it going to be 10 different attributes? |
|
Great question. I've not spent much time considering that as it relates to
Sure, seems like a reasonable perspective. The |
If we go with some variant of |
I think that's fine. It might be that they say
What consideration is needed here? Is this exposing a managed helper signature because exposing a managed method which takes I had already experimented with having ClangSharp recognize variadic functions and generate |
Yes basically. Consider how our prototype source gen works: [GeneratedDllImport(NativeExportsNE_Binary, EntryPoint = "sumi")]
public static partial int Sum(int a, <... or __arglist>); The above varargs would need to be marshalled in some manner. Which means the variable argument list would need to be inspected in an efficient way at run time to perform that marshalling and forward those arguments. |
Could the source generator not just do the concrete overloads. Say for example you had [GeneratedDllImport("msvcrt", EntryPoint = "printf")]
public static int Print(string format, __arglist); For this, the generator would create: [DllImport("msvcrt")]
private static extern printf(sbyte* format, __arglist); And for each unique invocation it found, it would generate a helper override that is a more exact match. The following two helpers would be generated: public static partial int Print(string format, string arg0, string arg1, string arg2)
{
fixed (byte* pFormat = Encoding.GetUTF8Bytes(format))
fixed (byte* pArg0 = Encoding.GetUTF8Bytes(arg0))
fixed (byte* pArg1 = Encoding.GetUTF8Bytes(arg0))
fixed (byte* pArg2 = Encoding.GetUTF8Bytes(arg0))
{
return printf(pFormat, pArg0, pArg1, pArg2);
}
}
public static partial int Print(string format, float arg0)
{
fixed (byte* pFormat = Encoding.GetUTF8Bytes(format))
{
return printf(pFormat, format);
}
} The only "loose" end would be that nothing was generated for the original |
Yep. Until a library decided to expose the P/Invoke directly at which point it may not observe any concrete calls. |
Isn't that the same world we already have today if someone does the following on Windows? [DllImport("msvcrt")]
public static extern printf(sbyte* format, __arglist) Can we not maintain the same support or can we not just block non-blittable parameters here (like we do for generics and a few other scenarios)? |
I don't think so. I believe a library can export that signature as is today without issue. But your proposal for the source generator approach wouldn't work for that case because it won't see the callsite when the application calls the libraries export.
I guess we could impose that, but my perspective is it isn't worth it. The proposed approach makes calling a vararg function possible and will naturally work with source generators in all scenarios since we tell the JIT how to pass the arguments. Supporting the gamut of varargs doesn't seem to be worth the cost at this point. It would impose a large burden on the source generator because the convention would need to be forwarded properly and the language updated. I'm simply not seeing the value in the cost to make it fully supported. |
Actually, I think it was also pointed out in #48796 (comment) that a lot of the JIT work will be the same - I agree with that. We can view this proposal with its requirement of a precise signature to be a down payment on enabling full support. Since if this proposal was accepted all we would need to do is address two additional issues:
I think this proposal simply starts the journey towards full support in an MVP manner. |
So you're basically thinking something like the following for the minimum viable product and then "full" varargs with language support might come later and would also enable the same on function pointers? [NativeVarargsAttribute(VarargBeginIndex = 1)]
[DllImport(@"NativeLibrary.dll", EntryPoint = "Varargs")]
static extern void Varargs1(int n, int a); That doesn't sound terrible, but it seems to me like you still have the problem that the user needs to know all overloads they require up front and that in the future you have two technologies to choose from. The only unique issues to
It would seem like the MVP would be to just block the first scenario and ask C# on the second (and fallback to something different if C# can't commit). |
@AaronRobinsonMSFT Instead of adding |
* Work around macOS arm64 varargs issues See dotnet/runtime#48752 dotnet/runtime#48796 * Better patch from Taloth
Is there any progress on supporting varargs in P/Invoke'd native libs? I just patched the padding for varargs to the 9th argument position into Libgit2sharp on OS X arm64, but there should be way to do this without this hack... (which may break again in the future) |
Just my two cents: C#, F#, and VB.Net all have a feature where variadic arguments can be passed via a specially marked array parameter at the end. Coupled with an attribute to identify it, perhaps you could have: [DllImport("libc.so", CallingConvention = CallingConvention.Cdecl, CharSet = CharSet.Ansi)]
public static extern void printf(string format,
[MarshalAs(UnmanagedType.VariableArguments)] params object[] args) |
.NET does not yet support variadic functions in PInvoke on macOS for arm64. dotnet/runtime#48796 Do some nonsense until it does.
When I read through the history of this issue, I can see a clear case of perfect becoming the enemy of good. Attempts to support declaring variadic parameters in metadata have soaked up time and energy that could have been spent supporting variadic methods on Mac and Linux at all. I came across this problem when wrapping Apple's Even if we could write a delegate and specify its variadic parameters, we don't want to because there are so many different ways to call
Here is a trimmed version of the code that I would like to write: public class NSRunningApplication : NSObject
{
// 1. Define a variadic native method
[DllImport("/System/Library/Frameworks/AppKit.framework/AppKit")]
private static extern nint objc_msgSend(nint receiver, nint selector, __arglist);
// 2. Pass an __arglist
public static NSRunningApplication? RunningApplicationWithProcessIdentifier(int pid) =>
FromPointer(objc_msgSend(Class, RunningApplicationWithProcessIdentifierSelector, __arglist(pid)));
} As we well know, this fails at runtime because Since we only need a limited number of Objective-C interop calls, we will proceed with the hack of padding the parameter list on Arm64. This is a very poor solution, but at least it avoids having to compile a new native library full of very specific |
It doesn't though. The This means that even if we decided to support There are definitely use cases for native varargs and there is interest, but the statement "I can see a clear case of perfect becoming the enemy of good" isn't an accurate description of the nature of it being lower priority. The lower priority is based on the narrow cases where it is is needed - real varargs. There are the |
I see, so Solving the problem at its root would require something even lower level than My opening statement wasn't describing the priority of the issue, I entirely understand why it's low. I was talking about how high-level parameter attributes were discussed for a long time, instead of starting with a low-level solution which unblocks the functionality and then refining/optimising it later. It was a bit frustrating to watch the years tick by. (Aside: I'm interested to know how casting |
It doesn't necessarily insert more, rather it instructs the JIT to pass the arguments in a certain way. That way conforms to the MSVC implementation of varargs and specifically how C++/CLI assumes they work. The details of how to pass floating point values via varargs is particularly treacherous on most platforms.
That is definitely an option. However, the minimum needed is simply to have an indication for the JIT to know to pass the arguments using the varargs calling convention that is appropriate for the current platform - encoding all of that in the JIT is the real cost. There are some differences of opinion on precisely how one might want to do this. One approach is to provide a "placeholder" (that is, Using [DllImport(@"...")]
extern static void printf(string fmt, __arglist);
// Usage:
printf("%d", 5);
printf("%f", 3.14); Use a new attribute: [NativeVarargsAttribute(VarargBeginIndex = 1)]
[DllImport(@"...")]
extern static void printf(string fmt, int a);
[NativeVarargsAttribute(VarargBeginIndex = 1)]
[DllImport(@"...")]
extern static void printf(string fmt, double a);
// Usage:
printf("%d", 5);
printf("%f", 3.14);
That is the agreed upon ABI for varargs. |
The attribute-plus-declared-parameters approach is certainly nice. But as I think Unless I misunderstand, the JIT needs to know not just which parts of the method should be variadic, but also which variadic convention should be used. To pose this as a question: does Assuming yes, how about this proposal: multiple keywords, one per supported variadic convention. extern static void printf(string fmt, argList_raw); // just pass as if normal arguments; would work for objc_msgSend
extern static void printf(string fmt, argList_msvc); // alias for the current __arglist
extern static void printf(string fmt, argList_libc); // other conventions added as deemed appropriate MSVC support is obvious, and I include "raw" partly because it's what I need, but also because it's the simplest possible convention. Going further with If it's not practical to add multiple keywords, then the variadic convention can instead be declared in a method attribute. Fully-typed signatures can still be achieved by wrapping the extern in a normal C# method: [DllImport(@"...")]
extern static void printf(string fmt, argList_msvc);
static void printf(string fmt, double a) => printf(fmt, argList_msvc(a)); I'm sure this would be slower to execute than an extern decorated with The biggest uncertainty I have is this: how much harder it is to support this approach in the runtime, compared to attributes?
Good, so that can be hardcoded on Arm64 regardless of the variadic convention in use. One less language feature required! |
It also looks like the new attribute solution wouldn't handle the situation of reverse p/invoke well. How well does each solution handle a function defined with |
any update ? |
Background and Motivation
Native varargs are a complicated interop scenario to support. At present, native varargs are only supported on the Windows platform through the undocumented
__arglist
keyword. Supporting varargs naturally in a P/Invoke scenario would be difficult from the C# language. However, it is possible to compromise by permitting support for the call with a fully specifiedDllImport
signature and a hint from the user.User scenario: #48752
Proposed API
Usage Examples
Consider the following native export with varargs:
The following P/Invoke declarations would enable users to call and properly forward the arguments in a supported multi-platform manner.
Alternative designs
Encode the information in the
CallingConvention
enum. This approach does remove the overhead of attribute reading, but does miss the added data of knowing where the varargs start - at present doesn't appear to be needed. This approach also impacts existing metadata tooling (for example, ILDasm, ILAsm, and ILSpy). See #48796 (comment).public enum CallingConvention { + VarArg = 6 }
Current state
Without this feature, calling functions with native varargs isn't possible on a non-Windows platforms. The proposed workaround is to create native shim libraries and instead P/Invoke into them. Continuing the example above, the shim library would export the following:
References
Support on Windows: dotnet/coreclr#18373
JIT details: #10478
The text was updated successfully, but these errors were encountered: