feat(variables): add initial version of variables.c test fixture - #1038
Open
JoshuaMoelans wants to merge 10 commits into
Open
feat(variables): add initial version of variables.c test fixture#1038JoshuaMoelans wants to merge 10 commits into
variables.c test fixture#1038JoshuaMoelans wants to merge 10 commits into
Conversation
- build.sh to generate binaryu (with O0 to avoid optimizing out everything) - .snap for validating - updated test_objects with new test_elf_variables - smol README with instructions how to run/rebuild/refresh the snapshot/add coverage/uncovered items Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
JoshuaMoelans
changed the title
variables.c test fixture
JoshuaMoelans
commented
Aug 5, 2026
JoshuaMoelans
commented
Aug 5, 2026
Fold VariablesDebug's duplicate tree walk into FunctionsDebug behind DebugOptions, and reduce VariablesDebug to a Debug impl for a single function's variables, replacing write_variables. Snapshots unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JoshuaMoelans
commented
Aug 13, 2026
| # See README.md for details. | ||
| set -eu | ||
|
|
||
| gcc -g -gdwarf-5 -O0 -o ../fixtures/linux/variables variables.c |
Member
Author
There was a problem hiding this comment.
(think about how this changes the DWARF format too + maybe add second test for inlining/ 'USED' , or 'USED' macro or 'volatile'...)
Comment on lines
+75
to
+90
| /* | ||
| * Inlined functions. `always_inline` forces inlining even at -O0, which is the | ||
| * only way this fixture can produce a `DW_TAG_inlined_subroutine`. | ||
| * | ||
| * The variables of the inlinee are not supported yet: their concrete DIEs | ||
| * carry only a location and a `DW_AT_abstract_origin` reference, with name and | ||
| * type living on the abstract DIE, which symbolic does not follow for | ||
| * variables. Both `param` and `doubled` have plain frame-base locations at | ||
| * -O0, so `inlined` rendering with no variables at all in the snapshot is | ||
| * purely the missing origin lookup. | ||
| */ | ||
| static inline __attribute__((always_inline)) int inlined(int param) | ||
| { | ||
| int doubled = param * 2; | ||
| return doubled + 1; | ||
| } |
Member
Author
There was a problem hiding this comment.
(IMO we can keep the fixture itself relatively clean of this slightly noisy type of clanker-aided comments, the PR with comments/intermittent commits can tell the full story if anyone's interested)
Suggested change
| /* | |
| * Inlined functions. `always_inline` forces inlining even at -O0, which is the | |
| * only way this fixture can produce a `DW_TAG_inlined_subroutine`. | |
| * | |
| * The variables of the inlinee are not supported yet: their concrete DIEs | |
| * carry only a location and a `DW_AT_abstract_origin` reference, with name and | |
| * type living on the abstract DIE, which symbolic does not follow for | |
| * variables. Both `param` and `doubled` have plain frame-base locations at | |
| * -O0, so `inlined` rendering with no variables at all in the snapshot is | |
| * purely the missing origin lookup. | |
| */ | |
| static inline __attribute__((always_inline)) int inlined(int param) | |
| { | |
| int doubled = param * 2; | |
| return doubled + 1; | |
| } | |
| /* | |
| * Inlined variables are not supported yet; DW_AT_abstract_origin is not followed | |
| * so we don't resolve these for now | |
| */ | |
| static inline __attribute__((always_inline)) int inlined(int param) | |
| { | |
| int doubled = param * 2; | |
| return doubled + 1; | |
| } |
- Pin the Docker image to gcc:14.4.0 (the version the committed fixture was built with) instead of the floating gcc:14 tag. - Drop the false claim that ./build.sh outside Docker gives the same result: DW_AT_comp_dir embeds the build directory. - Fix the garbled rebuild instructions in the README. - Fix the primitives() comment: the enum is PrimitiveTypeEncoding, and Address is deliberately not covered. - Rebuild the fixture with the pinned image (comment edit shifted line numbers in the debug info; snapshot unchanged). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
In relative mode, saturating_sub silently clamped a location address below the function base to 0x0, which prints as a plausible-looking whole-function range. A backend emitting wrongly-based addresses (e.g. module-relative vs. absolute) would produce an accepted snapshot instead of a failure. checked_sub().expect() makes the violation panic the test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Only two of the eight nominal flag combinations were ever constructed, and they were fully correlated: variables was always true and lines was always !relative. A two-variant enum (Full / RelativeVariables) encodes only the states that exist, deletes the dead variables guard, and removes the 0-sentinel base field on VariablesDebug. No output changes; all snapshots unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cd to the script's own directory before compiling, following the precedent in fixtures/wasm/emscripten/build.sh (the POSIX-sh form; the arm64 script's BASH_SOURCE variant is a bashism). Inside the documented Docker invocation this is a no-op: the working directory is already the script directory, so DW_AT_comp_dir and the fixture binary are unchanged (verified byte-identical). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
JoshuaMoelans
changed the title
variables.c test fixturevariables.c test fixture
JoshuaMoelans
marked this pull request as ready for review
August 14, 2026 11:04
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
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.
test_objects__elf_variables.snapfor validatingtest_objectswith newtest_elf_variablesresolves https://linear.app/getsentry/issue/INGEST-1087/dwarf-add-new-test-fixture-for-variables
To be followed up with #1042 (or maybe we merge that one in here, not sure what flows more naturally?)