mirror of
https://github.com/fluffos/fluffos
synced 2026-08-12 18:26:06 -04:00
2036 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
dependabot[bot]
|
6cf257cedb
|
build(deps): bump the npm_and_yarn group across 1 directory with 2 updates (#1346)
Bumps the npm_and_yarn group with 2 updates in the /docs directory: [fast-uri](https://github.com/fastify/fast-uri) and [js-yaml](https://github.com/nodeca/js-yaml). Updates `fast-uri` from 3.1.4 to 3.1.5 - [Release notes](https://github.com/fastify/fast-uri/releases) - [Commits](https://github.com/fastify/fast-uri/compare/v3.1.4...v3.1.5) Updates `js-yaml` from 3.14.2 to 3.15.1 - [Changelog](https://github.com/nodeca/js-yaml/blob/3.15.1/CHANGELOG.md) - [Commits](https://github.com/nodeca/js-yaml/compare/3.14.2...3.15.1) --- updated-dependencies: - dependency-name: fast-uri dependency-version: 3.1.5 dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: js-yaml dependency-version: 3.15.1 dependency-type: indirect dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Yucong Sun
|
1099b48236
|
Speed up diagnostic rendering, and let the caller own the compile arena (#1343)
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>
|
||
|
Yucong Sun
|
1e4d4145fd
|
Make sizeof()/indexing O(1) for pure-ASCII strings (#1344)
* Skip ICU entirely for pure-ASCII grapheme cluster iteration
sizeof(str), s[i] and s[a..b] all walk Unicode grapheme cluster
boundaries through ICU's RuleBasedBreakIterator. That costs a
utext_openUTF8()/setText() per string plus ~50 instructions per
cluster, and it ran unconditionally -- including for the pure-ASCII
strings that make up almost all mudlib text. Profiling the LPC
testsuite put RuleBasedBreakIterator at 20.6% of the entire run, with
f_sizeof() alone at 32% inclusive: ~300,000 instructions per sizeof()
call, or roughly 53 instructions per byte of string.
For text with no high-bit byte every byte is its own grapheme cluster,
so the EGC index is the byte offset and the cluster count is the byte
length -- no ICU needed. EGCIterator::reset() now scans for that case
(a flat, auto-vectorizing accumulate) and, when it holds, skips the ICU
setup completely; EGCSmartIterator answers count/index/walk queries
arithmetically. ICU is still wired up lazily via ensure_icu(), reached
through operator->(), so the callers that drive the underlying
icu::BreakIterator directly keep working on any input.
The ASCII test deliberately also rejects CR. UAX #29 rule GB3 joins
CR x LF into a single cluster, so "\r\n" is two bytes but one cluster
and the offset==index identity breaks. That is not hypothetical: it
was caught by the equivalence test below, which failed on a CRLF
string before the exclusion was added. Rejecting every CR (rather than
only CR-before-LF) keeps detection a single flat scan; a lone CR just
falls back to ICU, which is still correct.
Correctness is pinned by tests that compare the fast path against ICU
driven over the same bytes on a separate iterator, rather than against
a reading of the spec: count, index_to_offset and post_index_to_offset
are swept across every index from past-the-start to past-the-end so the
DONE edges are covered, and CrLfIsTheOnlyAsciiJoin checks all 128x128
ASCII pairs against ICU to prove CR-LF is the only joining pair -- an
extra one would silently corrupt sizeof() on any string containing it.
Measured (RelWithDebInfo -O3, same machine, A/B against the same tree
with only these two headers reverted):
sizeof(16-byte string) 741 ns -> 72 ns 10.3x
sizeof(72KB string) 2,434,974 ns -> 1,502 ns 1621x
string index s[5] 435 ns -> 75 ns 5.8x
string range s[3..8] 648 ns -> 107 ns 6.1x
Non-ASCII text pays one extra linear scan before ICU runs and is not
measurably slower -- 1853 -> 1659 ns for a small non-ASCII string,
313,950 -> 303,209 ns for a 6400-cluster one. The scan costs ~0.02 ns
per byte against ICU's ~47 ns per byte, so the added pass is ~0.04% of
the work it guards.
Also adds sizeof/index/range benchmarks (ASCII and non-ASCII) to
/command/bench so the two paths can be tracked separately.
Validated: LPC testsuite 3x, 334/334 GTest, Debug+ASan/UBSan with the
DEBUGMALLOC ref-count checker 2x, and RelWithDebInfo+ASan/UBSan -- all
clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MN9sz4nvGgBZw9oZiei3XR
* Memoize the pure-ASCII string tag in the block header
The ASCII fast path still had to prove a string was ASCII before it could
use it, so every sizeof()/index rescanned the bytes -- making the very
common `for (i = 0; i < sizeof(s); i++)` O(n^2).
Cache the answer per string instead. Both counted-string headers gain a
tri-state `ascii` byte (unknown / yes / no), which lands in the two bytes
of padding that already sat between `ref` and the string data -- so
sizeof(block_t) is unchanged in every build, debug included, now pinned
by a static_assert on the field offsets as well as the size.
UNKNOWN is the default at every creation site and is load-bearing: a
string-creation path that forgets to set the tag degrades to scanning
(slow but correct) rather than asserting a wrong answer. Only three sites
produce counted strings -- alloc_new_shared_string(), int_new_string()
and extend_string() -- and extend_string() additionally has to reset the
tag, since it grows a refcount-1 MALLOC string in place and the caller
then writes new bytes into it. Nothing else mutates a counted string's
bytes: `s[i] = c` builds a fresh string via new_string() (verified), and
there are no direct writes through svalue_t::u.string.
f_sizeof() now answers from the byte length when the tag says ASCII,
without constructing an EGC iterator at all.
sizeof(16-byte string) 72 ns -> 38 ns
sizeof(72KB string) 1502 ns -> 41 ns (now independent of length)
Against the original ICU walk that is 2,434,974 ns -> 41 ns. Non-ASCII
strings are unaffected; they cache MSTR_ASCII_NO and keep using ICU.
The regression test drives the stale-cache cases specifically -- every
case queries sizeof() BEFORE mutating so the tag is already populated,
then mutates and queries again. It was verified to FAIL with
extend_string()'s invalidation removed, which required building the
string at runtime: a shared literal is reallocated fresh on append and
never reaches the in-place realloc path at all.
Validated: LPC testsuite 3x, 334/334 GTest, Debug+ASan/UBSan with the
DEBUGMALLOC ref-count checker, and RelWithDebInfo+ASan/UBSan -- all clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MN9sz4nvGgBZw9oZiei3XR
* Propagate the ASCII string tag across concatenation
Each concatenation produced a fresh, untagged string, so a loop of
`s += x` with an accompanying sizeof(s) re-derived the tag from scratch
for every intermediate -- O(n^2) in scanning on top of the O(n^2) the
copying already costs.
All three concat macros now carry the tag through:
ascii(a + b) == ascii(a) && ascii(b), computed from the operands before
the result is built. That identity is exact only because the predicate
excludes CR: with no CR on either side no CR-LF (one cluster, UAX #29
GB3) can form across the seam. It is exact in the NO direction too --
a non-ASCII operand's bytes survive into the result -- so neither case
ever rescans the joined string.
The third macro, SVALUE_STRING_ADD_LEFT (f_add's int/float/object +
string paths), needs the same treatment as the two string+string ones.
Note the operand there is a raw stack buffer rather than an svalue, so
it is queried with counted=false: it has no block header to read a
cached tag out of, and treating it as though it did would be a wild
read.
Pinned by regression tests that tag "a\r" and "\nb" separately and then
join them: a propagated "both ASCII" would report 4 clusters where the
CRLF makes 3. Covered on both the string+string path and the
<non-string>+string one, the latter with the left operand held in a
variable so the expression cannot be constant-folded before f_add runs.
Verified to FAIL (2 checks) with the propagation forced to "ASCII".
append+sizeof loop, 4000 iters 875,913 ns -> 548,144 ns
The remainder is the string building itself, which is inherently
quadratic in copying.
Validated: LPC testsuite 3x, 334/334 GTest, Debug+ASan/UBSan with the
DEBUGMALLOC ref-count checker, RelWithDebInfo+ASan/UBSan -- all clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MN9sz4nvGgBZw9oZiei3XR
* Keep the ASCII cursor in step with ICU's after count()
count() on the ICU path walks the break iterator to the end of the string
as a side effect; the ASCII path answered arithmetically and left its own
cursor wherever it happened to be. A count() followed by next() therefore
meant two different things: DONE on non-ASCII input, offset 1 on ASCII.
No caller can reach that today -- all three EGCSmartIterator::count()
callers use the value and nothing else (f_sizeof returns it, f_range and
f_extract_range compare against it and then re-seek with
index_to_offset(), which resets both cursors), and u8_egc_split() drives
the real ICU iterator through operator-> rather than the fast path. So
this is a latent trap rather than a live bug: the next person to write
count() + next() would get an answer that depends on whether the string
happened to be ASCII.
Set the ASCII cursor to the end, matching what the ICU walk leaves
behind, and pin the two paths against each other rather than against an
assumed answer -- the new test asserts next() reports DONE after count()
on both an ASCII and a non-ASCII string.
Validated: 335/335 GTest, LPC testsuite.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MN9sz4nvGgBZw9oZiei3XR
* Carry the ASCII string tag through substrings
f_range and f_extract_range built their result with new_string()/
string_copy(), which leaves the tag UNKNOWN -- so slicing an already-known
ASCII string threw the answer away and the next sizeof()/index on the
result rescanned it. A loop that walks a string by slices paid that on
every iteration.
Both sites already hold the source's EGCSmartIterator, which knows
is_ascii() from the scan it did to decide its own fast path, so the tag
costs nothing to carry.
The propagation is ONE WAY, and that is the whole subtlety:
YES propagates exactly. Every byte of a substring came from the source,
so an ASCII-and-CR-free source cannot produce a substring that is
anything else -- no high bit can appear, and no CR-LF can form from
bytes that contained no CR.
NO does NOT propagate, and asserting it would be wrong in the common
case: a substring of a non-ASCII string is very often pure ASCII
("<CJK>abc"[2..4] is "abc"). Those results stay UNKNOWN and are derived
lazily if anyone asks -- slower than a tag, but never a wrong answer.
That asymmetry is why this is MSTR_TAG_SUBSTRING rather than reusing
MSTR_TAG_JOIN, which propagates in both directions because concatenation
genuinely is exact in both.
The regression test pins the direction that would not be self-correcting.
Cluster counts for a substring are right whether or not the tag is
carried -- an untagged result just gets rescanned -- so the assertions
target a substring answering from a byte length it is not entitled to: a
non-ASCII slice of a non-ASCII source, and a slice spanning a CRLF.
Verified to FAIL (2 checks) with the propagation forced unconditional.
Validated on RelWithDebInfo and Debug+ASan/UBSan: 335/335 GTest, LPC
testsuite 3x for order randomization, all clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MN9sz4nvGgBZw9oZiei3XR
* Settle the ASCII tag at compile time, and stop dragging ICU into the scan
Two related fixes, the second found by the first.
1. store_prog_string() now settles "is this literal pure ASCII?" while the
compiler has the bytes, writing the answer into the shared string's header.
Every program string literal is interned through that one function and every
shared string has the header field already, so a literal arrives at runtime
pre-tagged and sizeof()/indexing on it never scans -- however hot the loop
around it. Deriving it lazily costs the same scan, just on the runtime path
after the mud is live.
2. u8_string_is_ascii_cached() was constructing an EGCIterator purely to run
that byte scan -- and EGCIterator's constructor acquires from the break
iterator pool, which builds THIRTY-TWO icu::RuleBasedBreakIterators on first
touch. A scan needing no Unicode data at all was dragging in the entire ICU
machinery it exists to avoid, which made the "skip ICU entirely for ASCII"
claim only half true: setText/utext_openUTF8 were skipped, the pool acquire
was not. It now calls EGCIterator::scan_is_ascii(), a static that touches no
iterator.
Measured on the xkx100 mudlib (fluffos/xkx100, 侠客行 -- a real UTF-8 Chinese
lib, so the ASCII path is the one that does NOT pay off there), 200 files
through `lpcc --batch`, 5 runs each:
with tagging .377 .431 .444 .471 .517 median .444
without .408 .412 .433 .454 .517 median .433
Overlapping distributions -- the compile-time scan is not measurable, so the
runtime saving is free.
Worth recording how the ICU problem surfaced, because the first harness was
wrong in an instructive way: compiling the same 200 files one-lpcc-per-file
took 66.5s vs 59.6s, which read as an 11% regression from tagging. It was
not. Batch mode does the same work in 0.47s, so 99.3% of that harness was
process startup, and the entire "regression" was ICU pool construction paid
once per process. A cold-start-dominated benchmark measured initialization
and reported it as throughput.
Validated on RelWithDebInfo and Debug+ASan/UBSan: 335/335 GTest, LPC
testsuite (2x randomized on RelWithDebInfo, plus ASan), all clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MN9sz4nvGgBZw9oZiei3XR
* Invalidate the ASCII tag on in-place string range assignment
assign_lvalue_range() and copy_lvalue_range() (the value-producing and
void-statement backends for s[a..b] = x) both have a same-byte-length
fast path that strncpy()s the replacement bytes directly into the
owner string's existing buffer, in place. Unlike extend_string() and
new_string() -- the only other two places that write into a counted
string's bytes -- neither resets MSTR_ASCII, so a string that was
already cached ASCII (e.g. via the concatenation-propagation added in
|
||
|
Yucong Sun
|
3e3418179b
|
Remove per-svalue heap allocation from the VM's hot paths (#1342)
Profiling an allocation-free numeric LPC workload with callgrind put ~20%
of retired instructions in operator new / free / strlen / memcpy /
std::string -- on code that allocates nothing.
md_record_ref_journal() is a DEBUGMALLOC_EXTENSIONS-only diagnostic whose
entire body is #ifdef'd out of production builds -- but it took its
description as std::string BY VALUE. Every call site therefore paid a
strlen + operator new + memcpy + free to hand a ~50-character
__CURRENT_FILE_LINE__ literal to a function that does nothing. Those call
sites are assign_svalue_no_free(), int_free_svalue(), int_ref_string() and
int_free_string(): once per refcounted svalue assignment and release, and
once per shared-string ref/unref, across the whole VM.
Four call sites in stralloc.cc also built their argument by concatenation
("ref_string: " + std::string(desc)), which allocates twice more even when
the callee is an empty inline -- so the non-DEBUGMALLOC declaration is now
a variadic macro that discards its arguments unevaluated rather than an
empty inline function. Every argument at every call site is a pure read
(PTR_TO_NODET is pointer arithmetic, MSTR_REF/->refs are loads), so
discarding them unevaluated changes nothing observable.
Two smaller changes:
eval_instruction() re-read CONFIG_INT(__RC_TRACE_CODE__) and
CONFIG_INT(__RC_TRACE_INSTR__) from the global config_int[] array on
every dispatched instruction. Both are boot-time-only config, but the
opcode bodies call out to code the compiler cannot see through, so it
must reload them each iteration; they are now hoisted into locals.
debug_level is deliberately left alone -- set_debug_level() can change
it from LPC inside this very loop.
free_svalue() now tests for an owning type inline and only calls
int_free_svalue() for strings, refcounted pointers and error handlers.
For every other type the out-of-line body just sets T_FREED, and outside
DEBUG builds nothing reads that bit back (assign_svalue_no_free clears
it, sprintf.cc masks it, the remaining readers are #ifdef DEBUG). DEBUG
builds still always call out, keeping double-free detection intact.
svalue.h records why the matching inline fast path for
assign_svalue_no_free() is NOT present: it works and is worth ~4%, but
inlining the whole-struct store lets -fsanitize=object-size see through to
array_t's legacy `svalue_t item[1]` flexible-array member and report every
item[i>0] write, which would fail the RelWithDebInfo+sanitizer CI job.
Real flexible array members would unblock it.
Measured with callgrind against a pristine build of the same tree, on a 4D
simplex-noise workload (jemalloc is statically linked and really executes
under callgrind -- verified by je_* symbols in the profile, so these are
real allocator costs, not valgrind's):
retired instructions 4,520,240,337 -> 2,968,862,756 -34%
per-tile wall clock ~203 us -> ~129 us 1.57x
The speedup is general to any allocation-light LPC, not specific to that
workload.
Validated: LPC testsuite 3x (randomized order), 326/326 GTest,
Debug+ASan/UBSan with the DEBUGMALLOC ref-count checker 2x, and
RelWithDebInfo+ASan/UBSan -- all clean, with the workload's output
bit-identical throughout.
Claude-Session: https://claude.ai/code/session_01MN9sz4nvGgBZw9oZiei3XR
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
dependabot[bot]
|
2024182d68
|
build(deps): bump the npm_and_yarn group across 1 directory with 3 updates (#1340)
Bumps the npm_and_yarn group with 3 updates in the /docs directory: [brace-expansion](https://github.com/juliangruber/brace-expansion), [postcss](https://github.com/postcss/postcss) and [undici](https://github.com/nodejs/undici). Updates `brace-expansion` from 1.1.15 to 1.1.18 - [Release notes](https://github.com/juliangruber/brace-expansion/releases) - [Commits](https://github.com/juliangruber/brace-expansion/compare/v1.1.15...v1.1.18) Updates `postcss` from 8.5.15 to 8.5.25 - [Release notes](https://github.com/postcss/postcss/releases) - [Changelog](https://github.com/postcss/postcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/postcss/postcss/compare/8.5.15...8.5.25) Updates `undici` from 7.28.0 to 7.29.0 - [Release notes](https://github.com/nodejs/undici/releases) - [Commits](https://github.com/nodejs/undici/compare/v7.28.0...v7.29.0) --- updated-dependencies: - dependency-name: brace-expansion dependency-version: 1.1.18 dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: postcss dependency-version: 8.5.25 dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: undici dependency-version: 7.29.0 dependency-type: indirect dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Yucong Sun
|
16c57df887
|
websocket: drain output before driver-initiated close (supersedes #1338) (#1339)
* websocket: drain output before driver-initiated close * websocket: harden close-after-flush against input, pmd drain, and double-close Review fixes for the close-after-flush drain (PR #1338), each verified to fail on the unfixed driver via new ws-smoke coverage: - LWS_CALLBACK_RECEIVE with a nulled pss->user now DISCARDS input instead of returning -1. During the drain window a hard-close on one client keystroke truncated the very flush the close was waiting for (both subprotocols; smoke now types mid-drain and still requires the full burst + close 1000). Deliberately NOT lws_rx_flow_control(): with the libevent event lib, flipping POLLIN on a live wsi stalls POLLOUT and the drain dies at the deadline (observed directly). - The buffer-empty close now also requires !lws_send_pipe_choked(). Choked-with-empty-buffer means permessage-deflate still holds the compressed tail of the final message (tx_draining_ext); entering the lws close states discards that drain ("defeat tx draining" in ops-ws.c), truncating the final message for every pmd client -- i.e. every real browser. The smoke client now negotiates pmd (persistent raw inflater, RSV1/FIN tracking) and sweeps incompressible burst sizes across a full 2048-byte window ring so at least one attempt always lands the final drain window over pm-deflate's 1024-byte tx buffer. - The close is now one-shot (clear close_after_flush before return -1). A second already-queued WRITEABLE callback returning -1 again re-enters the close path in LRS_RETURNED_CLOSE and degrades the clean handshake into an immediate close(fd); with autotuned kernel buffers still holding undelivered output, a late client byte then RSTs it all away. This was a ~30% flaky truncation on the reused ascii smoke connection (fresh sockets' smaller buffers masked it on telnet). - ws_ascii's u8_truncate hold-back is skipped once the user is gone, mirroring ws_telnet's gate: the rest of an incomplete codepoint can never arrive post-close, and holding it back busy-spins the writable callback until the deadline. Also widens the permanently-choked smoke check's margin (the 5s deadline starts when the driver processes the destruct, not when the test sends it) and documents the four close-after-flush invariants in src/www/AGENTS.md. Validation: ws-smoke 22/22 x13 runs (10 instrumented + 3 clean), 19/22 on the unfixed driver (all three new checks fail there); full LPC testsuite passes ("Checks succeeded.", exit 0) on RelWithDebInfo. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014TX65yYUCStrDqworuFvhh --------- Co-authored-by: devaidendale <xarcos@gmail.com> Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Yucong Sun
|
19ffcc7aea |
wasm/index.html: settle NOWRAP_COLS at 200
Mirrors fluffos/mudlibs' scripts/web_shell_override/index.html. |
||
|
Yucong Sun
|
241e45708f |
wasm/index.html: make NOWRAP_COLS a real no-wrap ceiling (2000, not 160)
Mirrors fluffos/mudlibs' scripts/web_shell_override/index.html. "No wrap" should mean no wrap, not "wrap at a bigger number" -- 2000 cols is far beyond anything this genre's 80/100-col-authored ASCII art will ever hit. |
||
|
Yucong Sun
|
bc30e2f08b |
wasm/index.html: bump NOWRAP_COLS 80 -> 160 for wide ASCII banners
Mirrors fluffos/mudlibs' scripts/web_shell_override/index.html (kept byte-identical). A survey of every lib's login banner in that repo found real grid-aligned ANSI art up to 154 columns wide -- 80 meant even the purpose-built "Wrap off" toggle still silently mangled them. 160 covers the full range found. |
||
|
Yucong Sun
|
6899f66b9e |
Replace mobile command-input textarea with contenteditable div
Mirrors fluffos/mudlibs' scripts/web_shell_override/index.html (kept byte-identical). The invisible <textarea id="cmdReal"> was still a real HTML form control, which is very likely what iOS keys the AutoFill accessory bar off regardless of visual opacity/positioning. Switched to a contenteditable="plaintext-only" <div> instead, since it isn't a form element at all -- no name, not part of a <form>, not submitted -- and should be outside what AutoFill heuristics inspect. Rewrote all cmdReal.value/.selectionStart/.selectionEnd/.disabled call sites to use new helper functions for the div-based API, added an explicit paste handler, and replaced the native 'select' event with a selectionchange listener. Verified locally via Playwright: typing, backspace, caret repositioning, history recall, paste flattening, masked display, and disabled-state toggling all work with zero JS console errors. |
||
|
Yucong Sun
|
74ca5d21df |
Fix mobile input bar rendering ~3 lines tall with the prompt/text misaligned
Mirrors fluffos/mudlibs' scripts/web_shell_override/index.html (kept byte-identical, that repo's copy takes priority at publish time but this one ships in the release zip and is what every other mudlib repo's own Pages workflow downloads and packs). #cmd uses white-space:pre (needed to preserve the editable content's exact spacing), which also preserves any literal whitespace that's a direct text- node child of #cmd -- and the pretty-printed HTML had a newline+indentation before the first <span> and another before <textarea>. Those became real rendered blank/indented lines, inflating the input bar to ~3 line-heights (~116px on an iPhone-width viewport, measured) instead of one, with the placeholder text appearing indented on a lower "line" than the "> " prompt beside it. Collapsed the div to a single line with no inter-element whitespace, and matched #inputbar .prompt's font-size to #cmd's mobile 16px (previously 14px) so the prompt and input text share a baseline. |
||
|
Yucong Sun
|
4b623a700b |
feat(wasm-www): replace the native command input with a custom widget + history
Reported live on iOS Chrome (same WebKit engine as Safari underneath,
Apple mandates it): the system AutoFill accessory bar (passwords/cards/
contacts icons above the keyboard) kept showing even on the very FIRST
prompt of a session, on a plain type="text" field that had never been
type="password" and already had autocomplete=off/autocapitalize=off. This
rules out the type="password"-sticks-to-the-element theory the previous
CSS-masking fix (
|
||
|
Yucong Sun
|
eaa1a6ee13 |
fix(wasm-www): mask password prompts with CSS, never input.type=password
Reported live: iOS Safari kept showing its AutoFill quick-suggestion bar (passwords/cards/contacts icons) above the keyboard even during ordinary command typing, well after any password prompt had passed. Root cause: the command input toggled between type="text" and type="password" on the SAME persistent <input> element (driven by the telnet SGA/ECHO negotiation -- see onEcho()). WebKit's AutoFill heuristic sticks to an element as a credential field for the rest of the page's life once it's ever been type="password", regardless of the type reverting afterward. This mud never wants the browser's credential manager involved at all (autocomplete=off already says so) -- the masking need is purely visual. Replaced the type toggle with a `.masked` CSS class applying -webkit-text-security:disc (WebKit/Blink; not Firefox, but that only affects whether typed characters are legible, never functionality, and iOS/Chrome cover the overwhelming majority of this page's mobile traffic). input.type now never changes from "text"; updated the one other reader of it (the Enter-key handler's local-echo suppression) to check the session's serverEchoes flag directly instead. Verified against a real packed site: type stays "text" through the .masked toggle in both directions, the -webkit-text-security:disc computed style applies/clears correctly, and a masked field visibly renders bullet dots for typed text. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VyQCUoTo1Z93Py9aVFHQi1 |
||
|
Yucong Sun
|
5cc89de8ac |
feat(wasm-www): mobile keyboard fixes + a soft-wrap toggle for the terminal
Fixes reported live on iOS Safari (screenshot showed the shift key lit by default): the command input had autocomplete=off/spellcheck=false but no autocapitalize, so mobile keyboards auto-capitalized the first letter of every typed command -- wrong for a MUD command line where commands and most IDs are lowercase. Added autocapitalize="off" and its natural iOS sibling autocorrect="off" (mid-typing "correction" of names/commands that aren't real English words is equally unwanted). Also added enterkeyhint="send" so mobile keyboards label/route the Enter key for what it actually does here (submit the line, see the keydown handler). Three further mobile findings from re-reading the whole page: - Safe-area insets (env(safe-area-inset-*)) were only ever applied to top/bottom, in the portrait-oriented @media(max-width:600px) block. viewport-fit=cover already opts into edge-to-edge rendering, and the JS-driven body.compact mode is explicitly built for landscape phones (its own comment says so) -- exactly where a notch/rounded corner sits on a SIDE, not the top. Added left/right safe-area padding to the base body rule instead of threading env() through every compact-mode element: body's top/bottom are already claimed by the visualViewport keyboard-avoidance sync, but left/right are untouched by that, so this covers every child (header/tabs/term/inputbar) in one place. - touch-action:manipulation was scoped to button/.tab/#cmd but not the terminal pane itself, so double-tapping game text (the natural mobile gesture to select a word) could trigger native double-tap-zoom instead of/alongside xterm's own selection. Added #term .pane to that rule. - New: a Wrap toggle button in the header. xterm has no native "don't wrap" mode -- it always wraps at `cols`, and fit() always shrinks cols to match the pane's pixel width, which mangles this whole genre's ASCII-art login banners (authored for the traditional 80-col MUD client) on any phone. Wrap-off pins cols at 80 and lets the pane scroll horizontally instead (new .pane.nowrap CSS: let .xterm size to its own now-wider content -- xterm.js sets the .xterm-screen canvas layer's pixel size from cols/rows directly -- and scroll the pane under it). Centralized every raw fit() call through a new mode-aware refit(s) helper; char mode (full-screen TUIs that need real NAWS geometry) is deliberately exempted in both call sites that touch it. Persisted via localStorage, defaults to on (matches all prior behavior). Verified end to end against a real packed site (local WASM build + zzfy): wrap-on gives cols=40 on a 390px-wide viewport as before; toggling off gives cols=80, a "nowrap" class with real overflow-x (scrollWidth 720 vs clientWidth 378), and the ASCII banner renders at full width and scrolls correctly; toggling back and reloading both restore state correctly with zero console errors either way. Mirrored byte-for-byte into fluffos/mudlibs' scripts/web_shell_override/ index.html per that repo's existing convention (its own copy takes priority over this release zip's copy at publish time). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VyQCUoTo1Z93Py9aVFHQi1 |
||
|
gesslar
|
e059c891cb
|
docs: document the real rules for throw(), add a structured catch() example (#1335)
* docs: document the real rules for throw(), add a structured catch() example throw.md called throw() "forces an error to occur in an object" and treated the catch() requirement as a style suggestion. Neither matches what f_throw()/throw_error() do. Rewrite the page around the rules the driver actually implements, each verified against a running driver: - An uncaught throw() raises *Throw with no catch. and discards the value. This is a requirement, not advice. - Any type round-trips through catch() verbatim, not just strings. - throw(0) is indistinguishable from success, since catch() returns 0 for "no error". This was documented only on catch.md. - A thrown value skips the error machinery entirely: no traceback, no debug log, no error_handler() apply, and no leading '*'. That '*' is the discriminator handling code keys on, and it appeared on neither page. - Only the innermost catch() sees it; rethrow to propagate further. - It crosses ordinary calls (call_other, inherited functions, evaluate(), filter/sort_array callbacks, a create() running under load_object) but cannot escape a call the driver starts itself (call_out, input_to, driver applies), which each begin a fresh chain with no catch above them. The old example's `return;` after throw() was unreachable, and it concatenated a caught driver error into a new message, burying the '*' mid-string so callers testing err[0] would stop recognising it. Both fixed. catch.md gains a third example that throws a class: its existing two both use string errors and imply catch() only ever yields text. Written with dot accessors and named-argument new(), matching the ordering in lpc/types/classes.md. Chinese translations updated to match. The zh-CN catch.md description said throw() returns a non-zero value, narrower than "any value except 0" and the exact claim the new example rests on; corrected. Every example block was extracted from the markdown and compiled by the driver, Chinese comments and strings included. Both locales build clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014JuB4acdTho1rmAL7PwvS1 * docs: fix the rethrow example forging a driver-error '*' Review feedback on the rethrow example was right, and the bug is worse than a style nit: the example contradicted the rule its own page teaches. It stripped a leading '*' from the caught value and then unconditionally put one back, so anything that was not a driver error came back disguised as one. Verified against the driver: mudlib string -> "*move_or_fail(): took too long\n" (forged '*') structured -> "*Bad type argument to +. Had string and array\n" The second case is worse still: concatenating a string onto a thrown array destroys the original failure and replaces it with a type error raised inside the handler. Re-add the '*' only when it was there to begin with, add context without it for a non-driver string, and pass a non-string value through untouched. Now: driver error -> "*move_or_fail(): bad thing\n" mudlib string -> "move_or_fail(): took too long\n" structured -> ({ "insufficient_funds", 7 }) Also quote the uncaught-throw error as the full string it actually is, "*Throw with no catch.\n", matching the two example strings shown a few lines above that spell out their trailing newline. And drop the "any value, except 0" phrasing on catch.md, which reads as though throwing 0 were rejected. It is not rejected, merely undetectable, since catch() already returns 0 for "no error". The zh-CN page inherited the same ambiguity from the English; both now say so explicitly. All four example blocks recompiled from the markdown; both locales build clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014JuB4acdTho1rmAL7PwvS1 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Yucong Sun
|
8822499822
|
compiler: coerce return <expr>; to the function's declared return type (#1334)
Issue #1331: a function declared `int` whose return expression actually evaluates to a float (e.g. `return sqrt(x);`) genuinely handed back a T_REAL svalue at runtime. compatible_types() intentionally treats int and float as compatible (so no compile error), and until now nothing enforced the declared type at the return statement itself -- unlike '='/op= assignment, which rule_expr_assign() already coerces to a declared-type lvalue (#1303). A caller doing `int_var += that_call()` then hit F_ADD_EQ's untyped-lvalue promotion path meant only for mixed/mapping slots, silently promoting a genuinely int-declared variable to float -- exactly the reporter's own example (`int heat_close(...) { return (range - sqrt(dist_sq)) * ...; }` then `int_var += heat_close(...)`), cascading into switch()/array-index/% errors mudlib-wide. rule_return_expr() (grammar_rules_loops.cc) now applies the same int<->float promotion rule_expr_assign() already applies to an assignment RHS, scoped to scalar int/float only (exact-equality checks, matching the op= precedent, correctly exclude array return types like `int *`) and never touching untyped (mixed) returns. One nuance the compound-assignment precedent doesn't have to deal with: CREATE_NUMBER() gives a literal `0` node type TYPE_ANY, not TYPE_NUMBER (0 doubles as LPC's untyped "nil" across every type), so `float f() { return 0; }` needs the same `|| kind == NODE_NUMBER` fallback do_promotions() already uses (compiler.cc) -- otherwise it falls through unpromoted to the F_RETURN_ZERO fast path, which always returns an int-typed 0 regardless of the function's declared type. testsuite/std/percent.lpc's percent()/percent_of() were declared `int` but are genuinely polymorphic by design (float args in, float result out; int args in, truncated int result out) -- an intentional escape from their declared type that this fix would otherwise silently truncate. Redeclared `mixed` to accurately reflect that contract; this changes nothing for any caller; the compiler applies no coercion at all to a mixed-declared function's return in the first place, and this already-passing testsuite file is proof such a pattern needs to be called out by an honest `mixed` return type, not "int" while quietly depending on the driver not really enforcing it. New regression test testsuite/single/tests/operators/return_type_coercion.lpc, verified to fail (9 checks) on the unfixed binary via a worktree with just this commit reverted, and pass on the fix. Full LPC suite x3 (randomized order) + 373 GTests clean on RelWithDebInfo; full suite + GTests clean on Debug+ASan (one unrelated pre-existing leak surfaced in RcTest, in read_config()/rc.cc, unconnected to this change -- see PR description). Co-authored-by: Claude <noreply@anthropic.com> |
||
|
gesslar
|
5f7fba054f
|
docs: document the third argument (flag) of function_exists() (#1326)
The EN page only showed the two-argument form. Document the optional third argument, which admits protected and private functions when nonzero (matching the implementation in interpret.cc and the existing zh-CN page), and fix the SYNOPSYS -> SYNOPSIS heading typo. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
gesslar
|
7bbedece7c
|
docs: document that shuffle() reorders its argument array in place (#1332)
The shuffle.md page never mentioned the most important behavior: the array passed in is itself mutated (in-place Fisher-Yates in f_shuffle), and the "return value" is that same array left on the stack, not a shuffled copy. The zh-CN translation already warned about this; the English page did not. Rewrite the page with the mutation front and center, a RETURN VALUE section, an example showing the shared-array effect plus the shuffle(a[0..]) copy idiom, and a SEE ALSO in the standard name(3) format. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Yucong Sun
|
b0d3d2977b
|
tests: stress remove_interactive() net_dead teardown shapes (#1327) (#1330)
* tests: stress remove_interactive() net_dead teardown shapes
Five GTest stress loops driving remove_interactive() against fabricated
transport-less interactives (user_add() + a counted connection ref, all
transport/telnet/translator teardown branches null-guarded), asserting
exact refcount deltas and connection-state cleanup each iteration:
- NetDeadExecGhostStress: net_dead() exec()s the connection into a fresh
ghost body 150x -- the teardown must release the ghost's interactive
state, not decrement the old body again (the issue #1327 over-decrement)
nor leave ghost->interactive pointing at the freed interactive_t.
- NetDeadPlainStress: baseline net_dead(), exactly one ref released.
- NetDeadSelfDestructStress: net_dead() destructs the body; the recursive
remove_interactive() from destruct_object() is absorbed by CLOSING and
exactly one interactive ref is still released.
- NetDeadExecThenDestructStress: exec() to a ghost then destruct the old
body; teardown follows the connection to the ghost.
- RemoveInteractiveDestructedStress: the dested=1 path never runs
net_dead() and always operates on the original body.
Each loop periodically runs remove_destructed_objects() so the rewritten
next_destruct queue gets churned under the same load.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rw2P1MKGnjV8gn6CDuuyzQ
* fix: dealloc_object() leaked variable contents on early ref-0 drop; CI leak in net_dead stress tests
The clang+sanitizer RelWithDebInfo CI job failed the two exec-based
net_dead stress tests with LeakSanitizer reports: every ghost clone's
object_t/name/variable block leaked (450 allocations across the loops).
Root cause is a real driver leak the tests exposed, not test bookkeeping:
dealloc_object() frees the variable BLOCK but not the svalues inside it.
On the normal teardown path destruct2() zeroes the contents first, but an
object whose ref count drops to 0 while still on the destruct queue --
before its remove_destructed_objects() sweep; reachable since
|
||
|
Claude
|
032c00b798 |
testsuite: format two fixtures that landed unformatted in b1fb96f
testsuite/format.sh --check flagged single/tests/compiler/embedded_nul_hang.lpc and anon_func_bad_type_scope_leak.lpc (both from the AFL++ fuzzing-fixes commit). Whitespace-only; both tests re-verified passing on the driver. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rw2P1MKGnjV8gn6CDuuyzQ |
||
|
Claude
|
948b49ed7f |
Fix object-refcount over-decrement bugs behind #1327's corruption crashes
Issue #1327 reported two crashes on the v2026.07.17 build, both downstream
signatures of an object-ref over-decrement: a live object drained to ref 0
(dealloc_object "ref count 0, but not destructed" during a soul daemon's
global assignment) and an already-deallocated object freed again from a
mapping value (free_prog segfault on a dangling ob->prog). The primary
suspected origin -- the default-arguments closure-fill push_svalue() stack
corruption, present only in that build window (86d13cd..d917178/2d317e4)
and directly evidenced by #1295's garbage T_OBJECT stack slot -- is already
fixed on master. This commit fixes every further defect of the same class
found by auditing the implicated paths (command processing, applies,
mapping machinery, object lifecycle):
- remove_interactive(): when net_dead() exec()s the connection into a
different body (the classic linkdead-ghost idiom), the teardown released
the OLD body's interactive ref a second time -- draining a live object to
ref 0, the exact #1327 fatal -- and freed the interactive_t the new body
still pointed at. Re-resolve the owner from ip->ob after the apply.
- Destruct queue: obj_list_destruct rode on next_all/prev_all, which DEBUG
builds immediately reuse for the obj_list_dangling leak list -- once a
sweep left a still-referenced survivor, the next sweep's next_all walk
strayed into the dangling chain and destruct2()'d (= ref-decremented)
survivors a second time; non-DEBUG builds kept stale queue links that a
later dealloc_object() wrote through (gap in
|
||
|
gesslar
|
4d4ad6d9ce
|
docs: document the third argument (scroll_lines) of ed_start() (#1328)
The doc page only showed the two-argument form, but the efun spec (core.spec) and f_ed_start() accept an optional third argument that sets the number of lines used by the editor's scrolling commands (default 20). Also document the two-argument disambiguation: a second argument of 1 is treated as 'restricted', anything else as 'scroll_lines'. The zh-CN page is a verbatim copy of the EN page, so it receives the same update. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
dependabot[bot]
|
6436410c83
|
build(deps): bump the npm_and_yarn group across 1 directory with 4 updates (#1321)
Bumps the npm_and_yarn group with 4 updates in the /docs directory: [body-parser](https://github.com/expressjs/body-parser), [fast-uri](https://github.com/fastify/fast-uri), [shell-quote](https://github.com/ljharb/shell-quote) and [webpack-dev-server](https://github.com/webpack/webpack-dev-server). Updates `body-parser` from 1.20.5 to 1.20.6 - [Release notes](https://github.com/expressjs/body-parser/releases) - [Changelog](https://github.com/expressjs/body-parser/blob/master/HISTORY.md) - [Commits](https://github.com/expressjs/body-parser/compare/1.20.5...1.20.6) Updates `fast-uri` from 3.1.2 to 3.1.4 - [Release notes](https://github.com/fastify/fast-uri/releases) - [Commits](https://github.com/fastify/fast-uri/compare/v3.1.2...v3.1.4) Updates `shell-quote` from 1.8.4 to 1.10.0 - [Changelog](https://github.com/ljharb/shell-quote/blob/main/CHANGELOG.md) - [Commits](https://github.com/ljharb/shell-quote/compare/v1.8.4...v1.10.0) Updates `webpack-dev-server` from 5.2.5 to 5.2.6 - [Release notes](https://github.com/webpack/webpack-dev-server/releases) - [Changelog](https://github.com/webpack/webpack-dev-server/blob/v5.2.6/CHANGELOG.md) - [Commits](https://github.com/webpack/webpack-dev-server/compare/v5.2.5...v5.2.6) --- updated-dependencies: - dependency-name: body-parser dependency-version: 1.20.6 dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: fast-uri dependency-version: 3.1.4 dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: shell-quote dependency-version: 1.10.0 dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: webpack-dev-server dependency-version: 5.2.6 dependency-type: indirect dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
719f3e357b
|
build(deps): bump svgo (#1315)
Bumps the npm_and_yarn group with 1 update in the /docs directory: [svgo](https://github.com/svg/svgo). Updates `svgo` from 3.3.3 to 3.3.4 - [Release notes](https://github.com/svg/svgo/releases) - [Commits](https://github.com/svg/svgo/compare/v3.3.3...v3.3.4) --- updated-dependencies: - dependency-name: svgo dependency-version: 3.3.4 dependency-type: indirect dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Yucong Sun
|
b1fb96f381
|
Add AFL++ fuzzing harnesses; fix 5 pre-existing memory-safety/DoS bugs found by them, plus a Coverity-flagged ownership-transfer hardening (#1317)
* Add opt-in AFL++ harness for restore_variable()/save_variable()
Investigating a Discord report of printf("%O") returning corrupted data
after a driver update. Manual round-trip testing (lpcshell, ASan+UBSan,
the LPC testsuite) found no reproduction, so add a fuzzing harness to dig
further: src/main_fuzz_restore.cc, gated behind a new BUILD_FUZZERS CMake
option (default OFF, no effect on normal builds).
The harness boots the VM once and uses AFL's deferred fork server so each
test case forks cheaply post-boot. It feeds a SEQUENCE of restore_variable()
calls per exec (input split on a delimiter), not just one -- the known bug
class in this code path (AGENTS.md section 13 point 4) is a restore's
error() path leaving file-scope scratch state (save_svalue_depth, sizes[])
dirty for the NEXT restore in the same process, which a single-call-per-exec
harness structurally cannot find.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Fix two memory-safety bugs in restore_variable() found by AFL fuzzing
Both are pre-existing (confirmed present at
|
||
|
Yucong Sun
|
5db89572a2
|
Feat/wasm mobile ux (#1318)
* feat(wasm-www): mobile-responsive layout + keyboard-safe input bar The web terminal was desktop-only in practice: small fixed text sizes and cramped tap targets at phone widths, and -- the more disruptive bug -- the on-screen keyboard could cover #inputbar, since iOS Safari (and some Android browsers) only shrink window.visualViewport when the keyboard opens, never the layout viewport that <body>'s flex column and its height: 100% are sized against. * Keyboard-safe input bar: <body> is now position: fixed and a visualViewport 'resize'/'scroll' listener keeps its top/height synced to the *visual* viewport on every change. #term is the flex:1 child, so it shrinks to make room and #inputbar (the last child) always lands at the bottom of what's actually visible, never behind the keyboard. Falls back to input.scrollIntoView() on focus for browsers without window.visualViewport. Same listener also refits the visible xterm on window resize/orientationchange (with a second delayed pass post-rotation, since some mobile browsers report the new geometry late). * Mobile layout: a max-width: 600px query enlarges tab/input padding, truncates the status line instead of wrapping, and adds safe-area insets for notches/home-indicators; a pointer: coarse query (not width-gated, so it also covers wide touch devices) sets #cmd's font-size to >=16px -- the standard fix for iOS's auto-zoom-on-focus -- and gives tabs/reload/close real touch targets. xterm itself renders to canvas, so CSS can't resize its text: a matching JS breakpoint (fontSizeForViewport()) applies a modest 14->15px bump per session at phone widths and live-updates+refits on breakpoint crossing, without shrinking columns so far that game output loses its formatting. * Misc: overscroll-behavior: none on the page (no rubber-banding / pull-to-refresh fighting the fixed layout), overscroll-behavior: contain on the logs pane and xterm's own viewport (scroll it without chaining to the page), touch-action: manipulation on buttons/tabs/ input (removes the ~300ms tap delay and stray double-tap zoom). Desktop is unaffected: position: fixed + inset: 0 fills the viewport exactly like the previous height: 100% did absent a visualViewport (which desktop browsers largely don't fire keyboard-driven resizes through anyway), the two new media queries are no-ops above 600px CSS px / on mice, and fontSizeForViewport() returns the previous 14px outside the phone breakpoint. Verified headlessly: all four inline scripts still pass node --check; tools/wasm/pack-mudlib.sh packs a fresh bundle without error; the mudlib repo's scripts/pack_lib_for_web.sh path/anchor patcher (vendor/ telnet/fluffos script tags, locateFile, title/h1) still finds every anchor exactly once and produces a working patched per-lib page. Not verified: no real phone or headless-browser/screenshot tool was available in this environment, so the ~375px/~400px layout and the keyboard-avoidance behavior are reasoned from the CSS/JS logic above, not visually confirmed on-device. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VyQCUoTo1Z93Py9aVFHQi1 * fix(wasm-www): mobile issues found in Playwright visual verification Verified the prior (unverified) mobile-UX commit against a real full boot (packed lib + fluffos.wasm) in Chromium at 390x844/844x390 with touch emulation, and fixed what the screenshots showed: - The permanent first Game tab showed its supposedly-hidden close button on touch devices: the (pointer: coarse) display:inline-flex on .reload/.close overrides the UA's [hidden]{display:none}. Restore it with an explicit [hidden] rule. - A tap aimed at a tab (to switch) landed on the ↻ icon and silently dropped+redialed the session -- the icons sit a few px from the tab's natural tap point. On coarse pointers, ↻/× on an inactive tab now only activate it; the destructive action takes a deliberate second tap once the tab is frontmost. Mouse behavior unchanged. - The input placeholder clipped mid-word at phone widths; use a short hint under the 480px breakpoint (live-updated on rotation). - xterm.css hardcodes background:#000 on .xterm-viewport, which showed as a black strip in the sub-row remainder above #inputbar; repaint it in the page background. Verified via visualViewport-shrink simulation that #inputbar stays flush with the top of the on-screen keyboard in portrait and landscape. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VyQCUoTo1Z93Py9aVFHQi1 * fixup(wasm-www): remove dead connectSession truthiness check The mqNarrow 'change' handler's placeholder update was gated on `if (connectSession)` -- connectSession is a function reference (always truthy), not a connection-state flag, so the guard was a no-op; the statement already ran unconditionally in practice. Drop the misleading dead conditional; behavior is unchanged. * feat(wasm-www): boot progress bar over the blank terminal while loading A big packed mudlib (unknownlib20150716 packs to ~25MB of mudlib.data) gave the web client a long, completely silent boot on slow connections: a blank terminal, a disabled input saying "loading…", and only the tiny header status text -- indistinguishable from a hung page, and reported as exactly that ("can't boot on mobile, hangs with blank screen"). Add a #boot overlay centered in the (otherwise empty) terminal area: a progress bar plus a one-line phase text, mirrored into the header status. Wired to the progress signals the loaded code actually emits (verified against the generated glue + file_packager output in use): - file_packager's fetchRemotePackage() (mudlib.js) calls Module.setStatus("Downloading data... (loaded/total)") per received fetch chunk with real byte counts -- the boot's only determinate signal. Parsed into a real percentage bar with an "X / Y MB" label. The download starts at mudlib.js script-parse time, so the hooks are installed on Module in an inline script ahead of it. - Module.monitorRunDependencies(n) brackets the synchronous unpack of the data blob into the in-memory FS ("preparing game files…"). - the glue's run() calls setStatus("Running...") (never a percentage) right before initRuntime ("starting driver…"). - the wasm binary itself loads via fetch+instantiateStreaming with no progress callback; that stretch and the boot tail show an indeterminate sliding bar instead. The slide animates transform only, so the compositor keeps it moving through the long synchronous main-thread stretches (FS unpack, LPC preload compile). The overlay fades out once the first session is dialed and interactive (and on boot failure/abort/load error, where the error UI takes over); a done-latch keeps the glue's trailing setStatus('') from resurrecting stale text. Header status, connect flow, embedding API and the mobile/tab logic are untouched. Verified with Playwright/Chromium against real packed bundles served over HTTP (unknownlib20150716 25MB, xo 3.7MB), desktop 1280x800 and mobile 390x844 (touch, DPR3), fast and CDP-throttled (2MB/s, 150ms RTT): the bar climbs smoothly 0->100% with live MB counts during the ~16s throttled download, phase text follows, and the overlay is gone the moment the login banner appears; on a sub-second load it shows briefly and fades without leaving stale UI. mudlib repo's pack_lib_for_web.sh anchor patcher still finds every anchor once. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VyQCUoTo1Z93Py9aVFHQi1 * feat(wasm-www): compact chrome when the visible viewport is short On a phone the header (title + status), tab bar and input bar stack up to ~128 CSS px of chrome; in landscape (~412px tall) that plus the on-screen keyboard left about 3 terminal rows visible -- the keyboard "takes too much space in both portrait and landscape". Add a body.compact state, toggled by the same visualViewport plumbing that already drives the keyboard avoidance (the real trigger is "how much height is left right now", which an orientation media query cannot see -- a keyboard open in portrait is the same problem as landscape). Below 620px of visible height: - body switches from a flex column to a grid whose first row holds BOTH the header and the tab bar (title + status shrink and ellipsize; the title text drops entirely when the screen is also narrow, i.e. keyboard-open portrait); - the tab bar and input bar shed their touch-generous padding, down to but not below ~36-37px tap targets. Measured on a Pixel-7-sized viewport (Chromium, simulated keyboard via a controlled visualViewport): chrome px #term px xterm rows portrait (915px) 128.5 -> 128.5 786 -> 786 45 -> 45 (unchanged) portrait + keyboard 128.5 -> 75.2 466 -> 520 26 -> 30 landscape (412px) 128.0 -> 75.2 284 -> 337 17 -> 20 landscape + keyboard 128.0 -> 75.2 74 -> 127 3 -> 7 Tabs (+/reload/close), the Logs pane, the boot progress overlay and the keyboard round-trip (compact on open, back to the two-row layout on close) all verified in the compact state with Playwright. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VyQCUoTo1Z93Py9aVFHQi1 * feat(wasm-www): fullscreen toggle in the header After compact chrome squeezed the page's own bars, the biggest remaining space hog on a phone is the browser's chrome (address bar & co.) -- the Fullscreen API hands that back on request. A small corner-bracket icon button at the right edge of the header toggles document-element fullscreen; the icon swaps (corners-out / corners-in) off the fullscreenchange EVENT rather than a local flag, so Esc / back-swipe / system-UI exits can never desync it. The swap is driven by a .fs-active class because SVG elements do not reflect the .hidden IDL property -- setting el.hidden on them silently no-ops. Support detection: the button stays hidden unless requestFullscreen/webkitRequestFullscreen actually exists and fullscreenEnabled is not false (iPhone Safari has no element fullscreen at all; embedded webviews often block it). webkit-prefixed fallbacks cover older iPadOS Safari, the one mobile platform whose support was prefix-only. A rejected request (permissions policy, user-gesture rules) lands in the Logs pane instead of the crash modal. Fullscreen transitions rerun the existing viewport plumbing (updateCompact + syncVisualViewport + debounced xterm refit) with the same late second pass as orientationchange, so compact chrome and NAWS geometry settle correctly after the toggle. Verified with Playwright + headless Chromium against a packed lib at desktop (1280x800), portrait-phone (390x844 touch) and landscape-phone compact (844x390) viewports: headless Chromium genuinely enters fullscreen (document.fullscreenElement set), icon state tracks both button-driven and browser-driven exits, layout/cols/rows/compact state restore exactly, an unsupported-API context keeps the button hidden, and a rejecting requestFullscreen logs without opening the error modal. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VyQCUoTo1Z93Py9aVFHQi1 --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
Yucong Sun
|
691a74bd85 |
feat(wasm-www): multi-connection Game tabs + a driver Logs tab in the web terminal
The wasm web terminal mixed driver stdout/stderr (boot noise, compile warnings, debug_message) into the single game view as dim lines, and supported exactly one connection. Rework the page into a tab bar: * Game tabs -- each tab is a separate connection into the same in-page driver (own xterm + fit addon, own TelnetClient, own conn id and the two bridge-decoupling queues), like several telnet clients dialed into one mud. "+" dials another connection; every tab has a reload icon that redials just that connection (fluffos_disconnect + fluffos_connect with a fresh client-side TelnetClient so telnet negotiation restarts cleanly); tabs after the first also have a close button (the first tab is permanent). The shared input bar follows the active tab (echo masking, char-mode hiding, unsent draft preserved per tab). No page-level reset control: reloading the page already reboots the driver from the packed image. * Logs tab -- a plain scrollback pane (no second xterm, still dependency-free) fed by Module.print/printErr. Lines are timestamped, capped at 5000, auto-scrolled unless the user scrolled up; stderr renders red so compile errors read apart from warnings. * Unread badges -- output landing on any inactive tab (game or logs) lights a count badge on its tab, cleared on activation; no focus steal, no tab switch. Per-session queues keep the synchronous-bridge rules intact (sends flush from a 0-timeout, inbound chunks drain non-reentrantly); char mode, ECHO masking, NAWS refit (fit() is a NaN-guarded no-op while a pane is hidden; activation refits), boot flow, jsdemo handlers and the embedding API (createFluffOS / fluffos_* / M.fluffos.handlers) are unchanged. tools/wasm/pack-mudlib.sh's copied file set and downstream page-patch anchors (vendor/telnet/fluffos script tags, locateFile, title/h1) still occur exactly once. Verified headlessly: all four inline scripts pass node --check; the driver boots under node with print/printErr routed exactly like the page (compile warnings on stdout, fd-2 writes flagged as stderr), two simultaneous connections receive independently routed output, and the disconnect+reconnect ccall sequence yields a fresh working session; pack-mudlib.sh packs the page into a dist bundle byte-identically. Manual browser test: serve a packed dist (python3 -m http.server), open the page -- boot noise lands in Logs with a badge while Game 1 stays clean; "+" a second tab, log in on both, check unread badges and the per-tab reload icon, then click Logs (badge clears, stderr red). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VyQCUoTo1Z93Py9aVFHQi1 |
||
|
Yucong Sun
|
0096a6e70c |
feat(wasm): enable the pcre package on the WebAssembly target
Cross-build classic libpcre 8.45 (the library src/packages/pcre links) with emconfigure/emmake into the wasm-deps prefix and stop forcing PACKAGE_PCRE off under EMSCRIPTEN, so all pcre_* efuns exist in the browser driver. Mudlibs whose boot-critical code (e.g. simul_efuns doing ANSI handling with pcre_replace) calls these no longer fail to boot on wasm. - tools/wasm/build-deps.sh: new PCRE section following the ICU pattern (idempotent re-run guard extended, PCRE_VER override). Static, UTF-8 + Unicode properties on (the driver compiles every pattern with PCRE_UTF8), JIT off (no executable pages in wasm), default chartables (no host-run dftables needed). - src/CMakeLists.txt: drop the forced PACKAGE_PCRE OFF; FindPCRE locates the static lib through CMAKE_FIND_ROOT_PATH -> wasm-deps. - .github/actions/build-wasm: pcre-ver input feeds the deps cache key (old ICU-only caches no longer match) and PCRE_VER reaches build-deps.sh. - docs/build-wasm.md, src/wasm/README.md: pcre moved out of the absent-package lists; deps/build notes updated. Verified: wasm LPC testsuite passes with the 9 pcre efun tests now active (621 OK / 0 failed, same as native); native rebuild + testsuite unaffected. fluffos.wasm grows 3,390,869 -> 3,604,494 bytes (+209KB raw, ~0.84MB brotli over the wire). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VyQCUoTo1Z93Py9aVFHQi1 |
||
|
Yucong Sun
|
3bfb7483fe |
fix(lpc-syntax): refuse files with broken quoting instead of shredding them
A file with a PRE-EXISTING stray unbalanced '"' -- 1990s mudlib archives
ship many that never compiled -- was silently rewritten into garbage:
past the stray quote the tokenizer's string/code sense inverts, so real
string content lexes as code tokens (every CJK character a separate
'unknown' token the formatter space-separates, '\n' escapes torn into
'\ n') and everything the formatter renders from those tokens shreds.
200+ real files across a 91-mudlib corpus scan were corrupted this way.
Driver-lexer ground truth (src/compiler/internal/lexer.l): hitting EOF
inside a string, template literal, block comment, char literal, or text
block is a hard lexerror ("End of file in string" / "End of file in a
comment" / "End of file in template literal" / lpc_lex_char_error /
heredoc-terminator error). Such a file has NO well-defined token
stream, so per the documented safety contract (docs/lpc/formatter.md)
the only correct behavior is refusal: report, leave byte-identical,
exit nonzero. No speculative "fixed" formatting is invented.
Root cause of the silence: the corpus safety net (token-sequence
equivalence + literal byte-identity + idempotency) re-tokenizes the
output with the same tokenizer, and input and output mis-lex
IDENTICALLY -- the same self-check blind spot as the "(::" (
|
||
|
Yucong Sun
|
d1a491c1d3 |
fix(lpc-syntax): tokenizer mis-lexes unescaped quote-char literal '''
The unescaped quote character literal ''' -- an old MudOS-ism common
in 1990s Chinese mudlibs (`case ''':` for the say-shortcut quote key)
-- was mis-tokenized, and on the real-world shape
case ''': //'
cmd = "say " + cmd[1..];
(the trailing //' comment's quote balances editors' highlighting)
the formatter merged the case label, the comment, AND the next line's
statement into one output line:
case '' ': //' cmd = "say " + cmd[1..];
The char literal came out torn in two (`'' '`), and the re-flowed `//`
comment now swallowed the following assignment -- on recompile the
driver comments out the statement, silently deleting it.
Driver-lexer ground truth (src/compiler/internal/lexer.l): a char
literal's body is EXACTLY ONE unit -- <SC_CHAR_BODY>[^\\] matches any
single raw byte, explicitly "including a literal quote" per the rule's
own comment, or one escape sequence -- and then <SC_CHAR_CLOSE>
requires the closing quote. So ''' is a VALID literal (body = the
quote char, value 39): the mudlib containing this file compiled and ran
natively before formatting. If the closing quote is missing, the driver
reports an error and pushes the offending byte back (LPC_YYLESS(0)) for
the next scan.
Root cause: tokenizer.mjs's skipCharSpan() (and a duplicated inline
scan in the main loop) scanned "to the next quote" string-style. On
''' that terminates immediately, emitting an empty '' token, and the
leftover third quote then opens a bogus literal that runs to the next
quote ANYWHERE on the line -- here the one inside the trailing //'
comment -- producing char tokens `''` and `':<tab>//'`. The formatter
then laid out those tokens tight on one line, and everything after the
embedded // became comment text to the real compiler.
Fix: rewrite skipCharSpan() to mirror lexer.l's grammar exactly -- one
raw body byte (any byte, including a quote) or one escape (with the
variable-length forms scanned per lexer.l: "\x"[0-9a-fA-F]+ hex,
"\"[0-7]+ octal, "\<CR><LF>"/"\<LF>" escaped newline, otherwise
backslash + one char), then the closing quote; a missing close ends the
span there so the offending byte re-lexes, mirroring the driver's
push-back recovery. The main tokenizer loop now calls skipCharSpan()
instead of duplicating the old string-style scan; findInterpEnd() and
directiveLineEnd() pick up the corrected rule for free. The sibling
escaped forms ('\'', '\"', '\\', '\x41', '\101') lex identically
before and after.
Same self-check blind spot as the "(::" parent-call fix (
|
||
|
Yucong Sun
|
007bb86370 |
feat: synthetic resolve() on WASM instead of raising an LPC error
The WASM build's dns_stub.cc made resolve() raise "DNS resolver is not
available", which crashes mudlibs that call resolve() during boot/login
(e.g. security daemons whose create() resolves before initializing
state) -- an LPC error there aborts object setup entirely.
There is still no nameserver to consult on this target (the page is the
only peer), so instead of failing, resolve() now succeeds synthetically
while keeping the native resolver's exact contract (dns_libevent.cc):
- returns an incrementing int key immediately;
- schedules the callback (string apply or function pointer) on the
next gametick, never before resolve() returns;
- callback args are (name, ip, key): ip echoes the input when it is
already a numeric IPv4/IPv6 address (matching what a native
getaddrinfo() of a dotted quad yields) and is "127.0.0.1"
otherwise -- the same loopback every WASM connection reports from
query_ip_number();
- this_player() is preserved into the callback when the
'this_player in call_out' setting is on, and DEBUGMALLOC_EXTENSIONS
marking of in-flight queries mirrors the native implementation.
LPC callers cannot tell the difference from a native resolve() that
found the host at loopback. Docs (docs/build-wasm.md limits,
src/wasm/README.md) updated.
Native builds compile dns_libevent.cc, not this file: no native change.
Verified under node: resolve("mud.example.com", f) returns key 0 and
later delivers f("mud.example.com", "127.0.0.1", 0); resolve of
"10.20.30.40" echoes the address; full WASM testsuite passes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VyQCUoTo1Z93Py9aVFHQi1
|
||
|
Yucong Sun
|
e33bb5da6e |
fix: query_ip_number() returned uninitialized garbage under WASM
query_ip_number() passed sizeof(sockaddr_storage) (128) to getnameinfo()
instead of the connection's actual addrlen, and never checked the return
value. glibc tolerates an oversized salen so the native build worked by
accident, but emscripten's getnameinfo (readSockaddr in libcore.js)
requires salen to exactly match sizeof(sockaddr_in)/sockaddr_in6 and
fails with EAI_FAMILY, leaving the host buffer uninitialized -- the
stack garbage (e.g. "(" or "") was then interned and returned to LPC.
Downstream, mudlibs that parse the IP string (sscanf "%d.%d...",
ban lists, explode(ip, ".")) rejected or crashed every WASM/browser
connection.
Fix: pass ob->interactive->addrlen, check the getnameinfo() result, and
on failure fall back to inet_ntop() on the raw sin_addr/sin6_addr (and
"0.0.0.0" as a last resort) so callers always get a well-formed address
string regardless of libc quirks.
query_ip_name() shares this path when no resolved name is cached (the
WASM build has no resolver), so it now returns "127.0.0.1" as well.
Verified: WASM console connection now yields "127.0.0.1" from both
efuns (was ""/garbage); native and WASM testsuites pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VyQCUoTo1Z93Py9aVFHQi1
|
||
|
Yucong Sun
|
1654bf6297 |
fix(lpc-syntax): tokenizer mis-lexes bare (::name( parent-call guard
`if (::name(...))` -- a bare parent-call immediately inside a
control-flow condition's parens, with no whitespace between `(` and
`::` -- was mis-tokenized. tokenizer.mjs's functional-literal-open rule
only checked `src[i+1] === ':'` before greedily emitting the `(:`
token, without checking whether a second `:` follows. For `(::name`,
that swallows the `(` and the FIRST colon as `(:`, leaving a lone `:`
behind: the `::` scope-resolution operator gets torn into `: :`, and
every formatting decision built on top of that token (indentation,
brace placement, statement grouping) cascades into visibly broken
output -- in the worst observed case an `if`'s condition and body come
out with the wrong line breaks and a spurious extra `{`.
Root cause: this is a real lexical ambiguity in "(::" -- it can start
either a functional-literal open ("(:" ... ":)") or a bare parent call
("(" followed by "::"). The driver's own lexer.l resolves it explicitly
via a `"("{WS}*"::"` rule (see the "(::" longest-match guard comment
there) that returns just '(' and pushes the rest back so "::" scans as
its own token. tokenizer.mjs had no equivalent rule and always took the
functional-literal reading.
Fix: add isParentCallOpenParen() to tokenizer.mjs, mirroring lexer.l's
rule -- look past any intervening whitespace (lexer.l's WS class:
space/tab/CR/LF/VT/FF) for "::" before deciding a "(:"-shaped span is a
functional-literal open; if "::" follows, emit only '(' and let the
whitespace/"::" scan normally on subsequent iterations.
This is also a case study in the documented self-check blind spot
(tools/lpc-syntax/README.md, "Validating a formatter change"): the
corpus safety net (token-sequence equivalence + idempotency) reported
this file clean both before and after formatting, because the same
(buggy) tokenizer was used to check its own output -- it mis-lexed the
input and the corrupted output identically, so the "before"/"after"
token signatures trivially matched. Verified this reproduces on master
(both checks report `true`/no mismatch pre-fix) and that the driver
build has no such blind spot -- lexer.l already gets "(::" right, which
is how two real mudlibs' player-body classes (`::move()`/`::query()`
guards) compiled fine originally and only broke after this formatter
ran over them.
Found scanning ~91 real-world LPC mudlibs (sibling project, not part of
this repo) with `find . -name '*.lpc' | node
tools/lpc-syntax/bin/format-corpus.mjs`; the testsuite corpus itself
has no `(::` occurrences, which is why this slipped past `test.mjs` and
`testsuite/format.sh --check` -- both still pass unchanged (779 files,
0 errors, same single pre-existing unrelated `wouldChange` as before
this branch).
Also updates the token-merge safety-net comments/test (format.mjs,
test.mjs, README.md): `f( ::g() )` used to need a forced space to avoid
the tokenizer re-lexing its own tight `f(::g())` output wrong. With the
tokenizer fix that space is no longer needed -- `(` before a bare `::`
is exactly as safe to render tight as any other qualified-scope site
(`efun::foo()`), so it now does (`f(::g());`). Adds a dedicated
regression test for the original bug (single-line guard, `if (...) {`
shape, and a whitespace-separated `( ::` variant).
node tools/lpc-syntax/test.mjs: all pass (was 1 failure pre-fix, the
stale `f( ::g() )` expectation, once master's own test.mjs is patched
in isolation to reproduce -- see above).
testsuite/format.sh --check: 779 files, 0 errors, 1 pre-existing
unrelated wouldChange (testsuite/tmp_eval_file.c, present identically
on master).
build/src/driver etc/config.test -ftest: passes except one pre-existing,
unrelated failure (deep_macro_nesting.lpc) present with no changes from
this branch checked out (confirmed via `git diff --stat` touching only
tools/lpc-syntax/*).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VyQCUoTo1Z93Py9aVFHQi1
|
||
|
Yucong Sun
|
3ec802f6a7 |
Fix null backbone_domain crash; add lpcc --batch
- mudlib_stats.cc: init_domain_for_ob() dereferenced backbone_domain before
set_master() has a chance to set it -- master's own author_file()/
domain_file() applies run before set_backbone_domain(), and if either
causes an object load (e.g. via call_other into simul_efun), the reentrant
call crashes on a null backbone_domain. Guard with a null check.
- lpcc: new --batch mode compiles many files against ONE VM boot (master +
simul_efun loaded once) instead of a fresh boot per invocation --
15-70x faster for a sweep over a large mudlib, and arguably more
realistic (matches how a real boot compiles many objects with no state
reset between them). Rearms the per-file eval-cost timer (set_eval()) --
without that, wall-clock time accumulates against ONE budget across the
whole batch and every file compiled after it's exhausted spuriously fails
with "Too long evaluation."
Rebased onto master's iterative macro-expansion rewrite (
|
||
|
Yucong Sun
|
f3e5bfa799 |
compiler: eliminate C-stack recursion from macro expansion and lexer; raise nesting cap to 65535
The macro-expansion rescan path recursed one yylex() frame per nesting level, the textual argument pre-expander (lpc_lex_expand_string) recursed per level with per-level guard-vector copies, the #if/#elif evaluator recursed per unary/paren/ternary token, and the lexer's no-token recovery paths (malformed heredocs, over-long $N, template-interpolation close) retried via recursive yylex() -- error REPORTING stops after 5 parse errors but scanning does not, so runs of malformed constructs nested a frame each (a 600KB file of '@' lines segfaulted the driver). All of these are now iterative: - lpc_lex_resolve_identifier returns LPC_TOKEN_RESCAN (no token); the identifier rule falls through and the SAME yylex() frame keeps scanning the pushed expansion buffer -- one Flex buffer per level, zero C-stack growth. - lpc_lex_expand_string is an explicit work-stack machine (one shared guard stack + O(1) name-count lookups instead of per-level copies). - the #if evaluator is an explicit-stack machine (ifexpr_eval), depth bounded by token count on the heap; no cap needed. Keeps the audit's int64_t retyping (LLP64 correctness). - heredoc recovery / @@ splice, over-long $N, and template '}' close fall through instead of recursing (pinned by lexer_retry_chains.lpc, which segfaults the previous binary). This supersedes the 2026-07-20 audit's stack-overflow mitigations for the preprocessor (kMaxIfExprDepth, MAX_EXPANSION_NESTING lowered to 32 under the sanitizer build's measured crash boundary): the recursion itself is gone, so MAX_EXPANSION_NESTING becomes kLpcMaxExpansionNesting = 65535, shared by both expansion engines as a runaway/memory bound and counted in LIVE frames only via a live-index stack (dead same-line provenance frames no longer trip it, so 128+ sequential uses of a macro on one line compile again). deep_nesting_caps.lpc's deep cases now compile -- under ASan included -- instead of being rejected; its comments and AGENTS.md's cap-sizing guidance are updated to the new design. The self-reference guard is an O(1) hash lookup. innermost_real_buffer_index -- behind every current_line read, per matched token -- is O(1) via a maintained include-buffer index stack instead of walking the whole buffer stack (the walk made deep-chain compiles quadratic). Diagnostics keep 16 innermost + 16 outermost expansion notes with an elision marker instead of one note per level. Tests: deep_macro_nesting.lpc (60000-deep chains through both engines, same-line frame accounting, beyond-cap clean error, 262144-token #if shapes), lexer_retry_chains.lpc, deep_ternary_nesting.lpc (parser-stack bound pins). Validated post-rebase on Debug and clang ASan+UBSan full suites (621/621 files each) plus RelWithDebInfo pre-rebase; gtests 320/320. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Yucong Sun
|
25c87bbc29 |
Lower MAX_EXPANSION_NESTING 64 -> 32 for ASan stack margin
#1304 set this rescan-recursion cap to 64, calibrated against its own ASan build (measured C-stack overflow boundary ~70-80). On a clang WSL Debug+ASan build (ENABLE_SANITIZER=ON, 8MB stack) the boundary is lower: #1304's own testsuite/single/tests/compiler/deep_nesting_caps.lpc SEGFAULTs (C-stack overflow in lpc_lex_resolve_identifier -> yylex, ~63 frames deep) at 64, and passes cleanly at 32. #1304's CI is green because its runners have more headroom, but 64 leaves too little margin -- exactly the thin-margin trap #1304's own AGENTS.md §13 note warns about ("set the cap comfortably under the measured boundary"). 32 still rejects the pathological deep macro chain and keeps every shallow-chain assertion passing; the full LPC suite is green at 32 (5895 checks, no ref-count reports). This is a separate commit from the merge so it can be reviewed or dropped independently of the #1304 / audit-hardening changes it sits on top of. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Claude
|
4d5345f575 |
fix: 3 more sibling bugs from self-review (preprocessor recursion x2, db.cc lock symmetry)
Continued self-review for siblings of the already-fixed patterns turned up three more real issues: 1. #if/#elif expression evaluator (lexer_rules_pp.cc ifexpr_atom/binop/top) had no recursion cap -- its depth tracks paren/unary/ternary nesting, attacker-controlled. `#if ((((...))))` ~18k deep segfaults (C stack overflow). Same bug-3 class as ast_json(): a recursive walker over nestable input missing the guard its siblings have. Capped at 500. 2. lpc_lex_expand_string() (lexer_rules_pp.cc) recursed one C-frame per link of a DISTINCT-macro chain A->B->C->... -- the self-reference guard vector only stops a macro expanding ITSELF, not a chain. ~5k deep segfaults. This is a SEPARATE path from the rescan expansion that already had MAX_EXPANSION_NESTING (it backs function-like-macro argument pre-expansion and #include filename expansion), so bug 7's cap didn't cover it. Capped at 64, matching its sibling. Both confirmed SIGSEGV on the pre-fix binary and clean-error after; deep_nesting_caps.lpc extended with both (plus shallow positives, and a chunked file writer since the deep sources exceed __MAX_STRING_LENGTH__ as one LPC string). 3. db.cc: package/async runs db_exec on a detached worker thread holding db_mut around find_db_conn()+execute(); only sync f_db_exec() took that lock. f_db_close/fetch/commit/rollback/status touched a live dbconn_t with no lock -- a concurrent async_db_exec() + main-thread db_close() on the same handle is a use-after-free. Added an RAII DbAsyncLock to each, matching f_db_exec (the same "sibling path forgot the primitive" shape as the socket_acquire flag gap, §13 item 15). PACKAGE_DB isn't in the default build and the race needs real thread concurrency, so this is fixed on code-symmetry and compile-verified with a dedicated sqlite PACKAGE_DB+PACKAGE_ASYNC build. AGENTS.md §4/§13 and README updated with all three. Full ASan+UBSan suite green (617 files, no ref-count reports); both preprocessor repros verified deterministic. |
||
|
Claude
|
2d317e4531 |
fix: apply.cc's inline default-args fill had the same push_svalue() stack-corruption bug
Self-review after fixing fill_default_args() in interpret.cc turned up a second, independent copy of the same default-argument fill logic inline in apply_low() (apply.cc) -- the driver's apply path, reached by ordinary EXTERNAL calls (ob->foo() / call_other), master applies, heart_beat, etc. It had the identical defect: push_svalue(call_function_pointer(...)) runs STACK_INC before the closure is evaluated, so a default expression that error()s leaves sp one slot high pointing at garbage, and the trailing free_svalue + fp restore only ran on the normal path. Fixed identically: evaluate the closure into a local before pushing, and DEFER the ref-free and fp restore so they run on the throw path too. This is arguably the hotter of the two sites -- ob->foo() with a defaulted, erroring default is a very ordinary call shape. Confirmed SIGSEGV on the pre-fix binary (exit 139, same "Invalid permissions" signature as the interpret.cc instance) and clean after. Extended compiler/default_args_error_unwind.lpc with the external-call (call_other) path, which the original test missed -- it only covered the same-object, fp-local, and ::inherited paths, all of which route through interpret.cc, never apply.cc. |
||
|
Claude
|
a55e1572f2 |
docs+tests: replace standalone audit MD with regression tests and AGENTS.md/README lessons
Per review: the STABILITY_AUDIT_2026-07-20.md was a working document, not a
durable artifact. Its content now lives where it belongs:
- The ~39 scratch reproducers under testsuite/single/fuzz_tmp/ (most of
which were negative-result explorations that found no bug -- refloop/
hot-reload, math/matrix, vmcore, socket-write) are removed. The 10 fixes
are covered by 6 consolidated regression tests under testsuite/single/
tests/, which the suite actually runs (in randomized order, and under the
Debug ref-count checker):
crasher/disassemble_wide_push.lpc (bug 1)
compiler/deep_nesting_caps.lpc (bugs 2 + 7)
compiler/default_args_error_unwind.lpc (bug 6, all 3 call paths)
efuns/restore_variable_class.lpc (bug 8 + its 2 sub-findings)
efuns/ffi_lifecycle_safety.lpc (bugs 4 + 5)
efuns/error_handler_mapping_size.lpc (bug 10 + the 2 mapping leaks)
All 6 pass on both the ASan+UBSan Debug and RelWithDebInfo builds; the
full suite is green (617 files, 5858 checks, no ref-count reports).
- The durable engineering lessons fold into AGENTS.md's §4 (the push_svalue()
macro-evaluation-order trap; guard EVERY error site in a module not just
one; elements hidden below sp during aggregate construction) and §13's
recurring-bug-class checklist (three new classes: soft-capped push/pop
desync; cached handle outliving its owning resource + a callback freeing
itself mid-dispatch; cleanup-on-destruct gated on a flag an ownership path
forgets to set), plus the "every sibling tree-walker needs its own cap,
sized against the sanitizer build's stack" refinement and a note on crash
paths that have no driver-LPC regression route (lpcc-only, async-timing).
README's Memory-Safety Work section is updated to match.
Bugs 3 (ast_json, reachable only from offline lpcc) and 9 (socket_acquire
UAF, needs async event-loop timing a single-file test can't service) have
no driver-LPC regression route and are documented as such rather than given
a flaky test; both fixes stand on code-symmetry with their siblings.
|
||
|
Claude
|
d917178831 |
fix: resolve all 10 confirmed driver-stability bugs from the 2026-07-20 audit
Fixes every bug documented in STABILITY_AUDIT_2026-07-20.md, each
independently re-verified (crash -> clean error/pass) and validated
against 4 full ASan+UBSan Debug testsuite runs plus 1 RelWithDebInfo
run (611 files, ~5800 checks each, all "Checks succeeded", zero
sanitizer reports).
1. disassemble() F_PUSH stack-buffer-overflow: bound the accumulation
loop against buff's remaining space. Also fixed a second, previously
undocumented overflow of the identical shape one frame later in the
same function (the instruction hex-byte dump), found while
re-verifying the first fix.
2. push_function_context()/pop_function_context() depth-cap asymmetry:
track failed pushes so pops consume that budget before touching the
real stack, instead of walking a shared pointer into nullptr.
3. ast_json()/dump_tree() missing recursion cap: mirrored optimize()'s
depth guard and flattened the NODE_TWO_VALUES chain walk iteratively,
matching how i_generate_node()/optimize() already do.
4. FFI callback use-after-free: capture ret_code before calling into
LPC. Also found and fixed a deeper related issue during
re-verification -- freeing a callback's libffi trampoline while it's
still executing crashes inside libffi itself; a callback can no
longer free itself mid-dispatch.
5. FFI call-after-unload: FfiLibrary now tracks prepared FfiFunc ids and
invalidates them on unload instead of refusing to unload (the first
attempt at this fix broke the ordinary prepare-call-unload lifecycle,
caught by 13 regressed FFI tests in a full-suite run).
6. fill_default_args() VM eval-stack corruption -- the closest match to
the original report: push_svalue(x)'s STACK_INC ran before x
(call_function_pointer(...)) was even evaluated. Fixed by evaluating
into a local first; also DEFER-guarded the ref release and fp
restore for the exception path.
7. MAX_EXPANSION_NESTING lowered 128 -> 64, under the measured
ASan-build crash boundary.
8. restore_object() uninitialized-size abort: guarded the
previously-unguarded array/class size errors, added bounds
validation to allocate_class_by_size(), added the missing
ROB_CLASS_ERROR branch, and converted restore_array()/restore_class()
to RAII so a nested throw no longer leaks the outer array/class.
9. socket_acquire() now sets O_EFUN_SOCKET, matching its three siblings.
10. mudlib_error_handler()'s diagnostic mapping leak, plus two more
general leaks in the mapping subsystem found while tracing why the
first fix alone didn't fully resolve the repro: insert_in_mapping()
(mapping.cc) not releasing a key's shared-string conversion on its
error() path, and load_mapping_from_aggregate() not freeing
remaining aggregate elements on its own error() paths (invisible to
the normal stack-unwind since the caller already moves sp below the
whole aggregate first).
|
||
|
Claude
|
39e24504d0 |
audit: self-review consolidation -- fix formatting, a broken repro, and a factual error
Self-review pass over the whole audit before calling it done: - testsuite/format.sh --check was failing on 19 fuzz_tmp files (the lpc-syntax CI job globs all of testsuite/, not just tests/). Ran the formatter and confirmed --check now passes clean, then re-verified every affected crash repro still reproduces identically post-format. - inherited_err.lpc (cited as independent verification of the F_CALL_INHERITED path for bug #6, fill_default_args()) was actually broken: errbase.lpc defined both foo() and a gop() that called ::foo() on itself with no parent to inherit from, so it never exercised the code path it claimed to. Restructured so the child object inherits errbase and calls ::foo() itself -- now genuinely reproduces the same SIGSEGV signature, independently confirmed. Also confirmed the FP_LOCAL variant (fp_local_err.lpc) crashes identically, so all three fill_default_args() call sites are now actually verified rather than two asserted on an agent's say-so. - Bug #7's writeup said "the 256-level cap" when the actual constant (MAX_EXPANSION_NESTING) is 128 -- a transcription error introduced while drafting, not in the underlying finding. Fixed. - Verified two claims that were being passed through from an agent's report without independent confirmation: PACKAGE_DEVELOP defaults to ON in src/CMakeLists.txt (not just an artifact of this session's configure flags), and restore_svalue() is genuinely reachable from raw, pre-auth network bytes on PORT_TYPE_MUD connections (net/transport_libevent.cc:453, socket_efuns.cc:1413) -- which elevates bug #8's severity from "malicious mudlib code" to "unauthenticated remote crash if a MUD-mode port is exposed" and is now called out prominently instead of buried in a parenthetical. - Trimmed ~1500 lines of exploratory scratch chaff (an abandoned diamond-inheritance investigation and its helper files, a 40-file #include-depth probe superseded by the macro-chain bisection, and catch()-wrapped precursors superseded by the final minimal repro) that didn't support any claim in the report, while double-checking nothing removed was actually referenced anywhere first. - Spot-verified ~15 file:line citations across ffi.cc, simulate.cc, and socket_efuns.cc directly against source rather than trusting agent-reported line numbers; all checked out. - Re-ran the full battery of all 10 repros end-to-end after every change above to confirm nothing regressed. All still reproduce (exit codes 1/134/139 as documented, or exit 0 at the documented safe boundary for bug #7's chain_70.lpc control). |
||
|
Claude
|
20ec1ee867 |
audit: 8th confirmed crash (restore_object uninitialized-size abort) -- audit complete
restore_object()/restore_variable()'s dirty sizing-scratch state
(save_svalue_depth/sizes[], object.cc), combined with an unguarded
array-size error() throw and allocate_class_by_size()'s complete lack
of size validation, lets a crafted two-call restore sequence feed
genuine uninitialized heap memory into a DMALLOC() byte count --
aborting the process ("illegal size in debugmalloc()"). Independently
re-verified (exit 134/SIGABRT, deterministic). Reachable purely through
restore_svalue(), which also processes attacker-controlled network
save-data.
This closes out the dispatched audit at 8 independently-confirmed
crashing bugs plus 2 further well-evidenced findings at an adjacent
severity tier (documented in STABILITY_AUDIT_2026-07-20.md's Status
section). Reference-loops/hot-reload and math/matrix were audited with
real adversarial effort and found clean.
|
||
|
Claude
|
866aab8a2f |
audit: document mudlib_error_handler() ref-count leak; refloop/hot-reload clean
Reference-loop cycle efuns/orphan collector and recompile_object() hot-reload both held up under extensive adversarial testing -- no crash or ref-count corruption found in either, despite trying multi- edge cycles, cycle detection mid-foreach on the iterated container, FP_FUNCTIONAL staleness across recompiles, and reentrant/nested recompile_object() calls. Found one unrelated pre-existing bug while probing copy()'s interaction with mapping-size limits: mudlib_error_handler() builds its diagnostic mapping with a raw pointer and leaks it if populating that mapping itself triggers a second error() (e.g. a configured __MAX_MAPPING_SIZE__ too small). Confirmed deterministic via the debug ref-count checker (a hard CI gate per AGENTS.md), independently re-verified. 9 findings documented so far (7 confirmed crashes, 1 high-confidence static UAF pending live reproduction, 1 confirmed leak). One background audit (save/restore_object) still in progress. |
||
|
Claude
|
0196e71d68 |
audit: document socket_acquire() O_EFUN_SOCKET gap (static finding)
socket_acquire() sets lpc_socks[fd].owner_ob but never sets current_object->flags |= O_EFUN_SOCKET the way socket_create()/ socket_accept()/socket_connect() do, so destruct_object()'s close_referencing_sockets() gate never fires for an object that only ever acquired a socket -- destructing it while it still owns the socket leaves owner_ob dangling for the next network callback to dereference. Confirmed via direct code reading (the asymmetry and the exact destruct-time gate are unambiguous); not yet triggered as a live crash because the repro needs real async socket I/O + call_out timing that single-file -ftest: runs don't service (documented AGENTS.md §7 harness limitation). Labeled as a distinct, lower evidence tier from the 7 dynamically-confirmed bugs already documented. |
||
|
Claude
|
435a40939d |
audit: checkpoint in-progress scratch reproducers (save/restore, refloop/hotreload, sockets/parser, math/matrix)
Snapshotting exploratory test files from ongoing audit work before they land in a more complete commit. math/matrix directory corresponds to a completed sweep that found no crashes (documented in STABILITY_AUDIT_2026-07-20.md); the others are from investigations still in progress. |
||
|
Claude
|
7d6348809f |
audit: document 2 more confirmed bugs (default-args stack corruption, macro nesting overflow)
- fill_default_args() (src/vm/internal/base/interpret.cc, new this cycle
in the default-arguments feature) is not exception-safe: if a default
argument's closure calls error(), the throw skips the pending stack
push/free and the frame-pointer restore, leaving the VM eval stack in
a state where the next pop_n_elems() frees a corrupted slot and
crashes. This is functionally the same "eval-stack corruption from
ordinary LPC" class as the #1295 report fixed earlier today in
string/object concatenation (
|
||
|
Claude
|
b97b9e2808 |
audit: document 5 confirmed crashing bugs (compiler front-end + FFI)
Investigating a driver-instability report by reviewing all 2026 changes, building an ASan+UBSan Debug driver, and stress-testing recently-touched code paths. Confirmed so far (each independently re-run to rule out flakes): - disassemble()'s F_PUSH case overflows its 2048-byte buffer past ~85 packed push descriptors -- live-driver reachable via the develop package's dump_prog() efun, not just the lpcc tool. - push_function_context()/pop_function_context() desync past the closure-nesting depth cap (10), walking a global context pointer to null on the 11th nested closure. - ast_json()/dump_tree() (lpcc --ast) never got the recursion-depth guard added to its sibling tree-walkers optimize()/i_generate_node(), and segfaults on deep non-foldable expressions. - FFI callback use-after-free when a callback frees its own handle mid-dispatch. - FFI call-after-unload: no reference check between FfiFunc and its owning FfiLibrary, so calling a prepared function after ffi_unload() jumps into unmapped memory. Minimal reproducers for each live under testsuite/single/fuzz_tmp/ (kept outside testsuite/single/tests/ so they aren't picked up by the randomized full-suite run -- these are unfixed crashes, not yet regression tests for a fix). Full details and repro commands in STABILITY_AUDIT_2026-07-20.md. Audit continues across save/restore, sockets/parser, reference-loops/hot-reload, and default-args/preprocessor. |
||
|
Yucong Sun
|
8b0aee8aa2 |
Harden five latent memory-safety / correctness gaps found in a driver audit
A multi-subsystem audit (VM core, compiler front-end, network/socket, save/ restore + DB/async, string/sscanf/parser) turned up no new severe exploitable bug beyond what #1304 already fixes -- the driver is well-hardened. It did surface five lower-severity/latent issues, all fixed here: 1. #if preprocessor evaluator computed in `long` (lexer_rules_pp.cc/.h). On LP64 that's 64-bit and fine, but on the supported Windows/MinGW LLP64 target `long` is 32-bit: every #if arithmetic result truncated to 32 bits -- disagreeing with the 64-bit runtime opcode and trees.cc constant folder -- and the `<< (rhs & 63)` shift became UB when the masked count was 32..63 on a 32-bit operand. Widened IfTok::val, the ifexpr_* evaluator, and the public lpc_lex_eval_if_expr() to int64_t. Cross-platform correctness/UB; compile-time only. New regression: compiler/if_expr_64bit.lpc pins the 64-bit contract (passes on LP64 regardless; catches a 32-bit regression). 2. parse_recurse() word-segmentation backtracker was unmetered (packages/parser/parser.cc) -- the MAX_PARSE_RULE_STEPS budget added to parse_rule() only short-circuits the leaves; the segmentation tree itself, whose branching factor is the number of registered compound-noun prefixes, kept traversing after the budget tripped. A cooperating dictionary could make it exponential (a §13.12-class C-stack DoS invisible to max_eval_cost). Now honors the same shared step counter / abort flag, returning cleanly (plain returns run each frame's num_words--/FREE_MSTR cleanup, matching the flag's documented design). 3. mark_sockets() didn't mark the ref-counted TLS option strings that f_socket_set_option() stores in lpc_socks[].options[] (SNI hostname / cert / key). Off-graph refs per AGENTS.md §3 -- balanced-freed, so not a safety bug, but a Debug check_memory() would false-positive "bad ref count" for a socket holding a live TLS option. Mark them transitively. 4. readthread() (packages/async/async.cc) read with req->data.max_size() (~2^62) as the count and resize()d a possibly-negative return from a failed open(). Its only caller aio_read() is currently dead (the live path uses gzreadthread, which correctly uses .size()), so it's an unreachable typo -- but a heap overflow the moment it's wired up. Bound by .size(), guard fd<0 and a negative read. 5. Aggregate/argument element count emitted as a 16-bit short with no bound (icode.cc NODE_CALL / ins_short). A literal with >65535 elements would truncate the count while codegen pushed every element -> VM-stack corruption. Unreachable today only because the physical evaluator stack is hard-capped at exactly 65536 (== the truncation modulus), so the stack overflow fires first; guarded defensively (like ins_rel_short()/upd_short() already guard branch offsets) so raising the stack size can never turn it into silent corruption. Full LPC suite green 3x (randomized order) on a Debug build, no ref-count reports; testsuite/format.sh --check and node tools/lpc-syntax/test.mjs clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Yucong Sun
|
bf73c66ec8
|
fix: uninitialized float variables must stay undefined until first assignment (#1305)
The zero-init half of #1303 synthesized an implicit `= 0.0` for every declared `float` local/global with no initializer, so the variable was a genuine T_REAL 0.0 from birth. That broke a long-standing contract: `float f; undefinedp(f)` must report 1 -- an uninitialized variable is undefined (const0u) regardless of its declared type, and mudlib code uses undefinedp() to distinguish "never assigned" from "assigned 0.0". Revert just those two hunks (rule_new_name/rule_new_local_def in grammar_rules_decls.cc). The declared-type contract they were backing up is already fully enforced on first assignment by the OTHER half of that PR, which stays: plain `=` promotes the RHS via do_promotions(), and op= coerces the RHS to the lvalue's declared type at compile time (rule_expr_assign) with the runtime opcodes promoting a still-undefined T_NUMBER lvalue to float on a float RHS. So `float f; f += 1` still yields float 1.0 -- the variable remembers its declared type the moment it's first assigned, it just doesn't pretend to be assigned before then. Updated compound_assign_float.lpc to pin the corrected contract: undefinedp() == 1 on freshly declared float locals AND globals, undefinedp() == 0 plus floatp() == 1 after the first op= or plain assignment. These assertions fail on the previous commit's binary and pass now; full LPC suite passes 2x (randomized order) on a Debug build. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Yucong Sun
|
d0549220d8
|
Zero-init declared float variables as T_REAL; pin int op= float type contract (#1303)
* fix: f_xor/f_lsh/f_rsh leave stale T_UNDEFINED subtype on their result
Follow-up to the #1295/#1296 stack-corruption fixes: an audit for the
same "one arm forgot what its sibling arms do" bug shape turned up a
related but distinct defect in the same neighborhood of ops.cc.
"undefined" (the value read back from a missing mapping key) is
represented as {T_NUMBER, subtype=T_UNDEFINED, u.number=0}. f_xor(),
f_lsh(), and f_rsh() each reuse the left operand's stack slot for an
in-place numeric result (`sp--; sp->u.number OP= ...;`) but never reset
that slot's subtype -- so `x ^ 0`, `x << n`, or `x >> n` on an
undefined `x` produces a real computed 0 that still reports
undefinedp() == 1. Their true siblings f_and() and f_or() already do
this correctly (fixed for f_or() in
|