Skip to content

fix(bloat): drop intra-function jump labels from call graph (#484) - #486

Merged
zackees merged 1 commit into
mainfrom
fix/issue-484-intra-function-jump-labels
Jun 7, 2026
Merged

fix(bloat): drop intra-function jump labels from call graph (#484)#486
zackees merged 1 commit into
mainfrom
fix/issue-484-intra-function-jump-labels

Conversation

@zackees

Copy link
Copy Markdown
Member

Closes#484.

Summary

objdump -d annotates branches landing at known offsets inside an
existing function as <funcname+0xNN> (basic-block branches,
switch-table dispatch, loop back-edges). The pre-existing
is_real_call_target filter caught ARM mapping symbols, PLT shims,
and bare hex addresses — but not the +0xNN shape. As a result,
references_to and (via invert()) called_by were polluted with
non-call entries.

Discovered while verifying #478 end-to-end on the FastLED esp32s3 ELF
fl::basic_string::materialize() showed 17+ +0xNN entries.

Fix

In callgraph::is_real_call_target: drop any token whose tail matches
+0x<hex>. Conservative — doesn't try to recover the underlying
in-function offset (different bug, different fix).

Test plan

  • intra_function_jump_labels_are_droppedparse_disasm and
    the inverted map both clean of +0x entries.
  • plus_without_hex_suffix_passes_through — guards against an
    over-aggressive substring match (name+oddstuff survives).
  • All existing callgraph corpus tests (Xtensa / RISC-V / ARM /
    AVR / mapping-symbols / PLT / hex-address / etc.) stay green.
  • soldr cargo test -p fbuild-core --lib symbol_analysis::callgraph (16 passed)
  • soldr cargo clippy --workspace --all-targets -- -D warnings

🤖 Generated with Claude Code

objdump annotates branches that land inside an existing function as
`<funcname+0xNN>` (basic-block branches, switch-table dispatch, loop
back-edges). These are not calls — `is_real_call_target` already
filters the analogous noise sources (ARM mapping symbols, PLT shims,
bare hex addresses) but didn't catch the `+0xNN` shape, so the labels
leaked into `references_to` and propagated through `invert()` into
spurious `called_by` entries.
Now drops any token whose tail matches `+0x<hex>` from the call edge
set. Two regression tests:
- `intra_function_jump_labels_are_dropped` proves both `parse_disasm`
and the inverted map are clean.
- `plus_without_hex_suffix_passes_through` guards against an
over-aggressive substring match (`name+oddstuff` is not a jump
label and must survive).
Discovered during end-to-end verification of #478 on the FastLED
esp32s3 ELF, where `fl::basic_string::materialize()` showed 17+ such
`+0xNN` entries.
Closes#484
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

Copy link
Copy Markdown

Warning

Review limit reached

@zackees, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 9 minutes and 3 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fd68280b-efab-4dc3-9fa1-bbc1a7a743d4

📥 Commits

Reviewing files that changed from the base of the PR and between b88badf and 3e40ac1.

📒 Files selected for processing (1)
  • crates/fbuild-core/src/symbol_analysis/callgraph.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-484-intra-function-jump-labels

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@zackees
zackees merged commit 4abd6d6 into mainJun 7, 2026
87 checks passed
@zackees
zackees deleted the fix/issue-484-intra-function-jump-labels branch June 7, 2026 19:53
zackees added a commit that referenced this pull request Jun 7, 2026
Picks up:
- #480 — per-symbol back-references (`called_by`) + `fbuild bloat lookup`
- #486 — drop intra-function jump labels (`<funcname+0xNN>`) from the
call graph so `references_to` / `called_by` stay honest
- #482 — truncate subprocess output fed back to Claude in hooks
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

bloat: filter intra-function jump labels (<funcname+0xNN>) from references_to / called_by

1 participant

@zackees