Explorar o código

Frame displacements: fix the rule, and the coverage that was missing

EmLoadVar, EmStoreVar and EmPushVarAddr each encoded a [BP+off] access
themselves and each truncated the displacement with `off MOD 100H`, so a
variable past +127 was read from the wrong side of BP.  Three copies of one
decision is the actual defect, so it now lives in one place:

  EmBpDisp (disp8 = 46 d8 iff off <= 127, else disp16 = 86 lo hi)

There is no overflow case, and this is worth being explicit about because the
first attempt added one and it refused every local: locals are allocated
*downward* from 0FFFEh, so their offsets are negative, and disp16 covers the
whole 16-bit range as a signed value anyway.

The old truncation was accidentally correct across -32768..+127, which is
exactly where every real variable lives, so no existing fixture changed bytes
and the whole BP path had zero coverage.  Two fixtures make it reachable:

  t27_localvar  five locals, the common negative-offset case
  t28_farparam  70 declared parameters; p63 lands at BP+128 and p70 at BP+142,
                the first offsets a disp8 cannot hold (80h is -128, not +128)

t28 passes 16 arguments because that is the call-site cap (ParseCall raises
ECompOvf above 16) while the *declaration* list is uncapped -- and it is the
declaration that sets parmOff.  It tests the encoding and is never executed;
do not read it as a claim that a 70-argument call works.

check_framedisp.py guards it, and asserts the rule rather than one encoding:
always-disp16 is accepted.  nonvacuity.sh proves that by building the
over-long form and requiring the check to stay green, and proves the checker
can fail by restoring the truncation (t28 then reads [BP-128] and [BP-114]).
It computes its expectations from the frame layout rules -- locals from
0FFFEh by pre-decrement, parameters from 4 by +2 each -- rather than asking
the compiler.

Also in this commit:

- build_tpshell.sh was a second, hand-maintained copy of the Makefile, wrong
  twice: it ran `gm2 -c Posix.mod` (Posix is a foreign C module, built by
  `cc -c Posix.c`, and there is no Posix.mod), and it printed phase 2's return
  code then carried on regardless, so a failed link read as success whenever
  an older tpshell was around.  It was never run, because make is what people
  run.  Now a three-line delegate to make, with the reasons kept in the file.

- .gitignore at the root was missing shell/comtest, which is how a build
  artifact ended up committed in 7dada57.

- SUMMARY.md: the ModR/M table (measured, not remembered), the FCML and
  objdump oracles, the runtime's five emitter fixes, the encoding checks and
  what each cannot see, the corrected qemu finding, and the honest version of
  what is still missing -- nothing has run.  The stale "no 16-bit
  disassembler" claim is retracted where it was made.

- TP3-COMPILER.md: the fixture matrix was a second copy of expected.tsv and
  had gone stale (it still listed the multi-char literals as failing).  It now
  points at expected.tsv, which the runner asserts and whose re-baselining
  rationale lives in the file.  The `EmPushVarAddr` note is marked superseded
  rather than deleted, since it was accurate when written.

run_all.sh: all green.  nonvacuity.sh: 16 ok, 0 failed.
Eric Streit hai 1 semana
pai
achega
b3bb05d71a

+ 1 - 0
.gitignore

@@ -2,6 +2,7 @@
 shell/tpshell
 shell/tpshell.lst
 shell/compiletest
+shell/comtest
 shell/tests/ct.lst
 __pycache__/
 shell/tests/rtprobe

+ 371 - 142
SUMMARY.md

@@ -18,7 +18,9 @@ manual, not guessed.
 | Standard procedures + `rel16` fix | `v-TP3-STDPROCS` | done |
 | Runtime library + 8086 execution harness | `v-TP3-RUNTIME-BLOB` | assembled, **never run** |
 | Inline string literals (`writeln('hi')`) | `v-TP3-STRLITERAL` | done, **never run** |
-| Linker + `CmdRun` | — | **not started** |
+| Linker: real DOS `.COM` writer + independent byte checker | `v-TP3-COM-IMAGE` | done, **never run** |
+| Measured encodings: ModR/M table, runtime audit, golden disassembly, `[BP+off]` | `v-TP3-MEASURED-EMITTERS` | done, **never run** |
+| `CmdRun`, and a `.COM` that has actually executed | — | **not started** |
 
 ## Build
 
@@ -37,10 +39,21 @@ gm2 -fiso -fuse-module-list=modules.lst -o tpshell \
         Shell.mod Compiler.mod Term.o TextBuf.o Posix.o Editor.o   # phase B2
 ```
 
-Current clean build: `make clean && make` → rc=0, `tpshell` **125136 bytes**.
+Current clean build: `make clean && make` → rc=0, `tpshell` **163032 bytes**.
 The one diagnostic is `./Compiler.mod: ParseExpr: too many errors in pass 3`,
 which is the expected phase-1 rollup that the recipe tolerates — not a real
-error. `shell/build_tpshell.sh` and `shell/Makefile` are authoritative.
+error. `shell/Makefile` is the single authoritative build recipe.
+
+**`shell/build_tpshell.sh` is now a three-line wrapper around `make`**, and
+that is a fix, not a refactor. It used to be a second hand-maintained copy of
+the recipe, and a copy of a build recipe drifts — this one was wrong twice
+over: it ran `gm2 -c Posix.mod`, but `Posix` is a *foreign C module* built by
+`cc -c Posix.c` and there is no `Posix.mod`, so it died on the third module
+every time; and it printed phase 2's return code and then carried on regardless,
+so a failed link was reported as a success whenever an older `tpshell` was
+still lying around. The copy that is wrong is the one nobody runs, which is how
+it survived. The wrapper keeps the old invocation working and holds no build
+knowledge of its own.
 
 ## What is verified, and how
 
@@ -66,11 +79,11 @@ error position or a program that lost six bytes now fails the suite instead of
 needing a squint. It was checked for vacuousness by reverting the string
 scanner fix: 19/23 and exit 1, restored: 23/23 and exit 0.
 
-**25 of 27 fixtures compile**, up from 1 (the empty program) when the direct
+**27 of 29 fixtures compile**, up from 1 (the empty program) when the direct
 harness was first built.
 
 ```
-compile matrix: 27 passed, 0 failed (of 27)
+compile matrix: 29 passed, 0 failed (of 29)
 ```
 
 Compiling: `t01` minimal · `t04` var+assign+`writeln` · `t06` two args ·
@@ -79,7 +92,12 @@ Compiling: `t01` minimal · `t04` var+assign+`writeln` · `t06` two args ·
 `t15` label + goto · `t16` `writeln('a')` · `t18` bare `writeln` ·
 `t19` `writeln(1)` · `t20` 3 string args · `t21` mixed args ·
 `t22` `case` with two labels · `t23` `writeln('')` · `t24` `writeln('don''t')` ·
-`t26` mixed scalar/string args · `t02`/`t03`/`t05`/`t17` multi-char literals.
+`t26` mixed scalar/string args · `t02`/`t03`/`t05`/`t17` multi-char literals ·
+`t27` five locals · `t28` a 70-parameter declaration.
+
+`comtest` additionally links **every one of the 29** to a real `.COM` and
+re-verifies the bytes with an independent checker that restates the layout
+constants instead of asking the compiler: `26 checked, 0 failed`.
 
 Failing, all deliberately: `t14` `array [1..5] of integer` at its point of use
 and `t25` a string literal used as a *value* (`s := 'hi'`) both → `ENoLib`
@@ -107,12 +125,83 @@ changing `Run`'s signature, so `Editor.def` stays additive.
 feature: the latter move as the compiler grows, and a test whose
 expectations drift with it stops being a test.
 
+### The encodings — five checks that can each go red
+
+Nothing above looks at *machine code*. It all stops at "the compiler produced
+what it intended to produce", which is exactly where the bugs in this project
+live: a wrong ModRM byte is not a compile error and not a wrong code size, it is
+a perfectly well-formed instruction that does something else. So the encodings
+get their own stack, and it is built so that each check fails on a *different*
+class of mistake. `tests/run_all.sh` runs all of them; `tests/nonvacuity.sh`
+proves each one can go red.
+
+| check | what it asserts | what it cannot see |
+|---|---|---|
+| `probe/run_modrm19.py` | the mod=00/01/10 effective addresses, by **executing** 23 cases on a real 8086 under qemu and scanning for where the marker landed | mod=11 — see below |
+| `probe/modrm11.py` | the mod=11 register identities, by **encoding** with GNU `as` and decoding with FCML, against hard-coded bytes | the table agreeing with itself |
+| `audit_helpers.py` | every one-line emitter in `Runtime.mod` decodes to what its *name* says — 61/61 | anything longer than one instruction |
+| `check_runtime.py` + `runtime.golden` | the built runtime's 360-byte code region sweeps cleanly through FCML, every entry and all 37 branch targets land on an instruction boundary, and the whole disassembly is byte-for-byte the committed golden | whether the golden is *right* |
+| `check_framedisp.py` | `[BP+off]` uses disp8 iff `off <= 127`, for locals (negative) and far parameters (>127) | which of the two encodings was chosen, if the other also works |
+
+Two of these deserve the detail, because the reason they exist is the reason
+they are hard.
+
+**`audit_helpers.py` exists because a wrong ModRM that still decodes is
+invisible.** The mistake this project actually makes is not a malformed
+instruction — it is `MovSiBx` emitting `89 DC`, which decodes perfectly as
+`MOV SP,BX`, and `CmpSiBx` emitting `39 DC`, which decodes as `CMP SP,BX`. Both
+shipped for a long time. A structural check passes them. A golden passes them.
+Only asking "does this byte sequence mean what this procedure is called?" fails,
+which is what the audit does: it disassembles each one-line emitter and compares
+the decode against the name. Five bugs came out of it in one pass.
+
+**`modrm11.py` is deliberately not self-referential.** The obvious way to check
+a ModR/M table is to write the table in assembly and assemble it — but that can
+never fail, because editing the assembly makes `as` faithfully re-encode the new
+claim and the two then agree again. That failure mode was found by corrupting
+the `.s` and watching the check stay green. Two things close it: an `EXPECT`
+byte sequence hard-coded independently of the `.s` text, and four *anchor*
+encodings that are spelled as literal `.byte` directives because `as` would
+never choose them for a mnemonic (`83 C4 08` ADD SP,8 · `83 C6 02` ADD SI,2 ·
+`8B EC` MOV BP,SP · `8B E5` MOV SP,BP). No table shifted by one cell can
+satisfy all four.
+
+**`check_framedisp.py` exists because the bug it guards was accidentally
+correct.** The old code truncated the displacement with `off MOD 100H`, always
+emitting disp8. Locals are allocated *downward* from `0FFFEh`, so a local's
+offset is negative and −32768..+127 — the whole range where truncating to a
+byte happens to be right. The bug was only visible above +127, reachable from
+the 63rd parameter onward, and no fixture had one. The fix is `disp8` iff
+`off <= 127` else `disp16`, with no overflow branch at all: disp16 covers the
+entire 16-bit range as a signed value. The check also asserts the *rule* rather
+than one encoding — always-disp16 is accepted, and `nonvacuity.sh` proves that
+by building it and requiring the check to stay green.
+
+### Non-vacuity — `tests/nonvacuity.sh`
+
+Every assertion above is proved able to fail: **16 deliberate breakages, each
+asserted to turn exactly one named check red for the stated reason**, then
+restored and re-asserted green. Six break the runtime, five attack the mod=11
+table (including restoring the exact wrong table this project once shipped),
+and three target `EmBpDisp` — the truncation, the always-disp16
+over-encoding that must *stay* green, and the restored source.
+
+The one that produced the most information was restoring the original shifted
+table: it turns **three** cells red rather than one, because the error is
+invisible at code 100 and only visible from 101 down. See below.
+
 ## The executor problem (blocking everything downstream)
 
 The whole point of a Pascal→8086 compiler is that the output *runs*, and it
-still has not: no compiled image and no runtime entry has ever been executed
-on a correct CPU. So the plan stands: get a real CPU emulator, run the image,
-assert the exact stdout bytes.
+still has not: **no compiled image and no runtime entry has ever been executed
+on a CPU**, correct or otherwise. Everything in the five checks above is a claim
+about bytes. Whether the bytes work is the untested part, and it is the only
+part that cannot be closed by writing another checker.
+
+The rest of this section is about establishing what *can* be believed, because
+the first attempt at this used an emulator that was wrong, and a wrong oracle is
+worse than none: it cannot distinguish "my codegen is broken" from "the machine
+is broken".
 
 **Unicorn 2.1.4 cannot be used for this.** `UC_MODE_16` mis-decodes 16-bit
 ModRM memory operands. The measurement, by loading a byte-pattern image so a
@@ -144,6 +233,37 @@ emulator is wrong", which makes it worse than no emulator at all.
 `pip install --upgrade unicorn` resolves to the same 2.1.4, so this is not
 avoidable by upgrading.
 
+**This is no longer the whole picture: qemu-system-i386 is now a trusted
+oracle, and it was trusted by measurement, not by reputation.** `modrm19.s`
+assembles with GNU `as`, is wrapped in a 512-byte boot sector, and is booted
+under `/usr/bin/qemu-system-i386` with the serial port captured to a file. For
+each of 24 ModR/M encodings it stores a marker through the encoding under test
+and then *scans memory* for where the word landed, with `BX=1000 DI=2000
+SI=0030 BP=0040` so every candidate address is distinct. The answers come out
+as raw offsets, so this is arithmetic, not a judgement call, and the 23 cells it
+covers are the project's ground truth for effective addressing. It is also how
+we know **qemu's 8086 is right where Unicorn's is wrong**, on the same
+instruction class, by the same method.
+
+Two facts about the tooling came out of that work and are worth recording
+because both were believed wrong at first:
+
+- **`objdump -D -b binary -m i8086` disassembles 16-bit code correctly.** An
+  earlier note in this file said there was no usable 16-bit disassembler
+  available; that was wrong, and the cost of believing it was a hand-derived
+  ModR/M table. FCML (`fcml-disasm -m16`) is the other decoder and the two are
+  cross-checked by `tests/fcml_vs_objdump.py`. Note `fcml-disasm` linear-sweeps
+  and aborts (rc=134) on inputs of 16 bytes or more through the Debian wrapper,
+  which is why `tests/disasm16.py` windows input at 15.
+- **A qemu execution probe for mod=11 is structurally impossible**, not merely
+  awkward. The comparison register is itself a candidate target, and SP is
+  destroyed by the next `call` before any check can run — the first version of
+  that probe pushed its return address through `SS:0xBEEF`, so the evidence it
+  was about to collect had already been overwritten. It also "cleared AX
+  because AX is never an r/m target", which is true of the wrong table and
+  false of the right one. So mod=11 is measured by *encoding* instead, with
+  `as` as an oracle independent of both the runtime and qemu.
+
 Also ruled out, for the record:
 
 - **DOSBox-X 2025.02.01** (installed, `/usr/bin/dosbox-x`, plain root-owned
@@ -152,35 +272,27 @@ Also ruled out, for the record:
   commands never observably execute: a `md` never appeared on the host, and a
   `.COM` that creates `OUT.TXT` via `INT 21h AH=3Dh/40h` never produced the
   file. Shell `>` is intercepted by dosbox-x's own wrapper
-  (`SHELL:Redirect output to out.txt`). A `.BAT` route timed out.
-- **qemu-system-i386 is installed** (`/usr/bin/qemu-system-i386`) and is the
-  next candidate: a 512-byte boot sector can load the image and a `INT 21h`
-  shim can hand the output back. Not attempted yet.
-
-`tests/rt_exec.py` is the harness, written against Unicorn, and it is written
-to survive the switch: it loads the runtime, calls each entry with a known
-argument, and compares the bytes sent to `INT 21h` against expectations. It
-currently **fails**, and the failures are the emulator's, not the library's —
-`wrint` prints `-` for every value because `MOV AX,[BP+4]` reads the wrong
-address. Do not read those results as a verdict on the runtime.
-
-It holds **33 checks** (7 pass, 33 fail — the 7 are the `stackchk` returns and
-`rdln` cases that happen not to touch memory) covering `initmem`, `wrint`
-(10 values incl. both `INT16` extremes), `wrchar`, `wrbool`, `wrln`,
-`stackchk`, a composed `writeln(42) writeln TRUE` sequence, and the read
-entries against supplied input including EOF. It needs the venv that has the
-package — `unicorn` is **not** in the system Python:
-
-```sh
-/home/eric/.venvs/tp3emu/bin/python tests/rt_exec.py     # → 33 FAILURE(S), rc=1
-```
-
-It has **no `wrtinl` case yet**. That entry needs a harness change, not just a
-machine change, because its argument is not a stack word: the caller must place
-a length byte and the characters at the *return address*, which is precisely
-the property the compiler relies on — so testing it also tests the encoding
-contract between `Compiler.IoCall` and `Runtime.EmitWrInl`. Add it when the
-executor exists, and expect all 33 current checks to pass first.
+  (`SHELL:Redirect output to out.txt`). A `.BAT` route timed out. The user has
+  since installed **FreeDOS** (`freedos.qcow2`, FD14-LiveCD) which is very
+  likely the answer to this, and untried.
+
+**So what is still missing is narrow: qemu can decode, assemble and execute, but
+nothing has yet booted an image that was produced by *this* compiler.** The
+remaining piece is a boot sector that reads a `.COM` off the floppy with
+`INT 13h`, sets `SS:SP` at the segment top, hooks `INT 21h` for
+`AH=02h/09h/4Ch` to the serial port, and `JMP 0x100`. `tests/rt_exec.py` is
+the harness that will consume it: it loads the runtime, calls each entry with a
+known argument, and compares the bytes sent to `INT 21h` against expectations —
+33 checks covering `initmem`, `wrint` (10 values incl. both `INT16` extremes),
+`wrchar`, `wrbool`, `wrln`, `stackchk`, a composed `writeln(42) writeln TRUE`
+sequence, and the read entries against supplied input including EOF. It was
+written against Unicorn and **currently fails 33 of 33** — the failures are
+Unicorn's, not the library's, and `run_all.sh` does not run it. Re-point it at
+qemu and those numbers become a verdict on the runtime. It has **no `wrtinl`
+case yet**, which needs a harness change rather than just a machine change: that
+entry's argument is not a stack word, the caller must place a length byte and
+the characters at the *return address*, so testing it also tests the encoding
+contract between `Compiler.IoCall` and `Runtime.EmitWrInl`.
 
 ## Components
 
@@ -244,17 +356,16 @@ writeln('hi')                 ; CALL 70H ; 02 'h' 'i' ; CALL 40H
 readln(x)       LEA AX,[0104]; PUSH AX ; CALL 48H ; ADD SP,2 ; CALL 60H
 ```
 
-(Those are the `TU_*` **placeholders** — the real runtime is not wired in yet,
-see the limitations. The third line is the inline-literal form, which differs
-in kind: no value is pushed and the `ADD SP,2` is absent, because the length
-and the characters are the argument.)
+(Those `TU_*` names are the compiler's own; the offsets behind them are now
+**assigned from `Runtime.RT_Entry`** rather than written down, so the
+placeholder-versus-real distinction is gone. The third line is the
+inline-literal form, which differs in kind: no value is pushed and the
+`ADD SP,2` is absent, because the length and the characters *are* the
+argument.)
 
 `READ`/`READLN` push the *address* so the runtime can store
-(`EmPushVarAddr`: `8D 46 disp` / `8D 06 off`); a non-variable argument is
-`ETypeErr` (56), as in TP3. `TU_WrInt/Char/Bool/Real`, `TU_WrLn`,
-`TU_RdInt/Char/Bool`, `TU_RdLn`, `TU_WrInl`, `TU_Halt` continue the existing
-`TU_*` image-base space (`TU_InitMem=8H`, `TU_ProgEnd=10H`, `TU_StackChk=18H`,
-`TU_WrInl=70H`).
+(`EmPushVarAddr`: `LEA AX,[BP+off]` via `EmBpDisp` for locals, `8D 06 off` for
+globals); a non-variable argument is `ETypeErr` (56), as in TP3.
 
 Detail: `TP3-COMPILER.md`.
 
@@ -269,22 +380,27 @@ be checked by hand against an 8086 table. This is the same approach
 
 It mirrors the original's own mechanism: TPSRC7 `copyrt` copies the runtime
 into the front of the code buffer (`SI=DI=0`, `REPZ MOVSB`) and `pc` is then
-initialised past it (`MOV pc,#$2D7C`). Same shape here — the compiler is meant
-to copy the blob to the front of `cbuf` and start `pc`/`dc` past it. Runtime
-data therefore lives at fixed low offsets and needs no relocation, and because
-both sides of every `CALL` shift by the same amount, `EmCall`'s displacement
-arithmetic is unaffected by the runtime being prepended.
+initialised past it (`MOV pc,#$2D7C`). Same shape here, and now actually done:
+`pc := RT_Size`, `dc := RT_Size + 1000H`, so the image is
+`[runtime][program header][program code]` and every emitted address is
+image-absolute. **No relocation pass is needed** — worth having paid for, since
+a linker that has to walk fixups is a linker that can get them wrong.
 
-Current blob: **385 bytes**, 14 entries, offsets read back out of the
-assembled bytes by `tests/RtProbe.mod`:
+Current blob: **391 bytes**, 14 entries, 37 branch targets, offsets read back
+out of the assembled bytes by `tests/check_runtime.py` rather than asserted by
+hand:
 
 | entry | offset | entry | offset | entry | offset |
 |---|---|---|---|---|---|
-| `initmem` | 0 | `wrint` | 36 | `rdint` | 161 |
-| `progend` | 28 | `wrchar` | 97 | `rdchar` | 262 |
-| `stackchk` | 35 | `wrbool` | 106 | `rdbool` | 283 |
-| `halt` | 28 | `wrreal` | 126 | `rdln` | 329 |
-| | | `wrln` | 134 | `wrtinl` | 142 |
+| `initmem` | 0 | `wrint` | 36 | `rdint` | 167 |
+| `progend` | 28 | `wrchar` | 97 | `rdchar` | 268 |
+| `stackchk` | 35 | `wrbool` | 109 | `rdbool` | 289 |
+| `halt` | 28 | `wrreal` | 132 | `rdln` | 335 |
+| | | `wrln` | 140 | `wrtinl` | 148 |
+
+The `TU_*` constants in `Compiler.mod` are **assigned from
+`Runtime.RT_Entry` in `Inittur`**, not written down, so a runtime edit that
+moves an entry cannot leave the compiler calling the old address.
 
 `progend` and `halt` deliberately share one address (`XOR AX,AX / MOV AH,4C /
 INT 21h / RET`): the compiler already zeroes AX before `progend` and discards
@@ -301,16 +417,69 @@ Conventions, matching `Compiler.IoCall` exactly:
 | `ProgEnd/Halt` | nothing | exits, code 0 |
 | `WrInl` | **nothing** — reads its own text via `POP BX` | see below |
 
-`InitMem` reads the data base and end out of the header words at `+2`/`+6` and
-zeroes that range, because Pascal leaves globals undefined. `StackChk` is a
-bare `RET` — range and stack checking aren't compiled in yet, and the call site
-sits mid-expression, so it must not touch a register.
+`InitMem` receives the program-header offset **in AX** (not on the stack), reads
+the data base and end out of the header, and zeroes that range, because Pascal
+leaves globals undefined. `StackChk` is a bare `RET` — range and stack checking
+aren't compiled in yet, and the call site sits mid-expression, so it must not
+touch a register.
+
+**Five emitter bugs were fixed here in one session, all found by the
+`audit_helpers.py` name-vs-decode pass described above**, and all of them were
+invisible to everything else that was already in place:
+
+| emitter | emitted | actually was | correct |
+|---|---|---|---|
+| `MovSiBx` | `89 DC` | `MOV SP,BX` | `89 DE` |
+| `CmpSiBx` | `39 DC` | `CMP SP,BX` | `39 DE` |
+| `MovSiAx` | `8B C0` | `MOV AX,AX` (a no-op) | `8B F0` |
+| `initmem` zeroing loop | `MovAxDx` | loaded a value it then discarded | `XOR AX,AX` |
+| `initmem` header read | `+8` | `hdrMax` | `+6` (`hdrHeap`) |
+
+The third is the instructive one, because it is the same error pointed the
+other way. `8B C0` reads as `MOV AX,AX` under the shifted ModR/M table this
+project shipped, and the fix looked like it should be the byte that table said
+was SI. It is not: for opcode `8B` the **reg field is the destination**, so
+`8B F0` (reg=110=SI, r/m=000=AX) is `MOV SI,AX`, which is what the name asks
+for. Getting the direction backwards nearly caused a correct fix to be
+reverted.
 
 Two label-name plus fixup list: `rel8`, `rel16` and runtime-data addresses are
 all patched after the blob is placed, so nothing depends on a hand-computed
 displacement.
 
-**It has never executed.** See the emulator finding below.
+**It has still never executed.** The encodings are now audited, golden-pinned
+and non-vacuity-proved, which is a much stronger static claim than "it
+assembles" — but a static check cannot tell you the code *works*, only that it
+is what was intended. See the executor section.
+
+### The program image — `shell/Linker.mod`
+
+The runtime is copied to the **front** of the code buffer, then the program
+header, then the program. The header is our own format at `rtSz`:
+
+| off | field | |
+|---|---|---|
+| +0 | `hdrFlag` | 1, "header present" |
+| +2 | `hdrCS` | |
+| +4 | `hdrDS` | |
+| +6 | `hdrHeap` | = `dc`, the end of the data area |
+| +8 | `hdrMax` | |
+| +10… | | max-open-files, input buffer, output buffer words |
+
+`InitMem` gets the header offset in AX and reads `+6` for the heap limit; the
+independent checker in `run_com_tests.sh` restates these offsets as its own
+constants and asserts `initmem`'s SI displacements equal them, assertion by
+assertion, rather than asking the compiler where it thinks the header is.
+
+`CmdCompile` honours the Destination option: 0 = memory, 1 = `.COM`, 2 = `.CHN`
+(refused). A `.COM` is named after its source with the extension swapped at the
+last dot, padded with a zero gap to `max(pc, dc)`; its stack sits at the segment
+top (`SS = SP = CS:FFFE`), which is where DOS puts it.
+
+**Known limit:** the data area starts at a fixed `rtSz + 1000H` (391 + 4096 =
+4481), so a program whose code exceeds 4 KiB runs into its own data. Every
+fixture is at 4491 or 4493 bytes. Documented rather than fixed, because the
+original has the same fixed-offset behaviour.
 
 ### String literals — `writeln('hi')`
 
@@ -331,13 +500,13 @@ TPSRC4 `xwrtinl` is what makes that self-delimiting: `POP BX` takes the return
 address — which *is* the address of the length byte — and the entry ends with
 `JMP BX`, returning to just past the last character. So the literal needs no
 terminator, no length table, and **nothing at all in the data segment**. The
-arithmetic confirms it: `t26` emits `02 68 69` inline and its `data=` stays
-**260**, unchanged from a program with no strings at all.
+arithmetic confirms it: `t26` emits `02 68 69` inline and its data size is
+unchanged from a program with no strings at all.
 
-`wrtinl` is 19 bytes at offset 142, hand-checked against the 8086 table
-(`5B` POP BX · `31 C9` XOR CX,CX · `8A 0F` MOV CL,[BX] · `43` INC BX ·
-`B4 02` MOV AH,2 · `E3 07` JCXZ to the end label · `8A 07` MOV AL,[BX] ·
-`CD 21` · `43` · `E2 F9` LOOP · `FF E3` JMP BX).
+`wrtinl` is 19 bytes at offset 148 (`5B` POP BX · `31 C9` XOR CX,CX ·
+`8A 0F` MOV CL,[BX] · `43` INC BX · `B4 02` MOV AH,2 · `E3 07` JCXZ to the end
+label · `8A 07` MOV AL,[BX] · `CD 21` · `43` · `E2 F9` LOOP · `FF E3` JMP BX) —
+hand-checked once, and now also covered by the golden disassembly and the audit.
 
 **A string literal is a value in exactly one place: a `WRITE`/`WRITELN`
 argument.** Everywhere else it is a hard error, and it is enforced in a single
@@ -366,51 +535,49 @@ Details that are deliberate, not incidental:
   44 and the runtime would print 44 of them and silently drop the rest. TP3
   strings are at most 255 characters, so refusing is the faithful answer.
 - **A string *variable* is `ENoLib`, not wrong code.** `IoCall` knows the
-  difference and refuses, because `EmPushVarAddr`'s local form is still broken
-  (see bug 4 below) and a bad address prints garbage rather than failing.
+  difference and refuses. `EmPushVarAddr`'s local form is fixed now (see bug 4
+  below), so the blocker is no longer the encoding — it is that there is no
+  `string` type, no length word, no assignment path and no `WrStr` entry.
 
 
 ## Honest limitations
 
-- **A compiled image still cannot be executed.** `CmdRun` is a stub, the
-  linker is unwritten, and no 8086 executor on this machine has yet proved
-  trustworthy (above). The emitted code is verified *byte by byte* against the
-  offsets the compiler intends, but nothing has ever run it.
-- **The runtime is written but unproven.** `Runtime.mod` assembles to 385 bytes
-  and its entry offsets are derived from the emitted bytes, but it has never
-  been executed on a correct CPU, so treat every encoding in it as unverified.
-  Two of its own bugs were found by decoding the hex dump, one by running it
-  under the broken emulator; `wrtinl` is the newest and the only entry no test
-  touches even in principle.
+- **A compiled image has still never been executed.** This is the one
+  limitation that everything else is downstream of. `CmdRun` is a stub; the
+  linker *does* now write a real `.COM`; qemu is a proven oracle for encodings
+  but nothing has yet booted an image this compiler produced. Everything in the
+  checks section is a claim about the bytes, and the bytes have been checked
+  hard. Whether the bytes *work* is exactly the untested part.
+- **The runtime is audited but unproven.** 391 bytes, 14 entries, every one-line
+  emitter decoded against its own name, the whole code region golden-pinned, all
+  37 branch targets on instruction boundaries — and still never run on a CPU.
+  Nine of its own bugs have been found this way so far, so the prior is not
+  reassuring. `wrtinl` is the newest entry and the only one no check touches
+  even in principle: its argument lives at its own return address, so testing
+  it needs a harness that models the caller's contract.
 - **`wrreal` is a deliberate stub.** It writes the literal text `?REAL?` — the
   string lives in the runtime's own data block at `D_REAL=24`, which is what
   makes it a real 9-byte routine rather than a trap. Reals are not formatted
   yet, so `writeln(1.5)` "works" and prints nonsense. A trap would be louder;
-  this was chosen because the runtime is not yet reachable, and neither choice
-  is a real answer.
-- **The compiler is not yet wired to the runtime.** `Compiler.mod` still
-  carries the hardcoded placeholder `TU_*` constants (`TU_InitMem=8H`,
-  `TU_ProgEnd=10H`, `TU_WrInl=70H`, …) and still starts `pc` at 0, so emitted
-  images do not contain the runtime and those offsets are still wrong — note
-  the real `wrtinl` is at 142 (8EH) and the placeholder is 70H. `Runtime.mod`
-  is not in `make` or `run_compile_tests.sh` yet for the same reason. Note the
-  prologue's `CALL TU_InitMem` targets offset 8, which is currently the
-  `hdrMax` header word — coherent only once the blob is really prepended.
-  *(Correction to the earlier note in this file: the `TU_InitMem=8` "collision"
-  was a false alarm. Per TPSRC7 the runtime is copied to the *front* of the
-  code buffer and `pc` starts past it, so `TU_*` offsets are runtime-relative,
-  not image-absolute, and no rebasing of the displacement arithmetic is
-  needed. The placeholders stay on the ladder even though the real offsets are
-  now known, because the compiler cannot yet call the runtime — using the true
-  offsets would only make it look like it works.)*
+  neither choice is a real answer, and this is now reachable code rather than
+  an unreachable one, which raises the stakes on the choice.
+- **Code above 4 KiB overruns the data area.** The data base is fixed at
+  `rtSz + 1000H` = 4481 and a `.COM` is padded to `max(pc, dc)`, so the fixed
+  4 KiB code window is real and not advisory. Every fixture is 4491 or 4493
+  bytes, so nothing has hit this yet and nothing tests it.
+- **`EmMovAxSp` still emits a 386-only SIB byte** (`8B 44 24 00`). It is correct
+  on any 386+ but the SIB byte did not exist in 1984, and the whole premise of
+  this project is an 8086. Same class of bug as finding 2 below, unfixed.
 - **String *literals* work; string *variables* do not.** A literal in a
   `WRITE`/`WRITELN` argument list is emitted inline and needs no runtime
   support beyond `wrtinl`. Declaring `s : string`, assigning to it and
-  printing it are all `ENoLib` — there is no `string` type, no length word,
-  no assignment, and `EmPushVarAddr` is wrong for locals. The `chr` flag on
-  `ERes` is what keeps `writeln('a')` calling the *character* writer instead of
-  the integer writer; without it the compiler emitted the integer path and
-  printed 97 while the test still said OK.
+  printing it are all `ENoLib` — there is no `string` type, no length word, no
+  assignment path, no `WrStr` entry. The `chr` flag on `ERes` is what keeps
+  `writeln('a')` calling the *character* writer instead of the integer writer;
+  without it the compiler emitted the integer path and printed 97 while the test
+  still said OK.
+- **Comma-separated names are not supported.** `var i, c : integer;` is a
+  parse error. Pre-existing, unrelated to any of the above, and still open.
 - **Not implemented** (all `ENoLib`): real, set, record, file, string
   *variables*, and any type wider than 2 bytes. `with` is `ENoLib`. `case` *is*
   implemented (cascade `CMP`/`JNZ` per label, per RESUME-TP3.md §3.6) but only
@@ -450,7 +617,7 @@ hand — code sizes looked perfectly plausible throughout.
 Executing the hand-assembled runtime — even under a broken emulator — and
 running the emitted images back through the harness paid for itself
 immediately, because a wrong encoding *executes* rather than failing to
-assemble. Seven so far, none of which a compiler diagnostic would ever have
+assemble. Sixteen so far, none of which a compiler diagnostic would ever have
 reported.
 
 1. **`B()` silently truncated multi-byte opcodes.** `PROCEDURE B` emits exactly
@@ -466,13 +633,21 @@ reported.
 3. **`InitMem` read its argument from `[SP]`** — the return address. The
    convention is a register (`AX`), unlike the per-argument I/O entries which
    do take a stack word. Caught because the data area was never cleared.
-4. **`EmPushVarAddr` computes the wrong base register for locals** (pre-existing,
-   **not yet fixed**). It emits `8D 46 disp`, which is `LEA AX,[SI+disp8]`, but
-   intends `LEA AX,[BP+disp8]` = `8D 45 disp`. So `read` into a *local* variable
-   has always addressed the wrong cell, silently. It also truncates the offset
-   with `off MOD 100H`, losing displacements above 255. The global form
-   `8D 06 off` (`LEA AX,[disp16]`) is correct. Found while writing the runtime's
-   read entries, which needed the same encoding to be right.
+4. **`EmPushVarAddr` computed the wrong base register for locals, and truncated
+   the displacement** (pre-existing; **now fixed**). It emitted `8D 46 disp`,
+   which is `LEA AX,[SI+disp8]`, but intended `LEA AX,[BP+disp8]` = `8D 45 disp`
+   — so `read` into a *local* had always addressed the wrong cell, silently.
+   It also masked the offset with `off MOD 100H`, losing displacements above
+   255. The global form `8D 06 off` (`LEA AX,[disp16]`) was always correct. The
+   fix is `EmBpDisp`, one procedure that owns the choice: disp8 iff
+   `off <= 127`, else disp16, with no overflow branch, because disp16 covers
+   the whole 16-bit range as a signed value and every real offset is either
+   negative (locals, allocated down from `0FFFEh`) or small-positive. It is
+   now shared by `EmLoadVar`, `EmStoreVar` and `EmPushVarAddr` rather than
+   written three times, and guarded by `check_framedisp.py`. The old truncation
+   was *accidentally correct* across −32768..+127, which is the whole range
+   where every real variable lives — so the bug was unreachable from any
+   fixture that existed. `t28` exists to make it reachable.
 5. **A string literal was eating the rest of the source.** Every multi-character
    literal reported its error at *exactly* `Length()` — one past the last
    character of the buffer — so the editor landed past the final `.` of the
@@ -504,6 +679,44 @@ invisible to a check that only asks "does it compile": (5) still produced a
 plausible error *number*, (6) a plausible code *size*, and (7) a plausible
 *character*. Each is precisely the shape of bug a compile-only fixture ships.
 
+8. **The `[BP+off]` displacement was truncated to a byte** (see 4 above). Found
+   by reading `EmLoadVar` and asking what `off MOD 100H` means for a negative
+   local offset — at which point the answer is "correct by accident, and
+   unreachable from any fixture that exists", which is the most expensive kind
+   of wrong.
+
+9–13. **Five runtime emitters were one ModRM byte off** — `MovSiBx` `89 DC`,
+   `CmpSiBx` `39 DC`, `MovSiAx` `8B C0`, `initmem`'s zeroing loop, and
+   `initmem`'s header word. Tabulated with their correct encodings in the
+   runtime section above. All five decoded cleanly, all five passed a
+   structural check, and all five passed a golden disassembly. They were found
+   by the one check that asks a question the bytes can answer on their own:
+   *does this decode to what this procedure is called?*
+
+14. **The ModR/M table in the runtime's own documentation was wrong**, and it
+   had been wrong since the runtime was written. It read
+   `CX DX BX SP BP SI DI BX` — the correct list with `AX` dropped off the front
+   and a duplicate `BX` invented at the end. Every code was therefore one too
+   low except `100`, which lands on `SP` either way, so the error was invisible
+   at exactly the cell anyone would check first. This was a *documentation* bug
+   only: the emitters that followed the wrong table emitted `89 DE`/`39 DE`/
+   `8B F0`, which are right. The table is now measured, not remembered — see
+   `tests/probe/README.md`, which is the fuller account.
+
+15. **`DataBytes()` returned `dc`, the absolute end of the data area, not a
+   size.** Every program over-reported by 256, and every `expected.tsv` row had
+   been baselined to agree. The field is documented as "emitted data size in
+   bytes", so 4 is right and 260 was wrong. Re-baselining the whole matrix is
+   exactly the move that can turn a red suite green by hiding a bug, so it was
+   done *with* the semantic argument above written into the file, and the 6-byte
+   rows (the fixtures declaring one global) are the ones that carry the claim.
+
+16. **`build_tpshell.sh` was a broken duplicate of the Makefile** — it built
+   `Posix` as if it were Modula-2, and ignored its own link's return code. It
+   was never run, because the Makefile is what everyone runs, and a build script
+   nobody runs is documentation. The specific lesson: when two things must
+   agree, keep one.
+
 ## gm2 / ISO Modula-2 pitfalls hit along the way
 
 - **Two-phase link** (above) — a single whole-program pass 3 caps
@@ -532,6 +745,13 @@ plausible error *number*, (6) a plausible code *size*, and (7) a plausible
 - termios raw mode: every newline is an explicit `CR LF`.
 - Foreign modules and `Posix` cannot pass `argv`; the test harness reads
   fixture paths from **stdin** instead.
+- `subprocess.run(input=…)` needs **bytes**, not `str`, or it raises inside
+  Python rather than reporting the real error.
+- `as` always picks opcode `89` for a register-to-register `mov`, so it will
+  never emit `8B EC` for the mnemonic `mov bp,sp`. Test anchors that must
+  assert a specific opcode have to be written as literal `.byte`.
+- FCML prints immediates with an `h` suffix (`add sp,8h`), and the Debian
+  `fcml-disasm` wrapper aborts with rc=134 on inputs of 16 bytes or more.
 
 ## Reference material
 
@@ -543,6 +763,12 @@ plausible error *number*, (6) a plausible code *size*, and (7) a plausible
   per Pascal construct (types, skeletons, arithmetic, IF/CASE/REPEAT/WHILE/
   FOR, procedures, parameters, functions, I/O, typed constants, absolutes),
   plus the `TU_*` runtime entry list.
+- `shell/tests/probe/README.md` — **read this before touching any emitter.**
+  What each ModR/M artifact establishes, which oracle measures which half of
+  the table, why the mod=11 execution probe is structurally impossible, and
+  the shifted table this project shipped, in full.
+- `shell/tests/run_all.sh` / `nonvacuity.sh` headers — what runs, in what
+  order, and which breakage is supposed to turn which check red.
 - `Resources/turbopascal3source/TP3/` — TPSRC1-10, the disassembled original.
   **This is the ground truth**; when our behaviour and the book disagree, the
   disassembly wins.
@@ -550,36 +776,39 @@ plausible error *number*, (6) a plausible code *size*, and (7) a plausible
 
 ## Next steps
 
-1. **Get a trustworthy 8086 executor**, because it gates items 2–4 and nothing
-   else can be claimed until a compiled image runs. `qemu-system-i386` with a
-   boot-sector loader and an `INT 21h` shim is the plan; keep `rt_exec.py`'s
-   expectations and change only the machine behind them. (Do *not* reach for
-   Unicorn's 16-bit mode again, and do not re-derive the `rm` table by hand
-   again — check it against a real decoder.) A last resort is writing the 8086
-   interpreter `CmdRun` needs anyway, cross-checked against Unicorn only on
-   `disp16`-only code, which is the one addressing form Unicorn 2.1.4 gets
-   right.
-2. **Fix `EmPushVarAddr`** — `8D 45`/`8D 85` for locals, full `disp16`, per
-   bug 4 above. Two lines, and `read` into a local is wrong until it is done.
-3. **Wire the runtime in and write the linker**: `pc := RT_Size`,
-   `dc := RT_Size + 1000H` (a fixed 4 KiB code/data gap, so a program's data
-   can't collide with its code in a single 64 K `.COM` segment), take the
-   `TU_*` offsets from `RT_Entry` instead of the hardcoded constants, patch the
-   header words (`hdrDS` = data base, `hdrHeap` = data end, so `InitMem` can
-   zero globals), pad the image to cover the data area, and emit the `.COM`.
-   Add `Runtime` to the `make` and `run_compile_tests.sh` rebuild lists.
-4. **Prove it end to end**: compile a fixture, link, execute, assert the exact
-   stdout bytes (`writeln('hi')` → `hi`). That single assertion is what turns
-   this from "assembles" into "works".
-5. **`CmdRun`** — run the emitted image from the `R` menu key.
-6. **String *variables*** — `s : string`, `s := 'hi'`, `writeln(s)`. Blocked on
-   item 2: `IoCall` deliberately raises `ENoLib` for a string variable rather
-   than emitting the wrong address, so the moment `EmPushVarAddr` is fixed this
-   needs a length word, an assignment path, and a `WrStr` entry (TPSRC4
-   `xwrtstr`).
-7. Nested procedures / recursion, `var` parameters (the `SEG:OFF` push from
+1. **Boot a compiled `.COM` and read its output.** This gates everything and
+   nothing else can honestly be claimed until it works. The executor is no
+   longer the open question — qemu is a proven oracle and the FreeDOS image
+   gives a real DOS to run in. Concretely: a boot sector that reads a `.COM`
+   off the floppy with `INT 13h` to `0x100`, sets `SS:SP` at the segment top,
+   hooks `INT 21h` (`AH=02h/09h/4Ch` → serial), `JMP 0x100`; debug with
+   `qemu -d in_asm,exec -D trace.log`; assert the **exact stdout bytes** per
+   fixture against expected-output files under `shell/tests/fixtures/`
+   (`writeln('hi')` → `hi`). That single assertion is what turns "assembles"
+   into "works". Either route works and both are worth having: under FreeDOS
+   for realism, under bare qemu with an `INT 21h` shim for reproducibility.
+2. **Re-point `rt_exec.py` at qemu** and require all 33 of its checks to pass.
+   They currently fail 33/33 under Unicorn, and those failures are the
+   emulator's, not the runtime's. The expectations stay; only the machine
+   changes. Add the missing `wrtinl` case, which needs a harness that models
+   the caller's contract (length byte and characters at the return address).
+3. **`CmdRun`** as an in-process 8086 interpreter — the `R` menu key, and a
+   fallback executor for environments with no DOS. Validate it against qemu on
+   the *same images*, so the two oracles check each other.
+4. **String *variables*** — `s : string`, `s := 'hi'`, `writeln(s)`. The
+   encoding blocker is gone (`EmBpDisp`); what is left is a length word, an
+   assignment path, and a `WrStr` entry (TPSRC4 `xwrtstr`).
+5. Nested procedures / recursion, `var` parameters (the `SEG:OFF` push from
    RESUME-TP3.md §3.11), range/index checks (`TU_RANGE_CHECK`,
    `TU_INDEX_CHECK`), typed constants (RESUME-TP3.md §3.14), `array` at its
-   point of use (`t14`).
-8. Harden the program-header parameter loop against non-advancing input
+   point of use (`t14`), `case` with subrange labels.
+6. **Fix `EmMovAxSp`**, which still emits the 386-only `8B 44 24 00`. On an
+   8086 there is no SIB byte, so the right encoding is `8B 46 00`
+   (`MOV AX,[BP+0]`-adjacent form) or a register copy — this needs thinking
+   against the *measured* table rather than a habit, and a fixture that reads
+   `[SP]`.
+7. Make the 4 KiB code window an enforced limit rather than a documented one:
+   report an error when `pc` reaches `dc`, instead of writing over the data.
+8. Comma-separated names: `var i, c : integer;`.
+9. Harden the program-header parameter loop against non-advancing input
    (`program p(1;)`) with a `BOOLEAN` flag — **not** `EXIT`, which ICEs gm2.

+ 26 - 34
TP3-COMPILER.md

@@ -95,9 +95,11 @@ Three related defects fixed alongside:
 
 ## Tests
 
-Two harnesses live in `shell/tests/` -- in the repo on purpose, because
+Several harnesses live in `shell/tests/` -- in the repo on purpose, because
 `/tmp` is wiped between sessions and an earlier /tmp-only harness was lost
-with it.
+with it.  `shell/tests/run_all.sh` runs them all; its header says which is
+which.  `shell/SUMMARY.md` (at the repo root) is the state-of-the-project
+document.
 
 ### `run_compile_tests.sh` -- compiler front end, no pty, instant
 
@@ -113,39 +115,19 @@ cd shell && tests/run_compile_tests.sh            # all fixtures
 cd shell && tests/run_compile_tests.sh /some/dir  # another fixture set
 ```
 
-```
-cd shell && tests/run_compile_tests.sh            # all fixtures
-cd shell && tests/run_compile_tests.sh /some/dir  # another fixture set
-```
+**The fixture matrix is not written here.**  It used to be, and it went stale
+the moment it was copied -- a second copy of a table is a table that lies.
+The live one is `shell/tests/fixtures/expected.tsv`, which the runner *asserts*
+against: per fixture it pins the verdict plus the code size plus the data size
+(or the error number plus the error position), and the rationale for every
+re-baselining is written in the file rather than in a commit message.  Read that
+for what compiles; this document for how.
 
-Current matrix (23 fixtures) -- **17 compile, up from 1** (the empty
-program) when this work started:
-
-| fixture | result |
-|---|---|
-| `t01_minimal` `program t01; begin end.` | OK code=26 |
-| `t04_var` var decl + assignment + `writeln(x)` | OK code=45 |
-| `t06_two_args` `writeln('a','b')` | OK code=49 |
-| `t07_big` 10 assignments + `writeln(x)` | OK code=99 |
-| `t08_const` const decls, `'A'` char const | OK code=32 |
-| `t09_if` if/then/else | OK code=64 |
-| `t10_while` while | OK code=72 |
-| `t11_for` for/to | OK code=66 |
-| `t12_repeat` repeat/until | OK code=69 |
-| `t13_proc` procedure + value parameter | OK code=50 |
-| `t15_label` label + goto | OK code=35 |
-| `t16_str1` `writeln('a')` (1-char literal) | OK code=39 |
-| `t18_writeln_bare` `writeln` with no parens | OK code=29 |
-| `t19_int1` `writeln(1)` | OK code=39 |
-| `t20_str3` `writeln('a','b','c')` | OK code=59 |
-| `t21_mixed` `writeln(1,'a',2)` | OK code=59 |
-| `t22_case` `case x of 1: ..; 2: .. end` | OK code=93 |
-| `t02`, `t03`, `t05`, `t17` multi-char string literal | ERROR 102 (`ENoLib`) |
-| `t14_types` `array [1..5] of integer` | ERROR 102 (`ENoLib`) |
-| `uierror` deliberate `x := 1 + ;` | ERROR 41 (used by `uitest.py`) |
-
-`code=` is the emitted image size in bytes; 26 is just the prologue, so
-these are real code sizes, not placeholders.
+The two fixtures that fail are `t14` (`array [1..5] of integer` at its point of
+use) and `t25` (a string literal used as a value, `s := 'hi'`), both
+`ENoLib` (102) -- the original's own "not implemented" path, and pinned on
+purpose so that implementing them is a deliberate change to a test rather than
+an accident.  `uierror` is a deliberate syntax error used by `uitest.py`.
 
 ### Hex-dumping the emitted image
 
@@ -222,6 +204,16 @@ push the *address* of the variable (new `EmPushVarAddr`: `8D 46 disp` /
 `8D 06 off`) so the runtime can store; a non-variable argument is `ETypeErr`
 (56), as in TP3.
 
+  *Superseded.*  Both the `TU_*` offsets and the `8D 46 disp` encoding changed
+  after this was written.  The runtime is now prepended to the image and the
+  `TU_*` values are assigned from `Runtime.RT_Entry` rather than written down;
+  and `EmPushVarAddr` for locals was emitting the wrong base register
+  (`8D 46` is `[SI+disp8]`, not `[BP+disp8]`) while truncating the
+  displacement to a byte.  `EmBpDisp` now owns that choice -- disp8 iff
+  `off <= 127`, else disp16 -- and is shared by `EmLoadVar`, `EmStoreVar` and
+  `EmPushVarAddr`.  See `SUMMARY.md`, "Bugs found by actually running code"
+  item 4, and `shell/tests/check_framedisp.py`.
+
 A **single-character** literal needed care.  `RdConst` reports `'a'` as
 `TScalar` (its char code 97) and only longer literals as `TString`.  That is
 right for `c := 'a'` but would make `writeln('a')` print **97**, so

+ 43 - 13
shell/Compiler.mod

@@ -614,19 +614,52 @@ BEGIN
    Ebyte (48H)
 END EmDecAx ;
 
+PROCEDURE EmBpDisp (off : CARDINAL) ;
+(* Emit the ModR/M byte and displacement for a [BP+off] operand, picking the
+   encoding from the size of off.  This is the ONE place that choice is made,
+   because getting it wrong is invisible: 8B 46 d8 and 8B 86 lo hi are both
+   well-formed MOVs, both decode cleanly, and only one of them reads the
+   variable the symbol table names.  So the two are chosen here, once, rather
+   than re-derived at each of the four call sites.  See the ModR/M table in
+   Runtime.mod.
+
+      off <= 127 -> mod=01 rm=110 -> 46 <disp8>      3 bytes with the opcode
+      otherwise  -> mod=10 rm=110 -> 86 <disp16>     4 bytes with the opcode
+
+   Both displacements are SIGNED, and that is the whole subtlety:
+
+     - `off` is a 16-bit value and locals are allocated DOWNWARD from
+       0FFFEh (locFree starts there and is decremented per declaration), so
+       the first local of a procedure sits at off = 0FFFC, which is -4.  The
+       disp16 form reads those same two bytes as a signed value and addresses
+       [BP-4] correctly.  There is no overflow case: all 65536 values of `off`
+       are representable, and a frame larger than 64K is a different problem.
+     - The old code took `off MOD 100H` and always emitted disp8.  That is
+       the correct low byte for every displacement, so it was accidentally
+       right across -32768..+127, which is where locals actually live.  It
+       went wrong at +128, where disp8 80h is -128 and not +128.  So this
+       changes no existing program's bytes and fixes the one case that was
+       broken -- a bug nobody had hit yet, which is exactly why it wanted a
+       test rather than an argument. *)
+BEGIN
+   IF off <= 127 THEN
+      Ebyte (46H) ; Ebyte (VAL (BYTE, off))
+   ELSE
+      Ebyte (86H) ; Eword (off)
+   END
+END EmBpDisp ;
+
 PROCEDURE EmLoadVar (local : BOOLEAN ; off, nbytes : CARDINAL) ;
-VAR disp : CARDINAL ;
 BEGIN
-   disp := off MOD 100H ;
    IF nbytes = 1 THEN
       IF local THEN
-         Ebyte (8AH) ; Ebyte (46H) ; Ebyte (VAL (BYTE, disp))
+         Ebyte (8AH) ; EmBpDisp (off)
       ELSE
          Ebyte (0A0H) ; Eword (off)
       END
    ELSE
       IF local THEN
-         Ebyte (8BH) ; Ebyte (46H) ; Ebyte (VAL (BYTE, disp))
+         Ebyte (8BH) ; EmBpDisp (off)
       ELSE
          Ebyte (0A1H) ; Eword (off)
       END
@@ -634,18 +667,16 @@ BEGIN
 END EmLoadVar ;
 
 PROCEDURE EmStoreVar (local : BOOLEAN ; off, nbytes : CARDINAL) ;
-VAR disp : CARDINAL ;
 BEGIN
-   disp := off MOD 100H ;
    IF nbytes = 1 THEN
       IF local THEN
-         Ebyte (88H) ; Ebyte (46H) ; Ebyte (VAL (BYTE, disp))
+         Ebyte (88H) ; EmBpDisp (off)
       ELSE
          Ebyte (0A2H) ; Eword (off)
       END
    ELSE
       IF local THEN
-         Ebyte (89H) ; Ebyte (46H) ; Ebyte (VAL (BYTE, disp))
+         Ebyte (89H) ; EmBpDisp (off)
       ELSE
          Ebyte (0A3H) ; Eword (off)
       END
@@ -654,13 +685,12 @@ END EmStoreVar ;
 
 PROCEDURE EmPushVarAddr (local : BOOLEAN ; off : CARDINAL) ;
 (* LEA AX,[BP+disp] / LEA AX,[off] then PUSH AX - READ passes the address of
-   a variable, not its value.  8D 46 disp is LEA AX,[BP+disp] and 8D 06 off
-   is LEA AX,[off] (mod=00 rm=110 = direct disp16), both 8086-legal. *)
-VAR disp : CARDINAL ;
+   a variable, not its value.  8D 46 disp is LEA AX,[BP+disp8] and
+   8D 86 lo hi is LEA AX,[BP+disp16]; 8D 06 off is LEA AX,[off]
+   (mod=00 rm=110 = the direct disp16 form).  All three are 8086-legal. *)
 BEGIN
-   disp := off MOD 100H ;
    IF local THEN
-      Ebyte (8DH) ; Ebyte (46H) ; Ebyte (VAL (BYTE, disp))
+      Ebyte (8DH) ; EmBpDisp (off)
    ELSE
       Ebyte (8DH) ; Ebyte (06H) ; Eword (off)
    END ;

+ 37 - 28
shell/build_tpshell.sh

@@ -1,31 +1,40 @@
 #!/bin/bash
-# TP3 tpshell - two-phase ISO build, exactly the proven CocoGm2 recipe.
-#   Phase 1: -fgen-module-list=modules.lst   (generates the import closure list)
-#   Phase 2: -fuse-list=modules.lst          (links using that list)
-# This avoids gm2's whole-program pass-3 identifier cap on compiler-size programs.
-set -u
-D=/home/eric/Projets/Projets-Modula2/MyWork/TP3-comp/shell
-GM2=/home/eric/bin/Modula2/Gm2/bin/gm2
-cd "$D" || exit 9
-FLAGS="-fiso"
+# build_tpshell.sh -- build tpshell.  A thin wrapper; `make` is the build.
+#
+# This used to be a second, hand-maintained copy of the build recipe, and it
+# was wrong in two ways that a copy of a build recipe should never be:
+#
+#   * it ran `gm2 -c Posix.mod`, but Posix is a FOREIGN module written in C
+#     and built by `cc -c Posix.c` (see Posix.def).  gm2 has no Posix.mod, so
+#     the build died on the third module every time.
+#   * it printed phase 2's return code and then carried on regardless, so a
+#     failed link was reported as a successful build as long as some earlier
+#     `tpshell` happened to be lying around.  The Makefile does not have that
+#     hole, which is part of why it is the one that counts.
+#
+# Two copies of a build recipe drift, and the copy that is wrong is the one
+# nobody runs.  So there is now one: the Makefile.  This wrapper exists only
+# so the old invocation keeps working, and it deliberately has no build
+# knowledge of its own beyond that.
+#
+# The two-phase link itself is the real content of the Makefile:
+#   phase 1  -fgen-module-list=...  generates the import closure
+#   phase 2  -fuse-list=...         links against it
+# A single invocation trips gm2's whole-program pass-3 rollup ("too many
+# errors in pass 3") on a compiler-sized program.  Phase 1 exiting 1 is that
+# expected rollup, not a failure -- which is exactly the sort of thing a
+# hand-rolled script gets wrong, and is one more reason it is gone.
+set -eu
+cd "$(dirname "$0")" || exit 9
 
-echo "== compiling each module (isolated -c) =="
-for m in Shell Term Posix TextBuf Editor Compiler Runtime Linker ; do
-   $GM2 $FLAGS -c $m.mod >/tmp/tp_c_$m 2>&1 \
-      || { echo "COMPILE_FAIL $m"; grep -m3 "error:" /tmp/tp_c_$m; exit 1; }
-done
-echo "ok: Shell Term Posix TextBuf Editor Compiler Runtime Linker"
+if command -v make >/dev/null 2>&1; then
+   exec make "$@"
+fi
 
-echo "== Phase 1: generate module list =="
-rm -f modules.lst
-$GM2 $FLAGS -fgen-module-list=modules.lst -o /dev/null \
-    Shell.mod Term.o Posix.o TextBuf.o Editor.o Compiler.o Runtime.o Linker.o >/tmp/tp_p1 2>&1
-echo "p1_rc=$?  list: $(tr '\n' ' ' < modules.lst)"
-
-echo "== Phase 2: link with the list =="
-rm -f tpshell
-$GM2 $FLAGS -fuse-list=modules.lst -o tpshell \
-    Shell.mod Term.o Posix.o TextBuf.o Editor.o Compiler.o Runtime.o Linker.o >/tmp/tp_p2 2>&1
-echo "p2_rc=$?"
-grep -cE "error:|undefined" /tmp/tp_p2
-ls -l tpshell 2>/dev/null | awk '{print "tpshell bytes:",$5}'
+echo "build_tpshell.sh: make is not installed." >&2
+echo "  The build recipe now lives in exactly one place, the Makefile," >&2
+echo "  because a second copy of it was wrong in two ways.  Install make," >&2
+echo "  or read the Makefile and run its four commands by hand:" >&2
+echo >&2
+sed -n '1,20p' Makefile | sed 's/^/    /' >&2
+exit 1

+ 230 - 0
shell/tests/check_framedisp.py

@@ -0,0 +1,230 @@
+#!/usr/bin/env python3
+"""check_framedisp.py -- guard the [BP+off] displacement encoding.
+
+The compiler addresses a procedure's frame through BP, and there are two ways
+to encode an offset:
+
+    8B 46 d8      mod=01 rm=110   MOV AX,[BP+disp8]
+    8B 86 lo hi   mod=10 rm=110   MOV AX,[BP+disp16]
+
+Both are well-formed, both decode cleanly, and only one of them reads the
+variable the symbol table named.  Nothing else in the build can tell them
+apart, so this asserts the choice.
+
+The bug
+-------
+EmLoadVar, EmStoreVar and EmPushVarAddr all used to compute
+
+    disp := off MOD 100H
+
+and always emit the disp8 form.  That is the correct LOW BYTE for any
+displacement, and locals are allocated downward from 0FFFEh, so their offsets
+are negative -- and disp8 0FEh is -2, which is right.  The old code was
+therefore accidentally correct across -32768..+127, which is where almost
+every variable lives, and every existing fixture kept its exact bytes.
+
+It went wrong at +128: disp8 80h is -128 and not +128, so a read of
+[BP+128] became a read of [BP-128].  Procedure PARAMETERS are laid out
+upward from BP+4, so the 63rd parameter of a procedure is at 4 + 62*2 = 128
+and the 63rd word of every call's argument block was read from the wrong
+side of BP.  Nobody had hit it because no fixture declared that many
+parameters, and because a program with no local variables has no BP-relative
+access at all -- so the whole BP path had zero coverage.
+
+The rule now enforced
+---------------------
+EmBpDisp picks disp8 for off <= 127 and disp16 otherwise, where `off` is
+taken as a 16-bit value.  Note that every offset above 32767 is *negative* as
+a displacement, so "otherwise" covers all of them; there is no overflow case
+and no 32767 ceiling.  Both fixtures below therefore have every frame access
+encoded in the 4-byte form, and this check requires that form to be present
+and the 3-byte form to be ABSENT for those offsets -- so restoring the old
+`off MOD 100H` turns the test red.
+
+The expected offsets are computed here from the frame-layout rules rather
+than copied from the compiler's output, so this is a cross-check and not a
+restatement of what the compiler happens to do:
+
+    locals      locFree starts at 0FFFEh and the NEW SYMBOL IS GIVEN THE
+                PRE-DECREMENT VALUE, so the first local of a procedure is at
+                0FFFEh = -2, the second at 0FFFCh = -4, and so on down.
+    parameters  parmOff starts at 4 and grows by 2 per parameter, so
+                parameter k is at 4 + 2*(k-1).
+
+Fixtures
+--------
+t27_localvar.pas  five locals, assigned and read back.  Covers the negative
+                  half of the range, which is where every real variable is.
+t28_farparam.pas  seventy declared parameters, of which the call passes
+                  sixteen (the call site caps arguments at 16).  p63 and p70
+                  are at +128 and +142, the first offsets the disp8 form
+                  cannot represent.
+
+t28 is a compile-level fixture and is never executed: p63..p70 are declared
+but not passed, so at run time those reads would come from uninitialised
+stack.  What it establishes is the ENCODING, which is the thing that was
+wrong.  Do not read t28 as a claim that a 70-argument call works.
+
+Usage: check_framedisp.py [-v]     (from shell/)
+"""
+
+import os
+import subprocess
+import sys
+import tempfile
+
+HERE = os.path.dirname(os.path.abspath(__file__))
+SHELL = os.path.dirname(HERE)
+sys.path.insert(0, HERE)
+import disasm16  # noqa: E402
+
+RTSZ = 391              # re-stated, not asked of the code under test
+COMTEST = os.path.join(SHELL, "comtest")
+
+# (fixture, opcode, [signed offsets])
+# Each offset is checked three ways: the decoded displacement set must contain
+# it, the 4-byte mod=10 encoding of it must be present in the image, and the
+# 3-byte mod=01 encoding of the same offset must be ABSENT.  The last of those
+# is the one that goes red if `off MOD 100H` ever comes back.
+def expected_local_offsets(count):
+    """locFree starts at 0FFFEh; the symbol takes the pre-decrement value, so
+    the first local is at -2.  Signed, because that is what a displacement
+    is."""
+    return [-(2 + 2 * i) for i in range(count)]
+
+
+def expected_param_offsets(index):
+    """parmOff starts at 4 and grows by 2 per parameter."""
+    return 4 + 2 * (index - 1)
+
+
+CASES = [
+    # t27: `vN := const` then `g := v1 + ... + v5`.  Each local is stored once
+    # and loaded once, so each offset appears under both 89 (store) and 8B
+    # (load).
+    ("t27_localvar", "89", expected_local_offsets(5)),
+    ("t27_localvar", "8B", expected_local_offsets(5)),
+    # t28: `g := p63 + p70`.
+    ("t28_farparam", "8B", [expected_param_offsets(63),
+                            expected_param_offsets(70)]),
+]
+
+
+def link_fixtures(work):
+    """compile and link the two fixtures, returning {name: image bytes}"""
+    names = ["t27_localvar", "t28_farparam"]
+    paths = [os.path.join(HERE, "fixtures", n + ".pas") for n in names]
+    p = subprocess.run([COMTEST],
+                       input=("\n".join(paths) + "\n").encode(),
+                       stdout=subprocess.PIPE, stderr=subprocess.DEVNULL,
+                       cwd=work)
+    # ComTest writes the .COM next to the CWD, under the fixture's basename
+    out = {}
+    for n in names:
+        f = os.path.join(work, n + ".COM")
+        if not os.path.exists(f):
+            sys.stderr.write(p.stdout.decode("utf-8", "replace"))
+            raise SystemExit("FAIL: %s.COM was not written" % n)
+        with open(f, "rb") as fh:
+            out[n] = fh.read()
+    return out
+
+
+def bp_operands(code):
+    """every instruction in `code` that is an 8r/9r with a [BP+disp] operand,
+    as (opcode, modrm, disp_value, offset)"""
+    got = []
+    off = 0
+    while off < len(code):
+        t, n = disasm16.decode(code[off:], off)
+        if n == 0:
+            break
+        op = code[off]
+        if op in (0x8A, 0x8B, 0x88, 0x89, 0x8D) and n >= 3 \
+                and code[off + 1] in (0x46, 0x86):
+            modrm = code[off + 1]
+            if modrm == 0x46:              # mod=01 rm=110 -> disp8
+                disp = code[off + 2]
+                if disp > 127:
+                    disp -= 256
+            else:                           # mod=10 rm=110 -> disp16
+                disp = int.from_bytes(code[off + 2:off + 4], "little",
+                                      signed=True)
+            got.append(("%02X" % op, modrm, disp))
+        off += n
+    return got
+
+
+def main(argv):
+    verbose = "-v" in argv
+    if not os.path.exists(COMTEST):
+        print("FAIL: %s not built; run tests/run_com_tests.sh first" % COMTEST)
+        return 1
+
+    problems = []
+    work = tempfile.mkdtemp(prefix="framedisp.")
+    try:
+        images = link_fixtures(work)
+    finally:
+        subprocess.run(["rm", "-rf", work])
+
+    for fixture, want_op, offsets in CASES:
+        img = images[fixture]
+        code = img[RTSZ:]
+        op = int(want_op, 16)
+        mine = [d for (o, _, d) in bp_operands(code) if o == want_op]
+        if verbose:
+            print("%s  opcode %s  [BP+..] accesses found: %s"
+                  % (fixture, want_op, ["%+d" % d for d in mine]))
+        for off in offsets:
+            u = off & 0xFFFF
+            # 1. the displacement really is the one the layout rule predicts
+            if off not in mine:
+                problems.append("%s: no %s access at [BP%+d] (offsets seen: "
+                                "%s)" % (fixture, want_op, off,
+                                         ", ".join("%+d" % d for d in mine)))
+                continue
+            # 2. and it is encoded in the 4-byte mod=10 form ...
+            want = bytes([op, 0x86, u & 0xFF, (u >> 8) & 0xFF])
+            if want not in code:
+                problems.append("%s: expected the bytes %s for [BP%+d] and "
+                                "they are not in the image"
+                                % (fixture, want.hex(" ").upper(), off))
+            # 3. ... and NOT in the 3-byte mod=01 form, which EmBpDisp
+            #    reserves for offsets <= 127.  This is the assertion that
+            #    fails if the old `off MOD 100H` truncation returns.  Note
+            #    the two failure modes are not the same severity, so they
+            #    are reported differently: for a positive offset above 127
+            #    the disp8 form reads a DIFFERENT address, while for a
+            #    negative offset it reads the right one in fewer bytes.
+            bad = bytes([op, 0x46, u & 0xFF])
+            if bad in code:
+                landed = (u & 0xFF) - 256 if (u & 0xFF) > 127 else (u & 0xFF)
+                if landed != off:
+                    problems.append(
+                        "%s: [BP%+d] is encoded as %s, a disp8 that reads "
+                        "[BP%+d] instead -- a different address"
+                        % (fixture, off, bad.hex(" ").upper(), landed))
+                else:
+                    problems.append(
+                        "%s: [BP%+d] is encoded as %s, a disp8 form that "
+                        "EmBpDisp reserves for offsets <= 127 (it would read "
+                        "the right address, but by a different rule)"
+                        % (fixture, off, bad.hex(" ").upper()))
+
+    print("frame displacement: %d local offsets (negative) and %d parameter "
+          "offsets (+128, +142) encoded as mod=10/disp16"
+          % (len(expected_local_offsets(5)), 2))
+    if problems:
+        print("FAIL: %d problem(s)" % len(problems))
+        for p in problems:
+            print("  - %s" % p)
+        return 1
+    print("PASS: every [BP+off] outside -128..+127 uses the 4-byte form, and "
+          "no offset")
+    print("      is truncated to a disp8 that would read a different address")
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv))

+ 23 - 0
shell/tests/fixtures/expected.tsv

@@ -42,6 +42,27 @@
 #            "emitted data size in bytes", so 4 is the true size and 260 was
 #            the bug.  It is a real correction, not a fit-to-the-test change:
 #            the 6-byte rows are the fixtures that declare one global.
+#
+# ADDED for the BP displacement fix.  These two fixtures are the first with
+# any frame access at all -- every earlier fixture put its variables in the
+# DATA segment, addressed absolutely, so the whole [BP+off] path had zero
+# coverage and a truncation there was invisible.
+#
+#   t27_localvar  five LOCAL variables, assigned and read back.  Locals are
+#                 allocated downward from 0FFFEh and each access now costs 4
+#                 bytes (mod=10 + disp16) where it cost 3 (disp8), so its code
+#                 size reflects the fix rather than predating it.  Both
+#                 encodings address the same place; only the rule for choosing
+#                 between them changed.
+#   t28_farparam  70 declared parameters; the call passes 16, which is the
+#                 argument cap at the call site.  p63 lands at BP+128 and p70
+#                 at BP+142 - the first offsets a disp8 cannot represent,
+#                 since 80h is -128 and not +128.  Under the old code these
+#                 two reads came from the wrong side of BP.  This fixture
+#                 tests the ENCODING and is never executed; do not read it as
+#                 a claim that a 70-argument call works.  Its 123 bytes are
+#                 1 more than t27's because the body is one ADD and one
+#                 store rather than five of each.
 
 t01_minimal	OK	29	4
 t02_writeln	OK	38	4
@@ -69,4 +90,6 @@ t23_str_empty	OK	36	4
 t24_str_quote	OK	41	4
 t25_str_as_value	ERR	102	48
 t26_str_mixed_args	OK	58	4
+t27_localvar	OK	122	6
+t28_farparam	OK	123	6
 uierror	ERR	41	331

+ 21 - 0
shell/tests/fixtures/t27_localvar.pas

@@ -0,0 +1,21 @@
+program t27;
+var
+  g : integer ;
+procedure work ;
+var
+  v1 : integer ;
+  v2 : integer ;
+  v3 : integer ;
+  v4 : integer ;
+  v5 : integer ;
+begin
+  v1 := 10 ;
+  v2 := 20 ;
+  v3 := 30 ;
+  v4 := 40 ;
+  v5 := 50 ;
+  g := v1 + v2 + v3 + v4 + v5
+end ;
+begin
+  work
+end.

+ 10 - 0
shell/tests/fixtures/t28_farparam.pas

@@ -0,0 +1,10 @@
+program t28;
+var
+  g : integer ;
+procedure far (p1 : integer, p2 : integer, p3 : integer, p4 : integer, p5 : integer, p6 : integer, p7 : integer, p8 : integer, p9 : integer, p10 : integer, p11 : integer, p12 : integer, p13 : integer, p14 : integer, p15 : integer, p16 : integer, p17 : integer, p18 : integer, p19 : integer, p20 : integer, p21 : integer, p22 : integer, p23 : integer, p24 : integer, p25 : integer, p26 : integer, p27 : integer, p28 : integer, p29 : integer, p30 : integer, p31 : integer, p32 : integer, p33 : integer, p34 : integer, p35 : integer, p36 : integer, p37 : integer, p38 : integer, p39 : integer, p40 : integer, p41 : integer, p42 : integer, p43 : integer, p44 : integer, p45 : integer, p46 : integer, p47 : integer, p48 : integer, p49 : integer, p50 : integer, p51 : integer, p52 : integer, p53 : integer, p54 : integer, p55 : integer, p56 : integer, p57 : integer, p58 : integer, p59 : integer, p60 : integer, p61 : integer, p62 : integer, p63 : integer, p64 : integer, p65 : integer, p66 : integer, p67 : integer, p68 : integer, p69 : integer, p70 : integer) ;
+begin
+  g := p63 + p70
+end ;
+begin
+  far (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16)
+end.

+ 98 - 0
shell/tests/nonvacuity.sh

@@ -19,6 +19,8 @@
 #   check_runtime.py   entry goldens:     catches a broken prologue
 #   probe/modrm11.py   the mod=11 table:   catches the ModRM column itself
 #                      going wrong, which no amount of decoding will show
+#   check_framedisp.py the BP disp rule:   catches a displacement that reads
+#                      a different address than the symbol table named
 #
 # The mod=11 cases do not need a rebuild -- they read the probe sources
 # directly -- so they are cheap, and they are the ones that matter most: the
@@ -210,6 +212,102 @@ else
     fail=$((fail + 1))
 fi
 
+echo
+echo "== the BP displacement rule (check_framedisp.py)"
+# This one is about Compiler.mod rather than the runtime, and it needs the
+# whole toolchain rebuilt (comtest, not rtprobe), so it gets its own rebuild.
+SAVED_C=/tmp/opencode/nonvacuity.Compiler.mod
+cp Compiler.mod "$SAVED_C" || exit 1
+
+rebuild_compiler () {
+    $GM2 -fiso -c Compiler.mod >/dev/null 2>&1 || return 1
+    $GM2 -fiso -fgen-module-list=tests/ct.lst -o /dev/null \
+        tests/ComTest.mod TextBuf.o Posix.o Compiler.o Runtime.o Linker.o \
+        >/dev/null 2>&1
+    $GM2 -fiso -fuse-list=tests/ct.lst -o comtest \
+        tests/ComTest.mod TextBuf.o Posix.o Compiler.o Runtime.o Linker.o \
+        >/dev/null 2>&1 || return 1
+    return 0
+}
+
+# 1. the original bug: `off MOD 100H`, always disp8.  Restores exactly the code
+#    that was there before EmBpDisp existed.  t28's [BP+128] read becomes
+#    [BP-128], which is the failure this whole check is named after.
+python3 - "$SAVED_C" <<'PYEOF'
+import sys
+p = 'Compiler.mod'
+s = open(p).read()
+old = """BEGIN
+   IF off <= 127 THEN
+      Ebyte (46H) ; Ebyte (VAL (BYTE, off))
+   ELSE
+      Ebyte (86H) ; Eword (off)
+   END
+END EmBpDisp ;"""
+new = """VAR disp : CARDINAL ;
+BEGIN
+   disp := off MOD 100H ;
+   Ebyte (46H) ; Ebyte (VAL (BYTE, disp))
+END EmBpDisp ;"""
+assert old in s, "EmBpDisp body not found -- update this mutation"
+open(p, 'w').write(s.replace(old, new))
+PYEOF
+if rebuild_compiler; then
+    expect_red "displacement truncation reads a different address" \
+        "no 8B access at \[BP+128\]" python3 tests/check_framedisp.py
+else
+    echo "  FAIL: the compiler would not rebuild with the truncation"
+    fail=$((fail + 1))
+fi
+cp "$SAVED_C" Compiler.mod
+
+# 2. the other half of the rule: always use the 4-byte form, ignoring the
+#    <= 127 case.  This is over-cautious rather than wrong, so the checker must
+#    still be happy -- which is worth asserting, because a check that only
+#    ever fails on a smaller encoding is a check that pins one answer instead
+#    of the rule.
+python3 - <<'PYEOF'
+p = 'Compiler.mod'
+s = open(p).read()
+old = """   IF off <= 127 THEN
+      Ebyte (46H) ; Ebyte (VAL (BYTE, off))
+   ELSE
+      Ebyte (86H) ; Eword (off)
+   END"""
+new = """   Ebyte (86H) ; Eword (off)"""
+assert old in s, "EmBpDisp branch not found -- update this mutation"
+open(p, 'w').write(s.replace(old, new))
+PYEOF
+if rebuild_compiler; then
+    if python3 tests/check_framedisp.py >/dev/null 2>&1; then
+        echo "  ok: always-disp16 is accepted, so the check pins the rule"
+        echo "       and not one particular encoding"
+        pass=$((pass + 1))
+    else
+        echo "  FAIL: check_framedisp rejects a safe, over-long encoding"
+        python3 tests/check_framedisp.py 2>&1 | sed 's/^/       /'
+        fail=$((fail + 1))
+    fi
+else
+    echo "  FAIL: the compiler would not rebuild with always-disp16"
+    fail=$((fail + 1))
+fi
+cp "$SAVED_C" Compiler.mod
+
+if rebuild_compiler; then
+    if python3 tests/check_framedisp.py >/dev/null 2>&1; then
+        echo "  ok: check_framedisp passes on the restored source"
+        pass=$((pass + 1))
+    else
+        echo "NOT RESTORED: check_framedisp is red after restoring Compiler.mod"
+        python3 tests/check_framedisp.py 2>&1 | sed 's/^/       /'
+        fail=$((fail + 1))
+    fi
+else
+    echo "NOT RESTORED: the compiler would not rebuild"
+    fail=$((fail + 1))
+fi
+
 echo
 echo "non-vacuity: $pass ok, $fail failed"
 [ "$fail" -eq 0 ]

+ 7 - 0
shell/tests/run_all.sh

@@ -15,6 +15,12 @@
 #                       checked ON DISK.  This is the end-to-end path from
 #                       keystroke to artifact.
 #
+#   5. frame displ      links the two fixtures that touch a stack frame and
+#                       asserts the [BP+off] displacement encoding, which no
+#                       other check can see: disp8 and disp16 are both
+#                       well-formed and both decode cleanly, and only one of
+#                       them reads the variable the symbol table named.
+#
 #   RtProbe             dumps the runtime size and its 14 entry offsets, so a
 #                       runtime change that moves an entry is visible here.
 #
@@ -107,6 +113,7 @@ run "mod=11 table"     python3 tests/probe/modrm11.py
 run "runtime image"   python3 tests/check_runtime.py
 run "compile matrix"  tests/run_compile_tests.sh
 run "COM linker"      tests/run_com_tests.sh
+run "frame displ"     python3 tests/check_framedisp.py
 run "UI error path"   python3 tests/uitest.py
 run "UI success path" python3 tests/comtest.py