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

InlineArray reports wrong size when struct size is specified #110962

Open
hez2010 opened this issue Dec 27, 2024 · 5 comments
Open

InlineArray reports wrong size when struct size is specified #110962

hez2010 opened this issue Dec 27, 2024 · 5 comments
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner

Comments

@hez2010
Copy link
Contributor

hez2010 commented Dec 27, 2024

Description

InlineArray reports wrong size when struct size is specified.

Reproduction Steps

Console.WriteLine(Unsafe.SizeOf<S>());

[InlineArray(length: 2)]
[StructLayout(LayoutKind.Sequential, Size = 42)]
struct S { byte item; }

Expected behavior

42

This should be equivalent to

Console.WriteLine(Unsafe.SizeOf<S>());

[StructLayout(LayoutKind.Sequential, Size = 42)]
struct S { byte item1, item2; }

Actual behavior

84

Regression?

No

Known Workarounds

No

Configuration

No response

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Dec 27, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Dec 27, 2024
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

@tannergooding
Copy link
Member

The design doc covers this: https://github.com/dotnet/runtime/blob/main/docs/design/features/InlineArrayAttribute.md

When InlineArray attribute is applied to a struct with one instance field, it is interpreted by the runtime as a directive to replicate the layout of the struct Length times

Notably this does lead to some unexpected scenarios such as the following not holding:

  • The size of the entire instance is the size of its element type multiplied by the Length
    Example: the following holds: sizeof(MyArray<T>) == Length * sizeof(T)

Likewise the Span<T> produced by the C# compiler means that it will index as if n * sizeof(T) not n * LayoutSize and so many of the bytes are "inaccessible" by default.


This is potentially a case that should've just been blocked, much as LayoutKind.Explicit was blocked. But its also a niche edge case on an already niche feature, where users are defining structs that couldn't really exist in C/C++ or as a "valid" ABI definition anyways, so I'm not sure its critical to change or block.

CC. @jkotas, @VSadov

@VSadov
Copy link
Member

VSadov commented Dec 27, 2024

Yes, Size=X should be blocked. It was probably missed because you’d typically assume that explicit size is not used with sequential layout.
It seems to be an unusual corner case that can only result in problems.

@elgonzo
Copy link

elgonzo commented Dec 27, 2024

This is potentially a case that should've just been blocked,

Yes, Size=X should be blocked.

If it is decided to not block StructLayout.Size, the official API documentation for InlineArrayAttribute would need to be improved. In that case, its current API documentation is misleading regarding its length parameter, particularly in these pages:

If StructLayout.Size is going to be blocked, the API documentation can be left as it currently is.

@colejohnson66
Copy link

colejohnson66 commented Dec 27, 2024

If it's an unexpected, and frankly illogical, to do what @hez2010 did, I'd personally argue that is should be blocked. If one wants to do something like this (an inline array of padded structs), they can define two types: one with an explicit size and one with the [InlineArray(n)] containing the former. That would match what one would actually have to do in C to accomplish the same thing:

// or a union of `int` and `char[42]`
struct Padded
{
    int value;
    char[38] __padding;
};
struct Actual
{
    Padded[2] elements;
};
[StructLayout(LayoutKind.Sequential, Size = 42)]
public struct Padded
{
    public int Value;
}
[InlineArray(2)]
public struct Actual
{
    private Padded _element0;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

5 participants