Read DWARF registers through their guest-state offsets by mernst · Pull Request #91 · codespecs/fjalar

mernst · 2026-10-11T04:03:00Z

This pull request depends on #94, and it contains its commits. Please land #94 first. #94 removes an unused copy of the location evaluator that uses the get_reg and dwarf_reg_string tables, which this pull request deletes.

This pull request changes how Fjalar reads a register that DWARF names by number. It is a prerequisite of #90, from which it was split.

A table of guest-state offsets, dwarf_reg_guest_offsets, replaces the table of VG_(get_*) function pointers, get_reg. New functions dwarf_reg_is_readable and read_dwarf_reg use it. dwarf_reg_name (from binutils' regname) replaces the dwarf_reg_string table, whose x86 version had 9 entries where get_reg had 11.
Computing the frame base, and evaluating a DW_OP_reg* or DW_OP_breg* location, no longer call through a null or out-of-bounds entry for a register that Fjalar cannot read (such as x86 eflags or a floating-point register). Fjalar prints a debugging message instead, and such a variable's value is nonsensical.
A simple frame base of DW_OP_reg* (not just DW_OP_breg*) is handled.
The workaround for a frame base of DW_OP_deref assumed x86 (EBP+8). It now uses RBP+16 on amd64.
The entry and exit helpers declare that they read R8–R15 on amd64, so their guest values are up to date when Fjalar reads them.

At function exit, a DW_OP_reg* location, or a DW_OP_breg* location on a register other than the stack pointer, still reads the register's current value rather than its value at entry. #90 fixes that by saving the registers at entry.

The output of make fjalar-test is unchanged.

coderabbitai · 2026-10-11T04:03:11Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: codespecs/fjalar/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Advanced
Run ID: fa22326c-4a68-4e60-bee7-4bd750323362

📥 Commits

Reviewing files that changed from the base of the PR and between ea8fa94 and 50edff3.

📒 Files selected for processing (6)

valgrind/fjalar/dwarf.c
valgrind/fjalar/fjalar_include.h
valgrind/fjalar/fjalar_main.c
valgrind/fjalar/fjalar_main.h
valgrind/fjalar/fjalar_runtime.c
valgrind/fjalar/fjalar_traversal.c

💤 Files with no reviewable changes (1)

valgrind/fjalar/fjalar_main.h

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

The change adds shared helpers for decoding and reading DWARF registers. Frame-base evaluation and variable traversal use those helpers, with architecture-dependent register mappings and additional amd64 guest-state coverage. Variable matching now uses the calculated global-location or frame-offset address instead of evaluating the variable’s DWARF location expression.

Suggested reviewers: markro49

Priority: ➖ Normal

Change: Bug fix

Merge Risk | ⚪ Minimal · up to 50edf

Merge Risk: ⚪ Minimal · up to 50edf

No actionable issue remains from the examined register and variable-matching changes. The PR is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 50edf

The change bounds register reads and corrects temporary-value lifetimes without demonstrating new privileges. Handling of unavailable values has not been fully verified through every downstream consumer.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — The inspected reachability remains within existing Fjalar variable traversal and guest-state inspection. Fixing the temporary lifetime makes register-backed values usable by existing consumers, but does not itself provide arbitrary guest-state offsets or introduce a new caller authority.

Trust Boundaries and Controls

observed — DWARF-selected register numbers pass through readability checks before indexed access. Subsequent pointer traversal requires a nonzero value location; ordinary memory-backed pointer loads use allocation checks, while register-copy loads retain the existing no-guest-address convention. This does not establish the safety of every downstream callback.

Resilience and Maintainability Implications

observed — Frame-base-derived virtual-stack copying and mutation of shared FunctionEntry invocation fields remain existing lifecycle mechanisms. The PR changes their register inputs, not their ownership or cleanup design; the inspected comparison did not establish a newly introduced lifecycle vulnerability.

Pre-merge checks | 2 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 5 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

- Declare R8-R15 as read by the function-exit helper, too. - For DW_OP_bregN on the frame pointer, use its value at function entry. - Keep a register-located variable's value alive until visitVariable runs. - Share DW_OP_regN/DW_OP_bregN decoding via dwarf_op_reg_num. - Remove a redundant init_dwarf_regnames_by_elf_machine_code call. - Simplify the x86 register table; keep its size private. - Log when the CFA register cannot be read.

Also correct the comment on dwarf_reg_guest_offsets, which has no -1 entries.

returnArrayVariableWithAddr evaluated a formal parameter's DWARF location expression into var_loc but never used the result.

mernst · 2026-10-12T00:42:10Z

@coderabbitai review

coderabbitai · 2026-10-12T00:42:18Z

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

mernst · 2026-10-12T01:38:21Z

@coderabbitai review

coderabbitai · 2026-10-12T01:38:28Z

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…arf-register-access # Conflicts: # valgrind/fjalar/fjalar_runtime.c

mernst · 2026-10-12T02:52:50Z

@coderabbitai review

coderabbitai · 2026-10-12T02:52:57Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

mernst · 2026-10-12T17:34:01Z

@coderabbitai review

coderabbitai · 2026-10-12T17:34:09Z

⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…d-location-op As this pull request's description requires: * location_op_is_supported() accepts DW_OP_reg* for a register that Fjalar can read. extractOneFormalParameterVar sets locationUnsupported for any other register location, so no heuristic makes it valid. * visitVariableGroup asserts that the registers of a valid location were saved at entrance, instead of reading address 0. * The DW_OP_stack_value and DW_OP_piece tests move from omitted-params-test.sh, which codespecs#90 deletes, to register-params-test.sh. Their functions are in stack-params.c, a separate compilation unit, because an unoptimized function in register-params.c changes the comparability of scale's parameters. Use codespecs#91's FJALAR_NUM_DWARF_REGS instead of NUM_READABLE_DWARF_REGS, which counted entries of the get_reg table that codespecs#91 deletes.

Read DWARF registers through their guest-state offsets

674c534

mernst mentioned this pull request Oct 11, 2026

Trace formal parameters whose location is a register #90

Open

mernst added 5 commits October 11, 2026 16:45

Do not save the frame pointer at entry; codespecs#90 saves all registers

071458b

Also correct the comment on dwarf_reg_guest_offsets, which has no -1 entries.

Remove an unused copy of the location evaluator

f016945

returnArrayVariableWithAddr evaluated a formal parameter's DWARF location expression into var_loc but never used the result.

Rename a stale debug-message label

1f7e404

Document dwarf_op_reg_num at its definition

30557ba

Merge remote-tracking branch 'origin/remove-unused-evaluator' into dw…

50edff3

…arf-register-access # Conflicts: # valgrind/fjalar/fjalar_runtime.c

mernst requested a review from markro49 October 12, 2026 03:12

mernst assigned markro49 Oct 12, 2026

mernst mentioned this pull request Oct 13, 2026

Omit a formal parameter whose location contains an unsupported DWARF operation #95

Open

Navigation Menu

Repository navigation

Read DWARF registers through their guest-state offsets - #91

Read DWARF registers through their guest-state offsets#91
mernst wants to merge 7 commits into
codespecs:masterfrom
mernst:dwarf-register-access

mernst commented Oct 11, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 11, 2026 •

edited

Loading

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

mernst commented Oct 12, 2026

Uh oh!

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

mernst commented Oct 12, 2026

Uh oh!

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

mernst commented Oct 12, 2026

Uh oh!

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

mernst commented Oct 12, 2026

Uh oh!

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mernst commented Oct 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Oct 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

mernst commented Oct 12, 2026

Uh oh!

coderabbitai Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mernst commented Oct 12, 2026

Uh oh!

coderabbitai Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mernst commented Oct 12, 2026

Uh oh!

coderabbitai Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mernst commented Oct 12, 2026

Uh oh!

coderabbitai Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mernst commented Oct 11, 2026 •

edited

Loading

coderabbitai Bot commented Oct 11, 2026 •

edited

Loading

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading