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