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

Use number of slots intead of size in BuildBlockStore #100206

Closed
wants to merge 1 commit into from

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Mar 24, 2024

Proper fix for #99611

The #99611 crash was caused by a semi-valid struct defined in Microsoft.macOS.dll:

    [StructLayout(LayoutKind.Explicit, Size = 28)]
    private struct sockaddr_in
    {
      [FieldOffset(0)]
      private byte sin_len;
      [FieldOffset(1)]
      private byte sin_family;
      [FieldOffset(2)]
      private short sin_port;
      [FieldOffset(4)]
      private int sin_addr;
      [FieldOffset(4)]
      private uint sin6_flowinfo;
      [MarshalAs(UnmanagedType.ByValArray, SizeConst = 16)]
      [FieldOffset(8)]
      public byte[] sin6_addr8;
      [FieldOffset(24)]
      private uint sin6_scope_id;
    }

the size was explicitly set to 28 while in fact it's rounded to 32 anyway due to GC ref.
The bug basically is:

  1. LSRA: size is 28? then you don't need SIMD regs to copy it
  2. Codegen: struct has 4 slots, give me 2 SIMD regs

@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 Mar 24, 2024
@EgorBo EgorBo mentioned this pull request Mar 24, 2024
@EgorBo
Copy link
Member Author

EgorBo commented Mar 24, 2024

Although, it's possible that the problem is actually on NAOT size - what should getClassSize API return for this struct? cc @jkotas @MichalStrehovsky

28 (as requested via Size=28) or 32 (rounded to 8 because of gc field)?

@EgorBo
Copy link
Member Author

EgorBo commented Mar 24, 2024

Yeah, just checked - CoreCLR returns 32 here. NAOT - 28.

@jkotas
Copy link
Member

jkotas commented Mar 25, 2024

Yeah, just checked - CoreCLR returns 32 here. NAOT - 28.

Yes, this looks like a bug in native AOT. Opened #100220

@EgorBo
Copy link
Member Author

EgorBo commented Mar 26, 2024

Closing since this change shouldn't be needed with proper fix on VM side

@EgorBo EgorBo closed this Mar 26, 2024
@EgorBo EgorBo deleted the fix-cpblkunroll branch March 26, 2024 13:17
@github-actions github-actions bot locked and limited conversation to collaborators Apr 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants