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

[mono] Mono doesn't respect explicit size for structures when marked with LayoutKind.Sequential #101432

Closed
matouskozak opened this issue Apr 23, 2024 · 6 comments

Comments

@matouskozak
Copy link
Member

matouskozak commented Apr 23, 2024

Mono doesn't respect explicit struct size when combined with sequential layout, i.e., when using [StructLayout(LayoutKind.Sequential, Size = X)], the structure doesn't have the desired size X.

Repro code:

using System;
using System.Runtime.InteropServices;

[StructLayout(LayoutKind.Sequential, Size = 12)]
struct F3_S0_S0
{
    public nint F0;
    public uint F1;

    public F3_S0_S0(nint f0, uint f1)
    {
        F0 = f0;
        F1 = f1;
    }
}

private static void Main(string[] args)
{
    Console.WriteLine(Marshal.SizeOf(typeof(F3_S0_S0)));
}

Mono output:

16

CoreCLR output:

12
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Apr 23, 2024
@lambdageek
Copy link
Member

lambdageek commented Apr 23, 2024

This goes back a long time to 6fcc759

  * class.h, class.h: take minimum alignment into consideration so
   that the fields of a class remain aligned also when in an array.

Here:

if (!((layout == TYPE_ATTRIBUTE_EXPLICIT_LAYOUT) && explicit_size)) {
if (instance_size & (min_align - 1)) {
instance_size += min_align - 1;
instance_size &= ~(min_align - 1);
}
}

It might be possible to change this to respect the specified size also for sequential layouts, not just explicit. But I'm not sure how much code that will break downstream of mono.

@matouskozak matouskozak changed the title [mono] Mono doesn't allocate nested structures to the same offsets as CoreCLR [mono] Mono doesn't respect explicit size for structures when marked with LayoutKind.Sequential Apr 24, 2024
@jakobbotsch
Copy link
Member

jakobbotsch commented Apr 24, 2024

It's more evidence that something like #100896 is necessary, even without generics. These kind of struct layouts are necessary to model Swift structures in C#. cc @jkoritzinsky

@matouskozak if you are blocked on the Swift work I can give you some instructions on generating the Swift unit tests with ExplicitLayout.

@lambdageek
Copy link
Member

What happens in swift if you allocate an array of frozen structs that have a size that is not a multiple of the minimum alignment of any members? Do the array elements end up packing tightly and misaligning the members?

@jakobbotsch
Copy link
Member

Swift has the notion of "stride" for this:

https://github.com/apple/swift/blob/4b440a1d80a0900b6121b6e4a15fff2a96263bc5/docs/ABI/TypeLayout.rst#fragile-struct-and-tuple-layout

The final size and alignment are the size and alignment of the aggregate. The stride of the type is the final size rounded up to alignment.

I believe the stride is used for array allocations, but for struct layout Swift allows fields to pack into the "tail padding" of previous structures, like as follows:

[StructLayout(LayoutKind.Sequential, Size = 12)]
struct F3_S0_S0
{
    public nint F0;
    public uint F1;

    public F3_S0_S0(nint f0, uint f1)
    {
        F0 = f0;
        F1 = f1;
    }
}

[StructLayout(LayoutKind.Sequential, Pack = 16)]
public struct S
{
  public F3_S0_S0 S;
  public int X; // at offset 12
}

@lambdageek
Copy link
Member

I believe the stride is used for array allocations, but for struct layout Swift allows fields to pack into the "tail padding" of previous structures, like as follows:

that makes sense, thanks

@lambdageek
Copy link
Member

Fixed in #101529

@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label May 6, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jun 6, 2024
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

3 participants