[mono] Force Mono to respect explicit struct size when LayoutKind.Sequential is used

[matouskozak]

We encountered an issue with Mono not respecting explicit size when LayoutKind.Sequential was used (#101432). We fix this by eliminating addition of alignment padding when explicit size is set.

[matouskozak]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[lambdageek]

@grendello @jonpryor @lewing @rolfbjarne @dalexsoto We're going to change mono to respect the explicit Size property on a [StructLayout(LayoutKind.Sequential, Size=NN)] attribute, if given, for sequential layouts. (It was already respected for Explicit layouts).

Previously Mono would align the struct size up to the alignment of the most restricted struct element. This was so that arrays would always have their elements such that every struct member would be at its natural alignment. But as a result, if you had a struct inside another struct followed by a member with a less restricted alignment there would always be padding after the struct. So we would behave as if the Size property was always rounded up. CoreCLR doesn't do that. If a Size is given, it will always use it - even if it means that arrays of structs will end up with misaligned members in some elements.

This is needed for Swift interop (frozen swift structs also behave this way). (See #101432)

For example:

[StructLayout(Sequential, Size=12)]
struct DoubleAndInt1
{
   public double D1;
   public int I2;
};

[StructLayout(Sequential)]
struct DoubleAndInt2
{
   public double D1;
   public double I2;
}

Previously in Mono both DoubleAndInt1 and DoubleAndInt2 would have size 16 (12 rounded up to the alignment of the most restricted member - D1). In CoreCLR and in Mono after this change, DoubleAndInt1 will have size 12.

That means if you do a1 = new DoubleAndInt1[2] and a2 = new DoubleAndInt2[2], then a1[1].D1 will be mis-aligned (it will be at offset 12 from the start of a1, while a2[1].D1 will be at its natural alignment (offset 16 from the start of a2).

We're running a functional test on Android arm32 to check whether we need to do some work in the JIT in order to issue an unaligned load/store for arrays like a1

Questions

Is this change going to be a problem for your existing interop code in XI, XA or wasm?

[grendello]

I didn't find any StructLayout use with explicit size in XA code (in-repo or generated), so we should be fine with that change.

LibZipSharp is fine too, however Mono.Posix has one instance which might be affected?

[rolfbjarne]

Is this change going to be a problem for your existing interop code in XI, XA or wasm?

XI doesn't have any sequential StructLayouts with explicit size, so it's good on our side.

Additionally we use CoreCLR for macOS, and we haven't found any such issues there.

[matouskozak]

LibZipSharp is fine too, however Mono.Posix has one instance which might be affected?

That one should be unaffected by this PR. Here we only modify the behavior for LayoutKind.Sequential. LayoutKind.Explicit was already handled correctly when used together with explicit size.

[lewing]

I'm not aware of any issues on the wasm side

[matouskozak]

We have tested this change on Arm64 VMs emulating Arm32 without any issues. We also performed successful local testing on armv7 Raspberry (thanks to @akoeplinger).

[lambdageek]

lgtm, thanks!

[matouskozak]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[matouskozak]

The WASM failures are unrelated and are matched using Build Analysis. The tvos-arm64 Release AllSubsets_Mono Tar failure is also known #101692.