Skip to content

Record struct packing even when falling back to auto layout #116608

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 9 commits into from
Jun 18, 2025

Conversation

jkoritzinsky
Copy link
Member

Fixes #116475

@Copilot Copilot AI review requested due to automatic review settings June 12, 2025 21:42
@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 Jun 12, 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 pull request fixes the record struct packing for cases that fall back to auto layout. Key changes include:

  • Adding new tests in StructPacking.cs (TestAction and an overload for Test) to verify expected native sizes.
  • Setting the packing size explicitly when using Auto layout in methodtablebuilder.cpp.
  • Consolidating the SetPackingSize implementation in class.h by removing a duplicate definition.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/tests/Interop/StructPacking/StructPacking.cs Adds tests for struct packing validation using different layouts.
src/coreclr/vm/methodtablebuilder.cpp Explicitly sets the packing size for auto layout cases.
src/coreclr/vm/class.h Removes the duplicate SetPackingSize to avoid redundancy.
Comments suppressed due to low confidence (2)

src/tests/Interop/StructPacking/StructPacking.cs:139

  • The new TestAction method adds tests to validate struct packing behavior when falling back to auto layout. Verify that these tests comprehensively cover the scenarios for both 64-bit and 32-bit processes.
// Test data types that invalidate managed sequential layout

src/coreclr/vm/class.h:463

  • The duplicate implementation of SetPackingSize has been removed. Ensure that all external references now use the remaining version without any side effects.
void SetPackingSize(BYTE cbPackingSize)

jkoritzinsky and others added 2 commits June 12, 2025 14:44
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@jkotas jkotas added area-TypeSystem-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jun 13, 2025
@jkoritzinsky
Copy link
Member Author

/backport to release/10.0-preview6

Copy link
Contributor

Started backporting to release/10.0-preview6: https://github.com/dotnet/runtime/actions/runs/15720419045

@jkoritzinsky
Copy link
Member Author

/ba-g failures unrelated

@jkoritzinsky jkoritzinsky enabled auto-merge (squash) June 18, 2025 18:18
@jkoritzinsky jkoritzinsky merged commit 1ba24c4 into dotnet:main Jun 18, 2025
99 of 102 checks passed
@jkoritzinsky jkoritzinsky deleted the layout-fix branch June 18, 2025 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants