Repository navigation
[API Proposal]: System.Runtime.CompilerServices.RuntimeCompatibilityAttribute.DisableRuntimeMarshalling #60639
Description
Activity
- Reacted by Aleksey Kliger (λgeek)
@jkoritzinsky I like the global assembly level indication. Two things comes to mind though.
-
Will this global setting have an impact on consumers of the assembly if a P/Invoke is publicly accessible?
-
The global setting seems like it will restrict dual use which might be desired to work around limitations or bugs. At present the only way someone could work around this would be to create yet another assembly or manually define the call. Would an "opt-out" mechanism be desirable?
-
- Will this global setting have an impact on consumers of the assembly if a P/Invoke is publicly accessible?
Yes, this will impact customers who are consuming a public P/Invoke defined in an assembly with this attribute. There is no plan to use some sort of call-site checking to determine behavior based on the caller's assembly having this attribute in case they differ.
- The global setting seems like it will restrict dual use which might be desired to work around limitations or bugs. At present the only way someone could work around this would be to create yet another assembly or manually define the call. Would an "opt-out" mechanism be desirable?
If we decide that an opt-out would be desirable, I'd suggest that we should reconsider the opt-in design options. An opt-out design (instead of a single all-or-nothing switch) makes the "can Mono not ship the built-in system" decision nearly as difficult as the opt-in mechanisms. Even if we decide to add an opt-out mechanism, I'd suggest that we use the alternative opt-in designs as a basis to design the opt-out system.
I much prefer the global level indiciation (
RuntimeCompatibilityAttribute) as well. As a maintainer of explicitly blittable bindings, I would not appreciate having to annotate several thousandexterncalls (even if source generated) withMarshalAsor additionalCallConvannotations.Blittable bindings are blittable and one benefit of that is that they are more readable. Forcing users who want to be blittable to add additional magic annotations everywhere is a major pain point; while opting an entire assembly out via an attribute would not be.
It would be great if this came with an analyzer (or even compiler level warning/error) if
DisableRuntimeMarshallingwas set so you get notified whenDllImportmethods are non-blittable. That would likewise fit in if Roslyn needs to coordinate with its own setting ofWrapNonExceptionThrows.If we decide that an opt-out would be desirable, I'd suggest that we should reconsider the opt-in design options.
That seems a bit extreme for cases that should ideally be few. The concern here is for consumers that have perhaps a couple of P/Invokes that would an impediment to adopt the source generator for the vast majority of their assembly. This is similar thinking to @tannergooding's
ID3DIncludeexample discussed at #60143 (comment). A simple affordance that helps ease adoption. We could say this is a hard requirement too though which does force us to prioritize this impact.The concern here is for consumers that have perhaps a couple of P/Invokes that would an impediment to adopt the source generator for the vast majority of their assembly
@AaronRobinsonMSFT, are you saying they might have a situation where most of their
DllImportare blittable, but they have a couple that aren't and adopting the source generator for those couple might not be possible?The global setting seems like it will restrict dual use which might be desired to work around limitations or bugs.
You should be able to workaround limitations or bugs by writing the marshaler manually or tweaking the signature today. I do not think we should be promoting this attribute for this purpose.
It would be great if this came with an analyzer
Yes, I think the DllImportGenerator should produce hard errors (not warnings) for all non-blitable DllImports and unmanaged function pointer calls when it sees this attribute.
Reacted by Aaron R Robinson, Tanner Gooding and Clinton Ingramthat aren't and adopting the source generator for those couple might not be possible?
@tannergooding Yes. Consider the assembly owner that has a library that exposes an API that requires
StringBuilder. Rather than force that person to rev their assembly with a break change, we can permit them to use this and leave their one API alone with an "opt-out".I do not think we should be promoting this attribute for this purpose.
That is fair.
There are a few places in the libraries where the blitability concept leaks out.
GCHandle.Allocfor pinned handle is the example that I have mind, there may be others.Should we consider relaxing the argument validation in
GCHandle.Allocto check for the unmanaged type instead of blittable types? It should be an acceptable breaking change (exception thrown changing to success). It can be part of this API proposal or be separate proposal.Reacted by Tanner Gooding, Aaron R Robinson and Jeremy KoritzinskyI think removing the validation for
GCHandle.Allocwould make sense. I'll add that to the proposal above.Just leaving a comment to track...
We should consider how this impacts
SetLastError = trueand potentially otherDllImportflags that would become unavailable (or potentially unavailable) in this context.I believe, we also need to consider the case where the type loader may reorder or pack the type if it is determined to be non-blittable. If I recall during type load the layout of the type is managed auto and not managed sequential. This means if we apply the proposed attribute to a P/Invoke argument but the type was loaded with auto the native side must adhere to how the type loader decided to lay out the type.
@jkoritzinsky Can likely provide a better example but I think the gist here is the type loader has the option of doing something that will make just passing through a type difficult in some cases.
I believe, we also need to consider the case where the type loader may reorder or pack the type if it is determined to be non-blittable
Yes, it is possible today and it makes the field layout algorithm very complex. We should replace the IsBlittable checks in field layout with is IsUnmanaged` (ie does not contain object references).
It was attempted in #51416 (comment) , but we run out of time to stabilize that change for .NET 6. The plan is to do that in .NET 7. We should make this proposal dependent on that. cc @trylek
Reacted by Jeremy Koritzinsky and Aaron R Robinson- We changed this to be a separate attribute because of how it composes with the compiler-emitted
RuntimeCompatibilityAttribute - We had a philosophical discussion about generators emitting attributes like this, and concluded that they shouldn't because of the generator source-view model and composability. (When they're required that should be detected by an analyzer)
namespace System.Runtime.CompilerServices { [AttributeUsage(System.AttributeTargets.Assembly, AllowMultiple = false, Inherited = false)] public sealed class DisableRuntimeMarshallingAttribute : Attribute { } }
Reacted by David Maas- We changed this to be a separate attribute because of how it composes with the compiler-emitted
Made a tracking issue for Mono #61685
This is something I've wanted for quite some time, very excited to see this scheduled for .NET 7!
One concern I do have though is how this could be confusing when function pointers are allowed to cross between assembly boundaries.
For example, imagine I have these two assemblies:
// MyCoolLibraryWrapper [assembly: DisableRuntimeMarshalling] public unsafe static class MyCoolLibrary { // Returns an unmanaged function pointer which takes a 16 bit UTF16 character and returns a proper 8 bit bool. // (IE: No marshaling is necessary or desired to invoke the returned function pointer.) [DllImport("MyCoolLibrary.dll")] public static extern delegate* unmanaged<char, bool> GetFilter(); public static bool PassesFilter(char c) => GetFilter()(c); }
// MyCoolApp, depends on MyCoolLibraryWrapper // Note that DisableRuntimeMarshalling is not applied. const char c = 'a'; Console.WriteLine(MyCoolLibrary.PassesFilter(c)); Console.WriteLine(MyCoolLibrary.GetFilter()(c));
For the above:
PassesFilterwill work as expected regardless of who calls it since marshaling is disabled in the context of the function pointer invocation.MyCoolLibrary.GetFilter()(c)has undefined behavior in the context ofMyCoolAppsince the marshaler will mess with the function pointer invocation.- (This mistake is hard to notice because it can seem to work in some situations.)
Would it be reasonable to provide an in-box analyzer which warns if a non-
DisableRuntimeMarshallingassembly accesses an API involving a function pointer which uses non-blittable types from aDisableRuntimeMarshallingassembly and vice-versa?- ghost locked as resolved and limited conversation to collaborators
on Feb 20, 2022
Background and motivation
As the interop team is working on a source-generated DllImport solution, we've decided to take an extra look at the built-in marshalling rules per feedback from @stephentoub.
In the built-in marshalling system today, the runtime looks at all member fields of a type at runtime and uses this runtime type information to determine if a type is blittable and how to marshal it. For compile-time source generators, this sort of design is untenable for performance and feasibility reasons (we need a static type in the final "native call" signature to get correct calling-convention behavior). See https://github.com/dotnet/runtime/blob/main/docs/design/libraries/DllImportGenerator/StructMarshalling.md for a more information.
A complicating factor in this scenario is that the default marshalling rules for
boolandcharare non-blittable. In particular,boolmaps to a 4-byte Windows APIBOOL, andcharmaps to an ANSI (based on the current code page) 1-byte character. As a result, the C# language concept of "unmanaged" is not the same as the runtime concept of "blittable". In the proposal above, we took the route of building a new attribute and analysis system to ensure that a type marked as blittable is actually blittable.@stephentoub and @jkotas recommended that we could instead investigate changing the rules in source-generated P/Invokes to make the unmanaged and blittable concepts equivalent. To enable this mechanism at the interop boundary, we need to provide some mechanism to disable the built in marshalling rules. We've explored a few options in #59824, and I've selected one of the options to bring up for API review here (the other options are covered in Alternative Designs).
We propose adding a new property to
RuntimeCompatibilityAttributethat disables the built-in marshalling system for all P/Invoke and Reverse P/Invoke scenarios defined in the assembly (all methods marked[DllImport], all delegates that are defined in the assembly when used in interop scenarios, and allUnmanagedCallersOnly-attributed methods). When this property is set totrue, all unmanaged types are considered blittable, and all non-blittable types (in/ref/outparameters as well since they require pinning) are disallowed.We like the idea of disabling across a whole assembly for a few reasons:
API Proposal
namespace System.Runtime.CompilerServices { public class RuntimeCompatibilityAttribute { + public bool DisableRuntimeMarshalling { get; set; } } }Additionally, we propose a behavior change for
GCHandle.Allocto support all unmanaged types for allocating a pinned GC handle. There is no API change required for this portion of the request, as this only changes an exceptional scenario (non-blittable unmanaged types throw today in this case, and all other unmanaged types are blittable and as such succeed).API Usage
Alternative Designs
Alternative Design 1:
System.Runtime.InteropServices.UnmanagedType.UnmanagedBlittablenamespace System.Runtime.InteropServices { public enum UnmanagedType { + UnmanagedBlittable = /* TBD */ } }In this design, we'd either explicitly require or implicitly emit a
[MarshalAs(UnmanagedType.UnmanagedBlittable)]attribute on every unmanaged parameter or return type that doesn't have any marshalling attributes already applied.Pros:
Cons:
UnmanagedCallersOnly, which is the direction the interop team has been pushing.Alternative design 2:
System.Runtime.CompilerServices.CallConvUnmanagedBlittablenamespace System.Runtime.CompilerServices { + public sealed class CallConvUnmanagedBlittable {} }In this design, we introduce a new "calling convention" type that specifies the "unmanaged == blittable" behavior for all parameters and return values, and we'd implicitly apply it in source-generated scenarios.
Pros:
Cons:
Alternative design 3:
System.Runtime.CompilerServices.CallConvNoMarshallingnamespace System.Runtime.CompilerServices { + public sealed class CallConvNoMarshalling {} }In this design, we introduce a new "calling convention" type that disables the built-in marshalling behavior for all parameters and return values, and we'd implicitly apply it in source-generated scenarios.
Pros:
Cons:
Risks
Today, Roslyn emits the equivalent to
[assembly:RuntimeCompatibilityAttribute(WrapNonExceptionThrows = true)]into every assembly if theRuntimeCompatibilityAttributeis not defined by the user. If the user manually provides theRuntimeCompatibilityAttributeattribution themselves, they may forget to setWrapNonExceptionThrows=true. As scenarios whereWrapNonExceptionThrows=trueactually kicks in (requires manually written IL or C++/CLI to my knowledge), this shouldn't be much of an issue. If we view this as a problem, we can do one or both of the following things:RuntimeCompatibilityAttributeattribute application in scenarios where we'd recommend applying it (ie. with the DllImportGenerator or other interop source-generators).RuntimeCompatibilityAttributewith theDisableRuntimeMarshalling = trueproperty itself.