Skip to content

Skip static fields in FieldLayoutIntervalCalculator #115987

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

Merged
merged 1 commit into from
May 26, 2025

Conversation

MichalStrehovsky
Copy link
Member

It was reported on discord that:

using System.Runtime.InteropServices;

ref struct Inner1
{
    public const string SomeStr = "AAA";
}

ref struct Inner2
{
    public Inner1 F;
}

[StructLayout(LayoutKind.Explicit, Size = 100)]
ref struct Outer
{
    [FieldOffset(0)]
    public Inner2 F;
}


class Program
{
    static void Main(string[] args)
    {
        Outer x = new Outer();
        x.F.F = new Inner1();
        Console.WriteLine(Inner1.SomeStr);
    }
}

Will throw a "System.TypeLoadException: Could not load type 'Outer' from assembly 'TestProj, Version=1.0.0.0, Culture=neutral, PublicKey=null' because it contains an object field at offset '2147483647' that is incorrectly aligned or overlapped by a non-object field" at runtime in .NET 9.

It does not throw that on .NET 10, presumably thanks to #111584, but psychic debugging tells me we still have an issue here.

It was reported on discord that:

```csharp
using System.Runtime.InteropServices;

ref struct Inner1
{
    public const string SomeStr = "AAA";
}

ref struct Inner2
{
    public Inner1 F;
}

[StructLayout(LayoutKind.Explicit, Size = 100)]
ref struct Outer
{
    [FieldOffset(0)]
    public Inner2 F;
}


class Program
{
    static void Main(string[] args)
    {
        Outer x = new Outer();
        x.F.F = new Inner1();
        Console.WriteLine(Inner1.SomeStr);
    }
}
```

Will throw a `System.TypeLoadException: Could not load type 'Outer' from assembly 'TestProj, Version=1.0.0.0, Culture=neutral, PublicKey=null' because it contains an object field at offset '2147483647' that is incorrectly aligned or overlapped by a non-object field` at runtime in .NET 9.

It does not throw that on .NET 10, presumably thanks to #111584, but psychic debugging tells me we still have an issue here.
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 26, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses a runtime layout exception by ensuring that static fields are not considered when computing field layout intervals.

  • Added a guard to skip static fields in AddToFieldLayout
  • Prevents misaligned object fields caused by constants or other static members
Comments suppressed due to low confidence (1)

src/coreclr/tools/Common/TypeSystem/Common/FieldLayoutIntervalCalculator.cs:138

  • Consider adding a unit test to verify that static fields (including const and readonly) are properly skipped to prevent layout exceptions in nested types.
if (field.IsStatic)

@@ -135,6 +135,9 @@ private void AddToFieldLayout(List<FieldLayoutInterval> fieldLayout, int offset,

foreach (FieldDesc field in fieldType.GetFields())
{
Copy link
Preview

Copilot AI May 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Add a brief comment above this check explaining that static fields are skipped to avoid invalid object field offsets during layout computation.

Suggested change
{
{
// Skip static fields as they do not have valid offsets in the object layout.

Copilot uses AI. Check for mistakes.

@MichalStrehovsky MichalStrehovsky merged commit 6a32ee4 into main May 26, 2025
94 checks passed
@MichalStrehovsky MichalStrehovsky deleted the MichalStrehovsky-patch-1 branch May 26, 2025 20:37
@github-actions github-actions bot locked and limited conversation to collaborators Jun 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants