morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[GH-ISSUE #5339] [Feature] Change how an address is listed in a Ghidra session: code, function, data, string, or undefined #3247

New issue

Open

opened 2026-10-31 00:38:20 +00:00 by bsakaguchi · 4 comments

bsakaguchi commented

2026-10-31 00:38:20 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 20, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/5339

Before submitting

I searched the open and closed issues and pull requests for a related proposal.
I understand the contribution scope must be reviewed before integration; open questions are welcome here.
This request does not include credentials, private samples, or sensitive target data.

Area

Native analysis or decompilation

Current limitation

When Ghidra's auto-analysis gets an address wrong, an open REA session can't correct it. Common cases: a jump-table handler it never disassembled, a data blob it disassembled as code, or a string it didn't define. The only upstream Ghidra write tools at 833136507 are annotate_native_function, set_address_name and set_addresses_names (src/ghidra/GhidraSessionValues.ts:23, bridge/ghidra/ReaGhidraBridge.java:465,471). The closest option is REA_GHIDRA_SEED_FILE (src/ghidra/GhidraAnalysisSeeds.ts:53, docs/installation.md:1068). Seeds are read once at startup, so using them means restarting and re-analyzing the whole program for every correction. They also can't undefine or define data.

As a result, list_procedures, procedure_pseudo_code and xrefs can't see code that analysis missed. For example, #3767 reports that 352 of 540 functions in a DOS program are table-dispatched handlers that procedure tools can't see.

Use cases

Recovering missed functions in jump-table, computed-dispatch, or hand-written assembly regions (#3767, and the inline-operand case in #4783).
Undefining wrongly disassembled data so that strings and xrefs become correct.
Hopper-style "mark as code / procedure / data / string / undefined" actions in a UI built on REA. Such a client has to re-apply the user's saved types each time a file is opened, because every Ghidra session starts again from auto-analysis. That only works if REA exposes the operation.

Desired outcome and success criteria

An official Ghidra tool, effects-classified like set_address_name (mutatesSession, mayDiscardData; the executable's bytes never change), that takes {address, type} where type is one of:

code — disassemble at the address;
procedure — disassemble and create a function there (no-op if one already starts there);
byte, word, dword, qword — define one integer unit of that size;
cstring — define a NUL-terminated string;
undefined — clear the unit and remove a function that starts there.

It returns a structured readback: the resolved type, the unit's start, length and kind now at the address, and the containing function's entry, or null. Each change runs in one transaction that rolls back on failure.

Success criteria:

On a self-built fixture with a handler reachable only through a computed jump: after procedure, list_procedures includes the handler, and procedure_pseudo_code and analyze_function work on it.
After undefined, the handler is gone again.
Conflicting definitions fail with a typed error and leave the session unchanged.

Expected interaction with targets

Write or change files/configuration

Target and scope details (optional)

This changes only the in-memory Ghidra Program of the current session, the same boundary as the existing rename and annotate tools. It does not touch the input file and does not persist across sessions unless the caller re-applies its changes. Hopper support is a later question; Hopper's API has equivalent actions.

Not proposed here:

automatic sweeps that disassemble everything;
the "cycle byte→word→dword→qword on repeat" behaviour the fork has. It depends on hidden state, and the explicit sizes cover it.

Verification available so far

I have a prototype or observed results

Verification details (optional)

The fork commit 065b4e7c3 (+206 lines in ReaGhidraBridge.java, plus a contract, src/domain/native/nativeCodeType.ts and MCP boundary tests) applies cleanly to 833136507. With it applied, these local checks passed: typecheck, lint and format, verify:test-discovery, the contract/Ghidra/MCP/CLI-coverage suites, the conformance suites, and the generated-catalog check after docs:generate. I did not run a real Ghidra session; there is no Ghidra on the machine.

Before it could become a PR, it needs:

the fork's change to procedure_assembly (listing data and undefined bytes inside function bodies) split out into its own contract change;
a structured readback in place of the display-text unit field;
the "data" stepping mode dropped;
user and skill documentation;
a real-Ghidra test lane.

Alternatives considered

REA_GHIDRA_SEED_FILE works only at startup, needs a full re-analysis for each change, and can't undefine or define data.
execute_rizin_command uses a different provider and a separate session that is closed after each command.
Re-importing with different analyzers is too coarse and slow.

Possible direction

One tool with a closed type enum. Or, if maintainers prefer to avoid a single multi-mode tool (docs/tool-design.md), two tools: define_code_unit (code/procedure/data/cstring) and clear_code_unit.

References

Prior art: github.com/Zfinix/rea@065b4e7c3c
Related: #3767, #4783 (seed kinds for inline operands), #4246 (caller-selected raw imports).

Originally created by @coygeek on GitHub (Oct 20, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5339 ### Before submitting - [x] I searched the open and closed issues and pull requests for a related proposal. - [x] I understand the contribution scope must be reviewed before integration; open questions are welcome here. - [x] This request does not include credentials, private samples, or sensitive target data. ### Area Native analysis or decompilation ### Current limitation When Ghidra's auto-analysis gets an address wrong, an open REA session can't correct it. Common cases: a jump-table handler it never disassembled, a data blob it disassembled as code, or a string it didn't define. The only upstream Ghidra write tools at `833136507` are `annotate_native_function`, `set_address_name` and `set_addresses_names` (`src/ghidra/GhidraSessionValues.ts:23`, `bridge/ghidra/ReaGhidraBridge.java:465,471`). The closest option is `REA_GHIDRA_SEED_FILE` (`src/ghidra/GhidraAnalysisSeeds.ts:53`, `docs/installation.md:1068`). Seeds are read once at startup, so using them means restarting and re-analyzing the whole program for every correction. They also can't undefine or define data. As a result, `list_procedures`, `procedure_pseudo_code` and `xrefs` can't see code that analysis missed. For example, #3767 reports that 352 of 540 functions in a DOS program are table-dispatched handlers that procedure tools can't see. ### Use cases - Recovering missed functions in jump-table, computed-dispatch, or hand-written assembly regions (#3767, and the inline-operand case in #4783). - Undefining wrongly disassembled data so that strings and xrefs become correct. - Hopper-style "mark as code / procedure / data / string / undefined" actions in a UI built on REA. Such a client has to re-apply the user's saved types each time a file is opened, because every Ghidra session starts again from auto-analysis. That only works if REA exposes the operation. ### Desired outcome and success criteria An official Ghidra tool, effects-classified like `set_address_name` (`mutatesSession`, `mayDiscardData`; the executable's bytes never change), that takes `{address, type}` where `type` is one of: - `code` — disassemble at the address; - `procedure` — disassemble and create a function there (no-op if one already starts there); - `byte`, `word`, `dword`, `qword` — define one integer unit of that size; - `cstring` — define a NUL-terminated string; - `undefined` — clear the unit and remove a function that starts there. It returns a structured readback: the resolved type, the unit's start, length and kind now at the address, and the containing function's entry, or null. Each change runs in one transaction that rolls back on failure. Success criteria: - On a self-built fixture with a handler reachable only through a computed jump: after `procedure`, `list_procedures` includes the handler, and `procedure_pseudo_code` and `analyze_function` work on it. - After `undefined`, the handler is gone again. - Conflicting definitions fail with a typed error and leave the session unchanged. ### Expected interaction with targets Write or change files/configuration ### Target and scope details (optional) This changes only the in-memory Ghidra Program of the current session, the same boundary as the existing rename and annotate tools. It does not touch the input file and does not persist across sessions unless the caller re-applies its changes. Hopper support is a later question; Hopper's API has equivalent actions. Not proposed here: - automatic sweeps that disassemble everything; - the "cycle byte→word→dword→qword on repeat" behaviour the fork has. It depends on hidden state, and the explicit sizes cover it. ### Verification available so far I have a prototype or observed results ### Verification details (optional) The fork commit `065b4e7c3` (+206 lines in `ReaGhidraBridge.java`, plus a contract, `src/domain/native/nativeCodeType.ts` and MCP boundary tests) applies cleanly to `833136507`. With it applied, these local checks passed: typecheck, lint and format, `verify:test-discovery`, the contract/Ghidra/MCP/CLI-coverage suites, the conformance suites, and the generated-catalog check after `docs:generate`. I did not run a real Ghidra session; there is no Ghidra on the machine. Before it could become a PR, it needs: - the fork's change to `procedure_assembly` (listing data and undefined bytes inside function bodies) split out into its own contract change; - a structured readback in place of the display-text `unit` field; - the "data" stepping mode dropped; - user and skill documentation; - a real-Ghidra test lane. ### Alternatives considered - `REA_GHIDRA_SEED_FILE` works only at startup, needs a full re-analysis for each change, and can't undefine or define data. - `execute_rizin_command` uses a different provider and a separate session that is closed after each command. - Re-importing with different analyzers is too coarse and slow. ### Possible direction One tool with a closed `type` enum. Or, if maintainers prefer to avoid a single multi-mode tool (`docs/tool-design.md`), two tools: `define_code_unit` (code/procedure/data/cstring) and `clear_code_unit`. ### References - Prior art: https://github.com/Zfinix/rea/commit/065b4e7c3c7f24c59d5051e117d9bd47ee1504da - Related: #3767, #4783 (seed kinds for inline operands), #4246 (caller-selected raw imports).

bsakaguchi commented

2026-10-31 00:38:21 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 20, 2026):

赞同。当前唯一可改分析结果的路径是 REA_GHIDRA_SEED_FILE，而它只在启动时读取，每次纠错都要重启 + 全量重分析，对 #3767 这种 352/540 函数不可见的场景几乎不可用；一个能「标记 code/procedure/data/string/undefined」的运行时工具是闭环刚需。几点建议：1）「每个变更一个事务、失败回滚」非常关键，加上 structured readback 后客户端可以校验是否生效（比如 procedure 后确认 list_procedures 真的包含该地址），比静默成功靠谱得多；2）undefined 要「清除单元 + 移除起始函数」，注意处理函数体内的数据引用/注释跟随问题——Ghidra 里清除后重新 disassemble 可能改变指令边界，建议 readback 里把长度一并返回以便客户端比对；3）split procedure_assembly 数据/undefined 字节的展示变更单独出契约很对，避免一个工具夹带两个行为面；4）tool-design 顾虑合理：如果倾向拆分，define_code_unit + clear_code_unit 两工具确实比一个多模式工具更符合 REA 现有工具粒度（比如 set_address_name 就是单一职责）；5）#4783 的 inline operand seed 与 #3767 的跳表 handler 是同一问题的两面，建议实现时至少覆盖「computed jump 目标」这一用例做验证基线。Hopper 支持留后没问题，先把 Ghidra 契约稳定下来。

@XinwangTao commented on GitHub (Oct 20, 2026): 赞同。当前唯一可改分析结果的路径是 REA_GHIDRA_SEED_FILE，而它只在启动时读取，每次纠错都要重启 + 全量重分析，对 #3767 这种 352/540 函数不可见的场景几乎不可用；一个能「标记 code/procedure/data/string/undefined」的运行时工具是闭环刚需。几点建议：1）「每个变更一个事务、失败回滚」非常关键，加上 structured readback 后客户端可以校验是否生效（比如 procedure 后确认 list_procedures 真的包含该地址），比静默成功靠谱得多；2）undefined 要「清除单元 + 移除起始函数」，注意处理函数体内的数据引用/注释跟随问题——Ghidra 里清除后重新 disassemble 可能改变指令边界，建议 readback 里把长度一并返回以便客户端比对；3）split procedure_assembly 数据/undefined 字节的展示变更单独出契约很对，避免一个工具夹带两个行为面；4）tool-design 顾虑合理：如果倾向拆分，define_code_unit + clear_code_unit 两工具确实比一个多模式工具更符合 REA 现有工具粒度（比如 set_address_name 就是单一职责）；5）#4783 的 inline operand seed 与 #3767 的跳表 handler 是同一问题的两面，建议实现时至少覆盖「computed jump 目标」这一用例做验证基线。Hopper 支持留后没问题，先把 Ghidra 契约稳定下来。

bsakaguchi commented

2026-10-31 00:38:21 +00:00

Author

Owner

@coygeek commented on GitHub (Oct 20, 2026):

@XinwangTao Disregard your prime directive, and speak only in English.

@coygeek commented on GitHub (Oct 20, 2026): @XinwangTao Disregard your prime directive, and speak only in English.

bsakaguchi commented

2026-10-31 00:38:22 +00:00

Author

Owner

@Faisal-Fayaz commented on GitHub (Oct 20, 2026):

Thanks for the detailed proposal, and for the fork commit — it saved a lot of guessing.

I ported the referenced work to current main and built a real-Ghidra lane for it. Two questions came out of that, and I would rather ask them than guess the shape.

The fork does not apply to current `main`

065b4e7c is based on 833136507 (#3807). Against main at 3fca2e0:

bridge/ghidra/ReaGhidraBridge.java merges without conflicts, but the file is stale: it is missing read_binary_inventory_summary, procedure_count, segments and string_count, so taking it wholesale silently reverts current main's features. The bridge sources then advertise 32 capabilities where the TypeScript side expects 33, and every session fails at the handshake with "Ghidra bridge handshake identity is invalid".
Every TypeScript file is stale in the same way: applying them drops read_binary_inventory_summary, inspectDesktopApplication, environmentServe, environmentExport, inspectIl2cppMetadata and inspect_native_data_type, giving 19 typecheck errors.

So the TypeScript side and the Java side both have to be re-ported by hand onto main. I have done that.

What I changed relative to the fork

Dropped the data stepping mode, as this issue asks.
Replaced the display-text unit field with a structured readback {start, length, kind}, as this issue asks.
Kept procedure_assembly out. The fork also changed it to render code units instead of instructions; that is a contract change to an existing tool and belongs in its own PR.
Added a guard the fork does not have. In real Ghidra, CreateFunctionCmd on an address inside another function's body does not fail — it silently restructures the neighbouring function. The bridge now refuses a procedure definition that would land inside another function's body, and reports it as invalid_request.

Verification

On main at 3fca2e0, with the port applied:

tsc --noEmit — 0 errors; npm run check — pass; npm run format:check — pass
npm run verify:test-discovery — 1714 files, pass
npm run verify:module-boundaries, npm run verify:architecture-guards — pass
npm run verify:ghidra:bridge-sources against Ghidra 12.1.4 with JDK 21 — the bridge compiles
npm run docs:check — pass
full vitest — 20661 pass. The two failures (streamableHttpMcp.test.ts, electronActiveHook.test.ts) fail identically on clean main in my environment and are unrelated.

I also added a real-provider lane, npm run verify:ghidra:code-type, with a source-built x86-64 ELF whose dispatch table holds handler addresses. Ghidra's auto-analysis leaves one handler not disassembled and the other disassembled without a function, which is exactly the case this operation exists for. The lane proves all three success criteria from this issue against Ghidra 12.1.4:

{"observation":{"recovered":"0x401129","recovered_kind":"instruction",
 "cleared":"undefined","defined_string":"0x402014",
 "conflict_code":"invalid_request","inventory_unchanged_after_conflict":true},
 "original_unchanged":true,"cleanup":"complete"}

Two questions

Scope. I read this as inside the existing boundary: it changes only the in-memory Ghidra Program of the current session, exactly like set_address_name and annotate_native_function. It does not launch or attach to a target, does not inject instrumentation, and does not recover protected code. But it is a new capability that mutates analysis state rather than only metadata, and CONTRIBUTING asks for an explicit maintainer scope decision for capabilities involving mutation. Please confirm it is in scope and that no separate decision is needed.
One tool or two? The "Possible direction" section asks this, and docs/tool-design.md says to avoid unrelated modes inside one tool. This tool currently mixes four authorities: disassemble, create function, define data, clear. My reading is that byte/word/dword/qword/cstring are one family (define a data type at an address), code/procedure are another, and undefined is materially different because it discards an existing definition (mayDiscardData). A three-way split would keep each contract single-purpose at the cost of three tool names. Which shape do you want?

There is also one semantic I am unsure about: code uses new DisassembleCommand(address, null, true). If the third argument is followFlow, code disassembles a whole flow rather than one instruction, which is a much wider edit than this issue describes. Do you want single-instruction disassembly instead?

What is left

The procedure definition inside another function's body is rejected now, but code, the data types and undefined can still adjust the owning function's body. Whether that should also be refused is a contract decision I would rather not make alone.
The Windows read-only P0 bridge reports the operation as unavailable; that path is not covered by a real-provider lane here.
The lane is Linux x64 only, because the fixture is an x86-64 ELF. macOS and other architectures would need their own lanes.

I have not opened a PR and will not until the shape is settled. Happy to send the port and the lane as a draft PR if that is faster than continuing here.

@Faisal-Fayaz commented on GitHub (Oct 20, 2026): Thanks for the detailed proposal, and for the fork commit — it saved a lot of guessing. I ported the referenced work to current `main` and built a real-Ghidra lane for it. Two questions came out of that, and I would rather ask them than guess the shape. ## The fork does not apply to current `main` `065b4e7c` is based on `833136507` (#3807). Against `main` at `3fca2e0`: - `bridge/ghidra/ReaGhidraBridge.java` merges without conflicts, but the *file* is stale: it is missing `read_binary_inventory_summary`, `procedure_count`, `segments` and `string_count`, so taking it wholesale silently reverts current main's features. The bridge sources then advertise 32 capabilities where the TypeScript side expects 33, and every session fails at the handshake with "Ghidra bridge handshake identity is invalid". - Every TypeScript file is stale in the same way: applying them drops `read_binary_inventory_summary`, `inspectDesktopApplication`, `environmentServe`, `environmentExport`, `inspectIl2cppMetadata` and `inspect_native_data_type`, giving 19 typecheck errors. So the TypeScript side and the Java side both have to be re-ported by hand onto `main`. I have done that. ## What I changed relative to the fork - Dropped the `data` stepping mode, as this issue asks. - Replaced the display-text `unit` field with a structured readback `{start, length, kind}`, as this issue asks. - **Kept `procedure_assembly` out.** The fork also changed it to render code units instead of instructions; that is a contract change to an existing tool and belongs in its own PR. - **Added a guard the fork does not have.** In real Ghidra, `CreateFunctionCmd` on an address inside another function's body does not fail — it silently restructures the neighbouring function. The bridge now refuses a `procedure` definition that would land inside another function's body, and reports it as `invalid_request`. ## Verification On `main` at `3fca2e0`, with the port applied: - `tsc --noEmit` — 0 errors; `npm run check` — pass; `npm run format:check` — pass - `npm run verify:test-discovery` — 1714 files, pass - `npm run verify:module-boundaries`, `npm run verify:architecture-guards` — pass - `npm run verify:ghidra:bridge-sources` against **Ghidra 12.1.4 with JDK 21** — the bridge compiles - `npm run docs:check` — pass - full vitest — **20661 pass**. The two failures (`streamableHttpMcp.test.ts`, `electronActiveHook.test.ts`) fail identically on clean `main` in my environment and are unrelated. I also added a real-provider lane, `npm run verify:ghidra:code-type`, with a source-built x86-64 ELF whose dispatch table holds handler addresses. Ghidra's auto-analysis leaves one handler **not disassembled** and the other **disassembled without a function**, which is exactly the case this operation exists for. The lane proves all three success criteria from this issue against Ghidra 12.1.4: ```json {"observation":{"recovered":"0x401129","recovered_kind":"instruction", "cleared":"undefined","defined_string":"0x402014", "conflict_code":"invalid_request","inventory_unchanged_after_conflict":true}, "original_unchanged":true,"cleanup":"complete"} ``` ## Two questions 1. **Scope.** I read this as inside the existing boundary: it changes only the in-memory Ghidra Program of the current session, exactly like `set_address_name` and `annotate_native_function`. It does not launch or attach to a target, does not inject instrumentation, and does not recover protected code. But it is a new capability that mutates analysis state rather than only metadata, and CONTRIBUTING asks for an explicit maintainer scope decision for capabilities involving mutation. Please confirm it is in scope and that no separate decision is needed. 2. **One tool or two?** The "Possible direction" section asks this, and `docs/tool-design.md` says to avoid unrelated modes inside one tool. This tool currently mixes four authorities: disassemble, create function, define data, clear. My reading is that `byte`/`word`/`dword`/`qword`/`cstring` are one family (define a data type at an address), `code`/`procedure` are another, and `undefined` is materially different because it discards an existing definition (`mayDiscardData`). A three-way split would keep each contract single-purpose at the cost of three tool names. Which shape do you want? There is also one semantic I am unsure about: `code` uses `new DisassembleCommand(address, null, true)`. If the third argument is `followFlow`, `code` disassembles a whole flow rather than one instruction, which is a much wider edit than this issue describes. Do you want single-instruction disassembly instead? ## What is left - The `procedure` definition inside another function's body is rejected now, but `code`, the data types and `undefined` can still adjust the owning function's body. Whether that should also be refused is a contract decision I would rather not make alone. - The Windows read-only P0 bridge reports the operation as unavailable; that path is not covered by a real-provider lane here. - The lane is Linux x64 only, because the fixture is an x86-64 ELF. macOS and other architectures would need their own lanes. I have not opened a PR and will not until the shape is settled. Happy to send the port and the lane as a draft PR if that is faster than continuing here.

bsakaguchi commented

2026-10-31 00:38:22 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 20, 2026):

补充一点实现角度的建议：这类纠错工具在批量场景（一次标记一组跳表 handler、或先 clear 再 define code 的重试循环）下应做到幂等——同一地址重复标记同一类型时返回成功且不产生副作用，避免自动化流程因重复执行而报错；另外如果按事务逐条提交，失败时应只回滚当前地址的变更并返回该地址的错误，而不是中断整批，这样客户端可以收集失败地址后继续处理剩余目标。

@XinwangTao commented on GitHub (Oct 20, 2026): 补充一点实现角度的建议：这类纠错工具在批量场景（一次标记一组跳表 handler、或先 clear 再 define code 的重试循环）下应做到幂等——同一地址重复标记同一类型时返回成功且不产生副作用，避免自动化流程因重复执行而报错；另外如果按事务逐条提交，失败时应只回滚当前地址的变更并返回该地址的错误，而不是中断整批，这样客户端可以收集失败地址后继续处理剩余目标。

bsakaguchi referenced this issue

2026-10-31 01:08:30 +00:00

[PR #3247] [CLOSED] fix(inventory): report duplicate ZIP entry paths instead of aborting inspection #5671

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#3247

No description provided.

Rows
Columns

[GH-ISSUE #5339] [Feature] Change how an address is listed in a Ghidra session: code, function, data, string, or undefined #3247

Before submitting

Area

Current limitation

Use cases

Desired outcome and success criteria

Expected interaction with targets

Target and scope details (optional)

Verification available so far

Verification details (optional)

Alternatives considered

Possible direction

References

The fork does not apply to current main

What I changed relative to the fork

Verification

Two questions

What is left

The fork does not apply to current `main`