morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #4891] fix(ghidra): include callers that reach a function through its thunks #6459

New issue

Open

opened 2026-10-31 01:14:36 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:14:36 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4891
Author: @robotaro
Created: 10/17/2026
Status: 🔄 Open

Base: main ← Head: fix/ghidra-thunk-call-relationships

📝 Commits (1)

71f8418 fix(ghidra): include callers that reach a function through its thunks

📊 Changes

10 files changed (+245 additions, -11 deletions)

View changed files

📝 bridge/ghidra/ReaGhidraBridge.java (+38 -4)
📝 docs/ghidra-provider.md (+1 -1)
📝 docs/testing.md (+1 -0)
📝 package.json (+1 -0)
➕ scripts/verify-real-ghidra-thunk-calls.mjs (+175 -0)
📝 src/ghidra/GhidraAnalysisProfile.ts (+1 -1)
📝 src/ghidra/GhidraProvider.behavior.test.ts (+1 -1)
📝 src/ghidra/GhidraSnapshotCache.test.ts (+9 -4)
📝 tests/conformance/ghidra/README.md (+6 -0)
➕ tests/conformance/ghidra/shared-thunks.c (+12 -0)

📄 Description

Summary

In an ELF -fPIC shared library, calls between exported functions go through PLT thunks. Ghidra records the thunk's transfer to its target as a jump, so Function.getCallingFunctions omits every caller that arrives through a thunk. procedure_callers and analyze_function therefore reported no callers for nearly every exported function, with relationship_coverage.status: "complete", and call-graph traversal stopped at the PLT. No existing issue; found while analyzing a third-party 17.5 MB GCC 11 C++ shared library.

Problem and expected behavior

Smallest trigger: tests/conformance/ghidra/shared-thunks.c built with cc -shared -fPIC -O0. rea_ghidra_shared_entry calls rea_ghidra_shared_leaf@plt.

procedure_callers(rea_ghidra_shared_leaf) → [], coverage complete (expected: the entry function).
procedure_callees(<PLT thunk>) → [] (expected: the leaf).

On the large library, procedure_callers for an exported method returned [] while its PLT thunk had the real caller.

Change and scope

ReaGhidraBridge gains callingFunctions and calledFunctions, used by procedure_callers, procedure_callees and analyze_function:

callers include procedures that call any thunk of the function (getFunctionThunkAddresses(true)); the thunk itself is not listed, and its jump stays in incoming_references;
a thunk's callees include its target.

call_relationship_coverage moves to function-body-call-coverage-v6, so snapshots recorded with the old caller semantics are refused, following the existing snapshot-marker pattern. Not included: resolving callees that point at a thunk to its target (callees keep the thunk address, which carries thunk_target).

Contract and boundary impact

Semantic owner and earliest changed stage: Ghidra bridge call relationships.
Contribution scope fit and linked maintainer decision: static cross-references; not applicable.
CLI and MCP/tool-catalog contract: no schema change; caller/callee sets grow where thunks exist. Coverage diagnostics gain one sentence.
Provider, bridge, target-format, or platform compatibility: Ghidra only. Hopper and IDA are unchanged.
Evidence, artifact, provenance, or reconstruction contract: analysis profile marker call_relationship_coverage v5 → v6 (older snapshots refused).
Target selection, authority, upstream behavior, and cleanup details: read-only analysis.
Generated metadata, package, or installation impact: new verify:ghidra:thunk-calls script.

Expected effects:

Read existing files or captured data.
Launch or attach to a process. (Verification lane only: compiler and Ghidra headless.)
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated: scripts/verify-real-ghidra-thunk-calls.mjs (npm run verify:ghidra:thunk-calls) with tests/conformance/ghidra/shared-thunks.c. It checks, through MCP and the CLI, that the leaf's callers include the entry and not the thunk, that the thunk's callees include the leaf, and that the dossier callers match. GhidraSnapshotCache.test.ts refuses v5 snapshots.
Base reproduction or other evidence: with the bridge change reverted, the lane fails at the callers assertion (AssertionError: []).

User-visible CLI/MCP output (lane report):

{"entry":"0x101132","leaf":"0x101119","thunk":"0x101050","callers":["0x101132"],"thunk_callees":["0x101119"],"dossier_callers":["0x101132"],"cli_mcp_parity":true,"source_unchanged":true}

On the 17.5 MB library, an exported method's callers changed from [] to its real caller, and get_call_graph backward now reaches it.

Remaining proof gaps: Mach-O stubs and PE import thunks are not exercised; macOS dylibs bind intra-image calls directly, so the lane is Linux-only.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npm run verify:ghidra:thunk-calls (Linux x64, GCC 15.2, Ghidra 12.1.4, JDK 21.0.12) — passed, verified process cleanup; fails on origin/main's bridge.
npm run verify:ghidra:bridge-sources — passed.
npm run check:changed — 608 test files, 9762 passed, 8 skipped.
npm run docs:check — passed.
Rebased onto origin/main 23f984b1a without conflicts, then re-ran npm run verify:ghidra:thunk-calls (passed, verified process cleanup), npm run verify:ghidra:bridge-sources, npm run check:changed (10133 passed, 8 skipped) and npm run docs:check — all passed.

Compatibility, safety, and release

Breaking changes or migration steps: none for callers; older Ghidra analysis snapshots are refused because the call-relationship marker changed.
Real Hopper/Ghidra, browser, or OS coverage: real Ghidra 12.1.4 on Linux x64.
Package or release metadata impact: new npm script.
Security, privacy, process, or containment review: the lane compiles a source-owned fixture and never executes it.

Author readiness

Human owner:

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/4891 **Author:** [@robotaro](https://github.com/robotaro) **Created:** 10/17/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/ghidra-thunk-call-relationships` --- ### 📝 Commits (1) - [`71f8418`](https://github.com/morluto/rea/commit/71f841822a6bb54a012144ff9c3ca30071f624f8) fix(ghidra): include callers that reach a function through its thunks ### 📊 Changes **10 files changed** (+245 additions, -11 deletions) <details> <summary>View changed files</summary> 📝 `bridge/ghidra/ReaGhidraBridge.java` (+38 -4) 📝 `docs/ghidra-provider.md` (+1 -1) 📝 `docs/testing.md` (+1 -0) 📝 `package.json` (+1 -0) ➕ `scripts/verify-real-ghidra-thunk-calls.mjs` (+175 -0) 📝 `src/ghidra/GhidraAnalysisProfile.ts` (+1 -1) 📝 `src/ghidra/GhidraProvider.behavior.test.ts` (+1 -1) 📝 `src/ghidra/GhidraSnapshotCache.test.ts` (+9 -4) 📝 `tests/conformance/ghidra/README.md` (+6 -0) ➕ `tests/conformance/ghidra/shared-thunks.c` (+12 -0) </details> ### 📄 Description ## Summary In an ELF `-fPIC` shared library, calls between exported functions go through PLT thunks. Ghidra records the thunk's transfer to its target as a jump, so `Function.getCallingFunctions` omits every caller that arrives through a thunk. `procedure_callers` and `analyze_function` therefore reported no callers for nearly every exported function, with `relationship_coverage.status: "complete"`, and call-graph traversal stopped at the PLT. No existing issue; found while analyzing a third-party 17.5 MB GCC 11 C++ shared library. ## Problem and expected behavior Smallest trigger: `tests/conformance/ghidra/shared-thunks.c` built with `cc -shared -fPIC -O0`. `rea_ghidra_shared_entry` calls `rea_ghidra_shared_leaf@plt`. - `procedure_callers(rea_ghidra_shared_leaf)` → `[]`, coverage complete (expected: the entry function). - `procedure_callees(<PLT thunk>)` → `[]` (expected: the leaf). On the large library, `procedure_callers` for an exported method returned `[]` while its PLT thunk had the real caller. ## Change and scope `ReaGhidraBridge` gains `callingFunctions` and `calledFunctions`, used by `procedure_callers`, `procedure_callees` and `analyze_function`: - callers include procedures that call any thunk of the function (`getFunctionThunkAddresses(true)`); the thunk itself is not listed, and its jump stays in `incoming_references`; - a thunk's callees include its target. `call_relationship_coverage` moves to `function-body-call-coverage-v6`, so snapshots recorded with the old caller semantics are refused, following the existing snapshot-marker pattern. Not included: resolving callees that point at a thunk to its target (callees keep the thunk address, which carries `thunk_target`). ## Contract and boundary impact - Semantic owner and earliest changed stage: Ghidra bridge call relationships. - Contribution scope fit and linked maintainer decision: static cross-references; not applicable. - CLI and MCP/tool-catalog contract: no schema change; caller/callee sets grow where thunks exist. Coverage diagnostics gain one sentence. - Provider, bridge, target-format, or platform compatibility: Ghidra only. Hopper and IDA are unchanged. - Evidence, artifact, provenance, or reconstruction contract: analysis profile marker `call_relationship_coverage` v5 → v6 (older snapshots refused). - Target selection, authority, upstream behavior, and cleanup details: read-only analysis. - Generated metadata, package, or installation impact: new `verify:ghidra:thunk-calls` script. Expected effects: - [x] Read existing files or captured data. - [x] Launch or attach to a process. (Verification lane only: compiler and Ghidra headless.) - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `scripts/verify-real-ghidra-thunk-calls.mjs` (`npm run verify:ghidra:thunk-calls`) with `tests/conformance/ghidra/shared-thunks.c`. It checks, through MCP and the CLI, that the leaf's callers include the entry and not the thunk, that the thunk's callees include the leaf, and that the dossier callers match. `GhidraSnapshotCache.test.ts` refuses v5 snapshots. - Base reproduction or other evidence: with the bridge change reverted, the lane fails at the callers assertion (`AssertionError: []`). - User-visible CLI/MCP output (lane report): ```json {"entry":"0x101132","leaf":"0x101119","thunk":"0x101050","callers":["0x101132"],"thunk_callees":["0x101119"],"dossier_callers":["0x101132"],"cli_mcp_parity":true,"source_unchanged":true} ``` On the 17.5 MB library, an exported method's callers changed from `[]` to its real caller, and `get_call_graph` backward now reaches it. - Remaining proof gaps: Mach-O stubs and PE import thunks are not exercised; macOS dylibs bind intra-image calls directly, so the lane is Linux-only. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run verify:ghidra:thunk-calls` (Linux x64, GCC 15.2, Ghidra 12.1.4, JDK 21.0.12) — passed, verified process cleanup; fails on origin/main's bridge. - `npm run verify:ghidra:bridge-sources` — passed. - `npm run check:changed` — 608 test files, 9762 passed, 8 skipped. - `npm run docs:check` — passed. - Rebased onto origin/main `23f984b1a` without conflicts, then re-ran `npm run verify:ghidra:thunk-calls` (passed, verified process cleanup), `npm run verify:ghidra:bridge-sources`, `npm run check:changed` (10133 passed, 8 skipped) and `npm run docs:check` — all passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none for callers; older Ghidra analysis snapshots are refused because the call-relationship marker changed. - Real Hopper/Ghidra, browser, or OS coverage: real Ghidra 12.1.4 on Linux x64. - Package or release metadata impact: new npm script. - Security, privacy, process, or containment review: the lane compiles a source-owned fixture and never executes it. ## Author readiness Human owner:  - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-31 01:14:36 +00:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#6459

No description provided.

Rows
Columns