diff --git a/docs/guides/testing.md b/docs/guides/testing.md index 11e3c4a..9f2475b 100644 --- a/docs/guides/testing.md +++ b/docs/guides/testing.md @@ -121,6 +121,22 @@ linked into the kernel binary for a `ramroot` build. --- +## The suite is currently red + +`make schfs-check` **exits non-zero at baseline**, and has for some time. Two +separate causes, both in the harness rather than the filesystem, both itemised +in [../plan/1.0.md](../plan/1.0.md): + +- AddressSanitizer reports 2121728 bytes leaked in 518 allocations, all from the + test's own page allocator, which never frees. +- UndefinedBehaviorSanitizer reports 31 misaligned reads, from casting pointers + into tree leaves where records are not 8-aligned. + +Worth knowing before you run it, and worth fixing rather than living with: the +assertions still pass, so it is tempting to read the `[ok]` lines and ignore the +exit code — which is exactly how a suite stops being a check and becomes a +decoration. + ## What is not tested - **No automated test runs on real hardware.** Everything above is QEMU. diff --git a/docs/plan/1.0.md b/docs/plan/1.0.md index ca54fd2..c4f6209 100644 --- a/docs/plan/1.0.md +++ b/docs/plan/1.0.md @@ -139,6 +139,46 @@ These are the galling ones — the replacement exists, is tested, and is unused. suite runs before DHCP binds and the `ENFILE` above made the refusal look real. A test asserting a failure has to say *which* failure. +**The suite itself:** + +Both of these were found by running `make schfs-check` rather than by reading +it, and both predate the current work — confirmed by stashing and re-running. +They are here rather than filed as nice-to-haves because a suite that is red for +known reasons stops being read, and then it is red for unknown reasons too. + +- [ ] **`make schfs-check` fails at baseline.** AddressSanitizer reports + **2121728 bytes leaked in 518 allocations**, every one of them from + `md_palloc` (`tools/schfs_objtest.c:84`) — the test's own page allocator, + which hands out pages for the allocation map and never frees them. The + target exits non-zero, so the *whole* SchFS suite is failing whatever the + assertions say. + + The cost is not the leaked memory, it is that **the leak check is now + useless**: a real leak in the filesystem would arrive as noise inside + 2 MB of expected noise. Fixing it means the test freeing what it + allocated, not suppressing the report — a suppression keeps the number + quiet and keeps the detector off. + +- [ ] **31 UndefinedBehaviorSanitizer errors**, all one root cause: records are + read straight out of a tree leaf by casting a pointer into the block, and + a record's offset within a leaf is not a multiple of 8. Sites: + `obj.c` (9), `objw.c` (11), `attr.c` (7), `space.c` (5), across + `schfs_dirent_rec`, `schfs_extent`, `schfs_extref`, `schfs_index_rec` and + `schfs_snap`. + + **This is a real hazard, not sanitizer pedantry.** Unaligned access + happens to work on x86, which is why it has never shown as a bug — but it + is undefined in C, and a compiler that knows a pointer is 8-aligned is + entitled to emit an instruction that requires it. The failure would appear + under a different optimisation level or on a different architecture, and + would look like filesystem corruption rather than a codegen problem. + + The fix that keeps the format unchanged is to stop casting: copy the bytes + into an aligned local with `memmove`, or declare the records packed so the + compiler stops assuming. Padding the records to 8 bytes would also work + and costs on-disk space — and is now a **format change**, which is what + the version policy above is for. + **The identity item:** - [ ] **Boot from a USB disk.** The xHCI stack, bulk-only transport, SCSI and