[2] Defer GC while using the direct eval CacheLookupKey
The eval cache kept a raw pointer that nothing was keeping alive.
High. A raw pointer deliberately stripped of refcounting is held across the most allocation-heavy window in the direct-eval path, and the object that would keep its target alive stops being marked the moment a rope flattens. Reachable from plain script; escalation past the free depends on winning the timing and reclaiming the body.
JSC represents string concatenation lazily: a rope JSString holds pointers to other string cells rather than a resolved character buffer, and resolves itself in place when the characters are actually needed. Direct eval() at a given call site is cached so repeated evaluation skips reparsing — the cache is keyed on the source text plus a bytecode index, and its probe key deliberately holds a bare StringImpl* (WTF's refcounted string body) to avoid refcount traffic on a hot lookup. The expectation is that anything that could free that body stays provably reachable for as long as the bare pointer is live on the stack.
The angle: any script can build a rope, force it to flatten inside eval(), and have the engine keep using a raw pointer to string storage that a garbage collection in the same window frees — then read, compare, and refcount-increment through it.
The commit message documents the mechanism directly: DirectEvalCodeCache::CacheLookupKey holds a raw StringImpl* for performance, and that pointer can be the contents of a fiber of an on-stack rope JSString. Though the rope itself is live on the stack, it can be mutated in place by flattening, which leaves its fiber JSString unrooted and sweepable by the GC — dereferencing the String whose raw pointer sits in the key. The fix defers GC while the lookup key is live.
Source/JavaScriptCore/interpreter/Interpreter.cpp
JSTests/stress/direct-eval-cache-rope.js
Patch Details
The production change is one line in JSC::eval(): a DeferGC deferGC(vm); is constructed as the first statement of the cache-miss branch, immediately after cacheKey is built from programStr.data.impl() and the directEvalCodeCache().get(cacheKey) probe misses. The deferral then covers the allocation-heavy remainder of that branch — the auto programSource = programStr.data; copy, the SourceProfiler::g_profilerHook path, makeSource, parsing, and DirectEvalExecutable creation — for the block's whole scope. Because DeferGC is scoped to the whole if (!eval) { ... } block, the set(...) call that converts CacheLookupKey into the RefPtr-holding CacheKey is covered by the deferral as well.
The only other change is the regression test, run with --slowPathAllocsBetweenGCs=10, which repeatedly evals a rope built from a previously-atomized string plus "()". DirectEvalCodeCache.h is untouched: CacheLookupKey::m_source remains SUPPRESS_UNCOUNTED_MEMBER StringImpl* while the stored CacheKey::m_source remains RefPtr<StringImpl>.
Caching a raw, uncounted pointer into storage whose only owner is an object that an in-place structural mutation has already made collectable, while the pointer stays live across allocation points.
Background
Rope JSString. JSC represents string concatenation lazily. A JSRopeString stores up to three fiber pointers to other JSString cells instead of a resolved StringImpl; the low bit of the first fiber word (isRopeInPointer) marks the cell as a rope. JSString::visitChildrenImpl marks the fibers only while that bit is set.
Rope flattening. When a rope's characters are actually needed, JSC resolves it and mutates the same cell in place: convertToNonRope(...) replaces the fiber words with a resolved String, so the cell stops being a rope.
StringImpl. WTF's refcounted string body, holding length, an 8/16-bit flag, and the character buffer. A String or RefPtr<StringImpl> owns a reference; a raw StringImpl* does not. JSString::destroy destroys the cell's embedded String, releasing one reference.
GCOwnedDataScope. The wrapper returned by JSString::value() / view(). It pairs string data with an owner JSCell and keeps that owner alive for the scope's lifetime — the { this, ... } construction visible in JSRopeString::resolveRopeToAtomString.
DirectEvalCodeCache. A per-CodeBlock HashMap keyed by (source StringImpl, BytecodeIndex). It has two key types: the stored CacheKey holds a RefPtr<StringImpl>, while the probe-only CacheLookupKey holds SUPPRESS_UNCOUNTED_MEMBER StringImpl* m_source and is used through CacheLookupKeyHashTranslator so lookups avoid refcount churn. operator CacheKey() converts a lookup key into a stored key. The split exists so that the common case — a cache hit — costs no atomic refcount operations at all.
DeferGC. An RAII object that raises the heap's deferral depth, preventing a collection from running while it is in scope.
Conservative stack scanning. JSC treats live machine-stack and register words that look like cell pointers as roots, which is what keeps an on-stack JSString* such as programString alive without an explicit handle.
Atomization by property access. Indexing an object with a string key converts the key to an Identifier, which resolves a rope and associates an AtomStringImpl with that JSString cell.
Analysis
The bug is a use-after-free on a StringImpl whose sole owner stops being marked partway through the window in which a bare pointer to it is live:
rope programString cacheKey (stack) GC
────────────────── ──────────────── ──────────────
fiber0 ─► JSString A
flatten in place ──┐
fibers overwritten│ m_source = A's impl
A now unmarked └──────────► (uncounted)
mark: A unreachable
sweep: ~JSString
└─► ~String → free impl
hash() / equal() ─────────► freed memory
operator CacheKey() ──────► refcount++ into freed
Whether anything in that window holds a counted reference turns on the declared type of GCOwnedDataScope::data, which is not in the supplied context. The next statement is auto programSource = programStr.data; — if data were a String (or a const String& from which auto deduces a copy), that copy would take a reference and the new DeferGC would be a no-op. Since the fix is placed before that copy, and the in-code comment states the StringImpl can be deref'd when the JSString is swept, the load-bearing reading is that programStr.data is a borrowed, non-owning handle onto storage owned by a JSString cell — leaving the GCOwnedDataScope's owner cell as the only keep-alive in the window.
The missing invariant is that the object whose destruction would release the StringImpl must be provably reachable for as long as the raw pointer is live. Per the commit message and the in-code comment, that does not hold: the storage the key captures can be owned by a fiber JSString of programString, not by programString itself. Flattening mutates programString in place, and once the cell is a non-rope it reports only the resolved impl to the visitor — so the fiber becomes unreachable even though the outer rope is conservatively rooted on the stack.
The consumption after the free is not incidental. CacheLookupKey::hash() dereferences m_source->hash(), operator== calls WTF::equal(m_source, ...) which reads the impl's length and character data, and operator CacheKey() constructs a RefPtr<StringImpl> — a refcount increment written into freed memory — when the compiled executable is stored back into the per-CodeBlock cache. A dangling key that reaches m_cacheMap outlives the eval() frame entirely and is touched by every later probe of that cache.
Walking the regression test:
getRope(index)buildsa = "[" + index + ',[]'.repeat(0x100) + "]", a 771–773 character rope that is valid array-literal syntax and stays well under the on-stack resolve threshold.[][a]performs a property access keyed bya, converting the key to anIdentifierand therefore resolving and atomizing the rope in place —a's cell becomes a non-rope owning an atomized impl. Which resolution helper is taken (the on-stack path inresolveRopeToAtomStringversusresolveRopeToExistingAtomStringor the key-atom cache) is not in the supplied context, but all of them end in an in-placeconvertToNonRope.return a + bbuilds a freshJSRopeStringwhose fiber0 isa's cell and fiber1 is"()". The concatenated text parses but throws when run, so thetry/catchswallows it and each iteration presents distinct source text — guaranteeing a cache miss and thus entry into the patched branch.getRope(0)andgc()clear intermediate garbage whiles, and through it fiber0, is still reachable.eval(s)callsprogramString->value(globalObject), which flattens the rope in place; per the commit message the resulting data can be storage owned by a fiber cell, while the rope's own fiber words have just been overwritten, so that fiber is no longer marked.cacheKeycaptures itsimpl()as a bare pointer and the miss path begins parsing ~780 characters, allocating repeatedly; withslowPathAllocsBetweenGCs=10a collection runs inside that window, the fiber is unmarked, sweeping callsJSString::destroy, and the impl is freed.
Escalation beyond the ASAN-visible heap-use-after-free depends on reclaim conditions, and the reclaim story in turn depends on which allocator backs StringImpl bodies in this tree (plain fastMalloc versus a TZone/isoheap partition), which the supplied context does not show — a partitioned allocator would narrow the set of objects that can land in the freed slot. Conditional on a controlled object being placed there: if the freed slot is reclaimed with attacker-shaped bytes before the key is consumed, the RefPtr<StringImpl> construction in operator CacheKey() would perform a refcount increment at an attacker-chosen address offset, which could give a constrained increment primitive; if reclaim happens before a later probe, operator=='s call to WTF::equal(m_source, other.m_source.get()) would read length and character-buffer fields out of the forged object, which could yield a relative or absolute read expressed as a comparison oracle rather than direct disclosure; a forged equality match could return a DirectEvalExecutable compiled from different source text than the string passed to eval(), amounting to script-execution confusion inside the same realm; and because the converted key is inserted into a CodeBlock-lifetime map, a later DirectEvalCodeCache::clear() would run a deref() on the same forged pointer, supplying the matching decrement half of an increment/decrement pair. Realising any of these would require heap grooming into the freed body's size class and winning the GC-timing race, both practical from script.
This vulnerability weakens memory safety inside the JavaScript engine's heap, in a path fully reachable from web content. The security-model assumption at stake is that any pointer held across a collection point either keeps its target alive or is re-derived afterwards; before the fix the direct-eval cache key violated that for a StringImpl whose sole owner was a rope fiber unrooted by flattening. Because the key is inserted into a CodeBlock-lifetime cache, the dangling pointer would persist past the eval() call. This is a renderer-side memory-corruption foothold, not a boundary bypass by itself.
The dangerous ingredient is not the raw pointer alone but the combination of two facts stated far apart in the codebase: CacheLookupKey deliberately drops refcounting for speed, and GCOwnedDataScope names an owner cell that is not necessarily the cell whose destructor releases the data. Rope flattening is what breaks the implicit bridge — it is one of the few operations in JSC that mutates a live cell such that previously-marked children become unreachable, so any borrowed interior pointer obtained from a rope has a lifetime that ends at the next GC rather than at the end of the enclosing scope. The chosen fix is coarse but robust: rather than re-deriving or refcounting the key, it removes the GC point for the window. Worth noting because it leaves the uncounted-key design in place — the invariant is now upheld by a scope in one caller, not by the type.
Audit directions
-
Uncounted borrowed pointer into GC-releasable storage held across an allocation point. The invariant is a raw pointer may only outlive its owner's proof of reachability if nothing between here and its last use can allocate. Narrow: grep JavaScriptCore for
SUPPRESS_UNCOUNTED_MEMBERonStringImpl*/AtomStringImpl*/SymbolImpl*fields and for hash-translator lookup-key classes (theCacheLookupKey/CacheKeysplit shape inDirectEvalCodeCache.h) — match tell: a lookup key built from->value().data.impl()or->view()whose scope extends past any parse, allocation, orvisit-triggering call. Wider: the same class appears with any borrowed-buffer wrapper — auditGCOwnedDataScopeandStringViewlocals derived from aJSString, and typed-arraydata()pointers captured before a call that can allocate. Widest: this is the general borrowed-interior-pointer-into-managed-storage class — carry the invariant into V8 (String::FlatContentvalid only underDisallowGarbageCollection), SpiderMonkey (JSLinearString::charsunderAutoCheckCannotGC), and any FFI that hands out a pointer into a GC'd buffer. Match tell elsewhere: a raw pointer obtained from a managed object with no scope guard, no pinning, and an allocating call between acquisition and use. -
In-place structural mutation that orphans children of a still-live parent. The invariant is reachability of a container does not imply reachability of what it referenced a moment ago. Narrow: trace every caller that resolves a rope (
resolveRope,resolveRopeToAtomString,resolveRopeToExistingAtomString, and the fiber-walking helpers) and keeps any derived pointer,String&, orStringViewafterwards — match tell: code that reads->value()/->view()on a value that might be a rope, then allocates before last use. Wider: the same shape exists for other in-place representation transitions in JSC that drop marked edges — butterfly reallocation invalidating pointers to old property or element storage, structure transitions that discard a property table, substring bases replaced by resolved impls. Code-review tell: any method namedconvertTo*,flatten*,resolve*, ormaterialize*that writes over a field the visitor uses for marking. Widest: any runtime that normalizes a lazy tree in place — V8 ConsString flattening, SpiderMonkey rope flattening, persistent-data-structure libraries with path compaction; after a normalize step, every pointer derived from the pre-normalized shape must be re-derived, not reused. -
Keep-alive scopes whose declared owner is not the object that actually owns the data. The invariant is a liveness guard must name the exact object whose destructor would free the referenced bytes. Narrow: audit
GCOwnedDataScopeconstruction sites inSource/JavaScriptCore/runtime(the{ this, ... }form inJSString.cppis the template) and check, for each, whether the returned data can point into a different cell than the recorded owner — match tell: a return statement whose owner argument isthiswhile the data expression reaches throughsubstringBase(),fiber(i), or another cell. Wider: the same asymmetry shows up with any keep-alive idiom that takes an object rather than a region —ensureStillAliveHere(x)calls where the protected read is fromy, andRef/RefPtrlocals taken on a wrapper while the loop body dereferences its callee-owned payload. Widest: guard-aliases-the-wrong-owner covers reachability fences in managed languages (JavaReference.reachabilityFence, Objective-Cobjc_precise_lifetime) and Rustunsafecode tying a borrow to a lifetime derived from the wrong parent; for every keep-alive, ask which destructor frees the bytes being read and confirm the guard names that object. -
Coarse
DeferGC/ no-allocation windows used as a substitute for correct ownership. A later refactor can silently widen the window or move an allocation inside a supposedly quiet region. Narrow: review otherDeferGC/DeferGCForAWhile/DisallowGCscopes in JavaScriptCore that guard raw pointers rather than a specific allocation, and check that every use of the guarded pointer is actually inside the scope — match tell: a raw pointer declared before the guard and used after it, as with a lookup key constructed outside the deferred block. Wider: the same fragility applies to any ad-hoc critical section protecting an unowned pointer —AssertNoGCregions and lock-free fast paths whose safety argument is "nothing here allocates"; match tell: a comment asserting that a region cannot allocate, with no compile-time enforcement. Widest: a safety property enforced by a scope rather than by a type degrades under refactoring — Rustunsafeblocks documented only by comment, C++ RAII guards whose protected data outlives the guard. Verification here is largely manual because it requires reasoning about which callees can allocate, so pair it with an assertion-enabled stress run under--slowPathAllocsBetweenGCsand ASAN rather than relying on inspection alone.