TP3/shell/tests/check_framedisp.py

#!/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 sit below BP, 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 is the highest still-free byte below BP (0FFFFh = -1
                at entry), a local of size n is based so its LAST byte sits on
                locFree, and locFree then drops by n.  For the INTEGER locals
                t27 declares that gives the first at 0FFFEh = -2, the second at
                0FFFCh = -4, and so on down (a size-1 local would take the top
                byte, -1, and a string local n bytes at -n..-1 - the point of
                the rule is that its base is chosen so the whole variable, which
                grows UP from that base, stays below the saved BP).
    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):
    """Locals are packed top-anchored below BP: locFree is the highest still-free
    byte (0FFFFh = -1 at entry) and drops by each local's size.  For the INTEGER
    locals declared here (size 2 each) that puts the first base at -2, the second
    at -4, and so on.  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))