[SOL] Fix DW_OP_fbreg offsets for SBPFv3 stack variables - #195
Open
procdump wants to merge 1 commit into
Open
Conversation
For sBPFv3, the runtime auto-bumps R10 by FrameLength on each call so R10 ends up at the high end of the callee's frame slot, and SBFRegisterInfo::resolveInternalFrameIndex emits stack accesses as `r10 + (Offset_FI - FrameLength)` (negative displacement). The default TargetFrameLowering::getFrameIndexReference returned `Offset_FI + StackSize` instead, so DW_AT_location entries for stack-resident variables resolved to addresses in the next, uninitialized frame slot, making local variables appear as 0x0 in lldb/gdb. Override getFrameIndexReference in SBFFrameLowering to mirror the instruction-encoding adjustment so DW_OP_fbreg + N names the same byte the spill wrote. The override is gated on getHasNoStackGaps() && !getHasDynamicFrames() (i.e. v3+) and skips frame indices flagged by storeFrameIndexArgument (outgoing-call temporaries from LowerCall); SBPF stack args are listed as unsupported in the calling convention, so leaving those on the default path is fine. Affects DWARF emission (DwarfCompileUnit, LiveDebugValues) only; SBFRegisterInfo::eliminateFrameIndex still routes through resolveInternalFrameIndex, so instruction encoding is unchanged.
nagisa
reviewed
May 4, 2026
nagisa
left a comment
nagisa
left a comment
There was a problem hiding this comment.
Any chance this could have a test for the generated debuginfo?
From the looks of it tests for this tend to use llvm-dwarfdump.
Comment on lines
+69
to
+87
| // For SBPFv3+ the runtime auto-bumps R10 by FrameLength on each call so | ||
| // that R10 ends up at the high end of the callee's frame slot. | ||
| // SBFRegisterInfo::resolveInternalFrameIndex therefore emits stores as | ||
| // `r10 + (Offset_FI - FrameLength)` (a negative displacement). Mirror that | ||
| // adjustment in the DWARF location so DW_OP_fbreg + N names the same byte | ||
| // the store wrote, instead of the default `Offset_FI + StackSize` which | ||
| // resolves to an address in the next, uninitialized frame slot. | ||
| // | ||
| // The override fires only for v3+ (HasNoStackGaps && !HasDynamicFrames): | ||
| // - v1/v2 have HasDynamicFrames=true, so emitPrologue inserts an | ||
| // `add r10, -StackSize`, making R10 the low end of the frame; the | ||
| // default formula matches that. | ||
| // - v0 has HasNoStackGaps=false and short-circuits to the default branch | ||
| // below; its (gapped) layout is intentionally left untouched here. | ||
| // Stack-passed argument frame indices (containsFrameIndex) also use the | ||
| // default — their bytes live in the caller's frame, which needs a separate | ||
| // adjustment. | ||
| if (Subtarget.getHasNoStackGaps() && !Subtarget.getHasDynamicFrames() && | ||
| !SBFFuncInfo->containsFrameIndex(FI)) { |
There was a problem hiding this comment.
Hm, I would normally say to abstract this logic and decision tree into some method in SBFFunctionInfo so it can be shared between these multiple sources, but this seems harmless enough to have.
At the same time I have a feeling that needing this override at all is because we're missing setting some preexisting value somewhere (MFI.get/setOffsetAdjustment() maybe?) but figuring out what it is and if it wouldn't have unexpected effects also seems onerous enough and maybe not worth the time and effort… 🤔
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
3 participants
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.
On SBPFv3,
resolveInternalFrameIndexemitsr10 + (Offset_FI - FrameLength)(negative), but the defaultgetFrameIndexReferencereturnsOffset_FI + StackSize, soDW_OP_fbreg + Nresolves into the wrong frame slot and locals show as0x0in lldb. OverrideSBFFrameLowering::getFrameIndexReferenceto mirror the instruction-side adjustment for SBPFv3 (gated ongetHasNoStackGaps() && !getHasDynamicFrames()); skipcontainsFrameIndex(FI). It is DWARF-only related and instruction encoding is unchanged.