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

tests: one definition of the .COM image layout, and a sweep region that is measured

check_framedisp built its sweep region from RT_SZ = 391, a runtime size that
had drifted 61 bytes from the runtime it described, and reported PASS over a
region that began inside the runtime, part way through an instruction.  It
could not see where its own region began: the byte patterns it searches for
are in the image wherever they happen to be, and a decode that starts
mid-instruction reached the same operands anyway.

The region is now two independent readings of the file - where the entry jump
says execution starts, where the program header says the runtime ends -
measured, required to agree, and refused rather than guessed when they do not.

The layout itself moved into tests/comimage.py (ENT_SZ, HDR_SZ, LOAD_BIAS,
HEAD, both header words, find_header, entry_target, and an assert tying HEAD's
displacements to the header words).  comtest.py, the independent checker inside
run_com_tests.sh, check_framedisp.py and check_8086.py now import it instead
of keeping their own copy; check_8086 also cross-checks the probe size it is
given against find_header rather than trusting its caller.

tests/check_comimage.py asserts that this stays true: one definition of each
part, imported by every reader, no second find_header anywhere in tests/.  Its
scope is written down in its own docstring, because a check that hand-rolls
the layout from inline numbers defines none of those names - the readers catch
that a different way, each stating the layout twice from two sources and
refusing to proceed when the two disagree.

run_all.sh runs the audit as its own check.  nonvacuity.sh gains seven cases
that turn the readers red one at a time (the restated region, the unlocatable
header as reported by two different checks, a copied find_header, a copied
constant, the definition deleted, a caller reaching the helper through another
consumer) plus three restored greens, one per reader; the harness now restores
comimage.py and comtest.py through its exit trap as well, since neither is
recoverable from git once a mutation has been staged over them.

SUMMARY.md documents the milestone and drops the restated-RT_SZ next step.
Eric Streit 4 дней назад
Родитель
Сommit
4ba05b34bd

+ 60 - 27
SUMMARY.md

@@ -25,6 +25,7 @@ manual, not guessed.
 | Runtime entries under qemu, and the register contract stated | `v-TP3-BP-CONTRACT` | done, **36/36 entry checks; `wrchar`/`wrbool` no longer destroy BP** |
 | 8086-legal conditional branches and `SETcc` | `v-TP3-8086-LOWERING` | done, **the emitted code no longer contains an opcode the 8086 lacks** |
 | `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** |
 
 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
@@ -186,6 +187,7 @@ proves each one can go red.
 | `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 **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_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 |
@@ -247,6 +249,18 @@ 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.
 
+It had a **second, unrelated blind spot of its own**: the region it swept came
+from `RT_SZ = 391`, a runtime size that had drifted, so the sweep began inside
+the runtime, part way through an instruction — and the check passed anyway,
+because the byte patterns it searches for are in the image wherever they happen
+to be and a decode starting mid-instruction happened to reach the same offsets.
+Nothing in the check could see where its own region began, which is a wrong
+input that produces the right answers until the layout moves. The region is now
+**two independent readings of the file** — where the entry jump says execution
+starts, where the program header says the runtime ends — required to agree
+before a single byte is swept, with `tests/comimage.py` supplying both readings
+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
@@ -271,7 +285,8 @@ The two fixtures it found nothing in are the interesting ones: `t31_procparam`
 and `t32_forexit` were the two most expensive bugs in the project, and neither
 was visible as a wrong byte count. See `overProc` below.
 
-**The `.COM` layout constants are measured, not restated.** `run_com_tests.sh`
+**The `.COM` layout constants are measured, not restated — and now live in one
+file.** `run_com_tests.sh`
 and `comtest.py` both used to hard-code `RT_SZ = 391` against a runtime that
 had since grown to 432 bytes at the time, so they read the program header 41 bytes early
 and reported **30 false failures** — a red suite that meant nothing, which is
@@ -286,7 +301,18 @@ measured runtime size: 436 bytes (header at image offset 439)
 
 A restated constant that has drifted is worse than a derived one, and the two
 checkers had drifted from each other as well as from the runtime — which is why
-there are two of them and why both were wrong in the same way.
+there were two of them and why both were wrong in the same way.
+
+So the layout itself moved into **`tests/comimage.py`**, and the third reader
+joined the first two. Four places now read a linked image — `comtest.py`, the
+independent checker inside `run_com_tests.sh`, `check_framedisp.py` and
+`check_8086.py` — and none of them says what a `.COM` looks like; they import
+it. `tests/check_comimage.py` is the assertion that this stays true: one
+definition of each part, imported by every reader, no second `find_header`
+anywhere in `tests/`. Its scope is stated in its own docstring, because it
+cannot see a check that re-derives the layout from inline numbers — that is what
+the readers do instead, each stating the layout twice from two different
+sources and refusing to proceed when the two disagree.
 
 ### 8086 legality — `tests/check_8086.py`
 
@@ -410,21 +436,35 @@ goes red before a single instruction has run.
 
 ### Non-vacuity — `tests/nonvacuity.sh`
 
-Every assertion in this file is proved able to fail: **52 deliberate
+Every assertion in this file is proved able to fail: **62 deliberate
 breakages, each asserted to turn exactly one named check red for the stated
-reason, then restored and re-asserted green** — `non-vacuity: 52 ok, 0 failed`.
+reason, then restored and re-asserted green** — `non-vacuity: 62 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 8086 lowering,
-the interpreter itself, and the `R` key.
+`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.
 
-Eight of those 52 arrived with this milestone, and each one is the same shape:
-code that compiled clean and passed every byte-level check, until it was
-**run**. Two are behavioural (below), two come from the new interpreter, two
+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
+was **run**. Two are behavioural (below), two come from the new interpreter, two
 pin the two new grammar rows the helper audit gained for `EmXorAl01`, and two
 are the `R` key's mutation and its restored green.
 
+Ten arrived with this one, and they are a different shape: **a check whose own
+input had never been verified.** `check_framedisp` swept a region built from a
+literal that had drifted, and the region is now two independent readings of the
+file required to agree — so the first three cases break each reading in turn:
+the entry jump answered with the old literal, and the header's own equation one
+byte out, seen red by **two different readers** with two different sentences.
+The next four break the single copy itself — a `find_header` copied out of
+`comimage.py`, a layout constant copied out of it, the definition deleted (which
+must be a red report rather than a silent green one, because a rule that matches
+nothing looks exactly like a rule that passes), and a caller that reaches the
+helper through another consumer instead of through `comimage`. The last three
+are restored greens, one per reader.
+
 That last group is the newest and the least optional. Two of its five
 (`M4`, `M5`) invert the branch polarity, which is a *legal* 8086 opcode
 sequence, so no shape-based check can see it and only a source-derived
@@ -1531,8 +1571,9 @@ its branch sites declare, **read off the `.pas` sources** and written out with
 the reasoning beside each row — a table measured from the image would agree with
 any behaviour including a wrong one.
 
-Five mutations are now permanent cases in `tests/nonvacuity.sh` (`52 ok, 0
-failed` in total across every section; these five were `44 ok` when added):
+Five mutations are now permanent cases in `tests/nonvacuity.sh` (its final
+`non-vacuity:` line reports the total across every section; these five were
+`44 ok` when added):
 M1 and M2 restore each original defect, M3 swaps `>`/`>=`,
 M4 inverts the branch polarity everywhere, M5 inverts it for IF and CASE only.
 M5's first run was the one that came back green, and that is the case's whole
@@ -1665,42 +1706,34 @@ independently-scanned inventory at all.
 
 ## Next steps
 
-1. **Close the third restated `RT_SZ`.** `tests/check_framedisp.py` hard-codes
-   `RT_SZ = 391` where the runtime now measures 436, so `img[RTSZ:]` begins 61
-   bytes inside the runtime tail and the check passes by luck. `run_com_tests.sh`
-   and `comtest.py` carried the same constant and were fixed by *measuring* the
-   header from its own signature; this one needs that shared helper
-   (`tests/comimage.py`) and, being new, its own non-vacuity case — a green
-   nobody has ever seen red is the failure mode this project keeps
-   rediscovering, and this is the last known instance of it.
-2. **The multi-argument kind-2 clobber.** `f (a > b, x)` passes a wrong first
+1. **The multi-argument kind-2 clobber.** `f (a > b, x)` passes a wrong first
    value, and `f (a > b, c > d)` passes the second comparison twice.
    `SaveLeft` parks a computed operand across the parse of the *other* operand
    of a binary operator; the three call parsers park nothing. Fixture first, so
    the bug is red before the fix (see "Honest limitations").
-3. **String *variables*** — `s : string`, `s := 'hi'`, `writeln(s)`. The
+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
    refuses with `ENoLib`.
-4. Nested procedures / recursion, `var` parameters (the `SEG:OFF` push from
+3. 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`), `case` with subrange labels.
-5. **`readln` of a `BYTE`** calls `rdint`, which stores 2 bytes and overflows
+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.
-6. Make the 4 KiB code window an enforced limit rather than a documented one:
+5. 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.
-7. Both spellings of a multi-name declaration. `var i, c : integer;` is
+6. 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.
-8. Harden the program-header parameter loop against non-advancing input
+7. Harden the program-header parameter loop against non-advancing input
    (`program p(1;)`) with a `BOOLEAN` flag — **not** `EXIT`, which ICEs gm2.
-9. FreeDOS (`freedos.qcow2`, FD14-LiveCD) is still untried. Now that `R` runs
+8. 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

+ 27 - 9
shell/tests/check_8086.py

@@ -100,6 +100,10 @@ HERE = os.path.dirname(os.path.abspath(__file__))
 SHELL = os.path.dirname(HERE)
 sys.path.insert(0, HERE)
 import disasm16  # noqa: E402
+# The image layout: where the header is, how big it is, and the load bias this
+# file used to carry on its own account.  See tests/check_comimage.py, which
+# asserts that every reader of a linked image gets these from one place.
+import comimage  # noqa: E402
 
 COMTEST = os.path.join(SHELL, "comtest")
 
@@ -128,7 +132,6 @@ def negated(nib):
     """The Jcc nibble the image will carry for a site that declares `nib'."""
     return nib ^ 1
 
-LOAD_BIAS = 0x100        # Runtime.def: a .COM's byte 0 lands at DS:0100h
 CMP_AX_CX = b"\x3b\xc1"  # EmCmpAxCx
 CMP_AX_ZERO = b"\x3d\x00\x00"   # EmCmpAxi (0)
 
@@ -280,13 +283,27 @@ def program_code_region(img, rt_size):
     offsets, derived from the image rather than from a restated constant: the
     entry jump's displacement is the program's own answer for where the code
     begins, and hdrCS is `pc + LoadBias' with pc the end of the generated code,
-    so hdrCS - LoadBias is where it stops."""
-    hdr = 3 + rt_size
-    if len(img) < hdr + 16:
+    so hdrCS - LoadBias is where it stops.
+
+    Where the header is gets two INDEPENDENT answers: rt_size is the runtime
+    blob's size as the probe measured it, and comimage.find_header locates the
+    header inside this file by its own self-consistency equation.  They are
+    measurements of two different artefacts, so their agreement is evidence,
+    and a .COM whose two disagree has no code region this checker can state --
+    guessing one of them would be the restated constant this function exists
+    to avoid."""
+    hdr_probe = comimage.ENT_SZ + rt_size
+    hdr_file = comimage.find_header(img)
+    if hdr_file is None or hdr_file != hdr_probe:
         return None
-    start = (3 + int.from_bytes(img[1:3], "little", signed=True)) % 0x10000
-    hdr_cs = int.from_bytes(img[hdr + 2:hdr + 4], "little")
-    end = hdr_cs - LOAD_BIAS
+    if len(img) < hdr_file + comimage.HDR_SZ:
+        return None
+    start = comimage.entry_target(img)
+    if start is None:
+        return None
+    start %= 0x10000
+    hdr_cs = int.from_bytes(img[hdr_file + 2:hdr_file + 4], "little")
+    end = hdr_cs - comimage.LOAD_BIAS
     if not (start <= end <= len(img)):
         return None
     return start, end
@@ -330,7 +347,8 @@ def main(argv):
             region = program_code_region(img, rt_info["size"])
             if region is None:
                 problems.append("%s: could not locate the code region "
-                                "(entry jump and hdrCS disagree)" % name)
+                                "(entry jump, program header and hdrCS do not "
+                                "agree)" % name)
                 continue
             start, end = region
             code = img[start:end]
@@ -412,7 +430,7 @@ def main(argv):
             # whole thing is code -- assert no 0F opcode over it as well.  This
             # is extra coverage, not the backbone, and how much of the suite
             # it reached is printed rather than implied.
-            instrs, stopped = sweep(code, LOAD_BIAS + start)
+            instrs, stopped = sweep(code, comimage.LOAD_BIAS + start)
             if stopped is None:
                 swept_fixtures += 1
                 swept_bytes += len(code)

+ 149 - 0
shell/tests/check_comimage.py

@@ -0,0 +1,149 @@
+#!/usr/bin/env python3
+"""check_comimage.py -- the image layout is described in exactly one file.
+
+Any check that reads a linked .COM has to know where its parts are: the entry
+jump is three bytes, the header is sixteen, initmem's first bytes sit at ENT_SZ
+and the header is findable by its own signature.  Before tests/comimage.py
+existed that knowledge lived in three places - comtest.py had a find_header,
+the independent checker inside run_com_tests.sh had a second copy of it, and
+check_framedisp.py had a third answer of an entirely different kind: a literal
+RT_SZ that had drifted from the runtime it claimed to describe.
+
+Three copies are three chances to drift, and drift is SILENT here, because each
+copy is self-consistent on its own terms: every one of them reports confidently
+and only the world disagrees.  So the layout lives in one file, and this asserts
+that it stays that way:
+
+  A. comimage.py defines each part exactly once, and imports.  Counting rather
+     than merely searching is the point: a rule that matches nothing looks
+     exactly like a rule that passes (the lesson in tests/rt_exec.py's
+     "matched nothing" case), so a deleted definition would otherwise be a green
+     report about a subject nobody examined.
+  B. no other test source defines any of those names.
+  C. every test source that CALLS find_header takes it from comimage rather
+     than from some other consumer: `from comtest import find_header' would be
+     one helper reached through two of them, and the second of those two is a
+     copy waiting to drift.  Definitions quoted inside a mutation string are
+     not calls, so `def find_header(d):' written as a case's target does not
+     count - nonvacuity.sh would otherwise fail this audit for naming the very
+     thing it is breaking.
+
+Scope, stated so this is not read as more than it is
+----------------------------------------------------
+This cannot see a check that HAND-ROLLS the layout from literals - check_8086
+used to write `hdr = 3 + rt_size', which defines none of these names and would
+have passed here untouched.  Numbers written inline are caught a different way:
+check_8086.program_code_region and check_framedisp.main both state the layout
+twice, from two independent sources (the probe-measured runtime size against
+the header's own equation; the entry jump against the header's end), and
+require the two to agree.  A disagreement is reported instead of swept, and
+tests/nonvacuity.sh turns each of those red on purpose.
+
+Usage: check_comimage.py     (from shell/)
+"""
+
+import os
+import re
+import sys
+
+HERE = os.path.dirname(os.path.abspath(__file__))
+COMIMAGE = os.path.join(HERE, "comimage.py")
+
+# The names that make up the layout of a linked image.
+LAYOUT = ["ENT_SZ", "HDR_SZ", "LOAD_BIAS", "HEAD", "HDR_DS_WORD",
+          "HDR_HEAP_WORD"]
+
+# rt_exec.py states a load bias of its own, and deliberately so: the address
+# space it works in is the boot image IT builds (see build_image, and
+# exec/rtdrv.s, which carries a .set of the same value for the same reason).
+# Those two are independent statements checked against each other by the cases
+# failing if they disagree - and rtdrv.s cannot import a Python module.  It is a
+# different artefact's bias, not a copy of this one.
+EXEMPT = {("rt_exec.py", "LOAD_BIAS")}
+
+DEF_FN = re.compile(r"^\s*def\s+find_header\b")
+IMPORTS = re.compile(r"^\s*(?:import\s+comimage\b|from\s+comimage\s+import\b)")
+# A CALL, not a definition.  The lookbehind is what lets nonvacuity.sh quote
+# `def find_header(d):' as its mutation target without this audit reporting the
+# harness itself as a second copy of the helper.
+MENTIONS = re.compile(r"(?<!def )\bfind_header\s*\(")
+
+
+def sources():
+    """every test source this audit reads, as (relative path, lines)"""
+    out = []
+    for root, dirs, files in os.walk(HERE):
+        dirs[:] = sorted(d for d in dirs if not d.startswith("."))
+        for f in sorted(files):
+            if f == "comimage.py" or not (f.endswith(".py")
+                                          or f.endswith(".sh")):
+                continue
+            path = os.path.join(root, f)
+            with open(path, "r") as fh:
+                out.append((os.path.relpath(path, HERE),
+                            fh.read().splitlines()))
+    return out
+
+
+def main():
+    problems = []
+
+    # A: the helper exists, is complete, and loads.  The import runs
+    # comimage's own module-level assertion (initmem's displacements against
+    # the header word offsets), so a broken helper is caught here too.
+    if not os.path.exists(COMIMAGE):
+        print("FAIL: comimage.py does not exist")
+        return 1
+    try:
+        sys.path.insert(0, HERE)
+        import comimage  # noqa: F401
+    except Exception as exc:                      # noqa: BLE001
+        print("FAIL: comimage.py does not import: %s" % exc)
+        return 1
+
+    # Each name is matched on its own.  A single alternation would count one
+    # line for every name in it - six definitions each, from six lines total -
+    # which reports the layout as six times duplicated while it is defined once.
+    defs = dict((name, re.compile(r"^\s*%s\s*=" % re.escape(name)))
+                for name in LAYOUT)
+
+    with open(COMIMAGE) as fh:
+        comimage_lines = fh.read().splitlines()
+    for name in LAYOUT:
+        n = sum(1 for ln in comimage_lines if defs[name].match(ln))
+        if n != 1:
+            problems.append("comimage.py defines %s %d times, want exactly 1"
+                            % (name, n))
+    nfn = sum(1 for ln in comimage_lines if DEF_FN.match(ln))
+    if nfn != 1:
+        problems.append("comimage.py defines find_header %d times, want "
+                        "exactly 1" % nfn)
+
+    # B and C: every other test source.
+    for rel, lines in sources():
+        for ln in lines:
+            if DEF_FN.match(ln):
+                problems.append("%s defines its own find_header" % rel)
+            for name in LAYOUT:
+                if (rel, name) in EXEMPT:
+                    continue
+                if defs[name].match(ln):
+                    problems.append("%s defines its own %s" % (rel, name))
+        mentions = [ln for ln in lines if MENTIONS.search(ln)]
+        if mentions and not any(IMPORTS.match(ln) for ln in lines):
+            problems.append("%s calls find_header without importing it from "
+                            "comimage" % rel)
+
+    if problems:
+        print("FAIL: the image layout is not described in exactly one file")
+        for p in problems:
+            print("  - %s" % p)
+        return 1
+    print("image layout: defined once in comimage.py and imported by every "
+          "reader (%d names, %d sources scanned)"
+          % (len(LAYOUT) + 1, len(sources()) + 1))
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

+ 67 - 8
shell/tests/check_framedisp.py

@@ -51,6 +51,32 @@ restatement of what the compiler happens to do:
     parameters  parmOff starts at 4 and grows by 2 per parameter, so
                 parameter k is at 4 + 2*(k-1).
 
+The region this check reads
+---------------------------
+A displacement only means something about the instruction that contains it, so
+this check sweeps the program's OWN instructions, from the first one to the
+end of the image.  Two different things in the file say where the first one
+is, and they are read separately:
+
+    the entry jump  `E9 rel16' at file offset 0, so execution begins at
+                    ENT_SZ + rel16 (Compiler.Inittur)
+    the program     found by its own signature (comimage.find_header); the
+    header          program code follows the header immediately
+
+They must agree.  They are not the same measurement -- one is a jump target,
+the other is a self-consistency equation over header words -- so their
+agreement is evidence, and a disagreement means the region is undetermined,
+which this check reports instead of sweeping something anyway.
+
+Until this section was written the region was a literal: RT_SZ = 391, next to
+a comment saying it tracked Runtime.RT_Size().  It had drifted, the runtime
+having grown past it, so the sweep began inside the RUNTIME, part way through
+an instruction -- and the check passed.  It passed because nothing in it could
+see where its own region began: the byte patterns are found wherever they are
+in the image, and a decode sweep that starts mid-instruction happened to reach
+the same offsets.  A wrong input that produces the right answers is still a
+wrong input, and it stops being right the first time the layout moves.
+
 Fixtures
 --------
 t27_localvar.pas  five locals, assigned and read back.  Covers the negative
@@ -82,13 +108,18 @@ HERE = os.path.dirname(os.path.abspath(__file__))
 SHELL = os.path.dirname(HERE)
 sys.path.insert(0, HERE)
 import disasm16  # noqa: E402
-
-# Re-stated, not asked of the code under test.  The image starts with a
-# three-byte entry JMP (see Compiler.Inittur), so the program code begins at
-# ENT_SZ + RT_SZ rather than at the runtime size alone.
-RT_SZ = 391              # Runtime.RT_Size()
-ENT_SZ = 3               # E9 lo hi
-RTSZ = ENT_SZ + RT_SZ
+# Where the program header is, where the entry jump lands, and how big the
+# header is.  This check used to write the runtime's size down as RT_SZ = 391
+# beside a comment claiming it tracked Runtime.RT_Size(), and swept from
+# ENT_SZ + RT_SZ - which is inside the RUNTIME, part way through an
+# instruction, and nowhere near the program's first instruction.  It still
+# passed, for the two reasons a wrong region always passes: the byte patterns
+# below are in the image wherever they happen to be, and the decode sweep
+# reached the same answers from a start point that nothing in the check could
+# tell was wrong.  So the region is now measured from the file AND asserted to
+# agree with itself in main(), because a check whose own input is wrong reports
+# confidently about bytes the program never executes.
+import comimage  # noqa: E402
 COMTEST = os.path.join(SHELL, "comtest")
 
 # (fixture, opcode, [signed offsets])
@@ -180,7 +211,35 @@ def main(argv):
 
     for fixture, want_op, offsets in CASES:
         img = images[fixture]
-        code = img[RTSZ:]
+        # The region this check sweeps, measured twice from the file: where the
+        # entry jump says execution starts, and where the program header says
+        # the runtime ends.  Those are different facts read from different
+        # bytes, so a check that knows only one of them cannot tell that its
+        # region is wrong -- which is exactly what happened while this was a
+        # literal: the sweep began inside the runtime and reported PASS.  The
+        # two must agree or there is no measured place to start, and a guess
+        # would be reported here as a fact about the compiler.
+        entry = comimage.entry_target(img)
+        hdr = comimage.find_header(img)
+        if entry is None:
+            problems.append("%s: no entry jump at file offset 0, so the image "
+                            "says nothing about where the program's code "
+                            "begins and this check would have to guess"
+                            % fixture)
+            continue
+        if hdr is None:
+            problems.append("%s: no program header found, so the end of the "
+                            "runtime is unknown and this check would have to "
+                            "guess where the program's code begins" % fixture)
+            continue
+        if entry != hdr + comimage.HDR_SZ:
+            problems.append("%s: the entry jump lands on %d but the program "
+                            "header ends at %d -- the two measurements of "
+                            "where the program's code begins disagree, so "
+                            "neither region is swept" % (fixture, entry,
+                                                         hdr + comimage.HDR_SZ))
+            continue
+        code = img[entry:]
         op = int(want_op, 16)
         mine = [d for (o, _, d) in bp_operands(code) if o == want_op]
         if verbose:

+ 145 - 0
shell/tests/comimage.py

@@ -0,0 +1,145 @@
+#!/usr/bin/env python3
+"""comimage.py -- the layout of a linked .COM, measured from the file itself.
+
+Every check in this project that reads an emitted image has to answer the same
+three questions before it can say anything at all:
+
+    where is the program header?      find_header(d)
+    where does the runtime end?       find_header(d) - HDR_SZ
+    where does execution start?       entry_target(d)
+
+Until now each check answered for itself.  comtest.py carried one find_header,
+the independent checker inside run_com_tests.sh carried a second copy of it,
+and check_framedisp.py carried neither: it wrote RT_SZ = 391 beside a comment
+saying it tracked Runtime.RT_Size(), and swept the region beginning
+ENT_SZ + RT_SZ - which is inside the RUNTIME, part way through an instruction.
+The check passed anyway, for the two reasons a restated offset always passes:
+the byte patterns it searched for are in the image wherever they happen to be,
+and the decode sweep reached the same answers from a start point that nothing
+in the check could tell was wrong.  A region that is wrong in a way its
+assertions cannot see is the same fault as a constant that has drifted, only
+harder to notice, because the report it prints is a clean PASS.
+
+So the layout lives in one file, is measured rather than remembered, and every
+reader imports it.  tests/check_comimage.py asserts that there is exactly one
+definition of each part and that every reader gets it from here, because a
+shared helper that somebody copies again is four sources of truth instead of
+one - and four is how this started.
+
+What is written down here, and what is not
+------------------------------------------
+The FORMAT is written down: the entry jump is three bytes, the header is
+sixteen, initmem's first bytes and the two header words it reads, and the load
+bias.  Those are this project's knowledge of what it emits, and asking the code
+under test for them would let the code under test satisfy every check by
+agreeing with itself.
+
+The SIZES are not written down.  The runtime's size was written down twice
+(385, then 391), beside a comment saying it tracked the runtime; it did not,
+and each stale value sent a checker into the middle of the code, where it
+reported a well-formed image as a compiler fault.  A duplicated constant that
+has silently drifted is not an independent check, it is a second source of
+truth that lies, and it lies in the direction of looking like the thing under
+test is broken.
+
+Measuring is not the same as asking the compiler: everything here reads the
+emitted file.  tests/check_runtime.py is where the runtime's size is pinned on
+purpose, and tests/run_com_tests.sh prints the size it measures on every run.
+
+Usage: this is a library; import it from a check.
+"""
+
+# ---- the format, written down -------------------------------------------
+
+# ENT_SZ and HDR_SZ are the layout, and the load bias below is a decision this
+# project made about how a DOS .COM is loaded.  Those stay written down: they
+# are knowledge about the format, not sizes that move when code is edited.
+ENT_SZ = 3                    # E9 lo hi, the entry jump (Compiler.Inittur)
+HDR_SZ = 16                   # 5 header words + 3 buffer words (Compiler)
+
+# The load bias.  A DOS .COM's first byte is at CS:0100 and CS = DS, so an image
+# offset K is at DS:(K + 0100h); every ABSOLUTE address the image contains has
+# to carry it, or it points 0100h low and - since the code region and the
+# runtime are both below the bias - almost always lands inside the runtime
+# instead of inside the data.  Relative encodings must not carry it, because
+# both of their operands shift together.
+# Restated, not asked of the code under test.  See Runtime.LoadBias.
+LOAD_BIAS = 0x100
+
+# initmem's prologue, which is the runtime's only reader of the program header.
+# The displacements +4 and +6 below are the whole point of this constant: the
+# header checks in every checker read hdrDS at +4 and hdrHeap at +6, and
+# initmem has to read the SAME two words or it clears the wrong range.  It read
+# +8 (hdrMax, which the compiler patches to 0), so it zeroed nothing at all and
+# nothing here noticed - the emitted loop was perfectly well formed, it just
+# never ran.  Asserting these bytes together with the header offsets is what
+# closes that gap.
+#
+# It is the first eleven bytes of the runtime, so it sits at ENT_SZ, not 0.
+HEAD = "8B F0 8B 54 04 8B 4C 06"   # MOV SI,AX / MOV DX,[SI+4] / MOV CX,[SI+6]
+HDR_DS_WORD = 4               # header word holding the data base
+HDR_HEAP_WORD = 6             # header word holding the data end
+assert [int(HEAD.split()[4], 16), int(HEAD.split()[7], 16)] == \
+       [HDR_DS_WORD, HDR_HEAP_WORD], \
+       "initmem no longer reads the two header words the checks verify"
+
+
+# ---- what the file says, measured ---------------------------------------
+
+def find_header(d):
+    """The image offset of the program header in `d`, or None.
+
+    The header is eight words, and the layout says what they are (offsets here
+    are BYTES into the header, which is why HDR_DS_WORD is 4 and not 2 - the
+    words are two bytes each):
+
+        +0  1                 hdrFlag, always 1
+        +2  code end + bias   hdrCS
+        +4  data base + bias  hdrDS, where data base = hdrOff + 1000h
+        +6  data end  + bias  hdrHeap, which is hdrDS + dataBytes
+
+    hdrDS ties the header to its OWN offset: the data base is header offset +
+    1000h, and hdrDS is that with the load bias added, so the offset is
+    recoverable from the file with no remembered runtime size - which is the
+    whole point.  A candidate is accepted only if hdrFlag is 1, hdrDS satisfies
+    that equation, hdrHeap is above hdrDS (a heap below its own base is not a
+    layout, it is a coincidence), and hdrCS leaves room for the header itself.
+
+    initmem is the only code in the image that reads the header, so its bytes
+    are pinned at ENT_SZ: a candidate that also has them there is not a
+    coincidence in the code stream.
+
+    A header that cannot be found is returned as None rather than as the
+    nearest guess, because a caller that guesses reports confidently about the
+    wrong bytes - which is what the restated runtime size used to do.
+    """
+    head = bytes(int(x, 16) for x in HEAD.split())
+    if d[ENT_SZ:ENT_SZ + len(head)] != head:
+        return None                      # no runtime: nothing to measure
+    for off in range(ENT_SZ, len(d) - HDR_SZ + 1):
+        w = (lambda b: int.from_bytes(d[off + b:off + b + 2], "little"))
+        if w(0) != 1:                                    # hdrFlag
+            continue
+        if w(HDR_DS_WORD) != off + 0x1000 + LOAD_BIAS:    # hdrDS
+            continue
+        if w(HDR_HEAP_WORD) <= w(HDR_DS_WORD):            # hdrHeap
+            continue
+        if w(2) - LOAD_BIAS < off + HDR_SZ:               # hdrCS
+            continue
+        return off
+    return None
+
+
+def entry_target(d):
+    """The image offset the entry JMP at file offset 0 lands on, or None.
+
+    A .COM is entered at its first byte, so byte 0 is `E9 rel16' and the first
+    instruction of the program is at ENT_SZ + rel16 (Compiler.Inittur).  This
+    is where EXECUTION starts, which is a different statement from where the
+    runtime ends: the program header sits between the two, and a check that
+    measures only one of them cannot tell that it has started its sweep in the
+    wrong place.  A check that measures both is asked to make them agree.
+    """
+    if len(d) < ENT_SZ or d[0] != 0xE9:
+        return None
+    return ENT_SZ + int.from_bytes(d[1:ENT_SZ], "little", signed=True)

+ 20 - 71
shell/tests/comtest.py

@@ -34,78 +34,27 @@ FIXTURE = os.path.abspath(sys.argv[1]) if len(sys.argv) > 1 else os.path.join(
     os.path.dirname(os.path.abspath(__file__)), "fixtures", "t26_str_mixed_args.pas")
 COM = os.path.splitext(FIXTURE)[0] + ".COM"
 
-# Restated here on purpose - the checker must not ask the code under test.
-# ENT_SZ and HDR_SZ are the layout, and the load bias below is a decision this
-# checker made on its own account; those stay written down.
+# The image layout - the entry jump, the header's size, the load bias,
+# initmem's first bytes - and the function that MEASURES where the header is
+# live in tests/comimage.py and are imported, not copied.  This checker had one
+# copy of both and the independent checker in tests/run_com_tests.sh had a
+# second; the third reader (tests/check_framedisp.py) had neither and wrote
+# down RT_SZ = 391 instead, which had long since drifted from the runtime it
+# claimed to track, so its sweep began inside the runtime rather than at the
+# program's first instruction and reported a clean PASS over bytes the program
+# never executes.  Two copies of a constant that moves are two chances to be
+# confidently wrong, so there is one now, and tests/check_comimage.py asserts
+# that it stays one.
 #
-# The RUNTIME'S SIZE is not restated, and that is the correction.  It used to
-# be a literal (385, then 391) beside a comment saying it tracks
-# Runtime.RT_Size().  It did not: the runtime is 432 bytes, so this checker
-# read the program header 41 bytes early, out of the middle of the code, and
-# reported a .COM full of nonsense - hdrFlag=61579 - as a compiler fault.  A
-# duplicated constant that has drifted is not an independent check, it is a
-# second source of truth that lies, and it lies in a way that looks like the
-# thing under test is broken.  The size is now MEASURED, by the same
-# self-consistent-header argument documented in find_header below.
-ENT_SZ = 3                    # E9 lo hi, the entry jump
-HDR_SZ = 16                   # 5 header words + 3 buffer words
-# RTSZ, PROLOG and DATAB are derived per .COM from find_header(d).
-# The image starts with a three-byte JMP - see Compiler.Inittur.  A .COM is
-# entered at file offset 0, so before that jump existed this checker ASSERTED
-# that the runtime was at offset 0, which was precisely the bug: every .COM
-# began by executing initmem with whatever the loader left in AX.  A checker
-# that pins a wrong invariant is worse than none, because it makes the wrong
-# thing look tested.
-# The load bias.  A DOS .COM's first byte is at CS:0100 and CS = DS, so an
-# image offset K is at DS:(K + 0100h); every absolute address the image
-# contains has to carry it, or it points 0100h low and - since the code region
-# and the runtime are all below the bias - almost always lands inside the
-# runtime instead of inside the data.  Relative encodings must not carry it.
-# Restated, not asked of the code under test.  See Runtime.LoadBias.
-LOAD_BIAS = 0x100
-HEAD = "8B F0 8B 54 04 8B 4C 06"   # MOV SI,AX / MOV DX,[SI+4] / MOV CX,[SI+6]
-HDR_DS_WORD = 4               # header word holding the data base
-HDR_HEAP_WORD = 6             # header word holding the data end
-assert [int(HEAD.split()[4], 16), int(HEAD.split()[7], 16)] == \
-       [HDR_DS_WORD, HDR_HEAP_WORD], \
-       "initmem no longer reads the two header words this checker verifies"
-
-
-def find_header(d):
-    """Return the image offset of the program header, or None.
-
-    The header is eight words, and hdrDS ties it to its OWN offset: the data
-    base is header offset + 1000h, and hdrDS is the data base with the load
-    bias added, so header offset = hdrDS - 1000h - bias.  That makes the
-    offset recoverable from the file with no remembered runtime size, which is
-    the whole point - see the note on RT_SZ above.
-
-    A candidate is accepted only if hdrFlag is 1, hdrDS satisfies that
-    equation, hdrHeap is above hdrDS, and hdrCS leaves room for the header
-    itself.  initmem is the only code in the image that reads the header, so
-    its bytes are pinned at ENT_SZ and a match that also has them is not a
-    coincidence in the code stream.
-
-    Measuring is not the same as asking the compiler: this reads the emitted
-    file, so it cannot be satisfied by the code under test agreeing with
-    itself.  tests/check_runtime.py is where the runtime's size is pinned on
-    purpose, and tests/run_com_tests.sh measures and prints it on every run.
-    """
-    head = bytes(int(x, 16) for x in HEAD.split())
-    if d[ENT_SZ:ENT_SZ + len(head)] != head:
-        return None
-    for off in range(ENT_SZ, len(d) - HDR_SZ + 1):
-        w = (lambda b: int.from_bytes(d[off + b:off + b + 2], "little"))
-        if w(0) != 1:
-            continue
-        if w(HDR_DS_WORD) != off + 0x1000 + LOAD_BIAS:
-            continue
-        if w(HDR_HEAP_WORD) <= w(HDR_DS_WORD):
-            continue
-        if w(2) - LOAD_BIAS < off + HDR_SZ:
-            continue
-        return off
-    return None
+# RTSZ, PROLOG and DATAB below are derived per .COM from find_header(d) - never
+# written down.  The image starts with a three-byte JMP (Compiler.Inittur) and
+# a .COM is entered at file offset 0, so before that jump existed this checker
+# ASSERTED that the runtime was at offset 0, which was precisely the bug: every
+# .COM began by executing initmem with whatever the loader left in AX.  A
+# checker that pins a wrong invariant is worse than none, because it makes the
+# wrong thing look tested.
+from comimage import (ENT_SZ, HDR_SZ, LOAD_BIAS, HEAD, HDR_DS_WORD,
+                      HDR_HEAP_WORD, find_header)
 
 
 def check_com(path, expected_src_len):

+ 164 - 9
shell/tests/nonvacuity.sh

@@ -31,6 +31,12 @@
 #                      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
+#   tests/comimage.py the image layout:     catches a sweep region built from a
+#                      literal instead of from two independent readings of the
+#                      file - the old RT_SZ went on reporting PASS while its
+#                      region began inside the runtime - and the layout itself
+#                      being written down a second time, now that one file
+#                      defines it and all three readers import it
 #   rt_exec.py        the BP contract:      catches an entry that borrows BP to
 #                      reach its argument and does not hand it back.  The bytes
 #                      are well formed, the golden is satisfied, the audit says
@@ -87,18 +93,29 @@ DUMP=../tmp/nonvacuity.dump
 # restored only the sources would leave tpshell BUILT FROM the mutation - so
 # the trap rebuilds too.  All three copies live in the project's own tmp/ and
 # are taken here, before the first mutation, rather than beside each case.
+#
+# comimage.py and comtest.py are the image layout and one of its three readers;
+# both are mutated by cases further down, and neither is recoverable from git
+# once the mutation is staged over - so they join the trap here for the same
+# reason, and an interrupted run cannot leave the layout half-written.
 mkdir -p ../tmp
 SAVED_E=../tmp/nonvacuity.Exec86.mod
 SAVED_SH=../tmp/nonvacuity.Shell.mod
+SAVED_CI=../tmp/nonvacuity.comimage.py
+SAVED_T=../tmp/nonvacuity.comtest.py
 
 cp Runtime.mod "$SAVED" || exit 1
 cp Exec86.mod "$SAVED_E" || exit 1
 cp Shell.mod "$SAVED_SH" || exit 1
+cp tests/comimage.py "$SAVED_CI" || exit 1
+cp tests/comtest.py "$SAVED_T" || exit 1
 
 restore_all () {
     cp "$SAVED" Runtime.mod
     cp "$SAVED_E" Exec86.mod
     cp "$SAVED_SH" Shell.mod
+    cp "$SAVED_CI" tests/comimage.py
+    cp "$SAVED_T" tests/comtest.py
     "$GM2" -fiso -c Runtime.mod >/dev/null 2>&1
     "$GM2" -fiso -c Exec86.mod >/dev/null 2>&1
     # The shell is rebuilt as well: a test that runs against a binary built
@@ -402,6 +419,142 @@ else
     fail=$((fail + 1))
 fi
 
+# --- the region check_framedisp sweeps, and the one file it comes from -----
+echo
+echo "== the image layout, measured once (tests/comimage.py)"
+# check_framedisp.py decodes instructions over a REGION of the image, and a
+# wrong region does not stop it: the byte patterns it searches for are in the
+# image wherever they happen to be, and a decode that starts mid-instruction
+# reached the same offsets anyway.  That is how RT_SZ = 391 - a literal that had
+# drifted from the runtime it described - went on reporting PASS while its sweep
+# began inside the runtime, part way through an instruction.  Nothing in the
+# check could see where its own region began.
+#
+# So the region is two independent readings of the file - where the entry jump
+# says execution starts, and where the program header says the runtime ends -
+# and the two must AGREE or the region is undetermined.  Both readings, and the
+# layout itself, live in tests/comimage.py, which is also where comtest.py and
+# the independent checker in run_com_tests.sh now get the copy each of them
+# used to keep: a duplicated constant that has silently drifted is not an
+# independent check, it is a second source of truth that lies, and it lies in
+# the direction of looking like the code under test is broken.
+#
+# None of this needs a rebuild - comimage.py is Python the checkers import -
+# so these cases are cheap, and they turn the READERS red one at a time.
+
+# 1. entry_target answers with the literal the region used to be built from.
+#    The two readings now disagree, and there is no measured region to sweep.
+python3 - <<'PYEOF'
+p = 'tests/comimage.py'
+s = open(p).read()
+old = '    return ENT_SZ + int.from_bytes(d[1:ENT_SZ], "little", signed=True)'
+new = '    return ENT_SZ + 391                        # the literal RT_SZ was'
+assert s.count(old) == 1, "entry_target's return not found -- update this mutation"
+open(p, 'w').write(s.replace(old, new))
+PYEOF
+expect_red "a sweep region that is not where execution starts is rejected" \
+    "neither region is swept" python3 tests/check_framedisp.py
+cp "$SAVED_CI" tests/comimage.py
+
+# 2 and 3. The header's own equation, one byte out, so the header cannot be
+#    located at all - the case where a checker that fell back on a remembered
+#    offset would report a confident number instead.  Two readers, two
+#    different sentences: each must say in its own words that it does not know
+#    where the program's code begins.
+python3 - <<'PYEOF'
+p = 'tests/comimage.py'
+s = open(p).read()
+old = '        if w(HDR_DS_WORD) != off + 0x1000 + LOAD_BIAS:    # hdrDS'
+new = '        if w(HDR_DS_WORD) != off + 0x1000 + LOAD_BIAS + 1:  # hdrDS'
+assert s.count(old) == 1, "the hdrDS equation not found -- update this mutation"
+open(p, 'w').write(s.replace(old, new))
+PYEOF
+expect_red "check_framedisp reports a header it cannot locate" \
+    "no program header found" python3 tests/check_framedisp.py
+expect_red "check_8086 reports the same unlocatable header" \
+    "could not locate the code region" python3 tests/check_8086.py
+cp "$SAVED_CI" tests/comimage.py
+
+# 4-7. The single copy itself, now that there is one.  A second find_header,
+#      a second HDR_SZ, a caller that reaches the helper through another
+#      consumer instead of through comimage, and - the case that exists
+#      because of rt_exec.py's "matched nothing" lesson - the definition the
+#      audit counts.  A rule that matches nothing looks exactly like a rule
+#      that passes, so a deleted definition must be a red report rather than a
+#      silent green one.
+python3 - <<'PYEOF'
+p = 'tests/comtest.py'
+s = open(p).read()
+old = 'def check_com(path, expected_src_len):'
+new = ('def find_header(d):\n'
+       '    return None                    # a second copy of the helper\n'
+       '\n\n'
+       'def check_com(path, expected_src_len):')
+assert s.count(old) == 1, "check_com not found -- update this mutation"
+open(p, 'w').write(s.replace(old, new))
+PYEOF
+expect_red "the audit reports a find_header copied out of comimage" \
+    "comtest.py defines its own find_header" python3 tests/check_comimage.py
+cp "$SAVED_T" tests/comtest.py
+
+python3 - <<'PYEOF'
+p = 'tests/comtest.py'
+s = open(p).read()
+old = 'from comimage import (ENT_SZ, HDR_SZ, LOAD_BIAS, HEAD, HDR_DS_WORD,'
+new = ('HDR_SZ = 16                   # a second copy of the layout\n'
+       'from comimage import (ENT_SZ, HDR_SZ, LOAD_BIAS, HEAD, HDR_DS_WORD,')
+assert s.count(old) == 1, "the comimage import not found -- update this mutation"
+open(p, 'w').write(s.replace(old, new))
+PYEOF
+expect_red "the audit reports a layout constant defined elsewhere" \
+    "comtest.py defines its own HDR_SZ" python3 tests/check_comimage.py
+cp "$SAVED_T" tests/comtest.py
+
+python3 - <<'PYEOF'
+p = 'tests/comimage.py'
+s = open(p).read()
+old = 'def find_header(d):'
+new = 'def _find_header(d):                # the definition is gone'
+assert s.count(old) == 1, "def find_header not found -- update this mutation"
+open(p, 'w').write(s.replace(old, new))
+PYEOF
+expect_red "the audit counts its own subject rather than matching nothing" \
+    "defines find_header 0 times" python3 tests/check_comimage.py
+cp "$SAVED_CI" tests/comimage.py
+
+# 7. The import dropped while the CALLS stay - the shape a refactor makes when
+#    one reader starts taking the helper from another reader instead.  It would
+#    NameError the moment comtest ran, but the audit says so first, and in the
+#    vocabulary of the layout rather than in the vocabulary of Python: the
+#    failure this is guarding is "two sources of truth", not "undefined name".
+python3 - <<'PYEOF'
+p = 'tests/comtest.py'
+s = open(p).read()
+old = ('from comimage import (ENT_SZ, HDR_SZ, LOAD_BIAS, HEAD, HDR_DS_WORD,\n'
+       '                      HDR_HEAP_WORD, find_header)')
+new = '# MUTATION: the shared import is gone, but the calls remain\n'
+assert s.count(old) == 1, "the comimage import not found -- update this mutation"
+open(p, 'w').write(s.replace(old, new))
+PYEOF
+expect_red "the audit reports a caller that does not take it from comimage" \
+    "comtest.py calls find_header without importing it from comimage" \
+    python3 tests/check_comimage.py
+cp "$SAVED_T" tests/comtest.py
+
+# Green again, on all three readers at once: the case above mutated the one
+# file all of them import, so restoring it has to satisfy each of them, not
+# just the one that was last run.
+for c in tests/check_framedisp.py tests/check_comimage.py tests/check_8086.py; do
+    if python3 "$c" >/dev/null 2>&1; then
+        echo "  ok: $c passes on the restored sources"
+        pass=$((pass + 1))
+    else
+        echo "NOT RESTORED: $c is red after restoring comimage.py and comtest.py"
+        python3 "$c" 2>&1 | tail -3 | sed 's/^/       /'
+        fail=$((fail + 1))
+    fi
+done
+
 # --- 3. the operator bugs the nine dead fixtures were hiding ------------
 echo
 echo "== the bugs the never-executed fixtures were hiding"
@@ -1207,11 +1360,12 @@ cp "$SAVED_R3" Runtime.mod
 
 echo
 echo "== the .COM layout check, and the runtime size it now measures"
-# The checker used to RESTATE the runtime's size as a literal.  It was wrong
-# by 41 bytes for an unknown time, and every one of the 30 .COM files "failed"
-# on a header read out of the code stream.  A duplicated constant that has
-# drifted does not fail loudly; it re-reports the same falsehood, in which the
-# real failures hide.  The size is now MEASURED from the image.
+# The checker used to RESTATE the runtime's size as a literal.  It had drifted
+# from the runtime it described, so the header was read out of the code stream
+# and EVERY linked fixture "failed" on it - a whole file of "failures" that were
+# not findings at all.  A duplicated constant that has drifted does not fail
+# loudly; it re-reports the same falsehood, in which the real failures hide.
+# The size is now MEASURED from the image.
 #
 # These cases corrupt a real emitted .COM and require the checker to notice.
 # They need the images, so they are built once and copied; the checker has a
@@ -1230,9 +1384,10 @@ else
 
     # 0. The baseline.  Every case below is a claim that a specific assertion
     #    turns red, and none of them means anything if the copies of untouched
-    #    images already fail.  (The stale RT_SZ produced exactly that: 30
-    #    failures that were not findings.)  So this is asserted first, and a
-    #    failure here is reported as a broken baseline rather than a red test.
+    #    images already fail.  (The stale RT_SZ produced exactly that: a whole
+    #    file of "failures" that were not findings.)  So this is asserted first,
+    #    and a failure here is reported as a broken baseline rather than a red
+    #    test.
     #
     #    The baseline is also WHERE THE HEADER OFFSET COMES FROM.  Cases 1 and 2
     #    used to carry it as the literal 435 and 439, which were ENT_SZ + the
@@ -1269,7 +1424,7 @@ else
         fail=$((fail + 1))
         BASE_RT=0
     fi
-    # The layout constants are ENT_SZ 3 and HDR_SZ 16 (run_com_tests.sh), so the
+    # The layout constants are ENT_SZ 3 and HDR_SZ 16 (tests/comimage.py), so the
     # header sits at hdrOff = 3 + rtSz and the entry jump's displacement must be
     # (hdrOff + 16) - 3.  The wanted number is therefore computed from the header
     # OFFSET, not from rtSz: the two differ by 3, and getting that backwards

+ 10 - 0
shell/tests/run_all.sh

@@ -67,6 +67,15 @@
 #                       matched is itself a failure, because a helper that
 #                       drops out of the audit when you add a comment is worse
 #                       than a helper that is checked and found wanting.
+#   check_comimage       asserts that a linked .COM's layout is described in
+#                       exactly one file and that every reader imports it.
+#                       Each reader used to keep its own copy: two had their
+#                       own find_header, one a single constant of it, and one
+#                       nothing but a drifted literal for where the runtime
+#                       ends - which let its sweep begin inside the runtime
+#                       and report PASS over bytes the program never executes.
+#                       Silent, because each copy is self-consistent on its
+#                       own terms.
 #   check_runtime       sweeps the built runtime's code region with FCML: no
 #                       desync, every entry and every branch target on an
 #                       instruction boundary, and the whole disassembly equal
@@ -156,6 +165,7 @@ echo "make rc=0, tpshell $(stat -c%s tpshell) bytes"
 run "runtime probe"  python3 tests/rt_exec.py --probe
 
 run "helper audit"    python3 tests/audit_helpers.py
+run "layout helper"   python3 tests/check_comimage.py
 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

+ 33 - 121
shell/tests/run_com_tests.sh

@@ -92,88 +92,45 @@ fi
 fi
 
 # ---- independent verification of the bytes on disk ------------------------
-python3 - "$OUT" <<'PYEOF'
+python3 - "$OUT" "$D/tests" <<'PYEOF'
 import sys, os, re, glob
 out = sys.argv[1]
 
-# ENT_SZ and HDR_SZ are the layout constants, and they are RESTATED here on
-# purpose: the checker must not ask the code under test what the answer is.
+# The layout of a linked image - ENT_SZ, HDR_SZ, the load bias, initmem's
+# first bytes, and the function that MEASURES where the header is in a given
+# file - comes from tests/comimage.py and is not written down here.  This
+# checker carried its own copy of all of it and tests/comtest.py carried a
+# second; a duplicated constant that has silently drifted is not an
+# independent check, it is a second source of truth that lies, and it lies in
+# the direction of looking like the compiler is broken.  The runtime's SIZE
+# was such a constant (a literal 391 beside a comment saying it tracked
+# Runtime.RT_Size()), so the header was read out of the middle of the code and
+# every linked fixture "failed" on a header full of code bytes.
 #
-# RT_SZ is different, and it is NOT restated.  It used to be a literal, and it
-# was WRONG - 391, against a runtime of 432 bytes - so the header was read at
-# offset 394 instead of 435 and every one of the 30 .COM files "failed" on a
-# header full of code bytes.  A duplicated constant that has silently drifted
-# is not an independent check; it is a second source of truth that lies, and
-# it lies in the direction of looking like the compiler is broken.
+# The measurement is still independent of the compiler: find_header reads the
+# emitted file, not a Modula-2 variable, so the code under test cannot satisfy
+# it by agreeing with itself.  tests/check_runtime.py pins the runtime's size
+# explicitly, which is where a deliberate size change should be noticed.
 #
-# So the runtime's size is MEASURED, from the .COM itself: the runtime is the
-# region between the entry jump and the program header, and the header is
-# found by its own signature rather than by an assumed offset (hdrFlag = 1,
-# with the code END and the data base where the layout says they are).  If
-# the runtime ever changes size, this follows automatically; if the LAYOUT
-# changes, the header stops being found and the checker says so instead of
-# quietly measuring the wrong thing.
+# The image starts with a three-byte JMP at offset 0 (Compiler.Inittur): a
+# .COM is entered at file offset 0, and until that jump existed this checker
+# ASSERTED that the runtime was at offset 0, which was precisely the bug -
+# every .COM began by executing initmem with whatever the loader left in AX.
+# A checker that pins a wrong invariant is worse than no checker, because it
+# makes the wrong thing look tested.  RTSZ (the header's image offset), PROLOG
+# (RTSZ + HDR_SZ, where the entry jump must land) and DATAB (RTSZ + 1000h, the
+# compiler's data base) are all DERIVED PER FILE by find_header below.
 #
-# The measurement is still independent of the compiler - it reads the emitted
-# file, not a Modula-2 variable - so it cannot be satisfied by the code under
-# test agreeing with itself.  tests/check_runtime.py pins the size explicitly,
-# which is where a deliberate size change should be noticed.
-RT_SZ = None                    # measured per .COM by find_header, below
-
-# The image starts with a three-byte JMP at offset 0 -- see the layout comment
-# in Compiler.Inittur.  It has to be there: a .COM is entered at file offset
-# 0, and until the jump existed this checker ASSERTED that the runtime was at
-# offset 0, which is precisely the bug.  A checker that pins a wrong invariant
-# is worse than no checker, because it makes the wrong thing look tested.
-ENT_SZ = 3                     # E9 lo hi
-HDR_SZ = 16                   # 5 header words + 3 buffer words, see Compiler
-# RTSZ (image offset of the program header), PROLOG (RTSZ + HDR_SZ, where the
-# entry jump must land) and DATAB (RTSZ + 1000h, the compiler's data base) are
-# all DERIVED PER FILE by find_header below, not written down here.  They used
-# to be module constants built on the restated RT_SZ, which is the bug this
-# whole block exists to remove: every one of them was wrong by 41 bytes, and
-# a checker that is consistently wrong in a simple direction does not fail -
-# it re-reports the same false 30 failures, in which the real ones hide.
-
-# The load bias: DOS puts a .COM's first byte at CS:0100, and CS = DS, so an
-# image offset K lives at DS:(K + 0100h).  Every ABSOLUTE address in the
-# image must carry it; relative encodings (the entry jump, every CALL) must
-# not, since both operands shift together.  Restated here so that the header
-# checks below compare against the addresses the program will actually use,
-# and so that the +0100h in them is a decision this checker made rather than
-# an accident of the compiler's.  See Runtime.LoadBias.
-#
-# This is the THIRD bias of the same family in this file, and the subtlest:
-# the entry jump (a jump that landed on the end of the code), the RT_Entry
-# offsets (CALLs that landed inside a neighbouring runtime entry) and this
-# one (absolute addresses that landed 0100h low, inside the runtime) all
-# produce a program that STARTS, RUNS and PRINTS something.  Only running it
-# finds this one; the byte checks are all satisfied by an address that is
-# consistently 0100h wrong.
-LOAD_BIAS = 0x100
-
-# initmem's prologue, which is the runtime's only reader of the program
-# header.  The displacements +4 and +6 below are the whole point of this
-# constant: the header word checks further down read hdrDS at +4 and hdrHeap
-# at +6, and initmem has to read the SAME two words or it clears the wrong
-# range.  It used to read +8 (hdrMax, which the compiler patches to 0), so it
-# zeroed nothing at all, and nothing here noticed -- the emitted loop was
-# perfectly well formed, it just never ran.  Asserting the bytes and the
-# header offsets together is what closes that gap.
-#
-# It is the FIRST ELEVEN BYTES OF THE RUNTIME, so it sits at image offset
-# ENT_SZ, not 0.
-HEAD  = '8B F0 8B 54 04 8B 4C 06'   # 11 bytes of initmem, see below
-HDR_DS_WORD   = 4             # header word holding the data base
-HDR_HEAP_WORD = 6             # header word holding the data end
-# Byte 4 of HEAD is the displacement of initmem's MOV DX,[SI+?], and byte 7
-# the displacement of its MOV CX,[SI+?].  The checks below read the header
-# words at HDR_DS_WORD and HDR_HEAP_WORD, so tying those two displacements to
-# the same two constants is what makes the runtime and the compiler agree by
-# construction rather than by coincidence.
-assert [int(HEAD.split()[4], 16), int(HEAD.split()[7], 16)] == \
-       [HDR_DS_WORD, HDR_HEAP_WORD], \
-       'initmem no longer reads the two header words this checker verifies'
+# The load bias is the THIRD bias of the same family in this file, and the
+# subtlest: the entry jump (a jump that landed on the end of the code), the
+# RT_Entry offsets (CALLs that landed inside a neighbouring runtime entry) and
+# absolute addresses (which landed 0100h low, inside the runtime) all produce
+# a program that STARTS, RUNS and PRINTS something.  Only running it finds the
+# last one; the byte checks below are all satisfied by an address that is
+# consistently 0100h wrong.  See comimage.LOAD_BIAS and Runtime.LoadBias.
+sys.path.insert(0, sys.argv[2])
+from comimage import (ENT_SZ, HDR_SZ, LOAD_BIAS, HEAD, HDR_DS_WORD,
+                      HDR_HEAP_WORD, find_header)
 
 raw = open(os.path.join(out, 'raw.txt')).read()
 rows = re.findall(r'(\S+\.pas)\s+OK\s+com=(\d+)\s+image=(\d+)\s+data=(\d+)\s+nonzeroInGap=(\d+)', raw)
@@ -182,51 +139,6 @@ if not rows:
     sys.exit(1)
 
 
-def find_header(d):
-    """Locate the program header by its own signature.  Returns its image
-    offset, or None.
-
-    The header is eight words at image offset ENT_SZ + rtSz, and the layout
-    says what they are (offsets here are BYTES into the header, which is why
-    HDR_DS_WORD is 4 and not 2 - the words are 2 bytes each and 1-based by
-    two, not by one):
-
-        +0  1                 hdrFlag, always 1
-        +2  code end + bias   hdrCS
-        +4  data base + bias  hdrDS, where data base = hdrOff + 1000h
-        +6  data end  + bias  hdrHeap, which is hdrDS + dataBytes
-
-    hdrDS ties the header to its OWN offset, so the offset is recoverable from
-    the file without assuming a runtime size: hdrOff = hdrDS - 1000h - bias.
-    A candidate is accepted only if hdrFlag is 1, hdrDS satisfies that
-    equation, hdrHeap is above hdrDS (a heap below its own base is not a
-    layout, it is a coincidence), and the runtime's known first bytes are
-    where they belong.  initmem is the ONLY code in the image that reads the
-    header, so its bytes cannot themselves move: they are at ENT_SZ always.
-
-    Measuring beats restating the size, and it is not a loss of independence:
-    it reads the EMITTED FILE, so the compiler cannot satisfy it by agreeing
-    with itself.  tests/check_runtime.py is where the runtime's size is pinned
-    deliberately, and this checker reports the size it measured on every run,
-    so a change there is visible rather than absorbed.
-    """
-    head = bytes(int(x, 16) for x in HEAD.split())
-    if d[ENT_SZ:ENT_SZ + len(head)] != head:
-        return None                      # no runtime: nothing to measure
-    for off in range(ENT_SZ, len(d) - HDR_SZ + 1):
-        w = (lambda b: int.from_bytes(d[off + b:off + b + 2], 'little'))
-        if w(0) != 1:                                    # hdrFlag
-            continue
-        if w(HDR_DS_WORD) != off + 0x1000 + LOAD_BIAS:    # hdrDS
-            continue
-        if w(HDR_HEAP_WORD) <= w(HDR_DS_WORD):            # hdrHeap
-            continue
-        if w(2) - LOAD_BIAS < off + HDR_SZ:               # hdrCS
-            continue
-        return off
-    return None
-
-
 bad = 0
 last_rt = None
 for name, com, image, data, nzg in rows: