Verified Commit 3e75dcee authored by Yury Delendik's avatar Yury Delendik Committed by ma1
Browse files

Bug 2045435 - Prune dying entries from wasm::Realm::instances_ during sweeping. r=jpages 

wasm::Realm::instances_ is a weak list. Readers rely on the
instances()[i]->object() read barrier, but that barrier is a no-op once the
owning zone is being incrementally swept, and entries are otherwise only
removed at Instance finalization (~Instance -> unregisterInstance). So between
marking a zone's instance objects dead and finalizing them, the list could
still hand an about-to-be-finalized instance to a reader.

Prune such entries at the start of zone sweeping via a new
wasm::Realm::traceWeakInstances(), called from beginSweepingSweepGroup
alongside the other per-realm weak-collection sweeps. This makes instances_
behave like the engine's other weak collections, so it never exposes an
about-to-be-finalized instance to the mutator during sweep slices.

Differential Revision: https://phabricator.services.mozilla.com/D314032
parent 1f8e95ba

js/src/gc/GCRuntime.h

+1 −0
Original line number Diff line number Diff line
@@ -999,6 +999,7 @@ class GCRuntime { 
  void updateAtomsBitmap();

  void sweepCCWrappers();

  void sweepRealmGlobals();

  void sweepWasmInstances();

  void sweepEmbeddingWeakPointers(JS::GCContext* gcx);

  void sweepMisc();

  void sweepCompressionTasks();

js/src/gc/Sweeping.cpp

+12 −0
Original line number Diff line number Diff line
@@ -1387,6 +1387,13 @@ void GCRuntime::sweepRealmGlobals() { 
  }

}



void GCRuntime::sweepWasmInstances() {

  for (SweepGroupRealmsIter r(this); !r.done(); r.next()) {

    AutoSetThreadIsSweeping threadIsSweeping(r->zone());

    r->wasm.traceWeakInstances();

  }

}



void GCRuntime::sweepMisc() {

  SweepingTracer trc(rt);

  for (SweepGroupRealmsIter r(this); !r.done(); r.next()) {
@@ -1743,6 +1750,11 @@ IncrementalProgress GCRuntime::beginSweepingSweepGroup(JS::GCContext* gcx, 
  // This must happen before updating embedding weak pointers.

  sweepRealmGlobals();



  // Prune dying wasm instances from each realm's weak instance list now, at the

  // start of sweeping, before the mutator can observe them via the (now no-op)

  // instances() read barrier during later incremental slices.

  sweepWasmInstances();



  sweepEmbeddingWeakPointers(gcx);



  maybeWriteCoverageAndSpew();

js/src/wasm/WasmRealm.cpp

+14 −0
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


#include "wasm/WasmRealm.h"



#include "gc/Marking.h"

#include "vm/GlobalObject.h"

#include "vm/Realm.h"

#include "wasm/WasmDebug.h"
@@ -111,6 +112,19 @@ void wasm::Realm::unregisterInstance(Instance& instance) { 
  }

}



void wasm::Realm::traceWeakInstances() {

  // Registration/unregistration of instances_ is tied to Instance lifetime, so

  // an instance whose owning object is about to be finalized is still present

  // here until ~Instance runs. Remove such entries now, at the start of zone

  // sweeping, because the instances() read barrier that otherwise protects

  // readers is a no-op once the zone is being swept. erase order is preserved,

  // so the pointer-sorted invariant used by BinarySearchIf holds.

  instances_.eraseIf([](Instance* instance) {

    return js::gc::IsAboutToBeFinalizedUnbarriered(

        instance->objectUnbarriered());

  });

}



void wasm::Realm::ensureProfilingLabels(bool profilingEnabled) {

  for (Instance* instance : instances_) {

    instance->ensureProfilingLabels(profilingEnabled);

js/src/wasm/WasmRealm.h

+8 −1
Original line number Diff line number Diff line
@@ -51,10 +51,17 @@ class Realm { 
  // Return a vector of all live instances in the realm. The lifetime of

  // these Instances is determined by their owning WasmInstanceObject.

  // Note that accessing instances()[i]->object() triggers a read barrier

  // since instances() is effectively a weak list.

  // since instances() is effectively a weak list. This read barrier is only

  // effective while the owning zone is being marked; traceWeakInstances()

  // prunes dying entries at the start of sweeping so that the list never

  // exposes an about-to-be-finalized instance to the mutator.



  const InstanceVector& instances() const { return instances_; }



  // Remove instances whose owning object is about to be finalized. Called at

  // the start of zone sweeping, when the instances() read barrier is a no-op.

  void traceWeakInstances();



  // Ensure all Instances in this Realm have profiling labels created.



  void ensureProfilingLabels(bool profilingEnabled);