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

unsafe struct fails with primitive field only when at the end of the sequence #105064

Open
luislhg opened this issue Jul 18, 2024 · 7 comments
Open

Comments

@luislhg
Copy link

luislhg commented Jul 18, 2024

I have a project (tested with .NET6 and .NET8) with AllowUnsafeBlocks.
I then have a struct containing 2 fixed int arrays and a regular int.

The first two structs (SampleDataMemory_Works1 and SampleDataMemory_Works2) do work, the third one (SampleDataMemory_Crash1) crashes with System.TypeLoadException: Could not find or load a type. (0x80131522).

class and structs

public unsafe struct SampleDataMemory_Works1
{
    public int index;
    public fixed int processComplete[ClassTest.MAX_SAMPLE_BUFFER_SIZE];
    public fixed int data[ClassTest.MAX_SAMPLE_BUFFER_SIZE];
};

[StructLayout(LayoutKind.Sequential, Pack = 1)]
public unsafe struct SampleDataMemory_Works2
{
    public fixed int processComplete[ClassTest.MAX_SAMPLE_BUFFER_SIZE];
    public int index;
    public fixed int data[ClassTest.MAX_SAMPLE_BUFFER_SIZE];
};

[StructLayout(LayoutKind.Sequential, Pack = 1)]
public unsafe struct SampleDataMemory_Crash1
{
    public fixed int processComplete[ClassTest.MAX_SAMPLE_BUFFER_SIZE];
    public fixed int data[ClassTest.MAX_SAMPLE_BUFFER_SIZE];
    public int index;
};

public class ClassTest
{
    public const int MAX_SAMPLE_BUFFER_SIZE = 20000000;

    public static int GetSizeWorks1()
    {
        return Marshal.SizeOf<SampleDataMemory_Works1>();
    }

    public static int GetSizeWorks2()
    {
        return Marshal.SizeOf<SampleDataMemory_Works2>();
    }

    public static int GetSizeCrash1()
    {
        return Marshal.SizeOf<SampleDataMemory_Crash1>();
    }
}

csproj

<Project Sdk="Microsoft.NET.Sdk">
	<PropertyGroup>
		<TargetFramework>net6.0</TargetFramework>
		<ImplicitUsings>enable</ImplicitUsings>
		<Nullable>enable</Nullable>
		<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
	</PropertyGroup>
</Project>

As you can see all structs have the same size, the only thing different is the position of the the index property, all types are the same.
I wonder why we get this behaviour, assuming the size should be the same.

I can easily increase the size of the arrays even more and it still works for the first two structs, but a single int after the two fixed arrays makes it crash.

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jul 18, 2024
@huoyaoyuan
Copy link
Member

It's likely because field offset exceeding the limit. Add a more public fixed int processComplete2[ClassTest.MAX_SAMPLE_BUFFER_SIZE]; at the front of SampleDataMemory_Works2, or doubling the size of MAX_SAMPLE_BUFFER_SIZE will both result in TypeLoadException.

#95193 (comment) indicates that the maximum field offset is around 2^27. 20000000 * sizeof(int) is more than 2^26.

@steveisok steveisok removed the untriaged New issue has not been triaged by the area owner label Jul 18, 2024
@fanyang-mono
Copy link
Member

This looks like a similar issue to #97412

However, I would expect SampleDataMemory_Works1 and SampleDataMemory_Works2 trigger the TypeLoadException as well. I will look into it.

@steveisok steveisok added this to the 9.0.0 milestone Jul 18, 2024
@luislhg
Copy link
Author

luislhg commented Jul 18, 2024

It's likely because field offset exceeding the limit. Add a more public fixed int processComplete2[ClassTest.MAX_SAMPLE_BUFFER_SIZE]; at the front of SampleDataMemory_Works2, or doubling the size of MAX_SAMPLE_BUFFER_SIZE will both result in TypeLoadException.

#95193 (comment) indicates that the maximum field offset is around 2^27. 20000000 * sizeof(int) is more than 2^26.

I actually tried setting MAX_SAMPLE_BUFFER_SIZE to 1 << 25 (33.554.432) and it still fails when the field is after. If the field is positioned before then it works.

Below are new examples showing the real boundaries I found:

[StructLayout(LayoutKind.Sequential, Pack = 1)]
public unsafe struct SampleDataMemory_Works3
{
    public int index;
    public fixed int data[(1 << 25)];
};
[StructLayout(LayoutKind.Sequential, Pack = 1)]
public unsafe struct SampleDataMemory_Crash2
{
    public fixed int data[(1 << 25)];
    public int index;
};
[StructLayout(LayoutKind.Sequential, Pack = 1)]
public unsafe struct SampleDataMemory_Works4
{
    public fixed int data[(1 << 25) - 4];
    public int index;
};
[StructLayout(LayoutKind.Sequential, Pack = 1)]
public unsafe struct SampleDataMemory_Works5
{
    public fixed int data[(1 << 25) - 4];
    public fixed int data2[1 << 25];
};

As long as I declare my second field BEFORE (1 << 25) - 4 I can declare whatever I want (int, bool or even another huge array as you can see in SampleDataMemory_Works5)

@steveisok
Copy link
Member

As noted in #104393 (comment), we want to have the change apply for all runtimes. This will likely to through in .NET 10 as a result.

@steveisok steveisok modified the milestones: 9.0.0, 10.0.0 Aug 8, 2024
@steveisok
Copy link
Member

@MichalStrehovsky since nativeaot is the only runtime left, I'm assigning this issue to you.

@MichalStrehovsky
Copy link
Member

@MichalStrehovsky since nativeaot is the only runtime left, I'm assigning this issue to you.

Could you clarify what work you expect me to do? This is a customer running into this limit in the CoreCLR VM:

// Temporary values stored in FieldDesc m_dwOffset during loading
// The high 5 bits must be zero (because in field.h we steal them for other uses), so we must choose values > 0
#define FIELD_OFFSET_MAX ((1<<27)-1)
#define FIELD_OFFSET_UNPLACED FIELD_OFFSET_MAX
#define FIELD_OFFSET_UNPLACED_GC_PTR (FIELD_OFFSET_MAX-1)
#define FIELD_OFFSET_VALUE_CLASS (FIELD_OFFSET_MAX-2)
#define FIELD_OFFSET_NOT_REAL_FIELD (FIELD_OFFSET_MAX-3)

Managed type system (as used by crossgen2 or ILC) doesn't have such limit. I don't see #104393 doing anything for this. There was a suggestion from Jan to print a better exception message but even that wasn't done in that PR (and could be done in a different PR too, since it's unrelated).

@steveisok
Copy link
Member

My bad, I got crossed up between this and #97412

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants