1099b48236
Three related changes, plus the use-after-free the third one exposed.
1. read_source_line() walked the file with fgetc() to reach a target line,
and render_diagnostic() calls it once per level of a macro-expansion chain
-- so rendering one diagnostic cost O(levels x filesize) in one-byte stdio
calls. Profiling the LPC testsuite measured 45,430,932 fgetc() calls
reached from render_diagnostic, 12.1% of the entire run, for only 153
rendered diagnostics. It now reads through an 8K stack buffer and finds
line breaks with memchr.
Deliberately no heap buffer and no arena for that scan: it runs DURING a
compile (report_compile_diagnostic, from yyerror/yywarn) and well AFTER
one -- lpcshell renders stored diagnostics once the arena has been reset,
and the compiler GTests call it with no compile in flight at all. A stack
buffer is correct in all three contexts.
Output is unchanged, and checked rather than assumed: on the case that
exercises this hardest (compiler/deep_macro_nesting.lpc, 64 expansion
notes) all 396 lines of rendered diagnostics are byte-identical, and the
file's runtime drops from 1713 ms to 828 ms.
2. The compiler reset the scratchpad at the END of every compile -- freeing
its own output before the caller had read it, which is exactly why anything
a consumer reads afterwards could not live on the arena. Inverted:
compile_file()/compile_file_fd() take a ScratchArena*, allocate every
transient there, and leave it exactly as found. A caller that supplies none
gets a shared default arena, which IS the compiler's to recycle, so that
one is reset on the way IN -- the previous compile's transients stay
readable until the next compile starts. Arena state moves from file-static
globals into ScratchArena::Impl behind a plain RAII handle, and
scratchpad.{h,cc} moves to base/internal/ since ownership now sits outside
the compiler.
The default arena is process-lifetime deliberately. A fresh arena per
compile reads tidier but discards the retained chunk cache every time (that
cache is what drives a long-lived driver to zero chunk mallocs in the
steady state) and leaves scratch_stats()/scratchpad_status() describing an
arena that never took part in a compile. bench_compile caught it: "0
retained chunks, 0 resets" after 2000 compiles where master reports 1 and
2034 -- the "chunk mallocs delta MUST be 0" invariant had gone vacuous
rather than failing.
Because a second arena can no longer borrow the static base block, chunk 0
is sometimes a heap chunk, and base_is_static tells teardown whether to
free it. Every path that drops chunk 0 now keeps that flag honest via
release_base_claim(); without it the tiny-chunk test knob leaked its base
chunk on both calls (476 B and 1 MB, LeakSanitizer-confirmed).
3. lpcshell reaches the compiler through load_object_from_source(), which
now threads an optional ScratchArena* -- a single narrow entry point, so
the 24 general load_object() callers are untouched. lpcshell's Session owns
one, reset at the TOP of each Eval() rather than the end, since by then the
previous evaluation's diagnostics have been printed.
That unblocked the last piece: Diagnostic's variable-length fields are now
ScratchString/ScratchVector, as are the pending_* containers staging into
them. Two boundaries stay heap on purpose -- the lexer's provenance
accessors outlive any compile, and compiler_next_load_reason is set before
compile_file runs -- so both are copied onto the arena at capture.
That conversion carries a lifetime contract, learned twice the hard way: a
stale ScratchString is harmless to destroy, but a stale ScratchVector is
not (its destructor walks its own arena buffer to destroy elements), and
releasing must drop the BUFFER, not just clear() -- a cleared container
keeps capacity, so the next push_back wrote into memory the arena had since
handed to the lexer's Flex buffers, surfacing as a segfault in
yypop_buffer_state nowhere near the diagnostics.
Yucong Sun then found and fixed the remaining hole in that contract: an
arena could die while compiler_diags still referenced it, a
heap-use-after-free that the guard above did not cover. His fix adds the
missing teardown coupling plus regression coverage (test_compiler.cc, an
lpcshell .lpcs case) and the CMake wiring for it.
Validated on RelWithDebInfo and Debug+ASan/UBSan/LSan: 339/339 GTest and
the LPC testsuite clean on each, bench_compile steady-state matching
master, and an lpcshell run rendering a full clang-style error with snippet
and caret AFTER the compile returned -- the case this whole change exists
to make legal.
Claude-Session: https://claude.ai/code/session_01MN9sz4nvGgBZw9oZiei3XR
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|---|---|---|
| .. | ||
| .vim | ||
| clone | ||
| command | ||
| data | ||
| etc | ||
| include | ||
| inherit | ||
| log | ||
| lpcshell | ||
| single | ||
| std | ||
| u | ||
| .gitattributes | ||
| .gitignore | ||
| 2000.txt | ||
| ar_test.txt | ||
| crlf_test_file.crlf | ||
| crlf_test_file.lf | ||
| format.sh | ||
| README.md | ||
| speed.py | ||
| speed_str.py | ||
| telnet_test.expect | ||
| test.o | ||
FluffOS Testsuite
This directory is a minimal mudlib (a descendant of the classic Lil bootstrap mudlib) plus the driver's LPC regression suite. The driver boots it directly; every efun/compiler/VM behavior change is expected to come with a test here.
Source file extension
All LPC sources in this tree use the .lpc extension. The driver
resolves source files by these rules (implemented in load_object(),
src/vm/internal/simulate.cc, and pinned by
single/tests/efuns/dual_extension.lpc):
- An explicit extension is exact:
load_object("/foo.c")probes onlyfoo.c,load_object("/foo.lpc")probes onlyfoo.lpc— the other spelling is never looked up. - Extension-less names prefer
.lpcand fall back to.c(load_object("/foo")loadsfoo.lpcif present, elsefoo.c). - Object identity is extension-blind: object names never carry an
extension, and
find_object()strips either spelling, so"/foo","/foo.c", and"/foo.lpc"all find the same loaded object. The program name (prog->filename, whatinherit_list()and diagnostics report) carries the real extension of the file that was compiled. - The registry wins over the filesystem:
load_object()returns an already-loaded object for any spelling of its name — the exactness rule only applies when the load actually hits the disk. - When an exact probe misses, the load falls through to the master's
compile_object()virtual-object hook, which receives the stripped name; if it declines,load_object()returns0(no error is thrown). children(),save_object()/restore_object(),replace_program()andfunction_exists()treat both spellings equivalently (name stripping / suffix handling covers.lpcand.calike).
A few .c files exist on purpose: the clone/dual_*.c fixtures for
dual_extension.lpc, and /tmp_eval_file.c written at runtime by the
eval/codefor commands (live proof that genuinely .c-named
sources still compile).
Running the suite
The suite is a first-class ctest test and a set of CMake targets:
| Invocation | What it does |
|---|---|
ctest -R testsuite (or ctest -L testsuite) |
Runs the whole LPC suite through ctest, alongside ctest -LE testsuite for the GTest binaries. This is what CI runs. |
driver-autotest (CMake target) |
Same run, invoked directly; exits nonzero on any failure. |
driver-testsuite (CMake target) |
Boots the driver against this mudlib for interactive poking (log in and type tests). |
The runner (command/tests.lpc) prints a gtest-style protocol:
[ RUN ] /single/tests/efuns/dual_extension.lpc
[ OK ] /single/tests/efuns/dual_extension.lpc (24 ms)
[==========] 3401 checks from 297 file(s) ran. (7470 ms total)
[ PASSED ] 297 file(s).
Checks succeeded.
Failures do not stop the run: a failed check is printed with its
expected/actual diff and trace, recorded, and the run continues — one
run reports every failure (gtest semantics), then the recap lists each
[ FAILED ] file and the driver exits nonzero. Checks succeeded.
plus exit 0 is the machine-readable pass signal.
Run one file, or a glob over test paths:
./build/bin/driver testsuite/etc/config.test '-ftest:single/tests/efuns/dual_extension.lpc'
./build/bin/driver testsuite/etc/config.test '-ftest:efuns/dual*'
Without an argument the runner walks /single/tests/ recursively in
randomized order (run it 2–3× when touching the lexer/parser), so
tests must not depend on each other having run.
How the runner treats each directory
command/tests.lpc walks /single/tests/:
single/tests/**/*.lpc— regular tests: each file is loaded and itsdo_tests()is called; any uncaught error or failed assertion fails the suite..../fail/*.lpc— files that must fail to compile/load; the runner assertscatch(load_object(...))throws..../crasher/*.lpc— regression cases that only need to not crash the driver; errors are ignored.- In DEBUGMALLOC builds the runner calls
check_memory()after every file and fails on any leaked allocation, so tests must clean up (destruct clones, remove temp files).
Writing a test
Create single/tests/<area>/<name>.lpc with a do_tests() entry
point. etc/config.test auto-includes <globals.h> into every object,
which provides the assertion macros from include/tests.h:
void do_tests() {
ASSERT(intp(1)); // truthiness
ASSERT2(sizeof(x) == 3, "reason"); // with message
ASSERT_EQ("expected", actual); // equality with diff output
ASSERT_NE(a, b);
}
Failed assertions print file:line, Check failed (with expected/actual
and a trace for ASSERT_EQ/ASSERT_NE), are recorded, and the run
continues; any recorded failure makes the final recap fail the run with
a nonzero exit (that is how CI detects failure). Helper
fixtures that should not be executed as tests live outside
/single/tests/ — conventionally in /clone (e.g. inh0–inh2,
dual_*) or as #includes under /include. A fixture whose test
depends on it being unloaded (e.g. a fail/ test pinning that a
name does not resolve) must be private to that one test: the object
registry is extension-blind and survives across files, so any other
test loading the fixture first would change the outcome under the
randomized order.
Efun tests are named after the efun (single/tests/efuns/<efun>.lpc)
and should aim to cover every branch of the C++ implementation,
including error paths (catch(...)).
Formatting
The corpus is kept formatted with the grammar-driven LPC formatter in
tools/lpc-syntax/ (see its README). From the repo root:
testsuite/format.sh # format testsuite/**/*.lpc,*.c in place
testsuite/format.sh --check # exit 1 if anything is unformatted (CI)
Dependency-free (node ≥ 18 only). The script excludes the two
deliberately-malformed UTF-8 fixtures under
single/tests/compiler/fail/ (raw byte fixtures — see the exclusion
note in the script and AGENTS.md §7), and refuses to write any file
whose formatted output isn't token-sequence-equivalent to the input,
literal-content byte-identical, and idempotent. After a corpus reformat, run the driver suite before
committing.
Layout
| Path | What |
|---|---|
etc/config.test |
Driver config the suite boots with (mudlib dir, include dirs, global include, limits). |
single/master.lpc |
Master object: flag() test entry, compile_object() virtual-object hook (/test/virtual), get_include_path() cases, error handling. |
single/simul_efun.lpc |
Simul-efuns available everywhere in the suite. |
single/tests/ |
The test tree (efuns/, compiler/, operators/, applies/, std/, plus fail/ and crasher/ subdirs). |
command/ |
Interactive commands; tests.lpc is the suite runner, speed.lpc the benchmark entry (-fspeed). |
clone/, inherit/, std/, u/ |
Fixture objects, inheritance helpers, minimal std lib, user dirs. |
include/ |
Headers; tests.h (assertions), globals.h (auto-included). |
data/, log/ |
Runtime state and logs written by the suite. |