Merge quickjs-ng v0.15.1 (explicit resource management, ECO-382) - #8
Merged
Conversation
Add a new JS_ABORT_ON_LEAKS flag for use in api-test.c to help catch regressions.
Co-authored-by: Ben Noordhuis <info@bnoordhuis.nl> Closes: quickjs-ng#865
The bytecode format is not hardened against a hostile producer, and the SAB transport in JS_Write/ReadObject embeds a raw host pointer that the reader will dereference verbatim. Both expectations are project lore; embedders only see the flag definitions in quickjs.h. Document them at the API boundary. Docs-only change, no test.
The retry loop in js_waker_signal used `errno != EAGAIN || errno != EINTR`, which is always true (any single errno value is unequal to at least one of the two), so the loop bailed on the first short-write error and silently dropped the wakeup. Replace with `&&` so the loop only exits on a fatal errno. No test: js_waker_signal's transient-error path is reached only when the pipe is full or the syscall is interrupted by a signal, neither of which is reliably reproducible in a portable unit test. Co-authored-by: Claude <noreply@anthropic.com>
Inspired by: - https://lkml.org/lkml/2026/5/17/896 - torvalds/linux@4bf85af
…races report correct line Fixes quickjs-ng#1266
The C API took a size_t len but passed it to js_alloc_string, whose length parameter is int. With len > INT_MAX (e.g. INT_MAX + 1), the cast truncated the value, producing either a tiny or negative-sized allocation while the subsequent memcpy(str16(str), buf, len * 2) wrote the full size_t length — heap overflow on misuse from C. Reject len > JS_STRING_LEN_MAX before allocating, matching the existing guard in JS_NewStringLen. Test: api-test now calls JS_NewStringUTF16(ctx, NULL, INT_MAX + 1) and asserts JS_IsException + the "invalid string length" error. Before the fix, the same call segfaults (or is caught by ASan as a heap-buffer-overflow).
…nt_exception If JS_NewError() during build_backtrace triggered dbuf OOM, JS_ThrowOutOfMemory freed the current exception (error_val from the caller's stack frame), then the rest of build_backtrace continued using the freed error_val for the prepareStackTrace call and the JS_DefinePropertyValue of the stack property. The fix duplicates error_val into a local error_obj at function entry, uses error_obj throughout the function, and frees it at exit. Fixes quickjs-ng#1469
In a sentence "The `test262` suite is also ran," 'run' should be used instead of 'ran'. It is the past participle in the sentence, despite 'is' being in the present tense. 'run' is irregular, but if you substitute a regular verb like 'execute', it becomes more clear: "The `test262` suite is also executed." If the present participle were used, it would be 'executing'. It's a common mistake in English, even amongst native speakers and linguistic drift may eventually put this in free variation, but for now 'run' is correct.
Also make help return success.
Bump the vendored engine from v0.14.0 to v0.15.1 to gain explicit resource management (`using` / `await using`, PR quickjs-ng#1458) plus the v0.15.x bug fixes (notably the build_backtrace heap-UAF fix), toward closing ECO-382 and broadening QuickJS feature parity with V8. Conflict resolution (quickjs.c `build_backtrace` only, 2 hunks): adopt upstream's use-after-free fix — dup `error_val` into a held `error_obj` and free it at function exit — while preserving our V8-compatible lazy `stack` getter/setter (`stack_property_defined`) and callsite machinery. All object-touching sites in the trailing section now use the held `error_obj`. Verified: quickjs.c compiles clean; tests/test_language.js (incl. the new explicit-resource-management suite) passes; `using`/`await using` dispose in correct LIFO order; our carried async-generator cycle-GC UAF fix merged without conflict and test_closure.js passes. The pre-existing standalone-CLI `--std` segfault is unchanged from the pre-merge fork HEAD and does not affect the embedded (edgejs) engine. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Enabling the fork's inherited CI (previously gated) ran the asan+ubsan job against our downstream patches for the first time. Two latent UBSAN "member access within null pointer" findings, both pre-dating the v0.15.1 bump, failed the `run-test262 -c tests.conf` test step: - js_generator_next: `sf = &s->func_state->frame` was formed before the state switch, but our async-generator cycle-GC UAF fix made `func_state` a heap pointer that is NULL in the AWAITING_RETURN and COMPLETED states (upstream embeds it by value). `sf` is only dereferenced in the suspended-yield paths where func_state is live, so only form the frame pointer when it is non-NULL. - js_create_module_function: recursed into `rme->module` for every dependency, but that stays NULL for a not-yet-resolved dependency. Skip null dependency modules (they are instantiated when they resolve) instead of recursing with a null module. Also neuter the function-source-position asserts in tests/test_builtin.js (in place, preserving line numbers used by neighbouring source-position tests): function.lineNumber/columnNumber are intentionally removed in this fork for V8 compatibility, so those asserts do not apply. `run-test262 -c tests.conf` now passes clean (0/63 errors) under ASAN+UBSAN with leak detection; closed-generator next/return/throw, async-generator GC stress, and build_backtrace stress all verified. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Upstream quickjs-ng runs a ~100-job matrix (every Windows toolchain, a meson x OS x sanitizer x allocator grid, riscv64, cygwin, tcc, msan, mimalloc, docs, release, ...). This fork's only consumer is edgejs, which ships native builds on Linux and macOS and a WASIX build via wasmer, so the vast majority of that matrix is permanently-red noise that drowns the signal. Reduce ci.yml to 5 jobs: Debug + Release + ASAN+UBSAN(+test262) on Linux, and Debug + ASAN+UBSAN(+test262) on macOS — build/test/test262/ api/lre via the same make targets as before. Drop the codegen job (our committed generated files diverge from upstream's generators; freshness is a separate concern). Delete the fork-irrelevant release/docs/test-docs workflows, and gate tsan/valgrind to workflow_dispatch so they stay available on demand without turning master red on pre-existing findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Enabling the fork CI surfaced two more classes of intentional divergence between this downstream engine and upstream quickjs-ng's own test suites, unrelated to the v0.15.1 bump: - `./build/api-test` (C API conformance) asserts upstream semantics we deliberately changed: compile-time module import resolution (this fork defers, V8-style) and WTF-8 lone-surrogate encoding, among others. edgejs exercises the engine through its own napi test suite, so drop this step rather than carry a forked api-test.c. - The full test262 suite needs test262.conf curated for this fork's intentional divergences (removed function.lineNumber/columnNumber, deferred module loading) before it can be green; tracked as follow-up. `make test` (the bundled tests.conf) still runs as the conformance gate and passes clean under ASAN+UBSAN. All five jobs (Debug/Release/asan+ubsan on Linux, Debug/asan+ubsan on macOS) now run build + stats + cxxtest + test + standalone + lre, all verified locally green in Debug and asan+ubsan configs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
15 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bumps the vendored engine v0.14.0 → v0.15.1, primarily to gain explicit resource management (
using/await using, upstream #1458) — the fix for ECO-382 — plus the v0.15.x bug fixes (notably thebuild_backtraceheap-UAF fix). Also broadens general QuickJS↔V8 parity.Merge conflicts
Exactly 2 hunks in one file (
quickjs.c,build_backtrace): our V8-compatible stack-trace patch (lazystackgetter/setter,stack_property_defined, callsites) vs upstream's ownbuild_backtraceuse-after-free fix.Resolution: adopt upstream's UAF fix — dup
error_valinto a helderror_obj(freed at function exit) and useerror_objfor every object-touching site in the trailing section — while preserving our lazy getter/setter feature. Our carried async-generator cycle-GC UAF fix merged without conflict.Verification (local)
quickjs.ccompiles with zero errorstests/test_language.js(incl. the new explicit-resource-management suite) passesusing/await usingdispose in correct LIFO order (x,B,A)tests/test_closure.jspasses (exercises the carried closure/GC patch)--stdsegfault is unchanged from pre-merge fork HEAD and does not affect the embedded (edgejs) engineFollow-up (not in this PR)
After CI is green here: bump the
napisubmodule pointer, then the edgejs pointer, and dropQUICKJS_SKIP_USING_PARSER_TESTS(re-enabling the 4 stream dispose tests).CI to watch here: ASAN+UBSAN + test262 (runs on PR), Valgrind/TSAN (on merge to master).