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