Memory-safety redesign in .Net 11

[abenedik]

I am the author of Ab4d.SharpEngine, which uses Vulkan to render 3D graphics. Because of that, I must use a lot of unsafe code.

I have tested the engine with the .Net 11 preview 7 because of the new Memory-safety model.
From my perspective, I find most of the changes in the new model great - especially that stackalloc and the address of a local variable are no longer considered unsafe. Also, preventing overriding a safe method with an unsafe method is great.

But I have trouble with the new safe keyword that needs to be used for structures with explicit layout. Without that keyword, the following compiler error is produced: error CS9392: Field in an explicit or extended layout type must be marked 'unsafe' or 'safe'.

Here are two examples:

[StructLayout(LayoutKind.Explicit)]
public partial struct ClearColorValue
{
    [FieldOffset(0)]
    public float Float32_0;
    [FieldOffset(4)]
    public float Float32_1;
    [FieldOffset(8)]
    public float Float32_2;
    [FieldOffset(12)]
    public float Float32_3;
    [FieldOffset(0)]
    public int Int32_0;
    [FieldOffset(4)]
    public int Int32_1;
    [FieldOffset(8)]
    public int Int32_2;
    [FieldOffset(12)]
    public int Int32_3;
    [FieldOffset(0)]
    public uint Uint32_0;
    [FieldOffset(4)]
    public uint Uint32_1;
    [FieldOffset(8)]
    public uint Uint32_2;
    [FieldOffset(12)]
    public uint Uint32_3;
}

This code is generated from the vk.xml file that describes the Vulkan API.

[StructLayout(LayoutKind.Explicit)]
internal struct StandardMaterialUniformBuffer
{
    [FieldOffset(0)]
    public Color4 DiffuseColor;
    
    [FieldOffset(16)]
    public Color3 SpecularColor;

    [FieldOffset(28)]
    public float SpecularPower;

    [FieldOffset(32)]
    public Color3 EmissiveColor;

     // etc...

This is part of the struct that must exactly match the layout of what the shader expects (generated based on shader compiler output).

My problems:

1. Both structs contain only simple value types so there is no way a user would be able to write to the memory outside the struct or break an address of a referenced type.

2. What is the added value of adding the safe keyword to existing code? The code that was already considered to be safe now requires an explicit safe keyword. But isn't the unsafe keyword already enough to mark which code is safe and which is unsafe. This is true for all the code, except inside structs with explicit layout.

3. The biggest problem for me is that my source code needs to compile for multiple versions of .Net. Because the safe needs to be added after the public keyword, the only way to be able to compile this for .Net 10 (and older) and .Net 11 is to use #if NET11_0_OR_GREATER. There I have two options:

   A) To duplicate the whole struct. This is risky because very delicate field offset values may get out of sync, and this is then very difficult to spot.

   B) Use #if for each field. This produces a very ugly mess of code, for example:

        [StructLayout(LayoutKind.Explicit)]
        internal struct StandardMaterialUniformBuffer
        {
            [FieldOffset(0)]
            public
#if NET11_0_OR_GREATER
            safe
#endif
            Color4 DiffuseColor;
            
            
            [FieldOffset(16)]
            public
#if NET11_0_OR_GREATER
            safe
#endif
            Color3 SpecularColor;
            
            
            [FieldOffset(28)]
            public
#if NET11_0_OR_GREATER
            safe
#endif
            float SpecularPower;

My recommendations:

A. The compiler should analyze the struct, and in case there are no referenced types defined, then the struct is considered safe. Note that in my case only checking if the struct is blittable is not enough, because the second struct also defines Color3 and Color4 that are simple blittable structs but this makes the StandardMaterialUniformBuffer non-blittable but still safe.

B. Instead of marking each field, there should be an option to mark the whole struct as safe or unsafe. In my opinion, using only unsafe would be enough.

Additional notes:
The safe keyword must now also be used for pInvoke with extern code.

I use that rarely so this is not a problem for me. But there is a lot of older code "around" that uses pInvoke and this code will suddenly not compile anymore. The developers will probably need to add the ugly #if NET11_0_OR_GREATER and this will surely not make them happy. So maybe you can also consider removing the safe keyword there.

Those are my recommendations.

But otherwise, congratulations on the great work you are doing with each new version of .Net!

[tannergooding]

The code that was already considered to be safe now requires an explicit safe keyword. But isn't the unsafe keyword already enough to mark which code is safe and which is unsafe. This is true for all the code, except inside structs with explicit layout.

The code is notably not safe and wasn't before either. It is a "union" (and one where the "tag" is passed externally) and that comes with special aliasing considerations and other edge cases. Particularly for arbitrary types, types with padding exposed, types where not all bit representations are considered legal, and other scenarios it is explicitly dangerous and violates memory safety.

A. The compiler should analyze the struct, and in case there are no referenced types defined, then the struct is considered safe. Note that in my case only checking if the struct is blittable is not enough, because the second struct also defines Color3 and Color4 that are simple blittable structs but this makes the StandardMaterialUniformBuffer non-blittable but still safe.

This is not safe. A simple example is decimal overlapped with Int128. Since the former has bit representations that are "illegal", you cannot freely reinterpret them between eachother. A struct S1 { int x; byte y; } overlapped with some struct S2 { int x, int y; } is another case because S2.y exposes 3 padding bytes from S1.

Further, we may have reference assemblies which don't explicitly track all implementation fields; but a type is also free to version over time and add additional fields as well. Lots of special considerations exist and while many may not apply to your specific scenario, there is nothing that C# can do to prove the safety, its ultimately on the developer.

This code is generated from the vk.xml file that describes the Vulkan API.

I'd note that this is not a correct definition either. vk.xml defines this as:

<type category="union" name="VkClearColorValue" comment="// Union allowing specification of floating-point, integer, or unsigned integer color data. Actual value selected is based on image/attachment being cleared.">
	<member><type>float</type>                  <name>float32</name>[4]</member>
	<member><type>int32_t</type>                <name>int32</name>[4]</member>
	<member><type>uint32_t</type>               <name>uint32</name>[4]</member>
</type>

Which officially corresponds to the following in C:

typedef union VkClearColorValue {
    float       float32[4];
    int32_t     int32[4];
    uint32_t    uint32[4];
} VkClearColorValue;

And so the valid minimal C# implementation is something like:

[StructLayout(LayoutKind.Explicit)]
public partial struct VkClearColorValue
{
    [FieldOffset(0)]
    public InlineArray4<float> float32;

    [FieldOffset(0)]
    public InlineArray4<int> int32;

    [FieldOffset(0)]
    public InlineArray4<uint> uint32;
}

Strictly speaking, structs, unions, inline arrays, bitfields, signedness of types, and many other components have special ABI rules and that differs from manually inlining fields, specifying field offsets, or other considerations. This means that deviating from the 1-to-1 C definition can break things and lead to subtle incompatibilities on various platforms or configurations.

These "happen" to line up and be equivalent in many cases, but it isn't a guarantee and you're often pessimizing codegen, interop, optimizations and other considerations by deviating in the ways you do.

https://github.com/terrafx/terrafx.interop.vulkan is an example of correct bindings that are 1-to-1 generated using ClangSharp and so should be strictly ABI compatible on all current platforms that .NET supports.

[timcassell]

That's not even correct, though, is it? The new rules apparently are in effect with <LangVersion>preview</LangVersion> today, with no explicit way to disable, and old rules with any other LangVersion, regardless of the TFM. So public source generators shouldn't even be using the new rules yet until it's out of preview. And when that happens, I expect source generators will be able to distinguish which rules are in effect via the language version queried from the Roslyn API (unless a way to opt-in/out is added by then) and emit the appropriate code. Please correct me if I'm wrong.

[CyrusNajmabadi]

my point was not the explicit code check. Merely that i don't think it's hard for a code generator to generate appropriately. It's easier imo than it is for a human as the code to do the checking/emitting/etc can be nicely deterministically done.

[agocke]

The new rules apparently are in effect with preview today

I don't think this is correct. The rules are in a new language version, but they should not be enabled by default, even when preview is enabled.

[333fred]

A few of the relaxations around pointer safety apply regardless of the memory safety version; however, the requirement to annotate things as safe or unsafe should not be applying unless the new memory safety rules are turned on.

[timcassell]

I see, I was asking Claude about it and it was having trouble finding how it gets enabled. But now I see there's an experimental API CSharpCompilationOptions.MemorySafetyRulesVersion that will be queryable, independent of lang version and tfm, that looks to be the intended solution.

[abenedik]

You and your AI have missed my point. Trying to persuade me that my code is invalid is not what this discussion is about (btw, my code is 100% valid and correct and does not require .Net 10 as your version with InlineArray4).

Also, as I have written, there are cases where using explicit layout is unsafe. I have mentioned the referenced type. I agree that using decimal is also not safe. And there are probably some other types that are unsafe too. Access to 3 padding bytes (in your example with S1 and S2) is in my opinion not unsafe, but I agree that this is an edge case.

But both of my samples are 100% safe. There is no way that writing or reading from those two structs would violate memory safety. If you find a case where this is not true, please provide an exact explanation. If you cannot do that, then you should agree that both of my examples are safe. And if you can say that, then the compiler can determine that too.

Each year I read the whole blog post about new optimization techniques in new versions of .Net. It says the compiler can do very complex analyses of the code to provide optimized assembly code.

Therefore, I cannot agree with your statement that "there is nothing that C# can do to prove the safety". I am sure that the compiler can analyze the struct with an explicit layout and determine if it is safe.

For example:

1. If the fields in the explicit layout do not overlap and if the alignment is correct, then the struct is safe. This would already cover most of the cases.
2. If the fields overlap (union) and struct uses only simple types (no references, decimals or some other problematic types), then the struct is safe.

If the compiler finds that the struct is not safe, then the unsafe keyword would need to be used. In my opinion, it would be better to add that to the struct and not to the individual fields.

I am sure that your goal is for as many developers as possible to switch to the latest version of .Net. The great new features that come with each version are very good motivation to do that. But having breaking changes is not.

Increasing the .NET version in csproj and finding hundreds of errors after compiling is not a good experience. I think that if the compiler can analyze the structs and report errors only for truly unsafe structs, this would be much better.

Forcing developers with interop code to add safe keyword to already safe code will not make the .Net code any safer. And if they will also need to support older versions of .Net, then they will also need to use #if and make their code much uglier.

If those changes stay as they are, then this is not a problem for me. But in your last blog post about .Net 11 Preview 7, you asked for a review, and here it is from my side. And I still stand behind it.

[tannergooding]

Isn't this discussion about calling convention and registers irrelevant here - the convention is calling VkClearColorValue(<24 byte struct>)

@AartBluestoke, the struct layout is explicitly dependent on the ABI for the platform. Further, the way it is passed is dependent on the combination of the layout and the calling convention.

That means that the layout definition and whether it is accurate to the C definition or not is incredibly relevant and a matter of correctness. This is because void M(float f) vs void M(S f) where struct S { float value; } have different layouts and therefore may have different ABI and calling convention rules for how they are passed or returned. -- This exact case actually does differ for multiple calling conventions, where the former is typically passed in a floating-point register while the latter is typically passed in an integer register (typically, but not always, by value in both cases)

This then extends out to examples like the above, where float x, y, z, w is not strictly the same as float x[4] or where struct S { T x, U y; } is not strictly the same as struct S { struct { T x; }; struct { U y; } } and so on.

---

In general you can summarize it down to:

This is generally, outside some special edge cases, undefined behavior in C and/or C++

In particular, C and C++ alike allow common-initial-sequences to work. So for an example like struct S1 { int tag; float x; } and struct S2 { int tag; int x; } being used for union U { A a; B b; }, it is well defined to access u.a.tag or u.b.tag regardless of whichever the last stored was.

However, accessing u.a.x or u.b.x is then dependent. In C89 doing this was implementation defined and then in C99 it was updated to be defined as a reinterpretation of the memory. In C++, it remains undefined behavior because it is outside the realm of the common-initial-sequence

Then, while C defines it as a reinterpretation of the memory; whether or not it is "well-defined" is dependent on whether the reinterpretation produces a value representation or non-value representation, where the latter is effectively saying that the bit pattern is "invalid" for the type in question (not exactly, but high level overview).

C then also doesn't strictly define how all semantics all work here and so u.a being the last stored but u.b being the last accessed or vice versa can lead to different behavior. That is, u1.a = ... followed by u2.b = u1.b may lead to u1.a != u2.a (not bit identical); inversely you may also have u1.b = ... followed by u2.a = u1.a and then have u1.b != u2.b (not bit identical). You may even have things like faults (exceptions) silently raised by one of these but not the other, and so on.

---

So what it ultimately comes down to is that this is a very tricky space and one where not only do users often trivially get it wrong, especially for interop because they do not actually define 1-to-1 layouts; but it is a case where it is often explicitly undefined, partially defined, or generally unsafe behavior. It is likewise a place that can trivially lead to memory safety and other bugs.

While there is a very small subset of type punning involving primitives that is conceptually safe, it is far easier for the language to simply force users to consistently think and annotate such things explicitly; particularly given the languages lack of an explicit overlapping memory feature (and therefore no actual concept of it), that it is strictly undefined and unsafe in most other modern languages, and for all the other reasons given above (not to mention other responses). It also makes the language overall more consistent in that users simply have to always treat such attributed code as unsafe unless they explicitly state otherwise.

[AartBluestoke]

thanks,

• "the struct layout is explicitly dependent on the ABI for the platform"

  • i assumed the explicit layout tag would circumvent this

• "this is because void M(float f) vs void M(S f) where struct S { float value; } have different layouts and therefore may have different ABI"

  • yes, but the specific example here the call is with the struct, so pointer passing only
  • the calling convention of the extern function is well defined, so the square-peg-in-square-hole should be solved.

• " u.a being the last stored but u.b being the last accessed or vice versa can lead to different behavior."

  • agreed, i think i was expecting that value types this is 'funny values' rather than 'correctness, memory safety' as long as the data types doing the bit-cast are the same size

• would a solution here with non-overlapping definitions be 'safe'?
  public partial struct Color4 (where T in float,int,uint)
  {
  [FieldOffset(0)]
  public InlineArray4 values;
  }

and then a
[StructLayout(LayoutKind.Explicit)]
internal struct StandardMaterialUniformBuffer
{
[FieldOffset(0)]
public Color4 DiffuseColor;

[FieldOffset(16)]
public Color3<T> SpecularColor;

[FieldOffset(28)]
public float SpecularPower;

[FieldOffset(32)]
public Color3<T> EmissiveColor; 

}

[tannergooding]

i assumed the explicit layout tag would circumvent this

No, it still has to be classified based on the types of data it contains and ultimately passed according to the rules of the underlying calling convention.

yes, but the specific example here the call is with the struct, so pointer passing only

That is not the case and the example I gave is one such case where no pointer is involved. struct S { float f; } is sometimes passed by pointer, sometimes passed in a floating-point register, and sometimes passed in an integer register.

struct S { float f1; float f2 } is sometimes passed by pointer, sometimes passed by in 1 floating-point register, sometimes passed in 2 registers, sometimes passed in 1 integer register, and sometimes passed in 2 integer registers.

struct S { int32_t i1; int i2; } is sometimes passed by pointer, sometimes passed in 1 integer register, and sometimes passed in 2 integer registers.

and so on; it is entirely ABI and calling convention dependent, as is every unique struct shape and primitive type.

the calling convention of the extern function is well defined, so the square-peg-in-square-hole should be solved.

Not really! For starters VkClearColorValue is a struct and so how it passes depends on the calling convention of the function that uses it or the struct that uses it as a nested field.

Vulkan in general uses VKAPI_CALL which is defined as __stdcall on Windows and the default calling convention (namely __cdecl) elsewhere; so that's a deviation across Operating Systems in itself. However, these conventions themselves aren't stable across architectures and the way that Arm32 vs Arm64 vs x86 vs x64 all handle things also differs, so there isn't consistency there either.

There's also nuance here in that while all the Vulkan APIs currently taking VkClearColorValue directly are doing so via an explicit pointer today; there is no guarantee that remains true for all future functions. It is also not a guarantee that all structs that contain it as a field will do so in the future either.

It fundamentally remains an "incorrect" binding and one that could come back to bite the developer in the future. This one happens to be a case that is "probably" fine because most sane ABIs won't be doing anything particularly weird here. But that is also a bit beside the point in the overall discussion and what is safe for the language to assume is correct and safe. The language isn't going to and special case just this struct for Vulkan after all and there are plenty of other cases, including minor deviations, that are impossible to discern or which are not safe.

agreed, i think i was expecting that value types this is 'funny values' rather than 'correctness, memory safety' as long as the data types doing the bit-cast are the same size

These "funny values" are explicitly memory unsafe because it bypasses the normal initialization rules or handling of a type. It could break the invariants of the type and lead to memory unsafety. This is why it is explicitly defined as "undefined behavior" across essentially almost all language and even in C where it tries to make simple cases involving primitives "sensible", it explicitly calls out that it can be undefined for various types and aggregate types.

would a solution here with non-overlapping definitions be 'safe'?

You simply just don't use FieldOffset at all. This gives you a type with the correct layout that is better for the compilers to optimize and doesn't require unsafe in the first place.

You should never need to use [FieldOffset] for C structs that aren't themselves unions (*) and C unions are only ever overlapping things at offset 0. So to be "blittable" and the most efficient, you never have to do these "weird tricks". * The exception here is that some C compilers do have custom (non standard) extensions allowing over-alignment of data, which is not something that .NET supports and which is incredibly atypical even in C.

When you do need to overlap, it fundamentally becomes "unsafe" and requires a user to specify if they are in one of the rare edge cases when it is actually safe, typically because they know they are doing explicit interop with C (not any other language) and are only doing trivial type punning and will never change in the future (all things which the compiler itself cannot trivially prove or guarantee)

[MichalPetryka]

• i assumed the explicit layout tag would circumvent this

Explicit only affects the memory representation, not the ABI.

• the struct, so pointer passing only

Not all ABIs pass all structs via pointers, some can pass them via multiple registers depending on shape so for example S1 { byte a[16] } and S2 { ulong a[2] } could have S1 passed as pointer but S2 passed as 2 registers.

[AartBluestoke]

thanks. Today i learned some stuff i did in the past is more dangerous than i previously thought :)
edit: which i guess is the entire point of this safety redesign ...

[jjonescz]

nit:

because the safe needs to be added after the public keyword

That's not true, the safe keyword can be anywhere among the modifiers. There might be diagnostic "IDE0036: Modifiers are not ordered," but I imagine you can simply suppress that in generated code.