[JSC] `Heap::clearConcurrentRetainedDataIfPossible()` should not run while concurrent marking is active

c8e53c7

Source/JavaScriptCore/heap/Heap.cpp

JSC runs concurrent marker threads that traverse the object graph in parallel with the mutator. When a marker visits a JSString, it loads the underlying StringImpl pointer via fiberConcurrently() and dereferences it (e.g., in StringImpl::costDuringGC called from JSString::estimatedSize) without incrementing the refcount — the m_possiblyAccessedStringsFromConcurrentThreadsOrGCOwnedDataScope list is the sole mechanism keeping those impls alive during this window. A between-GC cleanup path introduced in 314730@main was designed to prune this list via the IncrementalSweeper timer, but it only guarded against JS execution, active GCOwnedDataScopes, and in-flight JIT compilations — not concurrent marking.

This commit closes that gap by adding a mutatorShouldBeFenced() early-return. mutatorShouldBeFenced() is the canonical flag already used by write barriers to mean "markers may be running." Applying it here means the IncrementalSweeper timer skips the clear whenever a marker could still be dereferencing one of the retained impls.

Mutator (IncrementalSweeper, ~100ms timer)        Collector (marker)

  clearConcurrentRetainedDataIfPossible()           SlotVisitor::drain()
          │                                                   │
          │                                       JSString::visitChildrenImpl()
          │                                                   │
          ▼                                       fiberConcurrently() → StringImpl*  ← no ref held
  retainedList.clear()  ◄────── UAF ─────────────────────────┤
  (StringImpl freed)                              StringImpl::costDuringGC(*impl)  ← dangling read

After fix:
  if (mutatorShouldBeFenced()) return;  // markers active → defer clear to next timer fire

Significance

This is an ASAN-confirmed UAF on the GC collector thread — a class of memory safety bug that is frequently exploitable. The fix itself is a one-liner, but the underlying pattern (GC-retained data accessed ref-free by marker threads, with a separate clearing path added later) is a template for similar oversights elsewhere in the heap subsystem. The fix only defers the clear; the timer re-fires every 100ms, so if marking runs long or is triggered rapidly, the retained list grows unboundedly during the deferral window.

Audit directions