JIT: Remove dataSection flexible array#124373
Conversation
Storing pointers and `emitLocation` instances in this `BYTE` flexible array is problematic due to alignment. We could use `alignas`, but the flexible array here is a micro optimization so just go with a simpler representation without the footguns.
github-actions
bot
added
the
area-CodeGen-coreclr
There was a problem hiding this comment.
Pull request overview
This pull request addresses a critical ARM32 crash caused by misaligned memory access in the JIT compiler's data section handling (issue #124350). The fix removes a problematic flexible array (dsCont[0]) from the dataSection struct and replaces it with a union of properly typed pointers. This eliminates alignment issues that arose from storing pointers and emitLocation instances in a BYTE array.
Changes:
- Replaced flexible array
dsCont[0]with a union containingdsData(BYTE*),dsBlocks(BasicBlock**), anddsLocations(emitLocation*) members - Updated all data section allocation code to separately allocate the descriptor and the data/blocks/locations arrays
- Updated all access sites to use the appropriate union member based on the section type
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/jit/emit.h | Replaced flexible array dsCont[0] with a union of typed pointers (dsData, dsBlocks, dsLocations) |
| src/coreclr/jit/emit.cpp | Updated memory allocation to separately allocate dataSection and data arrays; updated all access sites to use union members |
| src/coreclr/jit/emitxarch.cpp | Updated display code to use dsBlocks instead of casting dsCont |
| src/coreclr/jit/codegenlinear.cpp | Updated async resume info recording to use dsLocations |
| src/coreclr/jit/codegencommon.cpp | Updated async debug info reporting to use dsLocations |
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
It's quite a mystery to me why this wouldn't be failing widely on arm32. Any time we have a suspension we should have this misaligned load. |
|
Looks like the relevant CI leg got canceled: https://dev.azure.com/dnceng-public/public/public%20Team/_build/results?buildId=1293252&view=logs&s=6884a131-87da-5381-61f3-d7acc3b91d76&j=3873f3d5-afe1-57af-f8cc-0330a3675378, not sure if there is an easy way to trigger it |
|
/azp run runtime-libraries coreclr-outerloop |
|
No pipelines are associated with this pull request. |
|
/azp run runtime-libraries-coreclr outerloop-linux |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
PTAL @dotnet/jit-contrib |
|
/ba-g Deadletter and ILC failure is likely similar to #124370 |
4 participants
Storing pointers and
emitLocationinstances in thisBYTEflexible array is problematic due to alignment. We could usealignas, but the flexible array here is a micro optimization so just go with a simpler representation without the footguns.Fix #124350