JIT: fix DWARF register numbers and encoding for APX eGPRs - #133918
Open
DeepakRajendrakumaran wants to merge 1 commit into
Open
DeepakRajendrakumaran wants to merge 1 commit into
DeepakRajendrakumaran wants to merge 1 commit into
Conversation
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
The psABI numbers r16-r31 as 130-145; 16-31 aliased the return-address column and XMM0-XMM14. ULEB128-encode the DW_CFA_def_cfa[_register] operand so those values survive; push2 now takes one 16-byte CFA adjust.
DeepakRajendrakumaran
marked this pull request as ready for review
September 16, 2026 20:38
DeepakRajendrakumaran
requested a review
from MichalStrehovsky
as a code owner
September 16, 2026 20:38
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Unresolved findings require debug-register mapping updates, regression tests, and a comment correction.
Pull request overview
Fixes Unix DWARF/CFI generation for APX eGPRs and PUSH2 prolog instructions.
Changes:
- Corrects APX eGPR mappings to DWARF registers 130–145.
- Encodes CFA register operands as ULEB128.
- Emits accurate
PUSH2CFA adjustments and spill offsets.
File summaries
| File | Summary |
|---|---|
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/Dwarf/DwarfFde.cs |
Corrects ULEB128 encoding; automated regression coverage requested. |
src/coreclr/jit/unwindamd64.cpp |
Corrects APX eGPR mappings; debug-info register mapping also requires alignment. |
src/coreclr/jit/unwind.cpp |
Emits accurate PUSH2 CFI; automated APX regression coverage requested. |
src/coreclr/inc/cfi.h |
Documents DWARF register ranges; register-range comment needs correction. |
Review details
Suppressed comments (4)
src/coreclr/inc/cfi.h:23
- The register range in this comment is inaccurate: DWARF register 16 is the return-address (RIP) column, not a general-purpose register. Please document 0-15 as the general-purpose registers and 16 separately so the comment does not imply that R16 uses DWARF register 16.
short DwarfReg; // Dwarf register number. For x64: 0-16 general purpose and the
// return-address column, 17-32 XMM0-XMM15, and 130-145 for the
// APX eGPRs r16-r31.
src/coreclr/jit/unwind.cpp:201
- Could this add an automated regression test for the Unix APX CFI path? The existing unwind tests do not enable APX/PP2 or validate the emitted
.eh_frame, so they would not catch either the singlePUSH2CFA adjustment/slot ordering or a future regression in encoding register numbers 130–145 as ULEB128. The manual APX run described for this change is useful, but without a repeatable test these fixes can silently regress.
createCfiCode(func, cbProlog, CFI_ADJUST_CFA_OFFSET, DWARF_REG_ILLEGAL, 2 * REGSIZE_BYTES);
src/coreclr/jit/unwindamd64.cpp:76
- This APX mapping only fixes the CFI producer. The NativeAOT DWARF variable-location path still passes register numbers 16-31 through
DwarfExpressionBuilder.DwarfRegNum, where those values are defined as XMM0-XMM15 and encoded as DWARF 17-32. An APX method with debug variable info would therefore identify an R16-R31 value as the wrong register. Please update that mapping as part of this change, or explicitly scope the fix to.eh_frameand track the debug-info path separately.
// The x86-64 psABI assigns the APX eGPRs 130-145.
case REG_R16:
dwarfReg = 130;
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/Dwarf/DwarfFde.cs:85
- Please add an automated regression for the new extended-register encoding and PUSH2 CFI shape. The existing APX scenario exercises the current low-register callee-save set, so it does not produce a 130-145 operand and would not catch the original one-byte encoding desynchronizing the following CFI instructions; it also does not assert the single CFA advance and two slot offsets. A focused synthetic CFI byte-level test or an APX unwind test should cover these cases.
case CFI_OPCODE.CFI_DEF_CFA_REGISTER:
cfiCode[cfiCodeOffset++] = DW_CFA_def_cfa_register;
// The register operand is ULEB128, not a raw byte. Identical output for
// registers <= 127, but the APX eGPRs are 130-145.
cfiCodeOffset += DwarfHelper.WriteULEB128(cfiCode.AsSpan(cfiCodeOffset), (uint)dwarfReg);
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Lite
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three related fixes to the Unix CFI unwind path for APX.
1. unwindPush2Pop2CFI described PUSH2 as two separate pushes
. PUSH2 moves RSP by 2 * REGSIZE_BYTES in one step, so it needs a single CFA adjustment of the full amount. Emit one CFI_ADJUST_CFA_OFFSET plus a CFI_REL_OFFSET per callee-saved register, with reg1 at REGSIZE_BYTES and reg2 at 0, matching Intel's PUSH2 semantics ([rsp] = reg2, [rsp + 8] = reg1). This matches what LLVM emits: https://godbolt.org/z/z179oGeq5.
Details:
unwindPush2Pop2CFIwas a placeholder that calledunwindPushPopCFItwice.That is correct but describes a single instruction as two, and it left two
adjacent latent bugs on the eGPR path unexercised. Three changes:
unwind.cpp— emit oneDW_CFA_def_cfa_offsetof2 * REGSIZE_BYTESper
push2instead of two of one slot each, matching LLVM'sX86FrameLowering. IntelPUSH2 reg1, reg2stores[rsp]=reg2and[rsp+8]=reg1, soreg1takes the higher slot:CFI_REL_OFFSETgetsREGSIZE_BYTESforreg1and0forreg2.How the CFI changed
Measured with
unwindTest, a NativeAOT linux-x64 app built with--codegenopt:EnableAPX=1 + EnableApxPP2=1 + EnableApxPPHint=1, whose framesexhaust the callee-saved integer set so the JIT pairs the spills. Built and run
both ways on APX hardware.
For a prolog containing
push2p %r14,%r15(AT&T; IntelPUSH2 r15, r14):2. mapRegNumToDwarfReg assigned r16-r31 the numbers 16-31.
The x86-64 psABI assigns them 130-145; 16 is the return-address column and 17-32 are XMM0-XMM15. Emitting 16-31 would alias r16 onto the return address and r17-r31 onto XMM0-XMM14: https://lkml.rescloud.iu.edu/hypermail/linux/kernel/2605.3/09638.html.
3. DwarfFde.cs wrote the register operand of DW_CFA_def_cfa_register and DW_CFA_def_cfa as a raw byte.
That is a valid ULEB128 encoding only for values <= 127. 130 encodes as 0x82 0x01; a bare 0x82 sets the continuation bit, swallowing the next byte and desynchronising the rest of the CFI program. Use DwarfHelper.WriteULEB128, as the CFI_REL_OFFSET path already does.