[8] GetByStatus walks prototype chain for direct/private property opcodes

Severity: Medium | Component: JSC bytecode IC status + DFG | b07dca9

Rated Medium because the diff repairs a JIT IC status helper that allowed DFG to specialise GetByIdDirect/GetPrivateNameById under prototype-walk assumptions; whether this becomes a controlled property fetch from a non-own slot depends on downstream lowering that the diff does not establish in isolation.

When computing the GetByStatus, the property lookup should be checked for direct property access before doing a prototype walk, since direct accesses are not supposed to consult the prototype.

Source/JavaScriptCore/bytecode/GetByStatus.cpp

-GetByStatus GetByStatus::computeFor(JSGlobalObject* globalObject, const StructureSet& set, CacheableIdentifier identifier)
+GetByStatus GetByStatus::computeFor(JSGlobalObject* globalObject, const StructureSet& set, CacheableIdentifier identifier, GetByStatus::LookupMode mode)
 {
-    // Note that this code is also used for GetByIdDirect since this function only looks
-    // into direct properties. When supporting prototype chains, we should split this for
-    // GetById and GetByIdDirect.
     ...
-    if (auto result = attempToFold())
-        return result.value();
+    // We should do a prototype walk searching for the property only if this
+    // is a normal access. Don't consult proto for for direct accesses.
+    if (mode == GetByStatus::LookupMode::Normal) {
+        if (auto result = attempToFold())
+            return result.value();
+    }

Source/JavaScriptCore/dfg/DFGNode.h

+    GetByStatus::LookupMode propertyLookupMode()
+    {
+        switch (op()) {
+        case GetByIdDirect:
+        case GetByIdDirectFlush:
+        case GetPrivateNameById:
+            return GetByStatus::LookupMode::Direct;
+        case GetById:
+        case GetByIdFlush:
+        case GetByIdMegamorphic:
+            return GetByStatus::LookupMode::Normal;
+        default:
+            RELEASE_ASSERT_NOT_REACHED();
+        }
+    }

Patch Details

A new LookupMode enum (Normal/Direct) is added to GetByStatus and threaded through computeFor. The prototype-walk inside computeFor (the attempToFold() block) is now gated on mode == LookupMode::Normal. Node::propertyLookupMode() maps DFG opcodes to the correct mode. DFGAbstractInterpreterInlines.h::executeEffects and DFGConstantFoldingPhase::foldConstants now derive the mode from the node. op_get_from_scope in DFGByteCodeParser is explicitly pinned to Normal because global property lookup is supposed to walk the global object's prototype chain. The stale comment claiming the function only looks at direct properties is removed.

Speculation-soundness violation in the JIT IC status computation: a shared helper applies prototype-chain semantics to opcodes whose runtime semantics forbid prototype traversal.

Background

GetByStatus is JSC's summary of what a GetById-family bytecode has done at runtime: observed structures, offsets, and prototype walks, so the DFG can decide whether to inline a load, constant-fold a result, or fall back to a generic IC. GetById performs a full ECMAScript [[Get]] which walks the prototype chain. GetByIdDirect performs an own-property-only access; it must return undefined if the property is not own. GetPrivateNameById reads a private class field, keyed by a brand on the receiver, strictly own-property by spec. attempToFold inside computeFor is the proto-walk helper that searches the chain for the property and folds it into a Simple variant when found. The DFG's AbstractInterpreter and ConstantFoldingPhase consume the returned GetByStatus to specialise the IR.

Analysis

GetByStatus::computeFor always invoked attempToFold(), which walked the prototype chain. It was called from DFG passes for every GetBy* node, including GetByIdDirect, GetByIdDirectFlush, and GetPrivateNameById — opcodes whose runtime semantics require own-property-only access. The stale comment claimed the function only looked at direct properties, which was true when written but became false once attempToFold was added.

The result: DFG could fold or specialise a direct-property load as if it had resolved on the prototype, producing a GetByStatus::Simple variant whose offset/structure points at the prototype object's slot rather than reflecting the correct own-property-or-undefined semantics. If the JIT subsequently emits code that uses that variant's structure/offset to materialise a value, the generated code could return a prototype slot when runtime semantics require undefined (for GetByIdDirect) or could skirt the brand/own-slot semantics of private fields (for GetPrivateNameById). Depending on whether downstream lowering still inserts a sufficient structure check, the consequence ranges from a semantic deviation usable to confuse type-tracking up to a controlled property fetch from a non-own slot.

This vulnerability weakened the soundness of JIT speculation for own-property-only operations. The invariant being violated is that GetByIdDirect/GetByIdDirectFlush/GetPrivateNameById must not be optimised under prototype-walk assumptions; the pre-fix code allowed the DFG to build a Simple GetByStatus that promised a property resolution found via prototype traversal. Stale comments are a recurring source of latent JIT speculation bugs: the old comment correctly described the function's behaviour at the time it was written, but attempToFold was added later without updating either the comment or the call sites that depended on the original contract. The introduction of an explicit LookupMode with RELEASE_ASSERT_NOT_REACHED() in propertyLookupMode()'s default arm is the correct mitigation: every new GetBy* opcode must declare its lookup semantics at the call boundary.

Audit directions

Shared IC/status helpers that quietly assume one opcode family's semantics get reused by a sibling family with different semantics. Audit other JSC status computers — PutByStatus, InByStatus, DeleteByStatus, CheckPrivateBrandStatus, SetPrivateBrandStatus — for the same shape: a single computeFor overload consumed by both prototype-walking and own-property-only DFG nodes without a mode parameter. Start with Source/JavaScriptCore/bytecode/PutByStatus.cpp and InByStatus.cpp.
Private-name opcodes sharing lowering paths with normal property opcodes. Audit GetPrivateName*, PutPrivateName*, CheckPrivateBrand, and SetPrivateBrand in DFG, FTL, and B3 lowering for any helper that consults the prototype chain. Grep attempToFold, prototypeChainIsSane, and walkPrototypeChain callers and verify each treats private-name opcodes as direct.
Stale comments documenting a contract silently broken when the helper was extended. Grep IC/status helpers in Source/JavaScriptCore/bytecode/ for phrases like "this function only does X" / "only looks" / "only handles" / "does not" and verify the current code actually does only X.
Verify that Node::propertyLookupMode() exhausts all GetBy* opcodes that can reach the two updated callers — the default: RELEASE_ASSERT_NOT_REACHED() arm will crash in release if a new opcode is added without updating the switch. Check whether GetByIdWithThis, TryGetById, or any inline-cache megamorphic variants can reach those call sites without being listed.