From 1872fb4d87f90061c03afdf8c04718702d7ac58f Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Wed, 27 May 2026 00:23:40 +0000 Subject: [PATCH 1/2] Copy the message string when constructing Error/AggregateError from a ZigString MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zig::getErrorInstance built the Error message with Zig::toString, which for an untagged ZigString wraps the caller's bytes via StringImpl::createWithoutCopying — no copy, no ownership, no liveness link. Several callers pass stack-local buffers (AsyncModule's resolve_error/download_error format their message into a local Vec), so the JSString backing error.message aliases freed memory by the time user code reads it. The TypeError/SyntaxError/RangeError siblings already use toStringCopy; do the same here and in JSC__JSGlobalObject__createAggregateError, whose message has the same lifetime requirement. --- src/jsc/bindings/bindings.cpp | 5 ++++- src/jsc/bindings/helpers.h | 7 ++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index ae3d8986c710..e81914b4387b 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -3459,7 +3459,10 @@ JSC::EncodedJSValue JSC__JSGlobalObject__createAggregateError(JSC::JSGlobalObjec auto& vm = JSC::getVM(globalObject); auto scope = DECLARE_THROW_SCOPE(vm); - WTF::String message = Zig::toString(*arg3); + // toStringCopy, not toString: the AggregateError (and its message string) + // outlives this call, while an untagged ZigString message would alias the + // caller's buffer without owning it. See Zig::getErrorInstance. + WTF::String message = Zig::toStringCopy(*arg3); JSC::JSValue cause = JSC::jsUndefined(); JSC::JSArray* array = nullptr; { diff --git a/src/jsc/bindings/helpers.h b/src/jsc/bindings/helpers.h index cb9bc987755c..77f71d6ec103 100644 --- a/src/jsc/bindings/helpers.h +++ b/src/jsc/bindings/helpers.h @@ -393,7 +393,12 @@ static const WTF::String toStringStatic(ZigString str) static JSC::JSValue getErrorInstance(const ZigString* str, JSC::JSGlobalObject* globalObject) { - WTF::String message = toString(*str); + // toStringCopy, not toString: an untagged ZigString would otherwise become + // a JSString that aliases the caller's bytes with no ownership, and several + // callers (AsyncModule's resolve/download error paths) pass stack-local + // buffers that are freed before JS can ever read error.message. The + // TypeError/SyntaxError/RangeError siblings below already copy. + WTF::String message = toStringCopy(*str); if (message.isNull() && str->len > 0) [[unlikely]] { // pending exception while creating an error. return {}; From ae94dddce664e8d2adda2892ddd53d522512cd7a Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 28 May 2026 04:07:12 +0000 Subject: [PATCH 2/2] Keep the ownership-transfer path for external-tagged error messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback: switching getErrorInstance/createAggregateError to an unconditional toStringCopy dropped the external-tag (mark_global) ownership transfer, so callers that heap-allocate the message and rely on the JS string adopting and freeing it (VirtualMachine.rs's joined build-failure message for AggregateError) would leak it on every call. Guard like toIdentifier does: external-tagged messages keep toString (adopted as an ExternalStringImpl, freed via free_global_string); everything else — including the untagged stack-local buffers that motivated the change — is copied. --- src/jsc/bindings/bindings.cpp | 10 ++++++---- src/jsc/bindings/helpers.h | 16 ++++++++++------ 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index e81914b4387b..b3021c959b3a 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -3459,10 +3459,12 @@ JSC::EncodedJSValue JSC__JSGlobalObject__createAggregateError(JSC::JSGlobalObjec auto& vm = JSC::getVM(globalObject); auto scope = DECLARE_THROW_SCOPE(vm); - // toStringCopy, not toString: the AggregateError (and its message string) - // outlives this call, while an untagged ZigString message would alias the - // caller's buffer without owning it. See Zig::getErrorInstance. - WTF::String message = Zig::toStringCopy(*arg3); + // External-tagged messages transfer ownership of their heap buffer to the + // JS string (VirtualMachine.rs marks the joined build-failure message + // global and relies on C++ freeing it); everything else is copied so an + // untagged ZigString can't leave the AggregateError aliasing the caller's + // buffer. See Zig::getErrorInstance. + WTF::String message = Zig::isTaggedExternalPtr(arg3->ptr) ? Zig::toString(*arg3) : Zig::toStringCopy(*arg3); JSC::JSValue cause = JSC::jsUndefined(); JSC::JSArray* array = nullptr; { diff --git a/src/jsc/bindings/helpers.h b/src/jsc/bindings/helpers.h index 77f71d6ec103..996920fc149b 100644 --- a/src/jsc/bindings/helpers.h +++ b/src/jsc/bindings/helpers.h @@ -393,12 +393,16 @@ static const WTF::String toStringStatic(ZigString str) static JSC::JSValue getErrorInstance(const ZigString* str, JSC::JSGlobalObject* globalObject) { - // toStringCopy, not toString: an untagged ZigString would otherwise become - // a JSString that aliases the caller's bytes with no ownership, and several - // callers (AsyncModule's resolve/download error paths) pass stack-local - // buffers that are freed before JS can ever read error.message. The - // TypeError/SyntaxError/RangeError siblings below already copy. - WTF::String message = toStringCopy(*str); + // External-tagged strings transfer ownership: the caller heap-allocated + // the bytes and marked them global so the JS string adopts them (and + // frees them via free_global_string) — keep that path, like toIdentifier + // does. Everything else is copied: an untagged ZigString would otherwise + // become a JSString that aliases the caller's bytes with no ownership, + // and several callers (AsyncModule's resolve/download error paths) pass + // stack-local buffers that are freed before JS can ever read + // error.message. The TypeError/SyntaxError/RangeError siblings below + // already copy. + WTF::String message = isTaggedExternalPtr(str->ptr) ? toString(*str) : toStringCopy(*str); if (message.isNull() && str->len > 0) [[unlikely]] { // pending exception while creating an error. return {};