Struct promoted to auto-layout if it contains a non-blittable field type + explicit layout field type

xoofx commented 3 years ago

Description

We have discovered an unexpected behavior of the runtime regarding layout of a struct in memory with .NET CLR. In the following example, the struct BoolAndExplicitStruct layout will be promoted to an auto-layout because of the presence of the field public bool Bool and public ExplicitLayoutStruct ExplicitLayout together:

   [StructLayout(LayoutKind.Explicit)]
    struct ExplicitLayoutStruct {
        [FieldOffset(0)] public byte B00;
    }

    struct RegularStruct
    {
        public int Field;
    }

    struct BoolAndExplicitStruct
    {
        // bool AND a field with an explicit layout struct
        // change the layout of this struct to Auto instead of sequential
        public bool Bool;
        public ExplicitLayoutStruct ExplicitLayout;
        public int Int32;
        public RegularStruct RegularStruct;
        public long Int64;
    }

    // Field Int64: starts at offset 0
    // Field Int32: starts at offset 8
    // Field Bool: starts at offset 12
    // Field ExplicitLayout: starts at offset 16
    // Field RegularStruct: starts at offset 24

This is causing lots of trouble on the Burst compiler on our side, because as we are using these structs in native code, we expect the same sequential+explicit layout than .NET, but we were not expecting an implicit auto-layout promotion in this case (We haven't implemented today the auto-layout because we fear its implementation dependent behavior)

It seems to be related around this function CheckIfDisqualifiedFromManagedSequential or the caller of this function, but haven't identified exactly in the code when this flip occurs.

Do you know the reason of this auto-layout promotion? Could we revert that behavior? (We will be happy to make a PR)

Configuration

Happens on all OS, all CPU and all .NET version (including .NET framework) but not on Mono (because Mono doesn't have auto-layout promotion afaik)

Regression?

Nope.

tannergooding commented 3 years ago

I find it unfortunate that while the documentation expressly mentions that "primitives" like System.Bool and System.Char are not blittable, it fails to mention System.Decimal too.

This can likely be updated to include the fact. It may also be possible to force decimal to be blittable by manually forcing the alignment to be 8 in the VM, as we do with a few other types like Vector64/128/256, but I'd defer to @jkotas and @jkoritzinsky on if they thinks that's a good idea.

On a related note, do you know if that documentation is applicable for "constructed" types too? Or is there an unwritten assumption that it only applies to "plain" types? For example if I have the type constructor:

AFAIR, generic types should have the same guarantees provided TSome is blittable (and should be blocked from P/Invoke otherwise). I believe I added all the relevant tests and confirmed this behavior in the VM when I added the support back in https://github.com/dotnet/runtime/pull/103

zacknewman commented 3 years ago

@tannergooding, you're awesome. Thank you.

jkotas commented 3 years ago

It may also be possible to force decimal to be blittable by manually forcing the alignment to be 8 in the VM,

The current decimal implementation has long field that ensures the sufficient alignment. We used to have a quirk to force decimal alignment in the VM. The quirk was deleted in https://github.com/dotnet/runtime/pull/38603 .

decimal is non-blittable for backwards compatibility only. I am not sure whether it is worth fixing that. It is just going to break random old libraries like what happens every time we touch anything in build-in interop.

dotnet / runtime