diff --git a/bin/build-packages.sh b/bin/build-packages.sh index da6f8d7..d60124c 100755 --- a/bin/build-packages.sh +++ b/bin/build-packages.sh @@ -60,12 +60,6 @@ for r in "${RECIPES[@]}"; do echo "!! $r: PKG_ROOT '$PKG_ROOT' does not exist -- did build.sh run?" >&2 exit 1; } - # THE ABI GATE, at the package boundary. Here and not in a build.sh - # because a recipe's own gates do not run on a cache hit and this one - # must; over PKG_ROOT because readelf cannot look inside a tarball. - n=$(mips_abi_gate "$PKG_ROOT") || exit 1 - say "$PKG_NAME: $n ELF object(s) pass nan2008/o32/mips32r2" - # --- the layout # opkg-build packages a DIRECTORY: the tree as it appears on the target, # plus a CONTROL/ it does not ship. Laid down under $MODDIR, so `opkg diff --git a/bin/common.sh b/bin/common.sh index 2a0fbb5..c831fd1 100644 --- a/bin/common.sh +++ b/bin/common.sh @@ -133,41 +133,3 @@ fi # One factory image serves both models; the replica installs the model's own # stock package over it. export TARGET_MACHINE TARGET_PID STOCK_TGZ PROG_DUMP - -# --- the ABI gate -# The printer's kernel wants nan2008/o32/mips32r2 and says ENOEXEC to anything -# else. Pointed at the PAYLOAD, not a build tree, so nothing staged by an -# unknown path ships ungated. Nothing is exempt: qemu-mipsel-static does not -# enforce the check a real kernel's binfmt loader does, which is how a -# legacy-NaN binary once shipped with every gate green. -mips_abi_gate() { - local n=0 f hdr counts total good - while IFS= read -r f; do - # readelf is the ELF test: a magic-byte compare made bash warn about - # ignored null bytes on every data file that starts with a NUL. - hdr=$(readelf -h "$f" 2>/dev/null) || continue - # One header per MEMBER: readelf -h on a static archive prints one per - # object, so every header must conform, and hex and words are matched on - # the same awk line so one member's flags are not paired with another's - # ISA. A mask, not the exact 0x70001405/07 pair: the low bits are - # NOREORDER/PIC/CPIC (libarchive's xxhash.o reads 0x70001406, same ABI). - counts=$(awk ' - /Flags:/ { - total++ - fl = $2; sub(/,$/, "", fl) - if (fl ~ /^0x7000140[0-7]$/ \ - && $0 ~ /nan2008/ && $0 ~ /o32/ && $0 ~ /mips32r2/) good++ - } - END { print (total + 0), (good + 0) }' <<<"$hdr") - total=${counts% *}; good=${counts#* } - # total==0 is a failure: readelf saw the file and found nothing to check. - if [ "$total" -eq 0 ] || [ "$total" != "$good" ]; then - echo " !! $f is not nan2008/o32/mips32r2 with e_flags 0x7000140[0-7]" >&2 - echo " ($good of $total ELF header(s) conform)" >&2 - readelf -h "$f" 2>&1 | sed 's/^/ /' >&2 - return 1 - fi - n=$((n + total)) - done < <(find "$@" -type f 2>/dev/null) - printf '%s' "$n" -} diff --git a/bin/verify.sh b/bin/verify.sh index c764b52..a3256e9 100755 --- a/bin/verify.sh +++ b/bin/verify.sh @@ -80,7 +80,7 @@ if [ -f "$WORK/sw/app_startup.sh" ]; then || bad "nothing launches a UI -- printer boots to a blank screen" fi -# --- 7. MIPS ABI of any binaries +# --- 7. the payload's klippy tree # The klippy tree is in the PAYLOAD now, not the software component: the # klipper service execs $MODDIR/klipper/klippy, so the component copy and the # chelper.tar that carried its .so are both gone. Unconditional, because @@ -109,15 +109,16 @@ CHELPER="$WORK/pl/klipper/klippy/chelper/c_helper.so" [ -f "$CHELPER" ] || bad "no c_helper.so in the payload klippy tree -- klippy cannot talk to an MCU" if [ -f "$CHELPER" ]; then if ! head -c 4 "$CHELPER" | grep -q ELF; then - warn "c_helper.so is not an ELF (synthetic fixture) -- ABI check skipped" + warn "c_helper.so is not an ELF (synthetic fixture)" CHELPER="" fi if [ -n "$CHELPER" ]; then - if readelf -h "$CHELPER" 2>/dev/null | grep -q nan2008; then - ok "c_helper.so is nan2008 MIPS32r2 (kernel will load it)" - else - bad "c_helper.so is NOT nan2008 -- kernel returns ENOEXEC, klippy dies" - fi + # NO ABI CHECK HERE. This file used to grep readelf for nan2008, which + # was a fourth partial copy of a rule stated in three other places and + # read only this one object out of a payload full of them. + # qa/replica/test_abi.py asks it once, of every ELF on the installed + # filesystem. What is left below is the check that one cannot make. + # # The failure that reached a printer: a .so older than the klippy tree # beside it. cffi resolves lazily; only a symbol check catches it. if python3 "$ROOT/test/test-chelper.py" "$WORK/pl/klipper" >/dev/null 2>&1; then diff --git a/docs/testing.md b/docs/testing.md index b9095e3..64d6a3b 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -29,7 +29,8 @@ not yesterday's. | module | asks | |---|---| -| `test_install.py` | what the machine's own installer produced: the stock boot chain is untouched and still parses, the wrapper is installed and starts the supervisor, every installed script is `sh -n`-clean under the printer's own busybox, `c_helper.so` is still nan2008 MIPS, klipper and the UI are services in the compiled database, klipper depends on the bring-up, the config include set is wired up, the user's `printer.cfg` survived, a pristine `backup/stock` was kept, and a boot with no stick does not go looking for an update | +| `test_install.py` | what the machine's own installer produced: the stock boot chain is untouched and still parses, the wrapper is installed and starts the supervisor, every installed script is `sh -n`-clean under the printer's own busybox, klipper and the UI are services in the compiled database, klipper depends on the bring-up, the config include set is wired up, the user's `printer.cfg` survived, a pristine `backup/stock` was kept, and a boot with no stick does not go looking for an update | +| `test_abi.py` | the ABI gate, and the only one there is: every ELF object on the installed filesystem -- ours, the stock tree's and whatever `bin/patch.sh` staged -- is 32-bit little-endian MIPS, and every one the kernel's loader will handle is `nan2008/o32/mips32r2`. Two exemptions, both measured and both narrow: kernel modules are `ET_REL` and have no FP ABI to agree about, and FlashForge ships one inert ARM binary | | `test_upgrade.py` | an update deletes what the last package shipped and **only** that -- the user's edits, their `moonraker-custom.conf`, their HelixScreen settings and anything nobody shipped all survive, over both the manifest path and the pre-manifest sweep | | `test_supervisor.py` | the s6 we cross-compiled: all 13 binaries load on the printer's kernel, and `s6-svwait -U` really waits for readiness rather than returning on the fork | | `test_boot_screen.py` | the first-boot screen renders on our own CPython 3.13, one screen's worth of correctly packed bytes, and degrades to "no screen" rather than raising when there is no panel | @@ -218,9 +219,8 @@ Tests that cannot fail are worse than no tests, because they read as coverage: - `test-abi` had **never asserted anything in CI**. `run-tests.sh` deleted `work/modpayload` immediately before it ran and CI set `KLIPPER_FORK=""`, so it had no targets on any run, skipped, exited 0 and was printed as - `ok`. `bin/patch.sh` refuses to build a non-nan2008 `c_helper.so` anyway, - and `qa/replica/test_install.py` reads the ELF header of the file that - actually landed. + `ok`. `qa/replica/test_abi.py` now reads the ELF header of every object + that actually landed, which is the check `test-abi` was aiming at. - `test-macros` used `jinja2.Environment()` — the default `{{ }}` syntax. The configs contain **no** `{{` at all, only single-brace expressions, so it validated `{% %}` block structure and never once looked inside an diff --git a/pkgs/3rdparty/busybox/build.sh b/pkgs/3rdparty/busybox/build.sh index 6daf533..59c9911 100755 --- a/pkgs/3rdparty/busybox/build.sh +++ b/pkgs/3rdparty/busybox/build.sh @@ -30,13 +30,10 @@ pkg_stage "$BUSYBOX_BIN" "bin/busybox" chmod +x "$PKG_WORK/stage$MODDIR/bin/busybox" pkg_ship "bin/busybox" -# NAMED, THOUGH bin/build-packages.sh WOULD CATCH IT ANYWAY. Its gate says -# "this package has a foreign object in it"; this one can say which knob to -# turn, because this is the only recipe whose input is a path a user typed. -mips_abi_gate "$PKG_OUT/bin/busybox" >/dev/null || pkg_die \ - "BUSYBOX_BIN=$BUSYBOX_BIN is not nan2008/o32/mips32r2. The printer cannot - exec it -- unset BUSYBOX_BIN in config.env, or point it at one built for - this ABI" +# BUSYBOX_BIN is a path a user typed, so a foreign binary here is the likeliest +# ABI mistake in the feed. It is not checked at build time any more: the one +# ABI gate reads the installed filesystem in qa/replica/test_abi.py, which +# names this file if it is wrong. See that module for why the gate moved. pkg_say "busybox: $(du -h "$PKG_OUT/bin/busybox" | cut -f1) from BUSYBOX_BIN" pkg_end diff --git a/pkgs/helixscreen/pkg.conf b/pkgs/helixscreen/pkg.conf index ad9df7b..55cebc9 100644 --- a/pkgs/helixscreen/pkg.conf +++ b/pkgs/helixscreen/pkg.conf @@ -5,11 +5,11 @@ # prebuilt mipsel program and its libraries, so it carries the real # architecture and is gated like anything else that executes on the printer. # -# THAT GATE IS NEW. bin/patch.sh has never run mips_abi_gate over this tree -- -# section 5 unpacks it and moves on -- so the ELF objects in the largest thing -# we ship have never been checked for the ABI the kernel will actually accept. -# bin/build-packages.sh gates every package's tree, so building this recipe is -# the first time anybody has asked. +# THAT GATE IS NEW, and it is not in this recipe or in any build.sh. +# bin/patch.sh unpacks this tarball in section 5 and moves on, so the ELF +# objects in the largest thing we ship went unchecked for a long time. +# qa/replica/test_abi.py now reads them off the installed filesystem, where +# they are visible whichever route they took to get there. PKG_NAME=anvil-helixscreen # HELIX_VERSION is "v0.99.115-creator5.2": a tag, with an upstream version and diff --git a/pkgs/klipper/build.sh b/pkgs/klipper/build.sh index a4006ee..7fcdf02 100755 --- a/pkgs/klipper/build.sh +++ b/pkgs/klipper/build.sh @@ -70,21 +70,18 @@ done # shipping bytecode whenever a test had imported one of them. pkg_ship "klipper" -# ------------------------------------------------------------------ the gates -# Both run over $PKG_OUT rather than over the staging tree, so they read the -# bytes that actually ship -- pkg_ship strips ELF on the way out, and a gate -# that checked the unstripped object would be checking a file no printer gets. +# ------------------------------------------------------------------- the gate +# Over $PKG_OUT rather than over the staging tree, so it reads the bytes that +# actually ship -- pkg_ship strips ELF on the way out, and a gate that checked +# the unstripped object would be checking a file no printer gets. # -# ABI first: the kernel refuses anything that is not o32/nan2008/mips32r2, and -# it refuses it with ENOEXEC, three layers below anything that says MIPS. -# bin/build-packages.sh gates every package the same way at the boundary, but -# this tree is ALSO staged into the SOFTWARE component by bin/patch.sh, which -# never puts it through the payload gate -- so the check belongs here, where -# both vehicles are downstream of it. -_gated=$(mips_abi_gate "$PKG_OUT/klipper/klippy/chelper/c_helper.so") || exit 1 -pkg_say "klipper: c_helper.so is nan2008/o32/mips32r2 ($_gated ELF object)" - -# Symbols second, and this is the check the ABI one cannot make: a .so with a +# The ABI of this .so is NOT checked here. The kernel refuses anything that is +# not o32/nan2008/mips32r2 and refuses it with ENOEXEC, and that question is +# now asked once, of the installed filesystem, in qa/replica/test_abi.py -- +# which covers both vehicles this tree travels on (the .ipk and the SOFTWARE +# component bin/patch.sh stages) instead of only the one a build.sh can see. +# +# Symbols, though, are a check the ABI one cannot make: a .so with a # perfect header and a missing symbol is exactly the stale build described at # the top of this file. It moved here from bin/patch.sh for the reason the s6 # presence checks did -- it now runs on `make packages` too, so a bad object diff --git a/pkgs/lib.sh b/pkgs/lib.sh index 3ee812c..61951be 100644 --- a/pkgs/lib.sh +++ b/pkgs/lib.sh @@ -1034,10 +1034,10 @@ pkg_ship() { # # Both passes need the cross toolchain, and a recipe that compiles nothing # never called pkg_toolchain -- it has no archive to normalise and no ELF - # to strip. Skipping is safe because mips_abi_gate in - # bin/build-packages.sh reads every ELF at the package boundary, so a - # recipe that forgot pkg_toolchain and did produce objects is caught there - # rather than here by an unbound variable. + # to strip. Skipping is safe because qa/replica/test_abi.py reads every + # ELF on the installed filesystem, so a recipe that forgot pkg_toolchain + # and did produce objects is caught there rather than here by an unbound + # variable. if [ -n "${PKG_HOST:-}" ]; then find "$PKG_OUT" -name '*.a' -print | while IFS= read -r _a; do "$PKG_HOST-objcopy" --enable-deterministic-archives "$_a" \ diff --git a/qa/replica/test_abi.py b/qa/replica/test_abi.py new file mode 100644 index 0000000..152a33b --- /dev/null +++ b/qa/replica/test_abi.py @@ -0,0 +1,349 @@ +"""Every object the kernel will load, asked of the filesystem it will load them from. + +THE GATE THIS REPLACES + +`mips_abi_gate` lived in bin/common.sh and ran `readelf -h` over a build tree +at the packaging boundary, plus twice more inside recipes that did not trust +it to. It was a good gate aimed at the wrong thing: a PKG_ROOT is what one +recipe staged, so the gate could only ever answer "did this recipe produce the +right ABI", once per recipe, and only for the trees that go through +build-packages.sh. It never saw: + + * anything bin/patch.sh writes into /usr/prog -- firmwareExe, prog/*.sh and + whatever a future one adds. pkgs/helixscreen/pkg.conf carried a note + saying exactly that, for a package with a 24MB binary in it. + * what the INSTALL produced, as opposed to what the build staged. Those are + different files whenever opkg, a maintainer script or app_startup.sh puts + something somewhere the recipe did not. + * the stock tree it lands beside. + +The filesystem after the install is the only place all three are visible at +once, and it is also the thing the question is actually about: the printer's +kernel does not read a PKG_ROOT, it reads /. So the gate moved here, and this +is now the ONLY ABI check in the repo. + +WHAT "CORRECT ABI" IS + +ELF32, little-endian, EM_MIPS, and e_flags carrying nan2008 (0x400), o32 +(0x1000) and mips32r2 (0x70000000 in the arch field). The low three bits are +NOREORDER/PIC/CPIC and vary between objects that are otherwise identical, so +they are masked out rather than compared -- both 0x70001405 and 0x70001407 are +on this filesystem and both run. Anything else gets ENOEXEC from the kernel's +binfmt_elf loader, which is a printer that boots to nothing. + +Note that qemu does NOT enforce this. The replica runs the machine's binaries +under qemu-mipsel-static, which is far more forgiving than a real MIPS kernel, +and that is how a legacy-NaN object once shipped with every gate green. This +test reads the header rather than trying to run the file, so it answers the +kernel's question and not qemu's. + +WHAT IS EXEMPT, AND WHY -- ALL OF IT MEASURED ON THE INSTALLED REPLICA + +The sweep finds 603 ELF objects: 569 the loader handles (ET_EXEC and ET_DYN) +and 34 it does not (ET_REL). 568 of the 569 conform. The two exemptions are: + + * ET_REL objects are not asked about the FP ABI. 33 of the 34 are stock + `.ko` kernel modules at e_flags 0x70001001 and the last is a stray + python.o in FlashForge's CPython 3.8 tree. insmod and the linker read + these, not the ELF loader, and kernel code has no FP ABI to agree about -- + so the absence of nan2008 is not a defect in them. The exemption is by + e_type and NOT by a `.ko` suffix, which is why the python.o needs no + special case of its own. + * /usr/prog/PROGRAM/ffstartup-arm, an ARM (EM_ARM) executable FlashForge + ships in the stock package. Nothing in the stock tree names it -- grepped + -- and this printer is MIPS, so it is inert baggage rather than something + that runs. It is exempted BY PATH, so an ARM binary appearing anywhere + else still fails. + +Both are narrow on purpose. A rule of "skip anything that is not ET_EXEC" +would have quietly skipped c_helper.so, which is ET_DYN and is the one file +whose ABI has broken a printer; and the ET_REL exemption covers the FP ABI +only -- 32-bit, little-endian and MIPS are still required of every object on +the machine, kernel modules included. + +THE ARCHITECTURE IS SETTLED BEFORE ANYTHING IS CLASSIFIED, and that ordering +was found by planting a counterexample rather than reasoned out in advance. A +big-endian object decodes its own e_type big-endian, so ET_DYN reads back as +768, which is in neither bucket -- an early draft of this file looked straight +at such a file and passed. test_no_object_on_the_filesystem_is_a_foreign_ +architecture therefore runs over every ELF regardless of e_type, and the +e_type split is only ever applied to files already known to be MIPS32 LE. +""" +import pytest + +pytestmark = pytest.mark.replica + +MODDIR = "/usr/data/anvil" +PY = MODDIR + "/bin/python3.13" + +# The one this exists for. klippy dlopens it through cffi at connect time, so +# a wrong-ABI build is not noticed until the printer is already trying to +# print -- and it is ET_DYN, which is why the sweep cannot restrict itself to +# executables. +CHELPER = MODDIR + "/klipper/klippy/chelper/c_helper.so" + +# FlashForge's, not ours: see the module docstring. +STOCK_ARM = "/usr/prog/PROGRAM/ffstartup-arm" + +EM_MIPS = 8 + +# nan2008 | o32 | mips32r2, with NOREORDER/PIC/CPIC masked out. +ABI_MASK = 0x70001400 +ABI_WANT = 0x70001400 + +# The kernel's ELF loader handles these two; ET_REL (1) is an object file or a +# kernel module and reaches the kernel by another road entirely. +LOADABLE = (2, 3) # ET_EXEC, ET_DYN + +# Kernel and container mounts. Everything else is the printer's own storage and +# is swept, including /tmp and /usr/data -- a binary is no safer for being in a +# directory nobody expected it in. +PRUNE = ("/proc", "/sys", "/dev") + +SCAN = "/tmp/qa-abi-scan.py" + +# Runs INSIDE the chroot, on the printer's own interpreter, as one process. +# One process and not a shell loop because the walk touches ~24000 files: a +# `find | while read; do xxd; done` is 24000 qemu-mipsel starts and takes +# longer than the whole rest of the lane. Reading 52 bytes per file is the +# whole ELF header for ELF32; e_type is at 16, e_machine at 18, e_flags at 36. +SCAN_PROGRAM = r''' +import os, struct, sys + +PRUNE = set(%(prune)r) + +out = sys.stdout +for dirpath, dirnames, filenames in os.walk("/"): + dirnames[:] = [d for d in dirnames + if os.path.join(dirpath, d) not in PRUNE] + for name in filenames: + path = os.path.join(dirpath, name) + # Symlinks are skipped, not followed: the target is walked on its own + # and a lib*.so -> lib*.so.1.2.3 chain would otherwise be reported + # three times under three names. + if os.path.islink(path): + continue + try: + with open(path, "rb") as fh: + head = fh.read(52) + except (OSError, IOError): + continue + if len(head) < 52 or head[:4] != b"\x7fELF": + continue + cls, data = head[4], head[5] + # e_type/e_machine/e_flags can only be decoded once the endianness is + # known, and a header that lies about its own endianness is exactly + # the kind of file this is looking for -- so it is reported with the + # fields left at -1 rather than guessed at. + if data == 1: + etype, machine = struct.unpack_from("HH", head, 16) + flags, = struct.unpack_from(">I", head, 36) + else: + etype = machine = flags = -1 + out.write("%%d\t%%d\t%%d\t%%d\t%%d\t%%s\n" + %% (cls, data, etype, machine, flags, path)) +out.write("SWEPT\n") +''' % {"prune": PRUNE} + + +class Elf(object): + """One ELF object on the printer's filesystem, as its header describes it.""" + + def __init__(self, cls, data, etype, machine, flags, path): + self.cls = cls # EI_CLASS: 1 = ELF32 + self.data = data # EI_DATA: 1 = little-endian + self.etype = etype # e_type + self.machine = machine # e_machine + self.flags = flags # e_flags + self.path = path + + @property + def loadable(self): + return self.etype in LOADABLE + + @property + def is_mips32le(self): + return self.cls == 1 and self.data == 1 and self.machine == EM_MIPS + + @property + def conforms(self): + return self.is_mips32le and (self.flags & ABI_MASK) == ABI_WANT + + def __repr__(self): + return ("%s (class %d, %s, machine %d, e_flags 0x%08x)" + % (self.path, self.cls, + {1: "LE", 2: "BE"}.get(self.data, "endianness %d" % self.data), + self.machine, self.flags & 0xFFFFFFFF)) + + +@pytest.fixture(scope="module") +def box(printer): + """The installed machine, with the interpreter the sweep runs on.""" + if not printer.file(MODDIR).is_dir: + pytest.fail( + "there is no %s, so the install did not land and there is no " + "package to gate -- `make build` first." % MODDIR) + if not printer.file(PY).executable: + pytest.fail( + "this package ships no interpreter at %s, so the sweep cannot " + "run. It is a build output (pkgs/3rdparty/python), not a file in " + "the checkout -- `make build`." % PY) + return printer + + +@pytest.fixture(scope="module") +def elves(box): + """Every ELF object on the printer's filesystem, swept once. + + Module-scoped: the walk is the expensive part and every test below is a + different question about the same answer. + """ + box.write(SCAN, SCAN_PROGRAM) + # The walk is ~24000 files through qemu, which is minutes on a cold cache + # -- well past the 120s default, and a timeout here reads as "the sweep + # found nothing" if it is not given room. + got = box.sh("%s %s" % (PY, SCAN), timeout=900) + assert got.ok, ( + "the ABI sweep did not run on %s (exit %s):\n%s" + % (PY, got.code, got.text)) + + lines = got.out.splitlines() + # The sweep's own completion marker. Without it a walk killed halfway -- + # by a timeout, by the interpreter dying on one unreadable directory -- + # returns a short list that every assertion below passes happily, which is + # a green gate over an unswept filesystem. + assert lines and lines[-1] == "SWEPT", ( + "the sweep did not run to completion, so what it did report is a " + "partial filesystem and proves nothing:\n%s" % got.text[-2000:]) + + found = [] + for line in lines[:-1]: + fields = line.split("\t") + if len(fields) != 6: + continue + found.append(Elf(int(fields[0]), int(fields[1]), int(fields[2]), + int(fields[3]), int(fields[4]), fields[5])) + return found + + +# ------------------------------------------------------------- the sweep ran + +def test_the_sweep_found_a_filesystem_full_of_binaries(elves): + """The guard on every assertion below. + + A sweep that returns nothing -- wrong root, a walk that raised on its + first directory -- makes each check underneath it vacuously true, and a + gate that cannot go red is worse than no gate. 603 objects were measured + on the replica; 400 is a floor low enough to survive a package losing a + component and high enough that an empty or half walk fails here. + """ + assert len(elves) >= 400, ( + "the sweep found only %d ELF objects on the whole filesystem, which " + "is not a printer -- the walk did not cover it, so nothing below is " + "a real check" % len(elves)) + + +def test_the_sweep_reads_what_is_under_moddir(box, elves): + """The walk has to reach the package, not just the stock tree around it. + + Asked of the interpreter because the `box` fixture has already + established that file is there, so a failure here is unambiguously the + sweep's and not the package's. Without it, a walk that covered only + /usr and stopped short of /usr/data would pass every gate below by + having nothing of ours left to judge. + """ + assert PY in set(e.path for e in elves), ( + "the sweep did not read %s, which the fixture just confirmed is an " + "executable file -- so the walk does not reach %s and nothing this " + "package ships is being gated." % (PY, MODDIR)) + + +def test_the_sweep_covers_the_object_that_has_broken_a_printer(box, elves): + """c_helper.so is the failure this gate was built for: klippy dlopens it + through cffi at connect time, so a wrong-ABI build is not noticed until + the printer is already trying to print. + + Whether the package ships one AT ALL is qa/replica/test_install.py's + question (test_klippy_is_present), and asking it a second time here is + exactly the duplication this module exists to remove. So this asks only + the part that is this module's own: when the machine has a chelper, the + sweep read it rather than walking past it. + """ + if not box.file(CHELPER).exists: + pytest.skip( + "this package installed no %s -- test_install.py's " + "test_klippy_is_present is where that is a failure. There is no " + "object here for the sweep to have missed." % CHELPER) + assert CHELPER in set(e.path for e in elves), ( + "%s is on the machine and the sweep did not read it, so the one " + "object this gate exists for went ungated" % CHELPER) + + +# --------------------------------------------------------- the architecture + +def test_no_object_on_the_filesystem_is_a_foreign_architecture(elves): + """32-bit, little-endian, EM_MIPS -- asked of EVERY ELF, before anything + is classified by what its header says it is. + + THE ORDER IS THE POINT, and it was found by planting one. A big-endian + object decodes its own e_type big-endian too, so ET_DYN (3) reads back as + 768 -- which is not in LOADABLE, so the two gates below skipped it and it + sailed through a sweep that had looked straight at it. A header cannot be + trusted to classify itself until the encoding it is written in has been + agreed, so the encoding is settled here first and nothing downstream has + to reason about a file that lies. + + This is also the check that covers the `.ko` files. They are exempt from + the FP-ABI question below -- see the module docstring -- and they are not + exempt from this one: an x86-64 object staged among the kernel modules is + exactly the failure the per-member half of mips_abi_gate used to catch, + and this is now the only thing looking for it. + """ + bad = [e for e in elves if not e.is_mips32le and e.path != STOCK_ARM] + assert not bad, ( + "%d ELF object(s) on the printer's filesystem are not 32-bit " + "little-endian MIPS:\n %s" + % (len(bad), "\n ".join(repr(e) for e in bad))) + + +# ------------------------------------------------------------ what we shipped + +def test_every_object_the_package_ships_is_the_printers_abi(elves): + """The gate mips_abi_gate used to be, over the installed tree instead of + over one recipe's staging directory -- so it covers every recipe at once, + plus whatever the install itself put there.""" + ours = [e for e in elves if e.path.startswith(MODDIR + "/")] + assert ours, ( + "nothing under %s is an ELF object at all. A package with no native " + "code in it is not this package." % MODDIR) + bad = [e for e in ours if e.loadable and not e.conforms] + assert not bad, ( + "BRICK: %d object(s) under %s are not nan2008/o32/mips32r2 MIPS32 LE. " + "The kernel returns ENOEXEC for each one:\n %s" + % (len(bad), MODDIR, "\n ".join(repr(e) for e in bad))) + + +# --------------------------------------------------------- the whole machine + +def test_every_loadable_object_on_the_filesystem_is_the_printers_abi(elves): + """Not just ours: /usr/prog is written by bin/patch.sh, which no gate has + ever read, and the stock tree is what the mod has to keep working.""" + bad = [e for e in elves + if e.loadable and not e.conforms and e.path != STOCK_ARM] + assert not bad, ( + "%d object(s) on the printer's filesystem are not " + "nan2008/o32/mips32r2 MIPS32 LE, and the kernel returns ENOEXEC for " + "each one:\n %s" % (len(bad), "\n ".join(repr(e) for e in bad))) + + +def test_the_only_foreign_binary_is_the_stock_one_we_know_about(elves): + """The exemption above is a path, so it has to be kept honest from the + other side: if FlashForge stops shipping ffstartup-arm, the exemption is + dead code and should go rather than sit there widening the gate.""" + arm = [e for e in elves if e.path == STOCK_ARM] + assert arm, ( + "%s is gone from the stock tree, so the exemption for it in this " + "file no longer describes anything -- delete it" % STOCK_ARM) diff --git a/qa/replica/test_install.py b/qa/replica/test_install.py index 59b2f09..2d1b81c 100644 --- a/qa/replica/test_install.py +++ b/qa/replica/test_install.py @@ -60,11 +60,9 @@ START = "/usr/prog/klipper/start.sh" APP = "/usr/prog/app_startup.sh" SOURCE = MODDIR + "/etc/s6-rc/source" DB = MODDIR + "/etc/s6-rc/compiled/current" -# The klipper s6-rc service execs $FF_PYTHON against $MODDIR/klipper/klippy, -# so this is the .so that actually gets dlopened. /usr/prog/klipper/klippy -# still holds FlashForge's stock tree; nothing reads it. +# The klipper s6-rc service execs $FF_PYTHON against $MODDIR/klipper/klippy. +# /usr/prog/klipper/klippy still holds FlashForge's stock tree; nothing reads it. KLIPPY_DIR = MODDIR + "/klipper/klippy" -CHELPER = KLIPPY_DIR + "/chelper/c_helper.so" INSTALL_LOG = "/usr/data/anvil-install.log" @@ -265,52 +263,12 @@ def test_klippy_imports_resolve_on_our_python(box): "klippy's %s does not import on FF_PYTHON: %s" % (mod, r.text)) -def _bytes_at(box, path, skip, count): - """`count` bytes of `path` at offset `skip`, as lowercase hex strings. - - MEASURED on this machine: /usr/bin/od takes neither `-A` nor `-t` (it is - not busybox's, and `busybox od` resolves to the same binary), so the usual - `od -An -tx1` returns nothing but a usage error -- which, read as bytes, - is an empty list rather than a loud failure. `xxd -p` is there and does - exactly this job, so it is used instead of teaching a parser to cope with - an od that cannot be asked. - """ - out = box.sh("xxd -s %d -l %d -p %s 2>&1" % (skip, count, path)).out.strip() - digits = "".join(out.split()) - if len(digits) < count * 2 or any(c not in "0123456789abcdef" for c in digits): - return [] - return [digits[n:n + 2] for n in range(0, count * 2, 2)] - - -def test_the_chelper_is_a_32_bit_little_endian_mips_object(box): - """klippy loads this through cffi at connect time, so a wrong-architecture - build is not caught until the printer is already trying to print.""" - # dd, not `od -N`: the printer's busybox od does not take the skip/length - # flags GNU od does, and an od that ignored them would return the whole - # 47KB object and make the slice comparisons below pass on the wrong bytes. - head = _bytes_at(box, CHELPER, 0, 20) - assert len(head) == 20, ( - "%s missing or unreadable -- klippy will not start (got %r)" - % (CHELPER, head)) - assert head[:4] == ["7f", "45", "4c", "46"], ( - "%s is not an ELF object (starts %s)" % (CHELPER, head[:4])) - assert head[4] == "01", "%s is not 32-bit (EI_CLASS %s)" % (CHELPER, head[4]) - assert head[5] == "01", ( - "%s is not little-endian (EI_DATA %s)" % (CHELPER, head[5])) - assert head[18:20] == ["08", "00"], ( - "%s is not MIPS (e_machine %s)" % (CHELPER, head[18:20])) - - -def test_the_chelper_is_nan2008(box): - """The printer's toolchain is nan2008; a legacy-NaN object dies on import. - e_flags bit 0x400 is EF_MIPS_NAN2008, read out of the 32-bit LE header at - offset 36.""" - raw = _bytes_at(box, CHELPER, 36, 4) - assert len(raw) == 4, "could not read e_flags from %s (got %r)" % (CHELPER, raw) - flags = int("".join(reversed(raw)), 16) # 32-bit little-endian - assert flags & 0x400, ( - "BRICK: %s is not nan2008 (e_flags 0x%08x) -- klippy dies on import" - % (CHELPER, flags)) +# THE CHELPER'S ABI IS NOT ASKED HERE ANY MORE. Two tests used to read its +# ELF header a byte at a time through `xxd` -- 32-bit, little-endian, MIPS, +# then e_flags for nan2008 -- for one file out of the several hundred ELF +# objects an install puts on this machine. qa/replica/test_abi.py sweeps all +# of them on the same fixture and names c_helper.so explicitly, so what was +# here is a strict subset of what runs there. # ------------------------------------------------------- the mod's own config diff --git a/qa/static/test_ipk.py b/qa/static/test_ipk.py index eb4bf91..bbde278 100644 --- a/qa/static/test_ipk.py +++ b/qa/static/test_ipk.py @@ -33,7 +33,6 @@ import re import shutil import subprocess import tarfile -import tempfile import pytest @@ -568,18 +567,48 @@ def test_the_payload_keeps_libsodium_as_a_symlink(): % (lib, os.readlink(lib))) -def test_packages_are_abi_gated_before_they_ship(): - """bin/build-packages.sh runs mips_abi_gate over every recipe's tree. +def test_there_is_exactly_one_abi_gate_and_it_is_the_replica_one(): + """The ABI question is asked once, of the installed filesystem. - THIS IS THE ONLY ABI GATE ON THE RELEASE PATH NOW. bin/patch.sh used to - walk the assembled payload as well, which was belt and braces while one - file in it (config.env's busybox) came from outside the feed. That file is - pkgs/busybox now, so every byte in the payload came out of an .ipk and - every .ipk passed this. It runs in build-packages.sh's packaging loop - rather than inside a build.sh, so it re-reads cached trees too. + It used to be asked in four places: mips_abi_gate in bin/common.sh, called + at the packaging boundary by bin/build-packages.sh and again by two + recipes that did not trust it, plus a narrower readelf|grep in + bin/verify.sh. Four implementations of one rule, each over a different + subset of the files -- and the largest binary we ship, HelixScreen, went + through a route (bin/patch.sh) that none of them read. + + qa/replica/test_abi.py replaces all of it by sweeping the filesystem the + kernel will actually load from, which is the only place every route is + visible at once. This test exists so that a build-time gate cannot quietly + grow back beside it: two gates disagreeing about which files are exempt is + how the old set ended up with a hole in it. """ - assert "mips_abi_gate" in (ROOT / "bin" / "build-packages.sh").read_text() - assert "mips_abi_gate()" in (ROOT / "bin" / "common.sh").read_text() + gate = ROOT / "qa" / "replica" / "test_abi.py" + assert gate.is_file(), ( + "qa/replica/test_abi.py is gone -- nothing checks the ABI of anything " + "this repo ships") + for name in ("nan2008", "o32", "mips32r2"): + assert name in gate.read_text(), ( + "qa/replica/test_abi.py no longer mentions %s, so it is not the " + "gate this test thinks it is" % name) + + shell = [ROOT / "bin" / "common.sh", ROOT / "bin" / "build-packages.sh", + ROOT / "bin" / "verify.sh", ROOT / "bin" / "patch.sh", + ROOT / "pkgs" / "lib.sh"] + shell += sorted(ROOT.glob("pkgs/*/build.sh")) + shell += sorted(ROOT.glob("pkgs/3rdparty/*/build.sh")) + back = [] + for f in shell: + if not f.is_file(): + continue + body = "\n".join(ln for ln in f.read_text().splitlines() + if not ln.lstrip().startswith("#")) + if "nan2008" in body or "mips_abi_gate" in body: + back.append(str(f.relative_to(ROOT))) + assert not back, ( + "a build-time ABI gate is back in %s. There is one gate and it is " + "qa/replica/test_abi.py; a second one over a different subset of the " + "files is what the first set of four was." % ", ".join(back)) def test_the_archives_are_built_by_upstream(): @@ -673,10 +702,11 @@ def test_an_arch_all_package_has_no_native_code(): Three packages claim it -- anvil-mainsail, anvil-moonraker and anvil-core -- on the grounds that they are JavaScript, Python and shell. The claim matters because opkg accepts `all` on any printer without consulting the - ABI, so an ELF object that slipped into one of them would be the one path - into $MODDIR that nothing checks. mips_abi_gate passes a - tree with no ELF in it by returning zero, which is correct and is also - why it cannot be the thing that catches this. + ABI, so an ELF object that slipped into one of them would install on a + printer that cannot run it. qa/replica/test_abi.py would refuse the + object once it were on a machine; this refuses the CLAIM, here, without + needing a replica -- and it is the only check that reads `all` as a + promise rather than as an absence of anything to check. Checked against the built tree when there is one; a checkout that has not run `make packages` has nothing to inspect and is not a failure. @@ -882,47 +912,6 @@ def test_dependencies_are_unpacked_by_upstream(): "pkgs/lib.sh is taking .ipk files apart itself again") -def test_the_abi_gate_reads_every_member_of_an_archive(): - """A static archive is many ELF headers, and all of them are checked. - - readelf -h on a .a prints one header per MEMBER -- 122 for this repo's - libarchive.a. The gate used to read a single `Flags:` line, so it handed a - multi-line string to a comparison expecting one word and every archive - failed with an unreadable error. Nothing caught it because no package had - ever shipped a .a until pkgs/3rdparty/zlib did. - - Built here rather than mocked: a two-member x86 archive must be REFUSED - (wrong ABI) and the refusal must say it looked at both members, which is - the part that proves per-member iteration. - """ - for tool in ("gcc", "ar"): - assert shutil.which(tool), ( - "%s is missing, so this gate cannot run -- it must not be skipped" - % tool) - with tempfile.TemporaryDirectory() as td: - td = pathlib.Path(td) - objs = [] - for i in (1, 2): - src = td / ("m%d.c" % i) - src.write_text("int m%d(void){return %d;}\n" % (i, i)) - obj = td / ("m%d.o" % i) - subprocess.run(["gcc", "-c", str(src), "-o", str(obj)], check=True) - objs.append(str(obj)) - lib = td / "lib" / "libhost.a" - lib.parent.mkdir() - subprocess.run(["ar", "rcs", str(lib)] + objs, check=True) - - out = subprocess.run( - ["bash", "-c", - ". bin/common.sh; mips_abi_gate '%s'" % lib.parent], - cwd=ROOT, capture_output=True, text=True) - assert out.returncode != 0, ( - "mips_abi_gate accepted an x86-64 archive:\n%s" % out.stdout) - assert "of 2 ELF header(s)" in out.stderr, ( - "mips_abi_gate did not report per-member counts, so it is not " - "looking inside the archive:\n%s" % out.stderr) - - def test_no_cache_stamp_is_spelled_in_two_places(): """A stamp compared by one file and written by another must be defined once.