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

Reconcile blittable and unmanaged concepts #59824

Closed
elinor-fung opened this issue Sep 30, 2021 · 6 comments
Closed

Reconcile blittable and unmanaged concepts #59824

elinor-fung opened this issue Sep 30, 2021 · 6 comments

Comments

@elinor-fung
Copy link
Member

C# has the concept of an unmanaged constraint, but that does not map to blittability for interop in the runtime. We should investigate reconciling the two.

See #59658 (comment)

cc @AaronRobinsonMSFT @jaredpar @jkoritzinsky @lambdageek

@dotnet-issue-labeler dotnet-issue-labeler bot added area-Interop-coreclr untriaged New issue has not been triaged by the area owner labels Sep 30, 2021
@elinor-fung elinor-fung added this to the 7.0.0 milestone Sep 30, 2021
@elinor-fung elinor-fung removed the untriaged New issue has not been triaged by the area owner label Sep 30, 2021
@jkoritzinsky
Copy link
Member

Today, the blittable concept in the runtime is not equivalent to the unmanaged concept in the C# language. Blittable types are a subset of unmanaged types, mainly due to back-compat issues around .NET's initial Windows-focused nature.

All primitives except bool and char are blittable. In the built-in system, char is conditionally blittable when the CharSet is set to CharSet.Unicode, but by default the CharSet for a given struct is set to Ansi (since that was the default since .NET Framework 1.0). bool is never blittable for a few reasons. Its default marshalling behavior is to marshal to a 4-byte Windows BOOL. But even when custom marshalling is specified to marshal it as a 1-byte C bool, it is not blittable since the built-in marshalling system normalizes true and false to 1 and 0 and vice versa in the interop stubs.

For the source-generated marshalling system, we can make the choice to break these back-compat defaults and instead treat char as a UTF-16 character by default and treat bool as a 1-byte bool by default without normalization. This change would allow us to assume that "unmanaged" == "blittable" and significantly simplify some designs and concepts for our users with the new source generator. However, this does come with some possible unexpected breaking changes, as it changes both field size and alignment.

Given the following struct type:

struct MyStruct
{
      public char c;
      public bool f;
      public byte b;
}

In the current built-in system, this maps to the following native struct:

struct MyStruct // size 12, alignment 4
{
      char c; // Offset 0
      BOOL b; // Offset 4
      byte b; // Offset 8
};

whereas with an "unmanaged == blittable" system:

struct MyStruct // size 4, alignment 2
{
      char16_t c; // Offset 0
      bool b; // Offset 2
      byte b; // Offset 3
};

Because the differences between the two systems would be quite drastic, we would need to determine a good mechanism to warn users about structures that may have large differences between the old and new systems. Additionally, we would need to consider types that are defined in other assemblies (possibly in other NuGet packages) that may not have their full surface area exposed publicly (so it might not be possible to detect that it has bool and char fields that would cause problems).

Also, as part of this work, we should make sure to keep the defaults for bool and char aligned between the "struct" scenario and the "DllImport" scenario. When we have different defaults for each system, we end up causing confusion instead of clarity and more problems in the long run.

@jkoritzinsky
Copy link
Member

After thinking about this more and having some more conversations, here's a few implementation considerations that we need to think about if we go the reconciliation route (unmanaged == blittable route).

We need to consider how this new system interacts with the existing Marshal APIs like SizeOf, StructureToPtr, PtrToStructure, etc. The new behavior would not align with the old behavior, so we'd need to consider how we ensure that users do not shoot themselves in the foot.

Analyzers would be helpful here, but detecting when a struct is used in the new system and with these APIs might be a difficult problem. Thankfully, with the existence of the new Unsafe APIs, we can push users towards using Unsafe.Read to replace PtrToStructure, Unsafe.Write to replace StructureToPtr, and SizeOf to replace Unsafe.SizeOf. As these Marshal APIs will already be unable to process the new attributed default marshalling rules in the source-generated system, this change might not be as difficult as expected if we can detect enough concerning cases with an analyzer (a code fix might even be possible).

As the GeneratedDllImport design uses a regular DllImport under the hood, we need some mechanism (that's public so DllImportGenerator can use it) to indicate to the built-in system to treat unmanaged == blittable. Below I've included a few options:

  1. A new UnmanagedType enum member to use with MarshalAs. This member, named UnmanagedBlittable would only be valid on unmanaged structures and would tell the built-in system to pass the structure parameter/return-value as a blittable value (no marshalling).

This solution works for DllImportGenerator, but it prevents other developers from reusing the core of our marshalling logic to build their own generator based on function pointers. For example, being able to write a source-generator model to provide a "DllImport-like" surface for OpenGL/Vulkan APIs or other APIs that need #1906 by using function pointers to call into native instead of DllImport is something that the interop team has considered in our design around extensibility (the reason that
the Microsoft.Interop.SourceGeneration assembly exists today is as a proof-of-concept for allowing other generators to build on the base design of GeneratedDllImport). Since MarshalAs attributes cannot be applied on function pointers, this option would cause function pointers to be less useful in the new interop world. This design would also go against our general goal of moving towards enabling usage of lower cost primitives by pushing people to use delegate interop (where MarshalAs support would come basically for free as part of the DllImport support) instead of function pointers.

  1. A new CallConvUnmanagedBlittable type that would act as a modifier for a full signature. When this "calling convention" is applied, it would augment the already provided calling convention (similar to CallConvSuppressGCTransition) such that every unmanaged struct type in the signature will be treated as a blittable type.

This mechanism has its own pros and cons:

  • 👍🏻 It allows us to annotate DllImports (using the UnmanagedCalleeAttribute), function pointers (using the function pointer calling convention syntax), and UnmanagedCallersOnly (as it already takes a CallConvs type array).

  • 👍🏻 Using the calling convention route would also make it less discoverable in DllImport scenarios, limiting misuse by developers who don't know about the new interop system.

  • 👎🏻 It cannot be applied in a fine-grained manner on specific parameters/return value, it would be all or nothing.

    • Since this calling convention modifier's primary use case would be in source-generated code where we want the "all or nothing" behavior anyway, I don't think this is much of a problem.
  • 👎🏻 This isn't really a "calling convention" concept.

    • This is more of a philosophical objection, so I don't really particularly care.

@jkotas
Copy link
Member

jkotas commented Oct 1, 2021

we can push users towards using Unsafe.Read to replace PtrToStructure, Unsafe.Write to replace StructureToPtr, and SizeOf to replace Unsafe.SizeOf

You do not even need to use Unsafe.* APIs for this. The standard C# pointer derefence and sizeof works just fine to unmanaged types, and it is more concise and less ceremony.

BTW: It is not unusual to see bugs where people use Marshal.SizeOf, but actually should use sizeof.

indicate to the built-in system to treat unmanaged == blittable

I think it is important that we have something that works well for function pointers.

One more option to consider: Global assembly setting that says "this assembly does not depend on built-in interop support". It can be a new bit on CompilationRelaxations or its own attribute. It would make the auditing easy and allow us to introduce a clean boundary between old and new. The downside is that it would not be possible to mix&match old and new plans easily.

@jkoritzinsky
Copy link
Member

I like the idea of making something available assembly-wide to say "don't use the old marshalling system at all". That would be something that the Mono team could build off with their componentized runtime work. If they were to move the built-in marshalling subsystem to a component, they could exclude it if all assemblies either opt-in to using the new system or don't use DllImports/interop marshalling at all (which could be determined at build time).

@jkoritzinsky
Copy link
Member

This would probably be a good scenario for extending the System.Runtime.CompilerServices.RuntimeCompatibilityAttribute if we wanted to go the global assembly route.

@AaronRobinsonMSFT
Copy link
Member

Closing since #60639 now provides a reconciliation mechanism.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants