ebpf: simplify native unwinder opcodes #1092
Open
+310
−337
Conversation
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
fabled
force-pushed
the
tt-simplify-native-unwind
branch
7 times, most recently
from
f2d9746 to
5117aa9
Compare
fabled
force-pushed
the
tt-simplify-native-unwind
branch
from
5117aa9 to
79257c1
Compare
fabled
commented
Jan 20, 2026
Comment on lines
+611
to
+617
| union { | ||
| // regs is for the native unwinder to index the registers | ||
| u64 regs[16]; | ||
| // And the anonymous struct offers readable ebpf code access. | ||
| // stackdeltatypes.h must have corresponding indexes to these names. | ||
| struct { | ||
| u64 inval, cfa, pc, sp, fp, lr; |
Contributor
Author
There was a problem hiding this comment.
Technically using union here, and then the code to use alternatively both u64 regs[] and struct members to access the same data might be "invalid". But in practice this works.
If it is preferred to not have this kind of potentially invalid code, I can refactor and add perhaps #define reg_pc regs[UNWIND_REG_PC] and other similar accessors, and do the replace on all such instances.
| #define UNWIND_OPCODE_BASE_CFA_FRAME 0x06 | ||
| // An opcode flag to indicate that the value should be dereferenced | ||
| #define UNWIND_OPCODEF_DEREF 0x80 | ||
| #define UNWIND_REG_INVALID 0 |
Contributor
Author
There was a problem hiding this comment.
Wondering if all these #defines should be moved to types.h next to the places to where they apply, and just delete this file?
| u8 fpOpcode; // opcode to unwind FP | ||
| u8 flags; // flags | ||
| u8 baseReg; // base register to calculate CFA from | ||
| u8 auxBaseReg; // base register to calculate FP (x86-64) or RA[+FP] (aarch64) |
Contributor
Author
There was a problem hiding this comment.
chose aux because the meaning depends on arch; could be also baseReg2 and param2 if preferred?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Instead of having a custom set of unwinder opcoder and large switches, make the unwinder just use index to register state. This allows simplifying the
unwind_register_addressfunction to bare minimum. Based on my observation in comment #876 (comment)Additional refactor the code so that
unwind_register_addressgets simplified when the "dereference flag" is not possible/supported.On x86-64 the net reduction is -1209 instructions.
This also paves way for easier support of additional registers. One use case being the issue referenced above which triggered this work.
(This PR also includes some other PRs to avoid needing a rebasing the code in near future. Will rebase the PR as the included other commits get merged in to main.)