Bug 1502886 - Delete wasm breakpoints when the wasm instance's Zone is swept. r=jonco, a=RyanVM
authorBenjamin Bouvier <benj@benj.me>
Mon, 29 Oct 2018 18:55:17 +0100
changeset 501089 f65ddbf6c00545fcf8f56df06ee2a7e3811e5faf
parent 501088 522a5434e0a42890c5292c0228eea366e7dabafd
child 501090 12500028fe79390ae7d2d4128acf92d2c0d18733
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco, RyanVM
bugs1502886
milestone64.0
Bug 1502886 - Delete wasm breakpoints when the wasm instance's Zone is swept. r=jonco, a=RyanVM
js/src/gc/Zone.cpp
js/src/vm/Debugger.cpp
js/src/vm/Debugger.h
js/src/wasm/WasmDebug.cpp
js/src/wasm/WasmDebug.h
js/src/wasm/WasmInstance.h
--- a/js/src/gc/Zone.cpp
+++ b/js/src/gc/Zone.cpp
@@ -9,16 +9,17 @@
 #include "gc/FreeOp.h"
 #include "gc/Policy.h"
 #include "gc/PublicIterators.h"
 #include "jit/BaselineJIT.h"
 #include "jit/Ion.h"
 #include "jit/JitRealm.h"
 #include "vm/Debugger.h"
 #include "vm/Runtime.h"
+#include "wasm/WasmInstance.h"
 
 #include "gc/GC-inl.h"
 #include "gc/Marking-inl.h"
 #include "vm/Realm-inl.h"
 
 using namespace js;
 using namespace js::gc;
 
@@ -181,16 +182,28 @@ Zone::sweepBreakpoints(FreeOp* fop)
                 bool dying = scriptGone || IsAboutToBeFinalized(&dbgobj);
                 MOZ_ASSERT_IF(!dying, !IsAboutToBeFinalized(&bp->getHandlerRef()));
                 if (dying) {
                     bp->destroy(fop);
                 }
             }
         }
     }
+
+    for (RealmsInZoneIter realms(this); !realms.done(); realms.next()) {
+        for (wasm::Instance* instance : realms->wasm.instances()) {
+            if (!instance->debugEnabled()) {
+                continue;
+            }
+            if (!IsAboutToBeFinalized(&instance->object_)) {
+                continue;
+            }
+            instance->debug().clearAllBreakpoints(fop, instance->objectUnbarriered());
+        }
+    }
 }
 
 void
 Zone::sweepWeakMaps()
 {
     /* Finalize unreachable (key,value) pairs in all weak maps. */
     WeakMapBase::sweepZone(this);
 }
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -613,24 +613,26 @@ Breakpoint::Breakpoint(Debugger* debugge
   : debugger(debugger), site(site), handler(handler)
 {
     MOZ_ASSERT(handler->compartment() == debugger->object->compartment());
     debugger->breakpoints.pushBack(this);
     site->breakpoints.pushBack(this);
 }
 
 void
-Breakpoint::destroy(FreeOp* fop)
+Breakpoint::destroy(FreeOp* fop, MayDestroySite mayDestroySite /* true */)
 {
     if (debugger->enabled) {
         site->dec(fop);
     }
     debugger->breakpoints.remove(this);
     site->breakpoints.remove(this);
-    site->destroyIfEmpty(fop);
+    if (mayDestroySite == MayDestroySite::True) {
+        site->destroyIfEmpty(fop);
+    }
     fop->delete_(this);
 }
 
 Breakpoint*
 Breakpoint::nextInDebugger()
 {
     return debuggerLink.mNext;
 }
@@ -4346,18 +4348,18 @@ Debugger::removeDebuggeeGlobal(FreeOp* f
         DebuggerFrame* frameobj = e.front().value();
         if (frame.global() == global) {
             frameobj->freeFrameIterData(fop);
             DebuggerFrame_maybeDecrementFrameScriptStepModeCount(fop, frame, frameobj);
             e.removeFront();
         }
     }
 
-    auto *globalDebuggersVector = global->getDebuggers();
-    auto *zoneDebuggersVector = global->zone()->getDebuggers();
+    auto* globalDebuggersVector = global->getDebuggers();
+    auto* zoneDebuggersVector = global->zone()->getDebuggers();
 
     // The relation must be removed from up to three places:
     // globalDebuggersVector and debuggees for sure, and possibly the
     // compartment's debuggee set.
     //
     // The debuggee zone set is recomputed on demand. This avoids refcounting
     // and in practice we have relatively few debuggees that tend to all be in
     // the same zone. If after recomputing the debuggee zone set, this global's
@@ -4387,16 +4389,18 @@ Debugger::removeDebuggeeGlobal(FreeOp* f
                 bp->destroy(fop);
             }
             break;
           case BreakpointSite::Type::Wasm:
             if (bp->asWasm()->wasmInstance->realm() == global->realm()) {
                 bp->destroy(fop);
             }
             break;
+          default:
+            MOZ_CRASH("unknown breakpoint type");
         }
     }
     MOZ_ASSERT_IF(debuggees.empty(), !firstBreakpoint());
 
     // If we are tracking allocation sites, we need to remove the object
     // metadata callback from this global's realm.
     if (trackingAllocationSites) {
         Debugger::removeAllocationsTracking(*global);
@@ -7381,17 +7385,21 @@ DebuggerScript_getBreakpoints(JSContext*
 
 class DebuggerScriptClearBreakpointMatcher
 {
     JSContext* cx_;
     Debugger* dbg_;
     JSObject* handler_;
 
   public:
-    explicit DebuggerScriptClearBreakpointMatcher(JSContext* cx, Debugger* dbg, JSObject* handler) : cx_(cx), dbg_(dbg), handler_(handler) { }
+    DebuggerScriptClearBreakpointMatcher(JSContext* cx, Debugger* dbg, JSObject* handler)
+      : cx_(cx),
+        dbg_(dbg),
+        handler_(handler)
+    { }
     using ReturnType = bool;
 
     ReturnType match(HandleScript script) {
         script->clearBreakpointsIn(cx_->runtime()->defaultFreeOp(), dbg_, handler_);
         return true;
     }
     ReturnType match(Handle<LazyScript*> lazyScript) {
         RootedScript script(cx_, DelazifyScript(cx_, lazyScript));
@@ -7400,17 +7408,19 @@ class DebuggerScriptClearBreakpointMatch
         }
         return match(script);
     }
     ReturnType match(Handle<WasmInstanceObject*> instanceObj) {
         wasm::Instance& instance = instanceObj->instance();
         if (!instance.debugEnabled()) {
             return true;
         }
-        return instance.debug().clearBreakpointsIn(cx_, instanceObj, dbg_, handler_);
+        instance.debug().clearBreakpointsIn(cx_->runtime()->defaultFreeOp(), instanceObj, dbg_,
+                                            handler_);
+        return true;
     }
 };
 
 
 static bool
 DebuggerScript_clearBreakpoint(JSContext* cx, unsigned argc, Value* vp)
 {
     THIS_DEBUGSCRIPT_REFERENT(cx, argc, vp, "clearBreakpoint", args, obj, referent);
--- a/js/src/vm/Debugger.h
+++ b/js/src/vm/Debugger.h
@@ -1651,28 +1651,28 @@ class BreakpointSite {
     using BreakpointList =
         mozilla::DoublyLinkedList<js::Breakpoint,
                                   SiteLinkAccess<js::Breakpoint>>;
     BreakpointList breakpoints;
     size_t enabledCount;  /* number of breakpoints in the list that are enabled */
 
   protected:
     virtual void recompile(FreeOp* fop) = 0;
-    bool isEmpty() const;
     inline bool isEnabled() const { return enabledCount > 0; }
 
   public:
     BreakpointSite(Type type);
     Breakpoint* firstBreakpoint() const;
     virtual ~BreakpointSite() {}
     bool hasBreakpoint(Breakpoint* bp);
     inline Type type() const { return type_; }
 
     void inc(FreeOp* fop);
     void dec(FreeOp* fop);
+    bool isEmpty() const;
     virtual void destroyIfEmpty(FreeOp* fop) = 0;
 
     inline JSBreakpointSite* asJS();
     inline WasmBreakpointSite* asWasm();
 };
 
 /*
  * Each Breakpoint is a member of two linked lists: its debugger's list and its
@@ -1706,17 +1706,23 @@ class Breakpoint {
     /**
      * Link elements for each list this breakpoint can be in.
      */
     mozilla::DoublyLinkedListElement<Breakpoint> debuggerLink;
     mozilla::DoublyLinkedListElement<Breakpoint> siteLink;
 
   public:
     Breakpoint(Debugger* debugger, BreakpointSite* site, JSObject* handler);
-    void destroy(FreeOp* fop);
+
+    enum MayDestroySite {
+        False,
+        True
+    };
+    void destroy(FreeOp* fop, MayDestroySite mayDestroySite = MayDestroySite::True);
+
     Breakpoint* nextInDebugger();
     Breakpoint* nextInSite();
     const PreBarrieredObject& getHandler() const { return handler; }
     PreBarrieredObject& getHandlerRef() { return handler; }
 
     inline WasmBreakpoint* asWasm();
 };
 
--- a/js/src/wasm/WasmDebug.cpp
+++ b/js/src/wasm/WasmDebug.cpp
@@ -241,48 +241,47 @@ void
 DebugState::destroyBreakpointSite(FreeOp* fop, uint32_t offset)
 {
     WasmBreakpointSiteMap::Ptr p = breakpointSites_.lookup(offset);
     MOZ_ASSERT(p);
     fop->delete_(p->value());
     breakpointSites_.remove(p);
 }
 
-bool
-DebugState::clearBreakpointsIn(JSContext* cx, WasmInstanceObject* instance, js::Debugger* dbg, JSObject* handler)
+void
+DebugState::clearBreakpointsIn(FreeOp* fop, WasmInstanceObject* instance, js::Debugger* dbg,
+                               JSObject* handler)
 {
     MOZ_ASSERT(instance);
     if (breakpointSites_.empty()) {
-        return true;
+        return;
     }
-
-    // Make copy of all sites list, so breakpointSites_ can be modified by
-    // destroyBreakpointSite calls.
-    Vector<WasmBreakpointSite*> sites(cx);
-    if (!sites.resize(breakpointSites_.count())) {
-        return false;
-    }
-    size_t i = 0;
-    for (WasmBreakpointSiteMap::Range r = breakpointSites_.all(); !r.empty(); r.popFront()) {
-        sites[i++] = r.front().value();
-    }
-
-    for (WasmBreakpointSite* site : sites) {
+    for (WasmBreakpointSiteMap::Enum e(breakpointSites_); !e.empty(); e.popFront()) {
+        WasmBreakpointSite* site = e.front().value();
         Breakpoint* nextbp;
         for (Breakpoint* bp = site->firstBreakpoint(); bp; bp = nextbp) {
             nextbp = bp->nextInSite();
             if (bp->asWasm()->wasmInstance == instance &&
                 (!dbg || bp->debugger == dbg) &&
                 (!handler || bp->getHandler() == handler))
             {
-                bp->destroy(cx->runtime()->defaultFreeOp());
+                bp->destroy(fop, Breakpoint::MayDestroySite::False);
             }
         }
+        if (site->isEmpty()) {
+            fop->delete_(site);
+            e.removeFront();
+        }
     }
-    return true;
+}
+
+void
+DebugState::clearAllBreakpoints(FreeOp* fop, WasmInstanceObject* instance)
+{
+    clearBreakpointsIn(fop, instance, nullptr, nullptr);
 }
 
 void
 DebugState::toggleDebugTrap(uint32_t offset, bool enabled)
 {
     MOZ_ASSERT(offset);
     uint8_t* trap = code_->segment(Tier::Debug).base() + offset;
     const Uint32Vector& farJumpOffsets = metadata(Tier::Debug).debugTrapFarJumpOffsets;
--- a/js/src/wasm/WasmDebug.h
+++ b/js/src/wasm/WasmDebug.h
@@ -91,17 +91,19 @@ class DebugState
     // When the Code is debugEnabled, individual breakpoints can be enabled or
     // disabled at instruction offsets.
 
     bool hasBreakpointTrapAtOffset(uint32_t offset);
     void toggleBreakpointTrap(JSRuntime* rt, uint32_t offset, bool enabled);
     WasmBreakpointSite* getOrCreateBreakpointSite(JSContext* cx, uint32_t offset);
     bool hasBreakpointSite(uint32_t offset);
     void destroyBreakpointSite(FreeOp* fop, uint32_t offset);
-    bool clearBreakpointsIn(JSContext* cx, WasmInstanceObject* instance, js::Debugger* dbg, JSObject* handler);
+    void clearBreakpointsIn(FreeOp* fp, WasmInstanceObject* instance, js::Debugger* dbg,
+                            JSObject* handler);
+    void clearAllBreakpoints(FreeOp* fp, WasmInstanceObject* instance);
 
     // When the Code is debug-enabled, single-stepping mode can be toggled on
     // the granularity of individual functions.
 
     bool stepModeEnabled(uint32_t funcIndex) const;
     bool incrementStepModeCount(JSContext* cx, uint32_t funcIndex);
     bool decrementStepModeCount(FreeOp* fop, uint32_t funcIndex);
 
--- a/js/src/wasm/WasmInstance.h
+++ b/js/src/wasm/WasmInstance.h
@@ -53,16 +53,18 @@ class Instance
     const UniqueTlsData             tlsData_;
     GCPtrWasmMemoryObject           memory_;
     const SharedTableVector         tables_;
     DataSegmentVector               passiveDataSegments_;
     ElemSegmentVector               passiveElemSegments_;
     const UniqueDebugState          maybeDebug_;
     StructTypeDescrVector           structTypeDescrs_;
 
+    friend void Zone::sweepBreakpoints(js::FreeOp*);
+
     // Internal helpers:
     const void** addressOfFuncTypeId(const FuncTypeIdDesc& funcTypeId) const;
     FuncImportTls& funcImportTls(const FuncImport& fi);
     TableTls& tableTls(const TableDesc& td) const;
 
     // Only WasmInstanceObject can call the private trace function.
     friend class js::WasmInstanceObject;
     void tracePrivate(JSTracer* trc);