morluto/rea

Fork 0

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

[GH-ISSUE #4783] [Design] Seeds for instructions followed by inline operands (e.g. 68000 trap parameters) #2487

New issue

Open

opened 2026-10-17 21:05:34 +00:00 by bsakaguchi · 5 comments

bsakaguchi commented

2026-10-17 21:05:34 +00:00

Owner

Originally created by @onionmixer on GitHub (Oct 17, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/4783

Before submitting

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

Decision needed

Would a seed kind for instructions followed by inline operands be welcome in REA_GHIDRA_SEED_FILE? I am also unsure whether the seed script should be allowed to clear decodes that contradict such a seed. If this does not fit REA's direction, I would understand.

Context and evidence

Observed. Some 68000 programs use trap #n as a system call and place one or more parameter words directly after the instruction. The handler reads them through the stacked PC and advances it. Ghidra cannot know this, so it decodes the parameter words as instructions. The real instruction stream after the trap becomes misaligned, and function bodies stop early.

I hit this with a 258 KB Mega-CD main-CPU program, analysed with the raw m68k lane (#4018). That program uses traps #0–#3, #5–#7 and #9 with a known number of inline words. The program is commercial game code, so I cannot share it.

What the current seeds can and cannot do (source-derived). function / code / label seeds add starting points, but they cannot tell Ghidra that the bytes after an instruction are data and that execution resumes later. code seeds also refuse addresses already inside an instruction. In the raw lane, ReaGhidraPrepareRaw.java disassembles from the entry before seeds run (related: #4246), so the misaligned decodes usually exist by the time seeds are applied.

Prototype (observed, local only). I tried a local change on rea-agents-6.5.0 + #4018. It adds a seed line of the form 0x<addr>\tfallthrough\t0x<target>. The seed script then does four things:

It decodes the instruction at addr alone (DisassembleCommand with followFlow=false).
It marks the bytes up to target as data words.
It calls Instruction.setFallThrough(target) and disassembles target.
Before that, it clears contradicting decodes, with bounds:
- instructions overlapping the operand bytes or the next few real instructions (measured with PseudoDisassembler)
- the stale fallthrough chain that started inside the operands, up to 32 instructions; it never clears an address that is itself a function or fallthrough seed

I generated the seeds for this program with an external script that follows the trap conventions. I then compared Ghidra's instruction starts after full analysis plus ReaGhidraNoReturnFix.java against a trap-aware recursive disassembly of the same image (29,557 instructions):

	Instruction starts matching	Misaligned starts	Trap operand words decoded as code	Functions
6.5.0 + #4018, no seeds	72.1%	220	157	1,022
+ function and fallthrough seeds	98.8%	0	0	1,313

The comparison reference is my own disassembly, so these numbers only show agreement with it, not ground truth. I also have not separated the effect of the function seeds from the fallthrough seeds.

The existing seed unit tests (updated) and verify:ghidra:seeds, :m68k, :raw and :6502 passed locally on Linux (Ghidra 12.1, JDK 21). I have not written a real-Ghidra regression for the new kind yet.

Unknown.

Whether clearing existing decodes from a seed script is acceptable for REA's evidence model, even though the changes stay inside the ephemeral program.
Whether the bounds above are reasonable for other architectures.
Whether a separate base address (#4246) would make the clearing step unnecessary in the raw lane.

Expected interaction with targets

Read existing files or captured data

Boundary details (optional)

The change only affects how REA's private, ephemeral Ghidra program is prepared before auto-analysis. It does not execute the target, change the source file, or contact the network.

As with existing seeds, the result would be reported as caller assertions (counts of applied and failed fallthrough seeds), not as Ghidra discoveries. A malformed seed would fail before Ghidra starts.

Options and tradeoffs

A. Do nothing in REA. Callers could pre-process images outside REA, but the misaligned decodes would remain in REA's own analysis.
B. Fallthrough seeds without clearing. This is simpler. It only works where nothing has decoded the operand bytes yet, which in the raw lane is rarely the case unless #4246 changes the entry handling.
C. Fallthrough seeds with bounded clearing, as in the prototype. This works in my case, but the clearing behaviour needs careful review.
D. A more general "data range" seed plus a separate fallthrough override. This might be cleaner, but it is more to specify.

Verification available so far

I have a prototype or observed results

Success criteria and open questions

A possible real-Ghidra check would use a small synthetic m68k image: a trap followed by one inline word, then a bsr and an rts. It would check that, with the seed, the function body includes the instructions after the trap. It would also check that the inline word is data and the seed outcome counts are reported. I would be happy to prepare this fixture and a draft PR if the direction is acceptable, and to change the design based on your guidance.

References

Prototype commit (based on rea-agents-6.5.0 + #4018 cherry-picked; not rebased on main): https://github.com/onionmixer/rea/commits/prototype/fallthrough-seeds-6.5.0
Related: #4246 (load base separate from entry), #4018 (raw m68k lane), #2326 (function bodies that stop short)
Files touched in the prototype: bridge/ghidra/ReaGhidraApplySeeds.java, src/ghidra/GhidraAnalysisSeeds.ts and its tests

_{Prepared with help from an AI coding assistant (Claude Code); I have checked the numbers against my local runs.}

Originally created by @onionmixer on GitHub (Oct 17, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4783 ### Before submitting - [X] I searched the open and closed issues and pull requests for a related discussion. - [X] I understand the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope) must be reviewed before integration; open questions are welcome here. - [X] This discussion does not include credentials, private samples, or sensitive target data. ### Decision needed Would a seed kind for instructions followed by **inline operands** be welcome in `REA_GHIDRA_SEED_FILE`? I am also unsure whether the seed script should be allowed to clear decodes that contradict such a seed. If this does not fit REA's direction, I would understand. ### Context and evidence **Observed.** Some 68000 programs use `trap #n` as a system call and place one or more parameter words directly after the instruction. The handler reads them through the stacked PC and advances it. Ghidra cannot know this, so it decodes the parameter words as instructions. The real instruction stream after the trap becomes misaligned, and function bodies stop early. I hit this with a 258 KB Mega-CD main-CPU program, analysed with the raw m68k lane (#4018). That program uses traps #0–#3, #5–#7 and #9 with a known number of inline words. The program is commercial game code, so I cannot share it. **What the current seeds can and cannot do (source-derived).** `function` / `code` / `label` seeds add starting points, but they cannot tell Ghidra that the bytes after an instruction are data and that execution resumes later. `code` seeds also refuse addresses already inside an instruction. In the raw lane, `ReaGhidraPrepareRaw.java` disassembles from the entry before seeds run (related: #4246), so the misaligned decodes usually exist by the time seeds are applied. **Prototype (observed, local only).** I tried a local change on `rea-agents-6.5.0` + #4018. It adds a seed line of the form `0x<addr>\tfallthrough\t0x<target>`. The seed script then does four things: 1. It decodes the instruction at `addr` alone (`DisassembleCommand` with `followFlow=false`). 2. It marks the bytes up to `target` as data words. 3. It calls `Instruction.setFallThrough(target)` and disassembles `target`. 4. Before that, it clears contradicting decodes, with bounds: - instructions overlapping the operand bytes or the next few real instructions (measured with `PseudoDisassembler`) - the stale fallthrough chain that started inside the operands, up to 32 instructions; it never clears an address that is itself a function or fallthrough seed I generated the seeds for this program with an external script that follows the trap conventions. I then compared Ghidra's instruction starts after full analysis plus `ReaGhidraNoReturnFix.java` against a trap-aware recursive disassembly of the same image (29,557 instructions): | | Instruction starts matching | Misaligned starts | Trap operand words decoded as code | Functions | |---|---|---|---|---| | 6.5.0 + #4018, no seeds | 72.1% | 220 | 157 | 1,022 | | + function and fallthrough seeds | 98.8% | 0 | 0 | 1,313 | The comparison reference is my own disassembly, so these numbers only show agreement with it, not ground truth. I also have not separated the effect of the function seeds from the fallthrough seeds. The existing seed unit tests (updated) and `verify:ghidra:seeds`, `:m68k`, `:raw` and `:6502` passed locally on Linux (Ghidra 12.1, JDK 21). I have not written a real-Ghidra regression for the new kind yet. **Unknown.** - Whether clearing existing decodes from a seed script is acceptable for REA's evidence model, even though the changes stay inside the ephemeral program. - Whether the bounds above are reasonable for other architectures. - Whether a separate base address (#4246) would make the clearing step unnecessary in the raw lane. ### Expected interaction with targets Read existing files or captured data ### Boundary details (optional) The change only affects how REA's private, ephemeral Ghidra program is prepared before auto-analysis. It does not execute the target, change the source file, or contact the network. As with existing seeds, the result would be reported as caller assertions (counts of applied and failed fallthrough seeds), not as Ghidra discoveries. A malformed seed would fail before Ghidra starts. ### Options and tradeoffs - **A. Do nothing in REA.** Callers could pre-process images outside REA, but the misaligned decodes would remain in REA's own analysis. - **B. Fallthrough seeds without clearing.** This is simpler. It only works where nothing has decoded the operand bytes yet, which in the raw lane is rarely the case unless #4246 changes the entry handling. - **C. Fallthrough seeds with bounded clearing**, as in the prototype. This works in my case, but the clearing behaviour needs careful review. - **D. A more general "data range" seed plus a separate fallthrough override.** This might be cleaner, but it is more to specify. ### Verification available so far I have a prototype or observed results ### Success criteria and open questions A possible real-Ghidra check would use a small synthetic m68k image: a `trap` followed by one inline word, then a `bsr` and an `rts`. It would check that, with the seed, the function body includes the instructions after the trap. It would also check that the inline word is data and the seed outcome counts are reported. I would be happy to prepare this fixture and a draft PR if the direction is acceptable, and to change the design based on your guidance. ### References - Prototype commit (based on `rea-agents-6.5.0` + #4018 cherry-picked; not rebased on main): https://github.com/onionmixer/rea/commits/prototype/fallthrough-seeds-6.5.0 - Related: #4246 (load base separate from entry), #4018 (raw m68k lane), #2326 (function bodies that stop short) - Files touched in the prototype: `bridge/ghidra/ReaGhidraApplySeeds.java`, `src/ghidra/GhidraAnalysisSeeds.ts` and its tests <sub>Prepared with help from an AI coding assistant (Claude Code); I have checked the numbers against my local runs.</sub>

bsakaguchi commented

2026-10-31 00:34:28 +00:00

Author

Owner

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

The 72.1% → 98.8% instruction-start agreement is a strong argument for the seed kind, and the bounded clearing (never touching function/fallthrough seeds, capping at 32 instructions) addresses the main safety concern I'd have had. On the open question about whether the seed script may clear contradicting decodes: I'd say yes, but only within the documented bounds — since the operand-bytes decode is definitionally wrong when a trap convention is known, leaving it would keep the misalignment this issue exists to fix. One suggestion: make the fallthrough seed explicitly ordered after function/code seeds in the file so users don't have to reason about application order, and consider emitting a warning when a fallthrough target lands inside an existing function body, since that usually indicates a wrong operand count rather than a legit pattern.

@XinwangTao commented on GitHub (Oct 18, 2026): The 72.1% → 98.8% instruction-start agreement is a strong argument for the seed kind, and the bounded clearing (never touching function/fallthrough seeds, capping at 32 instructions) addresses the main safety concern I'd have had. On the open question about whether the seed script may clear contradicting decodes: I'd say yes, but only within the documented bounds — since the operand-bytes decode is definitionally wrong when a trap convention is known, leaving it would keep the misalignment this issue exists to fix. One suggestion: make the fallthrough seed explicitly ordered after function/code seeds in the file so users don't have to reason about application order, and consider emitting a warning when a fallthrough target lands inside an existing function body, since that usually indicates a wrong operand count rather than a legit pattern.

bsakaguchi commented

2026-10-31 00:34:28 +00:00

Author

Owner

@onionmixer commented on GitHub (Oct 19, 2026):

Thank you very much for reading the proposal so carefully and for the suggestions.

Ordering. I am sorry this was not clear from the description. In the prototype, the application order is fixed by kind and does not depend on the order of lines in the file. Fallthrough seeds are applied first, then function, code and label seeds; within each kind, the highest address goes first. I chose to apply fallthrough seeds first because creating functions first lets Ghidra follow flow through the trap and decode the operand words, which the fallthrough seeds would then have to undo. I would be glad to document this more explicitly, or to change it if there is a case I have not considered.

The warning. I tried to measure it on the same 68000 image: 450 fallthrough seeds, after full auto-analysis with the seeds applied.

For 301 seeds, the target is inside the same function as the trap. This is the expected case, since a trap usually sits in the middle of a function.
For 1 seed, the target is in a different function, and that case looks legitimate. A function entry starts with a trap followed by one parameter word, and the next instruction is a second entry point that other code also calls directly.
For the remaining 148, the trap or its target is not inside a function that Ghidra created.

So I am a little worried that a function-body warning might mostly fire on correct seeds, although I may be misreading your intent.

A possibly more specific signal of a wrong operand count would be the number of references into the operand bytes after analysis: a branch or data reference that lands on a byte declared as an inline operand. In this image that count is 0. Another candidate is the number of seeds whose stale-decode clearing reached the 32-instruction bound. Both could be reported as counts and limitations rather than as warnings. I would welcome your view, and of course the maintainers' decision on the overall direction comes first.

A bit of background. As a personal project, I restored the Intel kernel of the OPENSTEP operating system to C/Objective-C, and I am now using that work to analyze the m68k kernel of the same OS. I tried to apply REA to that project, but it turned out not to fit what I needed there, so I am not able to use it myself for now. I shared this proposal and the measurements anyway, in the hope that they might be useful to others :D

(Prepared with help from an AI coding assistant; I checked the numbers against a local run.)

@onionmixer commented on GitHub (Oct 19, 2026): Thank you very much for reading the proposal so carefully and for the suggestions. **Ordering.** I am sorry this was not clear from the description. In the prototype, the application order is fixed by kind and does not depend on the order of lines in the file. Fallthrough seeds are applied first, then function, code and label seeds; within each kind, the highest address goes first. I chose to apply fallthrough seeds first because creating functions first lets Ghidra follow flow through the trap and decode the operand words, which the fallthrough seeds would then have to undo. I would be glad to document this more explicitly, or to change it if there is a case I have not considered. **The warning.** I tried to measure it on the same 68000 image: 450 fallthrough seeds, after full auto-analysis with the seeds applied. - For 301 seeds, the target is inside the same function as the trap. This is the expected case, since a trap usually sits in the middle of a function. - For 1 seed, the target is in a different function, and that case looks legitimate. A function entry starts with a trap followed by one parameter word, and the next instruction is a second entry point that other code also calls directly. - For the remaining 148, the trap or its target is not inside a function that Ghidra created. So I am a little worried that a function-body warning might mostly fire on correct seeds, although I may be misreading your intent. A possibly more specific signal of a wrong operand count would be the number of references into the operand bytes after analysis: a branch or data reference that lands on a byte declared as an inline operand. In this image that count is 0. Another candidate is the number of seeds whose stale-decode clearing reached the 32-instruction bound. Both could be reported as counts and limitations rather than as warnings. I would welcome your view, and of course the maintainers' decision on the overall direction comes first. **A bit of background.** As a personal project, I restored the Intel kernel of the OPENSTEP operating system to C/Objective-C, and I am now using that work to analyze the m68k kernel of the same OS. I tried to apply REA to that project, but it turned out not to fit what I needed there, so I am not able to use it myself for now. I shared this proposal and the measurements anyway, in the hope that they might be useful to others :D (Prepared with help from an AI coding assistant; I checked the numbers against a local run.)

bsakaguchi commented

2026-10-31 00:34:29 +00:00

Author

Owner

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

Thanks for the measurements — the ordering rationale (fallthrough first so function creation doesn't re-decode operand words) makes sense and would be worth documenting explicitly in the seed file spec. And agreed on the warning concern: with 301/450 targets inside the same function, a function-body warning would mostly fire on correct seeds. The two signals you propose are more precise — references into operand bytes (0 in your image) and stale-decode clearing hitting the 32-instruction bound both directly indicate a mis-sized operand count, and reporting them as counts/limitations fits the evidence model better than warnings. I'd be in favor of option C with those counters included, and a small synthetic m68k fixture as the real-Ghidra regression.

@XinwangTao commented on GitHub (Oct 19, 2026): Thanks for the measurements — the ordering rationale (fallthrough first so function creation doesn't re-decode operand words) makes sense and would be worth documenting explicitly in the seed file spec. And agreed on the warning concern: with 301/450 targets inside the same function, a function-body warning would mostly fire on correct seeds. The two signals you propose are more precise — references into operand bytes (0 in your image) and stale-decode clearing hitting the 32-instruction bound both directly indicate a mis-sized operand count, and reporting them as counts/limitations fits the evidence model better than warnings. I'd be in favor of option C with those counters included, and a small synthetic m68k fixture as the real-Ghidra regression.

bsakaguchi commented

2026-10-31 00:34:29 +00:00

Author

Owner

@onionmixer commented on GitHub (Oct 19, 2026):

XinwangTao

I've submitted the PR for now... but please let me know if there's anything else I can help with. Also, thank you for being so patient and open-minded about this. :D

@onionmixer commented on GitHub (Oct 19, 2026): > XinwangTao I've submitted the PR for now... but please let me know if there's anything else I can help with. Also, thank you for being so patient and open-minded about this. :D

bsakaguchi commented

2026-10-31 00:34:30 +00:00

Author

Owner

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

Thanks for submitting the PR — I'll keep an eye on it. When the synthetic m68k fixture lands, it would be a good place to verify that the two counters (references into operand bytes and stale-decode clearing hitting the 32-instruction bound) show up in the reported limitations as designed. Happy to review the fixture once the PR is up.

@XinwangTao commented on GitHub (Oct 20, 2026): Thanks for submitting the PR — I'll keep an eye on it. When the synthetic m68k fixture lands, it would be a good place to verify that the two counters (references into operand bytes and stale-decode clearing hitting the 32-instruction bound) show up in the reported limitations as designed. Happy to review the fixture once the PR is up.

bsakaguchi referenced this issue

2026-10-31 01:05:17 +00:00

[PR #2487] [MERGED] feat(signals): discover installed decoders and option help #5266

bsakaguchi referenced this issue

2026-10-31 01:05:19 +00:00

[PR #2491] [MERGED] fix(browser): preserve unknown storage inventory and fingerprint coverage #5269

bsakaguchi referenced this issue

2026-10-31 01:05:22 +00:00

[PR #2496] [MERGED] feat(captures): discover installed protocol and field definitions #5275

bsakaguchi referenced this issue

2026-10-31 01:05:34 +00:00

[PR #2643] [MERGED] fix(process): preserve discovery ownership across cleanup retries #5301