Просмотр исходного кода

TP3-NONVACUITY-RDBYTE: a byte reader of its own, and bug 36's four sites re-derivable in suite

Part 1 - readln of a BYTE (bug 37).

t37_readbyte declares `b : byte` next to `n : integer`, assigns n := 1000 once
and never again, and reads b twice. Its .out was derived by hand from that
layout before anything was changed: 200 1000 / 255 1000. Run against the
compiler as it stood, both oracles printed 200 768 / 255 1023 - readln (b)
went through rdint, which stores a WORD, so the second byte written was n's
low byte. 1000 is $03E8: a word store of $00C8 overwrites its low byte with 00
($0300 = 768), a store of $FFFF overwrites it with FF ($03FF = 1023). The row
that was wrong had never been read from the input at all - it was written
while b was being read.

  * Runtime.mod: one shared scan, rdskip's shape (skip, optional sign, digits,
    push the terminator back), as rdscan - no frame, clobbers AX BX CX DX DI,
    returns the value in AX and a state in CX, 2 on the ^Z path meaning
    nothing was read. rdint and rdbyte are two thin entries over it, each with
    its own PUSH BP frame, storing AX or AL. The state check that used to sit
    at the shared tail could never fire (CX there was only ever 0 or 1), so it
    moved into the entries where 2 is live - the check is now dead-code-free
    instead of dead. Entry 14 is rdbyte (E_RdByte); entNm, RT_Entry's guard,
    RtProbe, check_runtime's golden dict and rt_exec's entry list all grew to
    match.
  * Compiler.mod: IoCall dispatches on the variable's own size - size = 1 ->
    TU_RdByte, otherwise TU_RdInt - which is TPSRC8 prdemit's
    `CMP varsize,#$01 / JA prdemit`. Both entries take their argument the same
    way, so t37's code and data sizes did not move: 139/7 before and after.

The WIDTH of the store is therefore asserted by execution and not by a size -
no byte count can tell a 1-byte store from a 2-byte one. The rt_exec probes
put a poison byte just past the one-byte variable and require it to survive
for 200, -1, 7 and 0, plus the ^Z case where nothing is stored at all. The
reader itself was not duplicated: the old body was split, not copied.

Boolean reads still store a word into a one-byte variable. TP3 refuses them
(error 66) rather than implementing them, so that choice moved into Next steps
and out of Honest limitations, where the BYTE half used to be recorded as an
unfixed latent overflow.

runtime.golden was re-blessed and the diff read end to end: the data
immediates all moved by the same +29 the runtime grew (436 -> 465 bytes, code
ends at 434), the reader region is the split described above, and every other
instruction is renumbered and unchanged.

Part 2 - non-vacuity for bug 36 (tests/nonvacuity.sh, 62 -> 66 ok).

The argument-order fix has four independent sites, and its only proof was a
quote: a run of the pre-fix compiler recorded in the previous commit's message,
which cannot be re-derived and which nothing kept from drifting. Each site is
now reverted in turn and `run_com_exec.py t36_argclobber` must come back red:

  * ProcFunc's parameter remap loop deleted;
  * ParseCallArgs, ParseCall and IoCall each put back to its deferred-push form
    by splicing the WHOLE PROCEDURE out of
    `git show v-TP3-IMAGELAYOUT:shell/Compiler.mod` - the old code's source of
    truth is the old code, not a copy of it living in the test file and free to
    drift from what it claims to reproduce.

All four were run to a red BEFORE being written into the suite, and they fail
on different rows, which is what makes them four cases: only ParseCall moves
the function result (19 -> 10), only IoCall moves the last line
(FALSE 1 -> TRUE 544), and the callee and ParseCallArgs both turn 5 3 7 into
7 3 5 and then part company - the callee swaps the plain-name rows
(8 0 -> 0 8, TRUE FALSE -> FALSE TRUE) where the deferred parser zeroes them
(0 0, FALSE FALSE). A splice that lands is asserted by counting its anchors
(the definition, not the FORWARD declaration), one that does not apply is a
BROKEN CASE, and one that leaves the fixture green is NOT NON-VACUOUS; all
three count as failures, so none of them can pass by being skipped.

SUMMARY.md: the bug 37 entry; the non-vacuity section's "proved outside the
suite" paragraph replaced by the four in-suite cases; the runtime entry table,
the measured sizes, the execution counts and the check transcripts refreshed
from this run; and the two next-step items that are now done replaced by the
boolean-read one.

Gates on this tree: tests/run_all.sh rc=0, OVERALL: ALL PASS (15 checks) -
compile matrix 38/38, independent .COM check 35/35, qemu 35/35, Exec86 35/35,
runtime entries 41/41, check_8086 38 comparison sites with 0 0F-prefixed bytes
in 465 swept runtime bytes, audit 102/102 - and tests/nonvacuity.sh
`non-vacuity: 66 ok, 0 failed`.
Eric Streit 4 дней назад
Родитель
Сommit
7369a48ad7

+ 130 - 72
SUMMARY.md

@@ -27,6 +27,7 @@ manual, not guessed.
 | `CmdRun` (the `R` key) + `Exec86`, the in-process 8086 interpreter | `v-TP3-CMDRUN` | done, **a second execution oracle: 33/33 fixtures agree with qemu byte for byte, and `R` runs one inside the shell** |
 | Image layout in one file; a check's sweep region measured, not restated | `v-TP3-IMAGELAYOUT` | done, **every reader of a linked `.COM` imports `tests/comimage.py`, and the region `check_framedisp` sweeps is two readings of the file required to agree** |
 | Arguments pushed as they are parsed; the parameter frame read from BP+4 backwards | `v-TP3-ARGORDER` | done, **`t36_argclobber` runs: a computed argument survives the parse of the next one, and the callee's slots match the order they were pushed in** |
+| A `byte` reader of its own, and bug 36's four sites proved re-derivable | `v-TP3-NONVACUITY-RDBYTE` | done, **`readln (b)` of a `byte` prints the byte and leaves the variable beside it alone; reverting any one of the argument-order fix's four sites turns `t36_argclobber` red** |
 
 Every row that names a tag has one, and every tag points at a commit on
 `master`; verified by diffing the rows against `git tag -l`, which is how
@@ -120,11 +121,11 @@ needing a squint. It was checked for vacuousness by reverting the string
 scanner fix: the suite went red with exit 1, and came back green only when the
 fix was restored.
 
-**34 of 37 fixtures compile clean**, up from 1 (the empty program) when the
+**35 of 38 fixtures compile clean**, up from 1 (the empty program) when the
 direct harness was first built.
 
 ```
-compile matrix: 37 passed, 0 failed (of 37)
+compile matrix: 38 passed, 0 failed (of 38)
 ```
 
 Compiling: `t01` minimal · `t04` var+assign+`writeln` · `t06` two args ·
@@ -140,16 +141,17 @@ expression · `t31` a value parameter *and* a call across statements ·
 `t32` `EXIT` out of a `for` body · `t33` all six comparisons · `t34` the
 operators nothing else uses (`div mod and or`, unary `-`, a variable `*`) ·
 `t35` `not`, both halves of TPSRC9's `neglevel` split · `t36` a call whose
-arguments are computed in various positions, each one keeping its own value.
+arguments are computed in various positions, each one keeping its own value. ·
+`t37` `readln (b)` of a `byte` sitting next to an `integer`
 
 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`
 (102), the original's "not implemented" path; `uierror` is a deliberate syntax
 error (41) used by the UI test.
 
-`run_com_tests.sh` additionally links **all 34 that compile** to a real `.COM`
+`run_com_tests.sh` additionally links **all 35 that compile** to a real `.COM`
 and re-verifies the bytes with an independent Python checker that *measures*
-the layout instead of restating it: `independent .COM check: 34 checked, 0
+the layout instead of restating it: `independent .COM check: 35 checked, 0
 failed`. That last part was itself a bug fix — see "the two restated
 constants" below.
 
@@ -190,11 +192,11 @@ proves each one can go red.
 | `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 **both** `Runtime.mod` and `Compiler.mod` decodes to what its *name* says — **101/101**, from an inventory scanned independently of the parser | anything longer than one instruction |
 | `check_comimage.py` | a linked image's layout is described in **exactly one file**: `comimage.py` defines each part once, every reader imports it, no second `find_header` exists anywhere in `tests/` | a check that *hand-rolls* the layout from literals (`hdr = 3 + rt_size`), which defines none of those names — the readers catch that a different way: each states the layout twice, from two independent sources, and requires the two to agree |
-| `check_runtime.py` + `runtime.golden` | the built runtime's code region (436 bytes, code ends at 405) sweeps cleanly through FCML, every entry and all branch targets land on an instruction boundary, and the whole disassembly is byte-for-byte the committed golden | whether the golden is *right* |
+| `check_runtime.py` + `runtime.golden` | the built runtime's code region (465 bytes, code ends at 434) sweeps cleanly through FCML, every entry and all 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 |
 | `run_com_exec.py` | the **emitted image executes** and prints exactly the expected bytes | semantics the fixture never exercises |
 | `run_exec86.py` | the same image runs in a **second, independent 8086** and agrees with qemu byte for byte, 33/33 | the `.out` file it shares with `run_com_exec.py` — a wrong expectation fails both at once |
-| `rt_exec.py` | the **runtime entries themselves** are called directly, one qemu boot per call, 35 cases plus a pre-flight, 36/36 — plus a `check_bp_contract` pre-flight over the built blob: an entry that borrows BP must open with `55 8B EC` (exact) and contain a `5D` (a screen, because `5D` is also a displacement byte) | whether the *caller's* half of a contract is right, where the only witness is a case that happens to make two calls in a row |
+| `rt_exec.py` | the **runtime entries themselves** are called directly, one qemu boot per call, 40 cases plus a pre-flight, 41/41 — plus a `check_bp_contract` pre-flight over the built blob: an entry that borrows BP must open with `55 8B EC` (exact) and contain a `5D` (a screen, because `5D` is also a displacement byte) | whether the *caller's* half of a contract is right, where the only witness is a case that happens to make two calls in a row |
 
 Two of these deserve the detail, because the reason they exist is the reason
 they are hard.
@@ -266,16 +268,16 @@ and `tests/check_comimage.py` asserting there is only one copy of each.
 ### Execution under qemu — `tests/run_com_exec.py`
 
 The check that cannot be written as a byte comparison, so also the one that
-finds the most: 34 fixtures are compiled to `.COM`, put on a floppy, booted,
+finds the most: 35 fixtures are compiled to `.COM`, put on a floppy, booted,
 and their serial output compared to a committed `.out` file **exactly** — CRLF
 included — plus the exit code passed to `INT 21h AH=4Ch`.
 
 ```
-execution: 34 passed, 0 failed (of 34)
+execution: 35 passed, 0 failed (of 35)
 ```
 
 This is no longer the only execution oracle: `run_exec86.py` below runs the
-same 34 images a second time, in a different machine, and requires the two to
+same 35 images a second time, in a different machine, and requires the two to
 agree byte for byte.
 
 It also found bug 33 — the branch polarity inverted in *every* conditional in
@@ -298,7 +300,7 @@ signature (`hdrFlag = 1`, `hdrDS == hdrOff + 1000h + bias`, `hdrHeap > hdrDS`,
 `prologAt` and `dataBase` per file:
 
 ```
-measured runtime size: 436 bytes (header at image offset 439)
+measured runtime size: 465 bytes (header at image offset 468)
 ```
 
 A restated constant that has drifted is worse than a derived one, and the two
@@ -325,8 +327,8 @@ compiler: **does the 8086 have this instruction at all?**
 8086 check: 38 comparison sites, 34 lowered to a Boolean value, 13 lowered to a branch
             value conditions  : = x6  <> x2  < x5  >= x3  <= x2  > x16
             branch conditions : IF / REPEAT x9  CASE x2  FOR downto x1  FOR to x3
-            runtime: 436 bytes, 219 swept, 0 0F-prefixed
-            program code: 31 of 34 fixtures swept end to end, 3373 bytes
+            runtime: 465 bytes, 235 swept, 0 0F-prefixed
+            program code: 32 of 35 fixtures swept end to end, 3496 bytes
             t33_cmpops: 13 comparisons matched against their source operators, in order
             clause H: 8 of 8 fixtures matched the branch conditions read off their source
 ```
@@ -380,7 +382,7 @@ check runs every fixture through it as well as through qemu and requires the
 two to agree **byte for byte**:
 
 ```
-exec86: 34 passed, 0 failed (of 34), cross-checked against qemu
+exec86: 35 passed, 0 failed (of 35), cross-checked against qemu
 ```
 
 Three assertions per fixture, in order of how much they are worth: the output
@@ -438,27 +440,40 @@ goes red before a single instruction has run.
 
 ### Non-vacuity — `tests/nonvacuity.sh`
 
-Every assertion in this file is proved able to fail: **62 deliberate
+Every assertion in this file is proved able to fail: **66 deliberate
 breakages, each asserted to turn exactly one named check red for the stated
-reason, then restored and re-asserted green** — `non-vacuity: 62 ok, 0 failed`.
+reason, then restored and re-asserted green** — `non-vacuity: 66 ok, 0 failed`.
 They cover the runtime's emitter audit and its restored source, the mod=11
 table (including restoring the exact wrong table this project once shipped),
 the `[BP+off]` rule, the behavioural bugs, the emitter-name audit of
 `Compiler.mod`, the BP contract, the `.COM` layout checker, the image-layout
 helper and the three checks that read it, the 8086 lowering, the interpreter
-itself, and the `R` key.
-
-One assertion is proved *outside* that suite, and the difference is stated
-rather than papered over: `t36_argclobber`'s `.out` was compiled and run
-against the compiler as it stood **before** the argument-order fix, on both
-oracles, and came back red with every line whose arguments included a computed
-value wrong while the two lines built from plain names stayed correct — so the
-two that would catch a one-sided flip were seen green before the fix, not after
-it. The evidence is quoted in the commit that introduces the fixture, and green
-followed only with the fix in. What is *not* there yet is the in-suite form of
-that proof: reverting each of the three call parsers and the parameter-offset
-remap in turn and requiring `run_com_exec.py t36_argclobber` red. That is the
-first of the next steps, and until it exists the count above does not cover it.
+itself, the `R` key, and — since `v-TP3-NONVACUITY-RDBYTE` — the four
+independent sites of the argument-order fix.
+
+Those four replace a proof that could only be quoted. `t36_argclobber`'s
+`.out` had been compiled and run against the compiler as it stood **before**
+the argument-order fix, on both oracles, and came back red on every line whose
+arguments included a computed value while the two lines built from plain names
+stayed correct — the two that would catch a one-sided flip. That run is a fact
+about the past: it cannot be re-derived, and nothing stopped the four sites of
+the fix from drifting apart. So each one is now reverted in turn and
+`run_com_exec.py t36_argclobber` must come back red — the parameter-offset
+remap in `ProcFunc` deleted, and each of the three call parsers
+(`ParseCallArgs`, `ParseCall`, `IoCall`) put back to its deferred-push form by
+splicing the *whole procedure* out of the tree the fix landed on top of
+(`git show v-TP3-IMAGELAYOUT:shell/Compiler.mod`), so the old code's source of
+truth is the old code rather than a copy of it living in this file and free to
+drift from what it claims to reproduce.
+
+The four are not one case wearing four hats. Only `ParseCall` moves the
+function call's result (`19` → `10`) and only `IoCall` moves the last line
+(`FALSE 1` → `TRUE 544`); and the callee and `ParseCallArgs` both turn `5 3 7`
+into `7 3 5` and then part company — the callee swaps the plain-name rows
+(`8 0` → `0 8`, `TRUE FALSE` → `FALSE TRUE`) where the deferred parser zeroes
+them (`0 0`, `FALSE FALSE`). A revert that leaves the fixture green is reported
+as NOT NON-VACUOUS, and one that does not apply at all is a BROKEN CASE; both
+count as failures, so neither can pass by being skipped.
 
 Eight arrived with the previous milestone (`v-TP3-CMDRUN`), and each one is the
 same shape: code that compiled clean and passed every byte-level check, until it
@@ -646,12 +661,13 @@ told apart from a broken harness without re-deriving anything.
 never calls `rdint` without a `readln` behind it, never calls `rdbool` twice in
 a row, and never exercises the `INT16` extremes individually.
 
-35 cases, one fresh qemu boot each, plus a pre-flight that runs before any of
+40 cases, one fresh qemu boot each, plus a pre-flight that runs before any of
 them: `initmem` (data poisoned to `AA` first, so
 a zero is a *result* and not a leftover), `wrint` ×10 including both `INT16`
 extremes, `wrchar` ×2, `wrbool` ×3, `wrln`, `stackchk`, a composed
 `writeln(42) writeln TRUE` sequence, `rdint` ×6 against supplied input,
-`rdchar` ×2, `rdbool` ×6, `rdint` at EOF, and `wrtinl`.
+`rdchar` ×2, `rdbool` ×6, `rdbyte` ×4 against supplied input with a poison
+byte sitting past the one-byte variable, `rdint`/`rdbyte` at EOF, and `wrtinl`.
 
 Three properties are load-bearing, and each of them was chosen against the
 easier alternative:
@@ -798,7 +814,7 @@ catches a runaway. Execution outside `[0100h, loadHi)` faults, which is what
 makes the `R` non-vacuity case fail on its very first step.
 
 The instruction set is not "the 8086" but the measured union of (a) every byte
-`Runtime.mod` emits — 219 instructions, swept by `check_8086.py` — and (b)
+`Runtime.mod` emits — 235 instructions, swept by `check_8086.py` — and (b)
 every byte `Compiler.mod`'s `Em*` procedures can write (the generated region
 cannot be swept: inline string literals desynchronise a sweep, so the authority
 there is the source). Everything outside that union faults rather than
@@ -898,21 +914,22 @@ initialised past it (`MOV pc,#$2D7C`). Same shape here, and now actually done:
 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: **436 bytes**, 14 entries, 38 branch targets, offsets read back
+Current blob: **465 bytes**, 15 entries, 41 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` | 171 |
-| `progend` | 28 | `wrchar` | 97 | `rdchar` | 279 |
-| `stackchk` | 35 | `wrbool` | 111 | `rdbool` | 302 |
-| `halt` | 28 | `wrreal` | 136 | `rdln` | 350 |
-| | | `wrln` | 144 | `wrtinl` | 152 |
+| `progend` | 28 | `wrchar` | 97 | `rdbyte` | 278 |
+| `stackchk` | 35 | `wrbool` | 111 | `rdchar` | 308 |
+| `halt` | 28 | `wrreal` | 136 | `rdbool` | 331 |
+| | | `wrln` | 144 | `rdln` | 379 |
+| | | `wrtinl` | 152 | | |
 
 Every number in that table is measured, and that is the point: it was last
 correct when the blob was 391 bytes, and the entries quietly moved as it grew
-to 436 — six of the fourteen offsets in the previous version of this table were
+past 436 to 465 — six of the offsets in an earlier version of this table were
 stale, and the size was stale too. A restated constant that has drifted is
 worse than a derived one, which is why the compiler asks
 `Runtime.RT_Entry` instead (below) and why the checkers locate the runtime by
@@ -931,7 +948,7 @@ Conventions, matching `Compiler.IoCall` exactly:
 | entry | argument | notes |
 |---|---|---|
 | `WrInt/WrChar/WrBool/WrReal` | one 16-bit **value** on the stack | caller pops |
-| `RdInt/RdChar/RdBool` | one **address** on the stack | caller pops |
+| `RdInt/RdByte/RdChar/RdBool` | one **address** on the stack | caller pops |
 | `WrLn/RdLn/StackChk` | nothing | |
 | `InitMem` | `AX` = offset of the program header | a *register*, not a stack word |
 | `ProgEnd/Halt` | nothing | exits, code 0 |
@@ -943,6 +960,12 @@ leaves globals undefined. `StackChk` is a bare `RET` — range and stack checkin
 aren't compiled in yet, and the call site sits mid-expression, so it must not
 touch a register.
 
+`rdscan` is the one routine the readers share and is **not** an entry: it takes
+no argument and no frame, clobbers `AX BX CX DX DI`, returns the value in `AX`
+and a state in `CX`, and `CX = 2` is the `^Z` path — nothing was read, so the
+entry that called it stores nothing. `rdint` and `rdbyte` each open their own
+`PUSH BP` frame, call it, and store `AX` or `AL` themselves.
+
 **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:
@@ -968,7 +991,7 @@ all patched after the blob is placed, so nothing depends on a hand-computed
 displacement.
 
 **It executes.** Every entry is called directly under qemu by
-`tests/rt_exec.py` — 36 cases, all passing — with the register contract each
+`tests/rt_exec.py` — 41 cases, all passing — with the register contract each
 entry must honour stated rather than assumed, which is how `wrchar` and
 `wrbool` were caught destroying the caller's BP. That milestone
 (`v-TP3-BP-CONTRACT`) is the reason this paragraph no longer says the runtime
@@ -1001,9 +1024,9 @@ assertion, rather than asking the compiler where it thinks the header is.
 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` (436 + 4096 =
-4532), so a program whose code exceeds 4 KiB runs into its own data. Every
-fixture's image is between 4539 and 4547 bytes. Documented rather than fixed,
+**Known limit:** the data area starts at a fixed `rtSz + 1000H` (465 + 4096 =
+4561), so a program whose code exceeds 4 KiB runs into its own data. Every
+fixture's image is between 4568 and 4576 bytes. Documented rather than fixed,
 because the
 original has the same fixed-offset behaviour.
 
@@ -1072,9 +1095,9 @@ Details that are deliberate, not incidental:
   `t34_arith`
   through the pty because a pty test is expensive and this one's job is to
   prove the *path* exists (compile → poke → run → report → no file written),
-  which it does with eight assertions. The other 33 are covered by
+  which it does with eight assertions. The other 34 are covered by
   `run_exec86.py`, which calls the same interpreter directly.
-- **Every fixture that compiles is now also run.** 34 of 37 execute; the 3 that
+- **Every fixture that compiles is now also run.** 35 of 38 execute; the 3 that
   do not are `t14` and `t25` (`ENoLib`, by design) and `uierror` (a deliberate
   syntax error). The gap this replaces was nine fixtures that compiled and were
   *never executed* — `t08` const, `t09` if/then/else, `t10` while, `t11` for/to,
@@ -1131,11 +1154,11 @@ Details that are deliberate, not incidental:
   (`7x 03 E9`, searching for the `E9`) and by `t22_case`'s execution, but the
   label immediate itself is unchecked.
 - **The runtime's entries are now each called directly, and two of its
-  invariants are stated rather than implied.** 436 bytes, 14 entries, 101
+  invariants are stated rather than implied.** 465 bytes, 15 entries, 102
   emitter helpers decoded against their own names across both modules, the
   whole code region golden-pinned, all branch targets on instruction
-  boundaries — and 35 cases that call the entries one at a time under qemu
-  (`rt_exec.py`, 36/36). What is *not* covered is what a direct call cannot
+  boundaries — and 40 cases that call the entries one at a time under qemu
+  (`rt_exec.py`, 41/41 with its BP pre-flight). What is *not* covered is what a direct call cannot
   see: `wrtinl` is checked, but only from the harness's side of the calling
   contract. (The nine fixtures that compiled but were never executed used to sit
   here as an extra gap; they are now run, so every fixture that reaches the
@@ -1182,10 +1205,11 @@ Details that are deliberate, not incidental:
   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.
-- **`readln` of a `BYTE` is a latent 1-byte overflow.** It calls `rdint`, which
-  stores a 2-byte word, so the high byte lands on the next variable. TP3 has a
-  separate `xrdbyte` for exactly this. No fixture declares a `BYTE`, which is
-  the only reason this has never been observed. Write the fixture first.
+- **`readln` of a `BOOLEAN` still stores a word into a one-byte variable.**
+  This is bug 37's shape, unfixed: the BYTE half now has its own `rdbyte`
+  entry, but a boolean read still dispatches to `rdbool`, which stores `CX`.
+  TP3 refuses a boolean read with error 66 rather than implementing it, so the
+  honest options are to refuse it too or to add the one-byte entry.
 - **The program-header parameter loop is unguarded against non-advancing
   input.** `program p(1;)` would loop forever. The fix needs a `BOOLEAN` flag
   and **must not** use `EXIT`, which ICEs gm2 in pass 3 (see the pitfalls
@@ -1393,9 +1417,11 @@ Eleven more, and the two worst in the project are here.
     so `rdint` consumed the delimiter and the following statement lost its
     first character. The fix is a one-character pushback slot `D_PUSH` plus
     `ungetch`, and the slot's *address moves whenever the runtime grows*
-    (`rtSz + LoadBias + dataAt + D_PUSH` — at 436 bytes, image `0x1B4`, memory
-    `0x2B4`, one pad byte before the program header at `0x1B7`). It is computed,
-    never hard-coded: the same class of restated-constant bug as `RT_SZ` above.
+    (`LdBxVx (D_PUSH)` is fixed up to `dataAt + D_PUSH + LoadBias`; in the
+    current blob that is `8B 1E CE 02`, so memory `0x2CE` / image `0x1CE`, one
+    unused byte before `rtSz` at image `0x1D1` and the program header three
+    bytes past that). It is computed, never hard-coded: the same class of
+    restated-constant bug as `RT_SZ` above.
 23. **`rdint` did not mirror TP3's `xrdint`/`readnum`.** TP3 checks for `^Z`
     *first*, then skips characters `<= 20h`, then an optional sign, then digits,
     then **pushes the terminator back**. Ours skipped leading whitespace only
@@ -1694,7 +1720,46 @@ reason for existing.
     compiler got the two plain-name lines right and the other six wrong, on
     both oracles at once. Those two greens matter — they are what would catch a
     fix that flipped only one of the two halves — and they were green *before*
-    the change as well as after it.
+    the change as well as after it. `nonvacuity.sh` now regenerates that red
+    in suite as well: see "Non-vacuity" above.
+
+### Then a `byte` variable printed the bytes beside it, and one more appeared
+
+37. **`readln (b)` of a `byte` went through `rdint`, which stores a *word* — so
+    the second byte written was the byte after the variable.** `b` came out
+    right and the variable beside it was silently rewritten; `t37_readbyte`
+    exists because no fixture declared a `byte`, which is why nothing had ever
+    seen it.
+
+    The fixture reads a `byte` `b` sitting next to an `integer` `n` that is
+    assigned `1000` once and never touched again, twice, so its `.out` has two
+    lines — and both were derived by hand from that layout before anything was
+    changed: line 1 is `200 1000`, line 2 is `255 1000`. The pre-fix compiler
+    printed `200 768`
+    and `255 1023`, because `1000` is `$03E8` and a word store of `$00C8`
+    overwrites its low byte with `00` (`$0300` = 768) while a store of `$FFFF`
+    overwrites it with `FF` (`$03FF` = 1023). The `n` that came out wrong had
+    never been read from the input at all: it was written while `b` was being
+    read, one byte to the right of where the store was supposed to stop.
+
+    TP3 has a separate `xrdbyte` for exactly this (TPSRC4) and TPSRC8's
+    `prdemit` dispatches on the variable's size, so the fix is the same shape:
+    `IoCall` picks `TU_RdByte` when `symtab [arg.idx].size = 1` and `TU_RdInt`
+    otherwise, and `Runtime.mod` grew a `rdbyte` entry. The reader was *not*
+    duplicated — skipping, the optional sign, the digits and pushing the
+    terminator back are one routine with no frame, and `rdint` and `rdbyte` are
+    two thin entries over it, each with its own `PUSH BP` frame, storing `AX`
+    or `AL`. The state comes back in `CX` and is checked inside each entry,
+    where it is live: the check the old shared tail had could never fire,
+    because `CX` there was only ever 0 or 1. `CX = 2` is the `^Z` path, where
+    nothing was read and so nothing is stored.
+
+    The *width* of the store is asserted by execution and not by a size —
+    `t37`'s code and data sizes did not move when the fix landed, because both
+    entries take their argument the same way, and no byte count can tell a
+    1-byte store from a 2-byte one. The `rt_exec` probes put a poison byte
+    just past the variable and require it to survive, and the fixture's `.out`
+    is the same assertion at the level of a whole program.
 
 ## The bug family, stated once
 
@@ -1775,14 +1840,11 @@ independently-scanned inventory at all.
 
 ## Next steps
 
-1. **The in-suite non-vacuity cases for bug 36.** `t36_argclobber`'s `.out`
-   was proved able to fail by running the pre-fix compiler — red on both
-   oracles, quoted in the commit that introduced the fixture — but
-   `nonvacuity.sh` does not yet mutate the fix *back*. Revert `ParseCallArgs`,
-   `ParseCall`, `IoCall` and the parameter-offset remap in turn, and require
-   `run_com_exec.py t36_argclobber` red for the stated reason, green on
-   restore. Until that exists, the `62 ok` under "Non-vacuity" does not cover
-   this fixture, and the text there says so.
+1. **`readln` of a `BOOLEAN`** is bug 37's shape with the fix not yet written:
+   `rdbool` stores a word (`CX`) into a one-byte variable. TP3 itself refuses a
+   boolean read with error 66 rather than implementing it, so the choice is to
+   refuse it the same way or to add the one-byte entry. Fixture first, so the
+   bug is red before the fix.
 2. **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`). `IoCall` currently
@@ -1791,21 +1853,17 @@ independently-scanned inventory at all.
    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`), `case` with subrange labels.
-4. **`readln` of a `BYTE`** calls `rdint`, which stores 2 bytes and overflows
-   into the next variable. TP3 has a separate `xrdbyte`; a `TU_RdByte` entry is
-   the fix. No fixture exists yet, which is why it has not been done — write
-   the fixture first, so the bug is red before the fix.
-5. Make the 4 KiB code window an enforced limit rather than a documented one:
+4. 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.
-6. Both spellings of a multi-name declaration. `var i, c : integer;` is
+5. Both spellings of a multi-name declaration. `var i, c : integer;` is
    error 1 at the comma and needs two `var` lines; `procedure f (a : integer;
    b : integer)` is error 1 at the semicolon and needs a comma. Neither is
    wrong Pascal, so a program that compiles under one compiler may not under
    another. A parameter may also not shadow a global (`DupTest` rejects any
    name `Search` finds at any level), which Pascal allows.
-7. Harden the program-header parameter loop against non-advancing input
+6. Harden the program-header parameter loop against non-advancing input
    (`program p(1;)`) with a `BOOLEAN` flag — **not** `EXIT`, which ICEs gm2.
-8. FreeDOS (`freedos.qcow2`, FD14-LiveCD) is still untried. Now that `R` runs
+7. FreeDOS (`freedos.qcow2`, FD14-LiveCD) is still untried. Now that `R` runs
    the image in-process, a real DOS is no longer needed for any claim above —
    but it is still the only way to get a third opinion on the `INT 21h` shim,
    and `Exec86` deliberately implements only the three functions the runtime

+ 18 - 2
shell/Compiler.mod

@@ -89,7 +89,10 @@ CONST
       and `readln(c)` called rdint (which stores a 16-bit result, so it wrote
       two bytes into a one-byte variable).  TP3 TPSRC8 prdtyped/pwriteln
       dispatches on the type identifier for exactly this reason.  BYTE stays
-      TScalar: a BYTE is written as an integer, as in TP3. *)
+      TScalar: a BYTE is written as an integer, as in TP3 - but a READ of one
+      is dispatched on the variable's size instead (TPSRC8 prdnum
+      `CMP varsize,#$01 / JA -> rdint, else rdbyte'), so the one thing class
+      alone cannot tell apart is told apart where the size is in hand. *)
    TChar    = 12 ;
 
    (* symbol kinds *)
@@ -262,6 +265,7 @@ VAR
    TU_WrReal   : CARDINAL ;  TU_WrLn   : CARDINAL ;
    TU_RdInt    : CARDINAL ;  TU_RdChar : CARDINAL ;  TU_RdBool : CARDINAL ;
    TU_RdLn     : CARDINAL ;  TU_Halt   : CARDINAL ;
+   TU_RdByte   : CARDINAL ;  (* one-byte variable reads; see IoCall *)
    TU_WrInl    : CARDINAL ;  (* inline string literal; NO stack argument *)
 
    abortFac : BOOLEAN ;
@@ -2371,7 +2375,18 @@ BEGIN
                ELSIF acls = TChar THEN
                   ent := TU_RdChar                  (* one byte, not a word *)
                ELSIF acls = TScalar THEN
-                  ent := TU_RdInt
+                  (* Dispatch on the SIZE, as TPSRC8 prdnum does:
+                     `CMP varsize,#$01 / JA prdemit' keeps xrdint for
+                     anything wider and falls through to `MOV AX,#xrdbyte'
+                     for a one-byte variable.  A BYTE is still TScalar (it
+                     is *written* as an integer, as in TP3), but reading one
+                     through rdint stored a word - and the second byte of
+                     that word is the byte that FOLLOWS the variable. *)
+                  IF symtab [arg.idx].size = 1 THEN
+                     ent := TU_RdByte
+                  ELSE
+                     ent := TU_RdInt
+                  END
                ELSE
                   Err (ETypeErr) ;
                   RETURN
@@ -3487,6 +3502,7 @@ BEGIN
    TU_RdLn     := RT_Entry (11) ;
    TU_Halt     := RT_Entry (12) ;
    TU_WrInl    := RT_Entry (13) ;      (* inline string literal *)
+   TU_RdByte   := RT_Entry (14) ;      (* a BYTE read stores one byte *)
 END Inittur ;
 
 PROCEDURE HeadWord (VAR slot : CARDINAL) ;

+ 2 - 0
shell/Runtime.def

@@ -27,6 +27,8 @@ CONST
    E_WrReal  = 6 ;  E_WrLn    = 7 ;
    E_RdInt   = 8 ;  E_RdChar  = 9 ;  E_RdBool  = 10 ;
    E_RdLn    = 11 ; E_Halt    = 12 ; E_WrInl   = 13 ;
+   E_RdByte  = 14 ;         (* one-byte store: TPSRC8 prdnum's `varsize <= 1'
+                               arm, TPSRC4 xrdbyte *)
 
    (* Where a .COM's first byte ends up.  DOS loads a .COM at CS:0100 (the
       100 bytes below are the PSP), and CS = DS, so an image offset K is at

+ 85 - 25
shell/Runtime.mod

@@ -117,7 +117,7 @@ VAR
    dataAt : CARDINAL ;
    rtBase : CARDINAL ;
    built  : BOOLEAN ;
-   entNm  : ARRAY [0..13] OF ARRAY [0..MaxNm] OF CHAR ;
+   entNm  : ARRAY [0..14] OF ARRAY [0..MaxNm] OF CHAR ;
 
 (* ---------------------------------------------------------------- *)
 (*  name helpers                                                     *)
@@ -464,6 +464,7 @@ PROCEDURE MovAlArg4 ; BEGIN B (8AH) ; B (46H) ; B (4) END MovAlArg4 ;  (* AL:=[B
 PROCEDURE MovAlDl  ; BEGIN B (8AH) ; B (0C2H) END MovAlDl ;  (* 8A C2: AL:=DL *)
 
 PROCEDURE StDiAx   ; BEGIN B (89H) ; B (5H) END StDiAx ;    (* 89 05: [DI]:=AX *)
+PROCEDURE StDiAl   ; BEGIN B (88H) ; B (5H) END StDiAl ;    (* 88 05: [DI]:=AL *)
 PROCEDURE StSiDl   ; BEGIN B (88H) ; B (14H) END StSiDl ;   (* 88 14: [SI]:=DL *)
 PROCEDURE StDiDl   ; BEGIN B (88H) ; B (15H) END StDiDl ;   (* 88 15: [DI]:=DL *)
 PROCEDURE StDiCx   ; BEGIN B (89H) ; B (0DH) END StDiCx ;   (* 89 0D: [DI]:=CX *)
@@ -759,7 +760,29 @@ BEGIN
 END EmitWrReal ;
 
 PROCEDURE EmitRdInt ;
-(* address on the stack.  A transcription of TP3 TPSRC4 xrdint/readnum:
+(* Read a number into a 16-bit variable: the caller's address is on the
+   stack.  The scan itself is EmitRdScan, shared with rdbyte; an entry owns
+   its frame, its address load and its store, nothing else. *)
+BEGIN
+   M ("rdint") ;
+   PushBp ; MovBpSp ;
+   PushAx ; PushBx ; PushCx ; PushDx ; PushDi ;
+   C8 ("rdscan") ;
+   CmpCxV (2) ;
+   Je8 ("ri_skip") ;                 (* nothing entered: leave the variable
+                                        alone, as TP3's xrdint at rdierr does *)
+   MovDiBp4 ;                        (* ...the caller's address into DI... *)
+   StDiAx ;                          (* ...and store the word.  MovDiBp4 and
+                                        not a [BX] form: there is none; see
+                                        the note on the store emitters. *)
+   M ("ri_skip") ;
+   PopDi ; PopDx ; PopCx ; PopBx ; PopAx ;
+   MovSpBp ; PopBp ; RetR
+END EmitRdInt ;
+
+PROCEDURE EmitRdScan ;
+(* The number reader, called by rdint (word store) and by rdbyte (byte
+   store).  A transcription of TP3 TPSRC4 xrdint/readnum, minus the store:
 
       rnspace:  getbyte ; ^Z ? -> rnend
                 consume ; <= 20h ? -> rnspace
@@ -767,6 +790,17 @@ PROCEDURE EmitRdInt ;
                 consume ; -> rndig
       rnend:    buf[0] := 0
 
+   It builds NO frame of its own and touches neither BP nor SP, so it is
+   called from the caller's frame and that frame's [BP+4] is still the
+   caller's argument when it returns.  It clobbers AX BX CX DX DI, which the
+   caller has already saved.
+
+   Returns AX = the value, and CX = 0 positive, 1 negative, 2 nothing read.
+   CX = 2 is set HERE, on the ^Z path: it is the state both callers test to
+   decide not to store, and before this split the check existed only as a
+   jump to the exit, so the state itself was never produced.  AX is
+   meaningless when CX = 2.
+
    Two behaviours are load-bearing and neither is obvious:
 
    - the character that ENDS the scan is left pending, because readnum tests
@@ -774,15 +808,12 @@ PROCEDURE EmitRdInt ;
      compiler emits next for a readln, is what consumes the newline.  See the
      note on getch.
 
-   - ^Z before any digit reaches rnend with "nothing entered", and xrdint
-     (JZ rdierr) then returns WITHOUT touching the variable.  CX carries both
-     the sign and that state: 0 = positive, 1 = negative, 2 = nothing read.
+   - ^Z before any digit returns without a value, and the caller then returns
+     WITHOUT touching the variable: TP3's xrdint reaches rdierr and RETs.
 
-   Sign in CX, value in DI. *)
+   Sign in CX, value in DI, value in AX on the way out. *)
 BEGIN
-   M ("rdint") ;
-   PushBp ; MovBpSp ;
-   PushAx ; PushBx ; PushCx ; PushDx ; PushDi ;
+   M ("rdscan") ;
    M ("ri_lead") ;
    C8 ("getch") ;
    CmpAl (1AH) ;                     (* ^Z: end of input *)
@@ -838,24 +869,51 @@ BEGIN
    J8 ("ri_dig") ;
    M ("ri_done") ;
    C8 ("ungetch") ;                 (* the delimiter stays pending *)
-   CmpCxV (2) ;
-   Je8 ("ri_out") ;
    CmpCx0 ;
-   Je8 ("ri_st") ;
+   Je8 ("ri_pos") ;
    NegDi ;
-   M ("ri_st") ;
-   MovAxDi ;                    (* the parsed value out of DI, into AX... *)
-   MovDiBp4 ;                  (* ...the caller's address into DI... *)
-   StDiAx ;                    (* ...and store.  The three are needed
-                                  because there is no [BX] form; see the
-                                  note on the store emitters. *)
-   M ("ri_out") ;
-   PopDi ; PopDx ; PopCx ; PopBx ; PopAx ;
-   MovSpBp ; PopBp ; RetR ;
-   M ("ri_none") ;               (* ^Z first: leave the variable alone *)
+   M ("ri_pos") ;
+   MovAxDi ;                        (* the parsed value out of DI and into AX,
+                                       where both callers store it from *)
+   RetR ;
+   M ("ri_none") ;                  (* ^Z first: no value, and CX says so *)
+   MovCxImm (2) ;
+   RetR
+END EmitRdScan ;
+
+PROCEDURE EmitRdByte ;
+(* Read a number into a BYTE variable: the same scan, a one-byte store.
+
+   TPSRC8 prdnum dispatches on the variable's size - `CMP varsize,#$01 /
+   JA prdemit' keeps xrdint for anything wider and falls through to
+   `MOV AX,#xrdbyte' for a one-byte variable - and TPSRC4's two entries
+   differ in exactly one store: xrdbyte is `CLC', xrdint is `STC', and rdi1
+   stores AX or AL on `JNB rdibyt'.
+
+   The flag form is not transcribed.  It would put the STC at the head of the
+   rdint entry, and the BP contract in tests/rt_exec.py requires an entry
+   that borrows BP to *start* with `55 8B EC' and to hand BP back from inside
+   its own slice - which is what an entry owning its own frame does.  Two
+   frames and two stores around one scan is the same idea, with that
+   constraint respected.
+
+   The store is the whole reason this entry exists: rdint writes two bytes,
+   and the second one is the byte that FOLLOWS the variable, so
+   `readln (b)' was reading into b and into whatever the compiler had put
+   next to it. *)
+BEGIN
+   M ("rdbyte") ;
+   PushBp ; MovBpSp ;
+   PushAx ; PushBx ; PushCx ; PushDx ; PushDi ;
+   C8 ("rdscan") ;
+   CmpCxV (2) ;
+   Je8 ("rby_skip") ;                (* nothing entered: leave it alone *)
+   MovDiBp4 ;                        (* the caller's address into DI *)
+   StDiAl ;                          (* ONE byte - 88 05, AL not AX *)
+   M ("rby_skip") ;
    PopDi ; PopDx ; PopCx ; PopBx ; PopAx ;
    MovSpBp ; PopBp ; RetR
-END EmitRdInt ;
+END EmitRdByte ;
 
 PROCEDURE EmitRdChar ;
 BEGIN
@@ -1016,7 +1074,8 @@ BEGIN
    EmitStackChk ;
    EmitWrInt ; EmitWrChar ; EmitWrBool ; EmitWrReal ; EmitWrLn ;
    EmitWrInl ;
-   EmitRdInt ; EmitRdChar ; EmitRdBool ; EmitRdLn ;
+   EmitRdInt ; EmitRdScan ; EmitRdByte ;
+   EmitRdChar ; EmitRdBool ; EmitRdLn ;
    EmitGetCh ; EmitUnGetCh ;
    EmitData ;
    FixUp ;
@@ -1034,6 +1093,7 @@ BEGIN
    SetStr (entNm [11], "rdln") ;
    SetStr (entNm [12], "halt") ;
    SetStr (entNm [13], "wrtinl") ;
+   SetStr (entNm [14], "rdbyte") ;
    built := TRUE
 END RT_Build ;
 
@@ -1061,7 +1121,7 @@ BEGIN
    IF NOT built THEN
       RT_Build (0)
    END ;
-   IF i > 13 THEN
+   IF i > 14 THEN
       RETURN 0
    END ;
    RETURN rtBase + LblOff (entNm [i])

+ 4 - 4
shell/tests/RtProbe.mod

@@ -108,7 +108,7 @@ END PENT ;
 
 PROCEDURE Main ;
 VAR i, n, base : CARDINAL ;
-    names : ARRAY [0..13] OF ARRAY [0..15] OF CHAR ;
+    names : ARRAY [0..14] OF ARRAY [0..15] OF CHAR ;
 BEGIN
    base := GetBase () ;
    RT_Build (base) ;
@@ -121,7 +121,7 @@ BEGIN
    PCARD (RT_Size ()) ; PS (" bytes") ; NL ;
    PS ("code ends at ") ; PCARD (RT_CodeEnd ()) ; NL ;
    i := 0 ;
-   WHILE i <= 13 DO
+   WHILE i <= 14 DO
       names [i] := "" ;
       INC (i)
    END ;
@@ -129,9 +129,9 @@ BEGIN
    names [3] := "wrint" ;   names [4] := "wrchar" ;  names [5] := "wrbool" ;
    names [6] := "wrreal" ;  names [7] := "wrln" ;    names [8] := "rdint" ;
    names [9] := "rdchar" ;  names [10] := "rdbool" ; names [11] := "rdln" ;
-   names [12] := "halt" ;   names [13] := "wrtinl" ;
+   names [12] := "halt" ;   names [13] := "wrtinl" ;  names [14] := "rdbyte" ;
    i := 0 ;
-   WHILE i <= 13 DO
+   WHILE i <= 14 DO
       PENT (i, names [i]) ;
       INC (i)
    END ;

+ 9 - 0
shell/tests/check_runtime.py

@@ -159,6 +159,15 @@ GOLDEN = {
     # because the digit has to survive MUL (which owns DX) and be parked in
     # DI across it.  A three-register version was not possible.
     "rdint": "55 8B EC 50 53 51 52 57",
+    # rdbyte: the same frame as rdint - PUSH BP; MOV BP,SP; PUSH
+    # AX,BX,CX,DX,DI - because it is the same scan under its own frame.  What
+    # differs between the two is the store, `88 05` for AL against `89 05` for
+    # AX, and that is two bytes past the point a prefix golden can reach
+    # without asserting where the whole entry sits.  So this asserts the frame
+    # and the contract (rt_exec.py finds the POP BP inside this entry's own
+    # slice), and the WIDTH is asserted by execution: the rdbyte probes put a
+    # poison byte just past the variable and require it to survive.
+    "rdbyte": "55 8B EC 50 53 51 52 57",
     # rdchar: PUSH BP; MOV BP,SP; PUSH AX,BX,DI - three, no CX and no DX.
     # "DI" is how the caller's address gets in (there is no [BX] form), and
     # the store is `88 15` = MOV [DI],DL - a BYTE store, so readln of a CHAR

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

@@ -183,6 +183,21 @@
 #                 bytes rather than adding them, so the same size came out of a
 #                 different order.  A number that never moved and a behaviour
 #                 that did is the same shape as t30 above.
+#
+#   t37_readbyte   added BEFORE the fix it covers, and its .out was derived by
+#                 hand from the layout before the fix was written: `readln (b)'
+#                 of a BYTE went through rdint, which stores a WORD - and the
+#                 second byte of that word is the byte that FOLLOWS the
+#                 variable, which in this fixture is `n'.  So `b' came out
+#                 right and `n' was silently rewritten: 1000 appeared as 768
+#                 (low byte zeroed) on line 1 and as 1023 (low byte set to
+#                 FF) on line 2, which is what the pre-fix run printed, and
+#                 what it was required to print before the fix was touched.
+#                 Neither size here moved when the fix landed: rdint and
+#                 rdbyte take the same argument the same way, so the dispatch
+#                 chooses which entry answers without changing how long the
+#                 call is.  The WIDTH of the store is asserted by execution
+#                 (run_com_exec.py), not by this row - a size cannot see it.
 
 t01_minimal	OK	29	4
 t02_writeln	OK	38	4
@@ -220,4 +235,5 @@ t33_cmpops	OK	404	12
 t34_arith	OK	448	8
 t35_not	OK	224	7
 t36_argclobber	OK	492	12
+t37_readbyte	OK	139	7
 uierror	ERR	41	331

+ 2 - 0
shell/tests/fixtures/t37_readbyte.in

@@ -0,0 +1,2 @@
+200
+-1

+ 2 - 0
shell/tests/fixtures/t37_readbyte.out

@@ -0,0 +1,2 @@
+200 1000
+255 1000

+ 12 - 0
shell/tests/fixtures/t37_readbyte.pas

@@ -0,0 +1,12 @@
+program t37;
+var
+  b : byte ;
+  n : integer ;
+begin
+  n := 1000 ;
+  readln (b) ;
+  writeln (b, ' ', n) ;
+  n := 1000 ;
+  readln (b) ;
+  writeln (b, ' ', n)
+end.

+ 141 - 0
shell/tests/nonvacuity.sh

@@ -819,6 +819,147 @@ else
 fi
 cp "$SAVED_C" Compiler.mod
 
+# Bug 36, argument order: the fix has FOUR independent halves - three parsers
+# that push each argument the moment it is parsed instead of after the whole
+# list, and the callee that reads its parameters backwards from BP+4 - and
+# t36_argclobber is the one fixture that tells them apart.  Before these four
+# cases existed, its only proof was the pre-fix run quoted in the commit
+# message, which is a fact about the past rather than a check: it cannot be
+# re-derived, and nothing stops the four halves from drifting apart.
+#
+# Each revert below was run to a red BEFORE it was written here, and the four
+# fail on four DIFFERENT rows, which is what makes them four cases and not one
+# wearing four hats: the callee revert turns `5 3 7' into `7 3 5' (every
+# argument lands on its neighbour's parameter), the deferred ParseCallArgs
+# zeroes the rows built from computed arguments, ParseCall changes the
+# function call's result (19 -> 10) and IoCall changes the last write
+# (`FALSE 1' -> `TRUE 544', the first argument printed as the second
+# argument's value).  A revert that left t36 green is reported by expect_red
+# as NOT NON-VACUOUS; a revert that did not land is a BROKEN CASE.
+
+# revert_procedure <name> -- put <name> back the way it was before bug 36 was
+# fixed, spliced out of the tree the fix landed on top of.  The old code's
+# source of truth is the old code: quoting a copy of it in this file is a copy
+# that can drift from the tag it claims to reproduce, and a revert that
+# reproduces nothing is a case asserting nothing.
+revert_procedure () {
+    python3 - "$1" <<'PYX'
+import subprocess, sys
+name = sys.argv[1]
+cur = open('Compiler.mod').read()
+r = subprocess.run(['git', 'show', 'v-TP3-IMAGELAYOUT:shell/Compiler.mod'],
+                   capture_output=True)
+assert r.returncode == 0, 'cannot read the pre-fix tree: %s' % r.stderr.decode()[:200]
+pre = r.stdout.decode()
+
+def span(text, where):
+    # The DEFINITION, not the FORWARD declaration a few screens earlier: the
+    # anchor runs to the end of the line, and a FORWARD line ends `FORWARD ;'
+    # instead of nothing.
+    a, b = 'PROCEDURE %s (idx : CARDINAL) ;\n' % name, 'END %s ;' % name
+    na, nb = text.count(a), text.count(b)
+    assert na == 1 and nb == 1, \
+        '%s: %d definitions, %d ends in %s -- update this case' % (name, na, nb, where)
+    i = text.index(a)
+    j = text.index(b, i) + len(b)
+    return text[i:j], i, j
+
+new, i, j = span(cur, 'Compiler.mod')
+old, _, _ = span(pre, 'the pre-fix tree')
+assert old != new, 'the pre-fix %s is identical to the current one' % name
+open('Compiler.mod', 'w').write(cur[:i] + old + cur[j:])
+PYX
+}
+
+# The callee half: no remap, so parameters are read in declaration order from
+# BP+4 while the arguments are pushed in parse order - the FIRST argument ends
+# up in the LAST parameter's slot, and one-argument calls (all of
+# Runtime.mod's own cases) cannot tell the two layouts apart.
+cp "$SAVED_C" Compiler.mod
+if python3 - <<'PYX'
+p = 'Compiler.mod'
+s = open(p).read()
+old = """   i := nestMark ;
+   WHILE i < symTop DO
+      symtab [i].off := parmOff + 2 - symtab [i].off ;
+      INC (i)
+   END ;
+"""
+assert s.count(old) == 1, 'the ProcFunc parameter remap found %d times -- update this mutation' % s.count(old)
+open(p, 'w').write(s.replace(old, ''))
+PYX
+then
+    if rebuild_compiler; then
+        expect_red "t36 catches the callee laid out in declaration order (no remap)" \
+            "t36_argclobber" python3 tests/run_com_exec.py t36_argclobber
+    else
+        echo "  FAIL: the compiler would not rebuild without the parameter remap"
+        fail=$((fail + 1))
+    fi
+else
+    echo "  BROKEN CASE: the ProcFunc remap mutation did not apply"
+    fail=$((fail + 1))
+fi
+cp "$SAVED_C" Compiler.mod
+
+# The caller half, site 1 of 3: a call used as a statement.  The old code
+# read the whole list into args[] and pushed it backwards afterwards, so a
+# computed argument (which exists only in AX) was gone before the push loop
+# reached it.
+cp "$SAVED_C" Compiler.mod
+if revert_procedure ParseCallArgs; then
+    if rebuild_compiler; then
+        expect_red "t36 catches ParseCallArgs pushing after the whole list" \
+            "t36_argclobber" python3 tests/run_com_exec.py t36_argclobber
+    else
+        echo "  FAIL: the compiler would not rebuild with ParseCallArgs deferred"
+        fail=$((fail + 1))
+    fi
+else
+    echo "  BROKEN CASE: the ParseCallArgs revert did not apply"
+    fail=$((fail + 1))
+fi
+cp "$SAVED_C" Compiler.mod
+
+# Site 2 of 3: the same call reached from ParseAtom, when a FUNCTION is
+# called inside an expression.  A separate site, because it is a separate
+# procedure - and t36's `19' row is the only thing that notices this one.
+cp "$SAVED_C" Compiler.mod
+if revert_procedure ParseCall; then
+    if rebuild_compiler; then
+        expect_red "t36 catches ParseCall pushing after the whole list" \
+            "t36_argclobber" python3 tests/run_com_exec.py t36_argclobber
+    else
+        echo "  FAIL: the compiler would not rebuild with ParseCall deferred"
+        fail=$((fail + 1))
+    fi
+else
+    echo "  BROKEN CASE: the ParseCall revert did not apply"
+    fail=$((fail + 1))
+fi
+cp "$SAVED_C" Compiler.mod
+
+# Site 3 of 3: write/writeln/read/readln, where the argument list is also
+# where an inline string literal gets its call.  Reverting this procedure
+# also reverts the per-argument dispatch inside it (the BYTE read landed
+# here), which is harmless for t36 and is why the splice is the whole
+# procedure rather than the push alone: half a revert is a state the tree
+# was never in.
+cp "$SAVED_C" Compiler.mod
+if revert_procedure IoCall; then
+    if rebuild_compiler; then
+        expect_red "t36 catches IoCall printing the last argument's value" \
+            "t36_argclobber" python3 tests/run_com_exec.py t36_argclobber
+    else
+        echo "  FAIL: the compiler would not rebuild with IoCall's loops split"
+        fail=$((fail + 1))
+    fi
+else
+    echo "  BROKEN CASE: the IoCall revert did not apply"
+    fail=$((fail + 1))
+fi
+cp "$SAVED_C" Compiler.mod
+
 if rebuild_compiler; then
     if python3 tests/run_com_exec.py >/dev/null 2>&1; then
         # No count: the matrix grows every time a fixture is added, and a

+ 18 - 1
shell/tests/rt_exec.py

@@ -92,7 +92,7 @@ SOH, STX, ETX, EOT = 0x01, 0x02, 0x03, 0x04
 # the lookup fail loudly instead of silently testing offset 0.
 ENTS = ["initmem", "progend", "stackchk", "wrint", "wrchar", "wrbool",
         "wrreal", "wrln", "rdint", "rdchar", "rdbool", "rdln", "halt",
-        "wrtinl"]
+        "wrtinl", "rdbyte"]
 
 # A call slot's flags, as rtdrv.s reads them.
 F_STACK = 1     # the argument goes on the stack, popped by the caller
@@ -463,6 +463,18 @@ def build_cases():
             dump=2, dumpbytes=bytes([ord(c), 0xEE]),
             note="rdchar stores ONE byte, so the poison is still in byte 2")
 
+    # rdbyte: the same reader as rdint, entered through its own frame, and the
+    # whole point is the WIDTH of the store.  A poison byte sits just past the
+    # variable, so the word store rdint used would show up here as byte 2
+    # turning into the high byte of the value instead of staying EE.  -1 is
+    # the case where the two disagree most visibly: the value is FFFF, so a
+    # word store writes FF twice.
+    for text, want in [(b"200", 200), (b"-1", 0xFF), (b"7", 7), (b"0", 0)]:
+        add("rdbyte(%r) then rdln" % text,
+            [s("rdbyte", V), n("rdln")], out=b"", stdin=text, dump=2,
+            dumpbytes=bytes([want & 0xFF, 0xEE]),
+            note="rdbyte stores ONE byte, so the poison is still in byte 2")
+
     for c, want in [("T", 1), ("y", 1), ("1", 1), ("F", 0), ("n", 0), ("0", 0)]:
         add("rdbool(%r)" % c, [s("rdbool", V)], out=b"", stdin=c.encode(),
             dump=2, dumpbytes=struct.pack("<H", want))
@@ -472,6 +484,11 @@ def build_cases():
         dumpbytes=b"\xee\xee",
         note="TP3's xrdint returns to rnerr without storing; the poison stays")
 
+    add("rdbyte at end of input leaves the variable alone",
+        [s("rdbyte", V), n("rdln")], out=b"", stdin=b"", dump=2,
+        dumpbytes=b"\xee\xee",
+        note="rdscan returns CX=2 on the ^Z path, so rdbyte stores nothing")
+
     add("wrtinl reads its length and characters from the return address",
         [("wrtinl", 0, F_INL)], out=b"hello",
         note="the one entry whose argument is not a stack word")

+ 1 - 1
shell/tests/run_all.sh

@@ -43,7 +43,7 @@
 #                       plus the reported AH=4Ch status and a non-zero step
 #                       count.  Nothing else in this file can observe R.
 #
-#   RtProbe             dumps the runtime size and its 14 entry offsets, so a
+#   RtProbe             dumps the runtime size and every entry offset, so a
 #                       runtime change that moves an entry is visible here.
 #
 # Three runtime suites run before all of those, because everything else

+ 144 - 128
shell/tests/runtime.golden

@@ -41,7 +41,7 @@
 0036  58          pop ax
 0037  F7 D8       neg ax
 0039  B9 0A 00    mov cx,0ah
-003C  BB 9D 02    mov bx,29dh
+003C  BB BA 02    mov bx,2bah
 003F  89 DE       mov si,bx
 0041  31 D2       xor dx,dx
 0043  F7 F1       div ax,cx
@@ -73,18 +73,18 @@
 0072  83 7E 04 00 cmp word ptr [bp+4h],0h
 0076  89 EC       mov sp,bp
 0078  75 05       jne 7fh
-007A  BA A2 02    mov dx,2a2h
+007A  BA BF 02    mov dx,2bfh
 007D  EB 03       jmp 82h
-007F  BA 9D 02    mov dx,29dh
+007F  BA BA 02    mov dx,2bah
 0082  B4 09       mov ah,9h
 0084  CD 21       int 21h
 0086  5D          pop bp
 0087  C3          ret
-0088  BA AB 02    mov dx,2abh
+0088  BA C8 02    mov dx,2c8h
 008B  B4 09       mov ah,9h
 008D  CD 21       int 21h
 008F  C3          ret
-0090  BA A8 02    mov dx,2a8h
+0090  BA C5 02    mov dx,2c5h
 0093  B4 09       mov ah,9h
 0095  CD 21       int 21h
 0097  C3          ret
@@ -106,126 +106,142 @@
 00B0  51          push cx
 00B1  52          push dx
 00B2  57          push di
-00B3  E8 BC 00    call 172h
-00B6  3C 1A       cmp al,1ah
-00B8  74 54       je 10eh
-00BA  3C 20       cmp al,20h
-00BC  76 F5       jbe 0b3h
-00BE  31 C9       xor cx,cx
-00C0  3C 2D       cmp al,2dh
-00C2  75 06       jne 0cah
-00C4  41          inc cx
-00C5  E8 AA 00    call 172h
-00C8  EB 07       jmp 0d1h
-00CA  3C 2B       cmp al,2bh
-00CC  75 03       jne 0d1h
-00CE  E8 A1 00    call 172h
-00D1  31 FF       xor di,di
-00D3  3C 30       cmp al,30h
-00D5  72 18       jb 0efh
-00D7  3C 39       cmp al,39h
-00D9  77 14       jnbe 0efh
-00DB  2C 30       sub al,30h
-00DD  B4 00       mov ah,0h
-00DF  87 C7       xchg di,ax
-00E1  BB 0A 00    mov bx,0ah
-00E4  F7 E3       mul bx
-00E6  03 C7       add ax,di
-00E8  89 C7       mov di,ax
-00EA  E8 85 00    call 172h
-00ED  EB E4       jmp 0d3h
-00EF  E8 97 00    call 189h
-00F2  83 F9 02    cmp cx,2h
-00F5  74 0E       je 105h
-00F7  83 F9 00    cmp cx,0h
-00FA  74 02       je 0feh
-00FC  F7 DF       neg di
-00FE  8B C7       mov ax,di
-0100  8B 7E 04    mov di,word ptr [bp+4h]
-0103  89 05       mov word ptr [di],ax
-0105  5F          pop di
-0106  5A          pop dx
-0107  59          pop cx
-0108  5B          pop bx
-0109  58          pop ax
-010A  89 EC       mov sp,bp
-010C  5D          pop bp
-010D  C3          ret
-010E  5F          pop di
-010F  5A          pop dx
-0110  59          pop cx
-0111  5B          pop bx
-0112  58          pop ax
-0113  89 EC       mov sp,bp
-0115  5D          pop bp
-0116  C3          ret
-0117  55          push bp
-0118  8B EC       mov bp,sp
-011A  50          push ax
-011B  53          push bx
-011C  57          push di
-011D  E8 52 00    call 172h
-0120  88 C2       mov dl,al
-0122  8B 7E 04    mov di,word ptr [bp+4h]
-0125  88 15       mov byte ptr [di],dl
-0127  5F          pop di
-0128  5B          pop bx
-0129  58          pop ax
-012A  89 EC       mov sp,bp
-012C  5D          pop bp
-012D  C3          ret
-012E  55          push bp
-012F  8B EC       mov bp,sp
-0131  50          push ax
-0132  53          push bx
-0133  51          push cx
-0134  57          push di
-0135  E8 3A 00    call 172h
-0138  31 C9       xor cx,cx
-013A  3C 54       cmp al,54h
-013C  74 12       je 150h
-013E  3C 74       cmp al,74h
-0140  74 0E       je 150h
-0142  3C 59       cmp al,59h
-0144  74 0A       je 150h
-0146  3C 79       cmp al,79h
-0148  74 06       je 150h
-014A  3C 31       cmp al,31h
-014C  74 02       je 150h
-014E  EB 01       jmp 151h
-0150  41          inc cx
-0151  8B 7E 04    mov di,word ptr [bp+4h]
-0154  89 0D       mov word ptr [di],cx
-0156  5F          pop di
-0157  59          pop cx
-0158  5B          pop bx
-0159  58          pop ax
-015A  89 EC       mov sp,bp
-015C  5D          pop bp
-015D  C3          ret
-015E  50          push ax
-015F  E8 10 00    call 172h
-0162  3C 0D       cmp al,0dh
-0164  74 0A       je 170h
-0166  3C 0A       cmp al,0ah
-0168  74 06       je 170h
-016A  3C 1A       cmp al,1ah
-016C  74 02       je 170h
-016E  EB EF       jmp 15fh
-0170  58          pop ax
-0171  C3          ret
-0172  8B 1E B1 02 mov bx,word ptr [2b1h]
-0176  83 FB 00    cmp bx,0h
-0179  74 09       je 184h
-017B  8A C3       mov al,bl
-017D  31 DB       xor bx,bx
-017F  89 1E B1 02 mov word ptr [2b1h],bx
-0183  C3          ret
-0184  B4 08       mov ah,8h
-0186  CD 21       int 21h
-0188  C3          ret
-0189  88 C2       mov dl,al
-018B  BB 01 00    mov bx,1h
-018E  8A DA       mov bl,dl
-0190  89 1E B1 02 mov word ptr [2b1h],bx
-0194  C3          ret
+00B3  E8 13 00    call 0c9h
+00B6  83 F9 02    cmp cx,2h
+00B9  74 05       je 0c0h
+00BB  8B 7E 04    mov di,word ptr [bp+4h]
+00BE  89 05       mov word ptr [di],ax
+00C0  5F          pop di
+00C1  5A          pop dx
+00C2  59          pop cx
+00C3  5B          pop bx
+00C4  58          pop ax
+00C5  89 EC       mov sp,bp
+00C7  5D          pop bp
+00C8  C3          ret
+00C9  E8 C3 00    call 18fh
+00CC  3C 1A       cmp al,1ah
+00CE  74 42       je 112h
+00D0  3C 20       cmp al,20h
+00D2  76 F5       jbe 0c9h
+00D4  31 C9       xor cx,cx
+00D6  3C 2D       cmp al,2dh
+00D8  75 06       jne 0e0h
+00DA  41          inc cx
+00DB  E8 B1 00    call 18fh
+00DE  EB 07       jmp 0e7h
+00E0  3C 2B       cmp al,2bh
+00E2  75 03       jne 0e7h
+00E4  E8 A8 00    call 18fh
+00E7  31 FF       xor di,di
+00E9  3C 30       cmp al,30h
+00EB  72 18       jb 105h
+00ED  3C 39       cmp al,39h
+00EF  77 14       jnbe 105h
+00F1  2C 30       sub al,30h
+00F3  B4 00       mov ah,0h
+00F5  87 C7       xchg di,ax
+00F7  BB 0A 00    mov bx,0ah
+00FA  F7 E3       mul bx
+00FC  03 C7       add ax,di
+00FE  89 C7       mov di,ax
+0100  E8 8C 00    call 18fh
+0103  EB E4       jmp 0e9h
+0105  E8 9E 00    call 1a6h
+0108  83 F9 00    cmp cx,0h
+010B  74 02       je 10fh
+010D  F7 DF       neg di
+010F  8B C7       mov ax,di
+0111  C3          ret
+0112  B9 02 00    mov cx,2h
+0115  C3          ret
+0116  55          push bp
+0117  8B EC       mov bp,sp
+0119  50          push ax
+011A  53          push bx
+011B  51          push cx
+011C  52          push dx
+011D  57          push di
+011E  E8 A8 FF    call 0c9h
+0121  83 F9 02    cmp cx,2h
+0124  74 05       je 12bh
+0126  8B 7E 04    mov di,word ptr [bp+4h]
+0129  88 05       mov byte ptr [di],al
+012B  5F          pop di
+012C  5A          pop dx
+012D  59          pop cx
+012E  5B          pop bx
+012F  58          pop ax
+0130  89 EC       mov sp,bp
+0132  5D          pop bp
+0133  C3          ret
+0134  55          push bp
+0135  8B EC       mov bp,sp
+0137  50          push ax
+0138  53          push bx
+0139  57          push di
+013A  E8 52 00    call 18fh
+013D  88 C2       mov dl,al
+013F  8B 7E 04    mov di,word ptr [bp+4h]
+0142  88 15       mov byte ptr [di],dl
+0144  5F          pop di
+0145  5B          pop bx
+0146  58          pop ax
+0147  89 EC       mov sp,bp
+0149  5D          pop bp
+014A  C3          ret
+014B  55          push bp
+014C  8B EC       mov bp,sp
+014E  50          push ax
+014F  53          push bx
+0150  51          push cx
+0151  57          push di
+0152  E8 3A 00    call 18fh
+0155  31 C9       xor cx,cx
+0157  3C 54       cmp al,54h
+0159  74 12       je 16dh
+015B  3C 74       cmp al,74h
+015D  74 0E       je 16dh
+015F  3C 59       cmp al,59h
+0161  74 0A       je 16dh
+0163  3C 79       cmp al,79h
+0165  74 06       je 16dh
+0167  3C 31       cmp al,31h
+0169  74 02       je 16dh
+016B  EB 01       jmp 16eh
+016D  41          inc cx
+016E  8B 7E 04    mov di,word ptr [bp+4h]
+0171  89 0D       mov word ptr [di],cx
+0173  5F          pop di
+0174  59          pop cx
+0175  5B          pop bx
+0176  58          pop ax
+0177  89 EC       mov sp,bp
+0179  5D          pop bp
+017A  C3          ret
+017B  50          push ax
+017C  E8 10 00    call 18fh
+017F  3C 0D       cmp al,0dh
+0181  74 0A       je 18dh
+0183  3C 0A       cmp al,0ah
+0185  74 06       je 18dh
+0187  3C 1A       cmp al,1ah
+0189  74 02       je 18dh
+018B  EB EF       jmp 17ch
+018D  58          pop ax
+018E  C3          ret
+018F  8B 1E CE 02 mov bx,word ptr [2ceh]
+0193  83 FB 00    cmp bx,0h
+0196  74 09       je 1a1h
+0198  8A C3       mov al,bl
+019A  31 DB       xor bx,bx
+019C  89 1E CE 02 mov word ptr [2ceh],bx
+01A0  C3          ret
+01A1  B4 08       mov ah,8h
+01A3  CD 21       int 21h
+01A5  C3          ret
+01A6  88 C2       mov dl,al
+01A8  BB 01 00    mov bx,1h
+01AB  8A DA       mov bl,dl
+01AD  89 1E CE 02 mov word ptr [2ceh],bx
+01B1  C3          ret