forked from arti/vt1200
660 lines
32 KiB
Markdown
660 lines
32 KiB
Markdown
# Ghidra TMS34010 module corrupts `CALLA` targets (`& 0xFFFFF`), so the call graph is empty
|
||
|
||
**Status:** root-caused to a single SLEIGH expression. Fix is a one-token change in the
|
||
processor module source; no Ghidra core fix is needed.
|
||
**Owner of the defect:** the third-party processor module, not this project and not stock Ghidra.
|
||
**Signed-off investigation:** all findings below were reproduced live against the open program
|
||
(`vt1200_v22.bin`, language `tms34010:LE:32:default`) with the Ghidra MCP server.
|
||
|
||
---
|
||
|
||
## 1. Summary
|
||
|
||
Ghidra's TMS34010 processor module decodes the `CALLA` (call absolute) operand with this
|
||
SLEIGH field definition:
|
||
|
||
```
|
||
addr32: addy is imm32 [addy = (imm32 / 8) & 0xfffff;] {export *:4 addy;}
|
||
```
|
||
|
||
`imm32 / 8` is the correct bit-address → byte-address conversion for this firmware (the ROM's
|
||
stored pointers are TMS34010 **bit** addresses; `bit / 8` is exactly the Ghidra byte address).
|
||
The trailing `& 0xfffff` is spurious and destroys the top 12 bits of the 32-bit address. For
|
||
every call target in this ROM it strips the `0x07F00000` nibbles and leaves a target such as
|
||
`0x07FBC018` rendered as `0x000BC018`.
|
||
|
||
`0x000BC018` lies **outside every defined memory block** (the ROM block is
|
||
`0x07E40000`–`0x07FFFFFF`; the lowest defined block is `0x00800000`). Ghidra therefore creates
|
||
an `UNCONDITIONAL_CALL` reference to an unmapped address with no function, so
|
||
`function_callees` / `function_callers` never link it. The call graph for this ROM is empty.
|
||
|
||
The **same** transform is applied to `ADDI`'s 32-bit immediate (a second site, line 185), where
|
||
even `/ 8` is wrong; see §7.
|
||
|
||
---
|
||
|
||
## 2. Impact
|
||
|
||
* `function_callers` / `function_callees` return **0** for functions that plainly call others.
|
||
Verified: `FUN_rom_07fbd000` (the level-1 error dispatcher) returns **0 callers, 0 callees**,
|
||
while it calls the printer routine and a helper many times.
|
||
* The decompiler renders those calls as calls to unknown externals
|
||
(`func_0x000bc018(...)`, `func_0x000bbf96()`), never as calls to real functions.
|
||
* Any analysis built on Ghidra's call graph for this ROM is unsound: reachability,
|
||
"is this function dead/unreachable", caller lookups, and any "0 callers" conclusion are all
|
||
artefacts of this bug. One such "0 callers" result was already recorded and retracted
|
||
(`harness/mame/BRINGUP_LOG.md`, around line 920).
|
||
* Data/pointer cross-references are **not** affected, because they are added by a separate
|
||
script (`ghidra_scripts/FixPtrConstants.java`) that converts bit→byte with `value >>> 3` and
|
||
adds `DATA` references. This is why `MOVI …` constants resolve but `CALLA` targets do not.
|
||
|
||
---
|
||
|
||
## 3. Environment / versions (for whoever fixes this)
|
||
|
||
| item | value |
|
||
|---|---|
|
||
| Ghidra | `12.1.2`, release `DEV`, build `2026-Jun-29 1434 GMT` (`/opt/ghidra/Ghidra/application.properties`) |
|
||
| Language | `tms34010:LE:32:default` |
|
||
| Processor module | third-party `ZPaul2Fresh8/Ghidra_TMS34010`, vendored in `tools/ghidra-tms34010/` |
|
||
| Upstream commit | `f4818e2738a1803849fabe25f826faba231c2610` ("Added ImportSymbols script for Python version 3") — still contains the defect |
|
||
| Installed spec | `/opt/ghidra/Ghidra/Processors/TMS34010/data/languages/tms34010.sinc` line **73** |
|
||
| Compiled spec | `/opt/ghidra/Ghidra/Processors/TMS34010/data/languages/tms34010.sla` |
|
||
| Installer | `tools/install-ghidra-plugin.sh` (copies vendored source to `/opt/ghidra`, recompiles `.sla`) |
|
||
| Program | `vt1200_v22.bin`, image base `rom:07e40000`, blocks: ROM `0x07E40000`–`0x07FFFFFF` (execute), I/O windows `0x00800000`–`0x00D00FFF` |
|
||
|
||
The project's local patch (`tools/ghidra-tms34010-local.patch`) only changes the **pspec**
|
||
(`data_space` → `rom`, and removal of the original Midway board memory map). The `.sinc` — where
|
||
this defect lives — is byte-for-byte identical to the vendored module and to the installed copy.
|
||
|
||
---
|
||
|
||
## 4. Exact reproduction
|
||
|
||
### 4.1 Open the program
|
||
|
||
```
|
||
tools.ghidra.project_program_open_existing
|
||
project_location = /home/arti/code/k-space/vt1200/ghidra
|
||
project_name = vt1200
|
||
program_name = vt1200_v22.bin
|
||
read_only = false
|
||
-> session_id = d05dfb7f99fe446ebb801c0609d2da8a
|
||
```
|
||
|
||
### 4.2 The instruction and its rendering
|
||
|
||
```
|
||
tools.ghidra.listing_code_unit_at { address: "0x07fbd020" }
|
||
-> { bytes: "5f0dc000de3f", mnemonic: "CALLA", text: "CALLA 0x000bc018" }
|
||
```
|
||
|
||
The raw operand is correct in memory: opcode `0x0D5F`, then 32-bit little-endian immediate
|
||
`c0 00 de 3f` = **`0x3FDE00C0`**. `smart34010` renders the same instruction as
|
||
`calla 0x3fde00c0` (see `reference/dis34-a-v22.txt.gz`, line 628112).
|
||
|
||
### 4.3 The transformation, computed from the raw bytes
|
||
|
||
```
|
||
python3 - <<'EOF'
|
||
d = open('vt1200_v22.bin','rb').read() # same bytes the Ghidra program holds
|
||
BASE = 0x07E40000 # Ghidra image base
|
||
def le32(o): return int.from_bytes(d[o:o+4],'little')
|
||
for foff in (0x17d020, 0x17d044, 0x17d062, 0x17d226, 0x17c000):
|
||
imm = le32(foff+2)
|
||
print(f"ghidra={BASE+foff:#010x} op={le32(foff):#06x} imm32={imm:#010x} "
|
||
f"imm/8={imm//8:#010x} masked={imm//8 & 0xfffff:#010x}")
|
||
EOF
|
||
```
|
||
Output:
|
||
```
|
||
ghidra=0x07fbd020 op=0x0d5f imm32=0x3fde00c0 imm/8=0x07fbc018 masked=0x000bc018
|
||
ghidra=0x07fbd044 op=0x0d5f imm32=0x3fde00c0 imm/8=0x07fbc018 masked=0x000bc018
|
||
ghidra=0x07fbd062 op=0x0d5f imm32=0x3fddfcb0 imm/8=0x07fbbf96 masked=0x000bbf96
|
||
ghidra=0x07fbd226 op=0x0d5f imm32=0x3f208660 imm/8=0x07e410cc masked=0x000410cc
|
||
ghidra=0x07fbc000 op=0x0d5f imm32=0x3fdf97f0 imm/8=0x07fbf2fe masked=0x000bf2fe
|
||
```
|
||
Every `masked` value equals the operand Ghidra prints.
|
||
|
||
### 4.4 Why the target does not resolve
|
||
|
||
```
|
||
tools.ghidra.function_at { address: "0x000bc018" }
|
||
-> ERR "no function found at rom:000bc018" # rendered (truncated) address
|
||
tools.ghidra.function_at { address: "0x07fbc018" }
|
||
-> ERR "no function found at rom:07fbc018" # correct address: currently a data byte
|
||
tools.ghidra.function_at { address: "0x07fbbf96" }
|
||
-> FUN_rom_07fbbf96 (exists; the helper)
|
||
```
|
||
|
||
* `0x000BC018` is below the lowest defined block (`0x00800000`), so it is **unmapped**; no
|
||
function can ever exist there. `reference_to 0x000bc018` returns a capped list of **100
|
||
`UNCONDITIONAL_CALL` references** (e.g. `from rom:07fbd020`), proving Ghidra *did* build call
|
||
references — just to the wrong, unmapped address.
|
||
* `0x07FBC018` (the correct address) is inside the ROM block but Ghidra currently types it as
|
||
`DataDB` byte `0x8F` (`DAT_rom_07fbc018`). Those bytes are a valid function prologue
|
||
(`8F 09 74 01` = `MMTM SP,0x0174`); the code was never disassembled there precisely because
|
||
no correct call reference ever reached it.
|
||
|
||
### 4.5 The empty call graph (the consequence that matters)
|
||
|
||
```
|
||
tools.ghidra.function_callers { function_start: "0x07fbd000" }
|
||
-> { count: 0, items: [] }
|
||
tools.ghidra.function_callees { function_start: "0x07fbd000" }
|
||
-> { count: 0, items: [] }
|
||
```
|
||
|
||
And the p-code emitted for the instruction calls the truncated address:
|
||
|
||
```
|
||
tools.ghidra.pcode_op_at { address: "0x07fbd020" }
|
||
-> CALL (rom, 0xbc018, 4) # should be (rom, 0x7fbc018, 4)
|
||
```
|
||
|
||
The decompiler output for the same function makes the damage visible:
|
||
|
||
```c
|
||
void FUN_rom_07fbd000(undefined4 param_1,undefined4 *param_2)
|
||
{
|
||
func_0x000bbf96(); /* real function is FUN_rom_07fbbf96 */
|
||
if (param_2 == 0) func_0x000bc018(0x3fdf5338,param_1); /* printer, target 0x3fde00c0 */
|
||
...
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
## 5. Derived transformation (exact arithmetic)
|
||
|
||
Let `I` = the raw 32-bit immediate little-endian stored after opcode `0x0D5F`.
|
||
|
||
```
|
||
Ghidra operand shown = (I / 8) AND 0xFFFFF # addr32, line 73
|
||
correct Ghidra address = I / 8 # because 0x3F200000 / 8 == 0x07E40000
|
||
```
|
||
|
||
where `0x07E40000` is the image base and `0x3F200000` is the firmware's bit-address base.
|
||
(The identity `0x3F200000 / 8 = 0x07E40000` is the whole reason `I / 8` is the right answer.)
|
||
|
||
The fault is **`& 0xFFFFF`**: a flat 20-bit truncation of the address. It is not a shift error,
|
||
not a signedness error, and not a relative-vs-absolute mix-up. The `/ 8` is correct for `CALLA`.
|
||
|
||
### Worked examples (opcode word `0x0D5F`, raw imm32 = stored value = `smart34010` rendering)
|
||
|
||
| Ghidra addr of instr | raw bytes | raw imm32 (= `smart34010`) | correct Ghidra target (`I/8`) | Ghidra shows | target meaning |
|
||
|---|---|---|---|---|---|
|
||
| `0x07FBD020` | `5f0d c000de3f` | `0x3FDE00C0` | `0x07FBC018` | `0x000BC018` | printer routine (`MMTM SP,0x174`) |
|
||
| `0x07FBD062` | `5f0d b0fcdd3f` | `0x3FDDFCB0` | `0x07FBBF96` | `0x000BBF96` | `FUN_rom_07fbbf96` |
|
||
| `0x07FBD226` | `5f0d 6086203f` | `0x3F208660` | `0x07E410CC` | `0x000410CC` | low-ROM routine |
|
||
| `0x07FBC000` | `5f0d f097df3f` | `0x3FDF97F0` | `0x07FBF2FE` | `0x000BF2FE` | routine |
|
||
| `0x07E40146` (`JAEQ`) | `80ca a024203f` | `0x3F2024A0` | `0x07E40494` | `0x00040494` | conditional absolute jump |
|
||
|
||
`smart34010` cross-checks (from `reference/dis34-a-v22.txt.gz`, file offset `f` shown as base-0
|
||
bit address `f*8`): `00be8100: calla 0x3fde00c0`, `00be8310: calla 0x3fddfcb0`,
|
||
`00be9130: calla 0x3f208660`, `00000a30: jaeq 0x3f2024a0`. All match the "raw imm32" column.
|
||
|
||
---
|
||
|
||
## 6. Scope
|
||
|
||
The defect is confined to the `addr32` field, which is used by exactly two instruction forms:
|
||
|
||
| instruction | field | affected? | evidence |
|
||
|---|---|---|---|
|
||
| `CALLA addr32` | `addr32` | **yes** | 16 939 `calla` sites / 1 767 distinct targets in `reference/dis34-a-v22.txt.gz`; all truncated |
|
||
| `JA^cc addr32` (conditional absolute jump) | `addr32` | **yes** | e.g. Ghidra `0x07E40146 JAEQ 0x00040494`; `smart34010` `jaeq 0x3f2024a0`. 103 sites / 82 distinct targets |
|
||
| `ADDI link,Rd` | `(imm32/8)&0xfffff` (line 185) | **yes** (separate site) | `0x07FBD128`: raw imm `0x00800000`; Ghidra `ADDI 0x0,A9`; `smart34010` `addi 0x800000, a9` |
|
||
| `CALL Rd` | register indirect | no | no immediate target |
|
||
| `JUMP Rd` | register indirect | no | no immediate target |
|
||
| `CALLR addr16` | `addr16` = `inst_next + s16*2` | no | relative; 0 absolute targets in reference |
|
||
| `JR^cc addr16` / `JR^cc addr8` | `addr8`/`addr16` | no (for this bug) | relative; 0 absolute targets in reference. NB the spec header itself notes a separate, unrelated "JRcc Long" arithmetic TODO |
|
||
|
||
Because the ROM's live code all sits at byte `≥ 0x07E40000`, **every** `addr32` absolute
|
||
target in this image is corrupted (all would need the `0x07` nibble that the mask deletes).
|
||
The defect is effectively total for absolute calls and jumps in this firmware.
|
||
|
||
The commented-out `MOVI link` variant at line 940 shows the module author applying the same
|
||
`(imm32 / 8) & 0xfffff` idea; the active `MOVI imm32,Rd` (line 942) does **not** use it, which is
|
||
why `MOVI 0x3fdca990`-style operands render correctly.
|
||
|
||
---
|
||
|
||
## 7. Root cause
|
||
|
||
* **Where:** `tms34010.sinc`, the `addr32` field.
|
||
* Vendored source: `tools/ghidra-tms34010/tms34010.sinc` line **73**
|
||
* Installed copy: `/opt/ghidra/Ghidra/Processors/TMS34010/data/languages/tms34010.sinc` line **73**
|
||
* Compiled into: `…/tms34010.sla`
|
||
* **Exact text:**
|
||
```
|
||
addr32: addy is imm32 [addy = (imm32 / 8) & 0xfffff;] {export *:4 addy;}
|
||
```
|
||
* **Origin:** the third-party module `ZPaul2Fresh8/Ghidra_TMS34010`, at upstream HEAD
|
||
`f4818e2738a1803849fabe25f826faba231c2610`; the `.sinc` is unmodified by this project. This is
|
||
**not** a stock-Ghidra module (Ghidra 12.1.2 has no in-tree TMS34010 SLEIGH source), so it is
|
||
not a stock-Ghidra regression.
|
||
* **Why the mask exists (inference):** the module was originally written against a Midway
|
||
arcade board map (the unpatched pspec maps ROM at `rom:0xff800000`, GFX at `rom:0x02000000`,
|
||
etc.). A flat `0xFFFFF` looks like a leftover assumption that the CPU address space is 20 bits;
|
||
the `addr32` value is otherwise a full 32-bit bit address. Whatever its intent, it discards the
|
||
high bits of every absolute target and cannot be correct for any machine whose code lives above
|
||
byte `0x0FFFFF`. The `pspec` in this project was already changed to `space="rom"` precisely
|
||
because the shipped board map did not describe the VT1200.
|
||
* **Known upstream issue?** No fix is present at the upstream commit above. The project treats
|
||
`tools/ghidra-tms34010/tms34010.sinc` as the source of truth and installs it with
|
||
`tools/install-ghidra-plugin.sh`.
|
||
|
||
---
|
||
|
||
## 8. Recommended fix
|
||
|
||
**Change belongs in the SLEIGH processor spec** (`addr32`), then recompile to `.sla`. In
|
||
`tms34010.sinc`:
|
||
|
||
```diff
|
||
-addr32: addy is imm32 [addy = (imm32 / 8) & 0xfffff;] {export *:4 addy;}
|
||
+addr32: addy is imm32 [addy = imm32 / 8;] {export *:4 addy;}
|
||
```
|
||
|
||
(equivalently `imm32 >> 3`). If a bound is genuinely wanted it must be the full 32-bit range,
|
||
but there is no reason to mask a 32-bit address. All bit addresses in this ROM are
|
||
`< 0x80000000`, so signed/unsigned division does not differ here.
|
||
|
||
Steps for the project (not performed by this investigation, per scope):
|
||
|
||
1. Edit `tools/ghidra-tms34010/tms34010.sinc` as above (and, if desired, the matching
|
||
`ADDI` site at line 185 and the commented line 940 — but note `/8` itself is wrong for
|
||
`ADDI`'s arithmetic immediate; that operand should be rendered as the raw imm32).
|
||
2. Reinstall: `tools/install-ghidra-plugin.sh` (needs `sudo`; copies sources into
|
||
`/opt/ghidra/Ghidra/Processors/TMS34010` and recompiles `tms34010.sla`).
|
||
3. Re-run auto-analysis over the ROM (or `listing_disassemble_seed` from each corrected call
|
||
target) so functions are created at the now-correct targets; the existing project's
|
||
references will remain pointing at the old truncated addresses until they are cleared.
|
||
4. Upstream: the same change should be offered to `ZPaul2Fresh8/Ghidra_TMS34010`.
|
||
|
||
**Consistency check:** with the mask removed, `0x3FDE00C0 / 8 = 0x07FBC018`, which is inside the
|
||
ROM block and holds the printer prologue `MMTM SP,0x0174`; the call reference and the `DATA`
|
||
reference created by `FixPtrConstants.java` (`value >>> 3`) would then agree, instead of
|
||
disagreeing (`CALL 0x0BC018` vs `DATA 0x7FBC018`).
|
||
|
||
---
|
||
|
||
## 9. Workaround (until the module is fixed)
|
||
|
||
**Use `smart34010` for call targets; use Ghidra for structure and decompilation.** Concretely:
|
||
|
||
* Treat the raw `CALLA` immediate as the truth. It is the firmware's bit address; the Ghidra
|
||
byte address is `imm32 / 8` (drop the `& 0xFFFFF`). The operand bytes are intact in the listing
|
||
(`listing_code_unit_at` returns them), so the correct target is recoverable by hand/script.
|
||
* `reference/dis34-a-v22.txt.gz` already contains the authoritative `calla`/`ja` targets
|
||
(16 939 `calla` sites, 1 767 distinct targets). `reference/dis34-a-v1.txt.gz` for v2.1.
|
||
* Do **not** build any conclusion on Ghidra `function_callers` / `function_callees`,
|
||
`function_report`'s `callers`/`callees`, or decompiler `func_0x…` names for this ROM.
|
||
* What still works in Ghidra: mnemonic/instruction structure, `MOVI` constants (raw bit
|
||
addresses), data xrefs created by `FixPtrConstants.java`, and decompilation *body* analysis
|
||
(the `func_0x…` call placeholders are the only call-graph damage).
|
||
* Caveat on `smart34010`: its own **analyze mode does not follow `CALLA` targets either** — the
|
||
printer prologue at base-0 bit address `0x00BE00C0` is emitted as `.word 0x098F` rather than
|
||
disassembled. So "use `smart34010` for call targets" means *use its correctly-rendered operand
|
||
values*, not its auto-discovered call graph. Both tools need the manual `bit/8` mapping.
|
||
* Also beware the separate `ADDI` corruption (e.g. `ADDI 0x0,A9` where the real immediate is
|
||
`0x800000`); read `ADDI`/`MOVI` immediates from the raw bytes when they matter.
|
||
|
||
---
|
||
|
||
## 10. Verified vs inferred
|
||
|
||
**Verified against the live Ghidra program and the raw image:**
|
||
* The exact rendering `CALLA 0x000bc018` and raw bytes `5f0dc000de3f` at `0x07FBD020`.
|
||
* The formula `(imm32 / 8) & 0xFFFFF` reproduces every observed operand (5 `CALLA` sites, 1
|
||
`JAEQ` site, 1 `ADDI` site), and the exact source line 73 produces it.
|
||
* `function_callers` and `function_callees` are 0 for `FUN_rom_07fbd000`; `pcode_op_at` emits
|
||
`CALL (rom, 0xbc018, 4)`; `reference_to 0x000bc018` returns `UNCONDITIONAL_CALL` refs from the
|
||
call sites.
|
||
* `0x000BC018` is outside all defined memory blocks; `0x07FBC018` and `0x07FBBF96` are inside the
|
||
ROM block; `FUN_rom_07fbbf96` exists; `0x07FBC018` is currently a `DataDB` whose bytes are the
|
||
`MMTM` printer prologue.
|
||
* `smart34010` targets for all sampled sites (from the committed reference disassembly).
|
||
* Module origin/commit, Ghidra version, and that the vendored/installed `.sinc` is unmodified.
|
||
|
||
**Inferred (not directly executed, because this task must not modify the install):**
|
||
* That removing `& 0xFFFFF` and re-running analysis will populate the call graph. This is
|
||
strongly supported: the helper target `0x07FBBF96` already exists as a function, the printer
|
||
bytes are a valid prologue, and Ghidra already generates the call references — only the
|
||
address is wrong.
|
||
* The original rationale for `& 0xFFFFF` (a 20-bit-address assumption / Midway-board leftover).
|
||
* The exact post-fix behaviour of `function_callees` (whether Ghidra auto-creates the printer
|
||
function at `0x07FBC018` on re-analysis) was not run.
|
||
|
||
---
|
||
|
||
## 11. Appendix — raw evidence
|
||
|
||
Raw instruction bytes (`vt1200_v22.bin`, little-endian; `xxd -s 0x17c000`):
|
||
|
||
```
|
||
0017c000: 5f0d f097 df3f 8a11 2b14 efc5 484d eeb7 <- CALLA @0x07FBC000, imm=0x3fdf97f0
|
||
0017c010: 2001 af09 a83e 6209 8f09 7401 cd4d a9b7 <- 0x07FBC018 = 8f 09 74 01 = MMTM SP,0x0174
|
||
```
|
||
|
||
Spec excerpt (`/opt/ghidra/Ghidra/Processors/TMS34010/data/languages/tms34010.sinc`):
|
||
|
||
```
|
||
73 :addr32: addy is imm32 [addy = (imm32 / 8) & 0xfffff;] {export *:4 addy;}
|
||
...
|
||
294::CALLA addr32 is word=0x0D5F; addr32
|
||
296: call addr32;
|
||
...
|
||
560::JA^cc addr32 is cc & bits_12_15=0b1100 & bits_0_7s=0b10000000; addr32
|
||
185::ADDI link,Rd is bits_5_15=0b00001011001 & Rd; imm32 [link=(imm32 / 8) & 0xfffff;]
|
||
940:#:MOVI link,Rd is bits_5_15=0b00001001111 & Rd; imm32 [link=(imm32 / 8) & 0xfffff;] (commented out)
|
||
942::MOVI imm32,Rd is bits_5_15=0b00001001111 & Rd; imm32 (correct: raw)
|
||
```
|
||
|
||
MCP calls used (all with `session_id` from §4.1): `listing_code_unit_at`,
|
||
`listing_disassemble_function`, `listing_disassemble_range`, `pcode_op_at`, `function_at`,
|
||
`function_callers`, `function_callees`, `function_report`, `reference_to`, `reference_from`,
|
||
`memory_blocks_list`.
|
||
|
||
---
|
||
|
||
# FIX + VALIDATION
|
||
|
||
**Status:** DONE. The SLEIGH defect is fixed in both places, the `.sla` is rebuilt and
|
||
installed, the existing program is repaired in place, and every affected instruction in the
|
||
program is validated against an independent reference disassembler with **0 mismatches**.
|
||
|
||
Environment used: Ghidra `12.1.2` at `/opt/ghidra`, language `tms34010:LE:32:default`,
|
||
program `vt1200_v22.bin` (image base `rom:07e40000`).
|
||
|
||
## 1. The change
|
||
|
||
Two functional sites in the vendored SLEIGH module
|
||
`tools/ghidra-tms34010/tms34010.sinc` (CRLF preserved):
|
||
|
||
```diff
|
||
@@ line 73 (shared by CALLA and JA^cc) @@
|
||
-addr32: addy is imm32 [addy = (imm32 / 8) & 0xfffff;] {export *:4 addy;}
|
||
+addr32: addy is imm32 [addy = imm32 / 8;] {export *:4 addy;}
|
||
|
||
@@ line 185 (ADDI) @@
|
||
-:ADDI link,Rd is bits_5_15=0b00001011001 & Rd; imm32 [link=(imm32 / 8) & 0xfffff;]
|
||
+:ADDI imm32,Rd is bits_5_15=0b00001011001 & Rd; imm32
|
||
{
|
||
- Rd = link;
|
||
+ Rd = imm32;
|
||
}
|
||
```
|
||
|
||
* `addr32` (line 73) is used by both `CALLA addr32` (line 294) and `JA^cc addr32`
|
||
(line 560), so one edit fixes both instructions. `imm32 / 8` is the correct
|
||
bit→byte conversion; the `& 0xfffff` is gone.
|
||
* The naive ADDI edit `[link=imm32;]` is **rejected by the SLEIGH compiler**
|
||
(`Error: operand link has an issue: for table "instruction" constructor from
|
||
tms34010.sinc:185`). The operand was therefore renamed to `imm32`, mirroring the
|
||
already-correct `MOVI imm32,Rd` form (line 942). Semantics are unchanged
|
||
(`Rd = imm32`, previously `Rd = link` with `link` the same value); only the displayed
|
||
operand changes from the truncated `(imm32/8)&0xfffff` to the raw immediate.
|
||
* The commented-out `MOVI link` form at line 940 was left untouched (inert; not compiled).
|
||
|
||
## 2. Rebuild and install
|
||
|
||
`tools/install-ghidra-plugin.sh` was used unchanged (sudo available):
|
||
|
||
1. It copies `tools/ghidra-tms34010/tms34010.{slaspec,sinc,ldefs,pspec,cspec}` into
|
||
`/opt/ghidra/Ghidra/Processors/TMS34010/data/languages/` and touches `Module.manifest`.
|
||
2. It runs `/opt/ghidra/support/sleigh -a <that dir>`, recompiling `tms34010.sla`.
|
||
|
||
Verification that the installed artefact is the one built:
|
||
|
||
| file | md5 | note |
|
||
|---|---|---|
|
||
| `tools/ghidra-tms34010/tms34010.sinc` | `316d065a44e8fb4ddfed1455f5b0da9c` | source of truth |
|
||
| `/opt/ghidra/…/TMS34010/data/languages/tms34010.sinc` | `316d065a44e8fb4ddfed1455f5b0da9c` | installed copy, identical |
|
||
| `/opt/ghidra/…/TMS34010/data/languages/tms34010.sla` | `e0e1a6792f37b669539bda654a7dac7b` | recompiled `10:52:17` |
|
||
|
||
A throw-away build of the same sources into `/tmp/opencode/sleightest/` produced the
|
||
byte-identical `.sla` (`e0e1a679…`), confirming the module compiles from the edited source.
|
||
The pre-fix file hashes were `tms34010.sinc = 7b11c07300fb636432f247a00be5701c`,
|
||
`tms34010.sla = 31633dd3985ee92e5fa5d826d2bbc393`.
|
||
|
||
The running Ghidra JVM caches the language, so the MCP server was restarted
|
||
(the harness reconnected it via `POST /api/experimental/mcp/ghidra/connect`). A fresh JVM
|
||
then decoded the three forms correctly **before any program edit**:
|
||
|
||
```
|
||
0x07fbd020 -> CALLA 0x07fbc018 (was 0x000bc018)
|
||
0x07e40146 -> JAEQ 0x07e40494 (was 0x00040494)
|
||
0x07fbd128 -> ADDI 0x800000,A9 (was 0x0,A9)
|
||
```
|
||
|
||
## 3. Repairing the already-analysed program — route chosen
|
||
|
||
**Route: re-disassemble the affected instructions in the existing program, in place**
|
||
(not a fresh re-import).
|
||
|
||
Why: Ghidra re-resolves an instruction's *prototype* against the loaded language on open,
|
||
so text and p-code already update automatically once the fixed `.sla` is installed — but the
|
||
**stored references** (`REF_CALL`, `REF_JUMP`), the stored **function bodies**, and the
|
||
resulting **function table** are persisted in the program database and were computed with the
|
||
buggy module. In-place re-disassembly regenerates exactly those, while preserving every
|
||
annotation. The existing analysis holds 25 functions (some named), 9 memory-block comments
|
||
and the `FixPtrConstants.java` data cross-references; a fresh re-import would have discarded
|
||
all of them and produced a different function set. In-place was therefore both cheaper and
|
||
lossless.
|
||
|
||
Procedure (single Ghidra transaction each, via `ghidra_eval`):
|
||
|
||
1. Collect every `CALLA`/`JA*`/`ADDI` instruction (9 934 sites at that moment), then for each:
|
||
`listing.clearCodeUnits(addr, addr, true)` followed by
|
||
`new DisassembleCommand(addr, null, true).applyTo(program, monitor)`. This re-decodes the
|
||
instruction with the fixed language and emits the corrected reference (following the call
|
||
target discovers the previously unreachable code).
|
||
2. Create functions at every `UNCONDITIONAL_CALL` destination inside the ROM block that had no
|
||
function: `CreateFunctionCmd(addr)` → 1 563 functions created (13 targets already had one,
|
||
4 were mid-function). Function count 25 → 1 588.
|
||
3. `program_save` to persist.
|
||
|
||
Not re-run: a wholesale auto-analysis. The surgical re-disassembly + `CreateFunctionCmd`
|
||
already restores the call structure; a full re-analysis would re-derive a large amount of
|
||
unrelated state and risk perturbing the preserved annotations.
|
||
|
||
Effect on the program: instructions 101 052 → 225 794 (code now reaches the real call targets,
|
||
750 642 bytes = 40.9 % of the ROM; the independent reference disassembles 308 512, so this is
|
||
*more* conservative, not over-disassembled). References into the truncated `0x000bc018`
|
||
alone went from 168 to 0; references into the true `0x07fbc018` went from 0 to 265.
|
||
|
||
**Annotations preserved:** all 25 original functions still present (including
|
||
`video_regs_init`, `led_control_1/2`, `io5600000_screen_init`, `nvr_user_settings_validate`,
|
||
`io_settings_copy_expand/linear/restore`, `keyboard_mouse_irq_handler`, `io5800_5900_init`,
|
||
`io5800_status_read`) and all 9 memory-block comments intact.
|
||
|
||
## 4. Exhaustive validation
|
||
|
||
Every affected instruction in the repaired program was checked twice:
|
||
|
||
* **self-check** — the operand Ghidra renders must equal the value computed directly from the
|
||
instruction's own bytes, independently of Ghidra's SLEIGH: `imm >> 3` for `CALLA`/`JA^cc`,
|
||
the raw `imm` for `ADDI`/`MOVI`.
|
||
* **reference check** — `tools/smart34010/dis34 -l -b 3f200000 -o vt1200_v22.bin` (full linear
|
||
sweep, 697 412 lines) decoded at the same address must agree on the raw immediate/target.
|
||
Ghidra address `G` ↔ dis34 address `G*8`.
|
||
|
||
| instruction | sites checked | self-match | dis34-match | mismatches |
|
||
|---|---|---|---|---|
|
||
| `CALLA` | 15 955 | 15 955 | 15 955 | **0** |
|
||
| `JA^cc` (`JA`/`JAEQ`/`JAHI`/`JANE`) | 56 | 56 | 56 | **0** |
|
||
| `ADDI` | 6 742 | 6 742 | 6 742 | **0** |
|
||
| `MOVI` | 8 938 | 8 938 | 8 938 | **0** |
|
||
| **total** | **31 691** | **31 691** | **31 691** | **0** |
|
||
|
||
The 8 938 `MOVI` include 12 whose dis34 line carries a trailing symbolic register name
|
||
(`… c000 HESYNC`, `DISPLAY`, `CONVSP`, `DPYCTL`, `CONTROL`, `HSTCTLH`, `INTENB`,
|
||
`INTPEND`); each was checked by hand and the three hex words agree with Ghidra. At no site did
|
||
Ghidra and dis34 disagree on the instruction boundary (0 boundary diffs), and every Ghidra
|
||
site had a matching reference line (0 missing).
|
||
|
||
Distinct targets: the repaired program has **1 577 distinct `CALLA` targets** (up from 1 767 in
|
||
the standalone reference; every site present was cross-checked). The set difference is
|
||
analysis-coverage, not a value error — 317 Ghidra targets do not appear in the reference's
|
||
*analyze-mode* output (which emits `.word` for code it does not classify) but every one of them
|
||
was confirmed against the reference's linear sweep; 507 reference targets belong to call sites
|
||
Ghidra's analysis still has not disassembled.
|
||
|
||
Known concrete sites, before → after:
|
||
|
||
| Ghidra address | bytes | before | after |
|
||
|---|---|---|---|
|
||
| `0x07FBD020` | `5f0dc000de3f` | `CALLA 0x000bc018` | `CALLA 0x07fbc018` |
|
||
| `0x07FBD128` | `290b00008000` | `ADDI 0x0,A9` | `ADDI 0x800000,A9` |
|
||
| `0x07E40146` | `80caa024203f` | `JAEQ 0x00040494` | `JAEQ 0x07e40494` |
|
||
| `0x07E404FC` | `5f0d90cadd3f` | `CALLA 0x000bb952` | `CALLA 0x07fbb952` |
|
||
|
||
## 5. Call-graph evidence (empty before → populated now)
|
||
|
||
Measured with the MCP tools on the program itself.
|
||
|
||
`FUN_rom_07fbd000` — `function_callees`:
|
||
|
||
```
|
||
BEFORE { count: 0, items: [] }
|
||
AFTER { count: 2, items: [
|
||
FUN_rom_07fbbf96 (rom:07fbbf96, body 07fbbf96..07fbc015),
|
||
FUN_rom_07fbc018 (rom:07fbc018, body 07fbc018..07fbc073) ] }
|
||
```
|
||
|
||
`FUN_rom_07fbbf96` (the helper) — `function_callers`:
|
||
|
||
```
|
||
BEFORE { count: 0, items: [] }
|
||
AFTER { count: 6, items: [ FUN_rom_07e6a9a8, FUN_rom_07e6ad48,
|
||
FUN_rom_07fbc262, FUN_rom_07fbcabe, FUN_rom_07fbce0e,
|
||
FUN_rom_07fbd000 ] }
|
||
```
|
||
|
||
`pcode_op_at 0x07fbd020`:
|
||
|
||
```
|
||
BEFORE CALL (rom, 0xbc018, 4)
|
||
AFTER CALL (rom, 0x7fbc018, 4)
|
||
```
|
||
|
||
(The BEFORE values are from the live program under the buggy module. They were re-confirmed
|
||
independently by opening the pre-fix project export in the fixed JVM: `callees(0x07fbd000)=0`,
|
||
`callers(0x07fbbf96)=0`.)
|
||
|
||
**Site that still does not resolve, investigated:** `FUN_rom_07fbd000` still reports 0
|
||
*callers*. This is correct, not residual damage — there is no `CALL`/`JUMP` reference to
|
||
`0x07fbd000` anywhere in the image, in either Ghidra or the reference disassembler. The
|
||
function is entered by **fall-through** from the tail of `FUN_rom_07fbce0e`
|
||
(`0x07fbcf…` ends with `MOVI 0x22,A9` at `0x07fbcffc`; `0x07fbd000` is the next instruction),
|
||
so a static direct-caller list is legitimately empty. Its *callee* list — the part the bug
|
||
destroyed — is now populated as shown above.
|
||
|
||
## 6. Over-correction check (addresses that were already right)
|
||
|
||
The fix touches only `addr32` and the `ADDI` immediate. Ghidra's absolute `MOVI` operands were
|
||
already correct and must still be. All 8 938 `MOVI` instructions were re-validated (see §4):
|
||
**8 938/8 938 match, 0 broken**. Examples compared against `smart34010`:
|
||
|
||
| Ghidra address | Ghidra | `smart34010` (raw words) |
|
||
|---|---|---|
|
||
| `0x07E400B0` | `MOVI 0xff,A0` | `movi 0xff, a0 ; 09c0 00ff` |
|
||
| `0x07FC17F6` | `MOVI 0xc0000120,A9` | `movi … ; 09e9 0120 c000 INTPEND` |
|
||
| `0x07EAF146` | `MOVI 0xc0000080,A3` | `movi … ; 09e3 0080 c000 DPYCTL` |
|
||
| `0x07E40078` | `MOVI 0x55555555,A0` | `09e0 5555 5555` |
|
||
|
||
The 4-byte `MOVI`/`ADDI` short forms (3 317 + 3 781 sites) were included in the check and are
|
||
unchanged. No previously-correct absolute operand regressed.
|
||
|
||
## 7. Files changed
|
||
|
||
| file | change |
|
||
|---|---|
|
||
| `tools/ghidra-tms34010/tms34010.sinc` | lines 73 and 185 (fix) |
|
||
| `/opt/ghidra/Ghidra/Processors/TMS34010/data/languages/tms34010.sinc` | installed copy of the above |
|
||
| `/opt/ghidra/Ghidra/Processors/TMS34010/data/languages/tms34010.sla` | recompiled |
|
||
| `ghidra/vt1200.rep/` (`vt1200_v22.bin`), gitignored | program repaired in place + saved |
|
||
| `docs/ghidra-tms34010-calla-bug.md` | this section |
|
||
|
||
The `.cspec/.pspec/.ldefs/.slaspec` were re-copied by the installer with unchanged content
|
||
(timestamps bumped only). Nothing under `harness/` was touched.
|
||
|
||
## 8. Revert
|
||
|
||
* **SLEIGH / install**: restore `tools/ghidra-tms34010/tms34010.sinc` from
|
||
`/tmp/opencode/backup/vendored-tms34010.sinc.orig` (md5 `7b11c073…`; the pre-fix `.sla`
|
||
backup is `/tmp/opencode/backup/installed-tms34010.sla.orig`, md5 `31633dd3…`), then re-run
|
||
`tools/install-ghidra-plugin.sh`. (`git checkout -- tools/ghidra-tms34010/tms34010.sinc`
|
||
also works while the pre-fix blob is reachable.)
|
||
* **Program**: the untouched pre-fix project was exported before any edit to
|
||
`/tmp/opencode/backup/vt1200_v22_pre-fix.gzf/` (a full `vt1200.gpr` + `vt1200.rep` copy).
|
||
Copy it over `ghidra/` to restore the original analysis. The pre-fix instruction/reference
|
||
dump is `/tmp/opencode/before_dump.json`.
|
||
|
||
## 9. Provenance note
|
||
|
||
The `tms34010.sinc` fix was swept into HEAD by the concurrently-running worker's broad
|
||
`git add -A` commit `5000b8bc07a8` before this task committed; HEAD therefore already contains
|
||
lines 73/185 as shown in §1. This task's own commit adds only §1–§8 of this document.
|
||
|
||
---
|
||
|
||
# FURTHER FINDING: the decompiler is not trustworthy for this ROM either
|
||
|
||
Found 2026-10-16, after the `CALLA`/`ADDI` fix above, while investigating why a worker
|
||
reported "Ghidra failed to decompile the DUART test code".
|
||
|
||
## Symptom
|
||
|
||
`decomp_function` does **not** error — it returns C, but with a warning embedded, e.g.:
|
||
|
||
```c
|
||
/* WARNING: Control flow encountered bad instruction data */
|
||
|
||
void FUN_rom_07fbbf96(uint param_1, undefined4 param_2, undefined4 param_3)
|
||
{
|
||
...
|
||
```
|
||
|
||
## Measurement
|
||
|
||
Twelve functions sampled across the ROM, decompiled via the MCP:
|
||
|
||
```
|
||
sampled: 12 warned ("bad instruction data" or other WARNING): 10 clean: 2 hard errors: 0
|
||
```
|
||
|
||
So roughly **85% of sampled functions** produce output that carries a warning that the
|
||
decompiler could not follow the real instruction stream. The returned C is therefore
|
||
**unreliable**, even though it looks plausible.
|
||
|
||
## Cause
|
||
|
||
This is a **different and deeper defect than the `CALLA` mask**: it is not an address
|
||
computation error but **missing/incomplete instruction definitions in the SLEIGH module**
|
||
(`tools/ghidra-tms34010/tms34010.sinc`). Where an instruction has no p-code, the decompiler
|
||
cannot model its effect and reports "bad instruction data" on that path.
|
||
|
||
## Consequence — the working rule for this project
|
||
|
||
| Ghidra layer | verdict |
|
||
|---|---|
|
||
| **disassembly** (`listing_disassemble_range`, `search_instructions`) | **trustworthy** — validated at 31,691/31,691 affected sites after the fix |
|
||
| **xrefs / call graph / function recovery** | **trustworthy** — 25 → 1,588 functions |
|
||
| **decompiler / p-code** (`decomp_function`) | **NOT trustworthy** — ~85% of sampled functions flagged |
|
||
|
||
Use Ghidra for structure and disassembly; use `tools/smart34010/dis34` when reading
|
||
instruction semantics. Do not build conclusions on decompiled C for this ROM until the
|
||
SLEIGH instruction coverage is completed.
|
||
|
||
## To fix later
|
||
|
||
Extend the SLEIGH module's instruction definitions so every opcode used by this firmware has
|
||
p-code. That is a substantially larger job than the one-line mask fix — it needs the full
|
||
TMS34010 instruction set audited against `tms34010.sinc`, with each missing instruction
|
||
added and the `.sla` rebuilt and re-validated the same way (see the FIX + VALIDATION section
|
||
above for the validation method: cross-check against `smart34010` and check for
|
||
over-correction on the instructions that were already correct).
|