Bug 1520778 - Ensure implicit edges are marked on all paths through the marking code r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Mon, 21 Jan 2019 12:40:52 +0000
changeset 514709 48810cb8ba3fb643308bbf2c5400e1b42042ca45
parent 514683 c311812c530c03588f7ac2fac4fd9f6c38103529
child 514710 d323b050b0ce2ae624fc09e8f7d204c5016d3f9e
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1520778
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1520778 - Ensure implicit edges are marked on all paths through the marking code r=sfink
js/src/gc/Marking.cpp
js/src/jit-test/tests/gc/bug-1520778.js
js/src/vm/JSObject.cpp
js/src/vm/JSScript.cpp
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -90,30 +90,30 @@ using mozilla::PodCopy;
 //                           '-----------------'                                                //
 //                              /          \                                                    //
 //                             /            \                                                   //
 //                       .---------.   .----------.         .-----------------.                 //
 //                       |DoMarking|   |DoCallback|-------> |<JSTraceCallback>|----------->     //
 //                       '---------'   '----------'         '-----------------'                 //
 //                            |                                                                 //
 //                            |                                                                 //
-//                        .--------.                                                            //
-//      o---------------->|traverse| .                                                          //
-//     /_\                '--------'   ' .                                                      //
-//      |                     .     .      ' .                                                  //
-//      |                     .       .        ' .                                              //
-//      |                     .         .          ' .                                          //
-//      |             .-----------.    .-----------.   ' .     .--------------------.           //
-//      |             |markAndScan|    |markAndPush|       ' - |markAndTraceChildren|---->      //
-//      |             '-----------'    '-----------'           '--------------------'           //
-//      |                   |                  \                                                //
-//      |                   |                   \                                               //
-//      |       .----------------------.     .----------------.                                 //
-//      |       |T::eagerlyMarkChildren|     |pushMarkStackTop|<===Oo                           //
-//      |       '----------------------'     '----------------'    ||                           //
+//                     .-----------.                                                            //
+//      o------------->|traverse(T)| .                                                          //
+//     /_\             '-----------'   ' .                                                      //
+//      |                   .       .      ' .                                                  //
+//      |                   .         .        ' .                                              //
+//      |                   .           .          ' .                                          //
+//      |          .--------------.  .--------------.  ' .     .-----------------------.        //
+//      |          |markAndScan(T)|  |markAndPush(T)|      ' - |markAndTraceChildren(T)|        //
+//      |          '--------------'  '--------------'          '-----------------------'        //
+//      |                   |                  \                               |                //
+//      |                   |                   \                              |                //
+//      |       .----------------------.     .----------------.         .------------------.    //
+//      |       |eagerlyMarkChildren(T)|     |pushMarkStackTop|<===Oo   |T::traceChildren()|--> //
+//      |       '----------------------'     '----------------'    ||   '------------------'    //
 //      |                  |                         ||            ||                           //
 //      |                  |                         ||            ||                           //
 //      |                  |                         ||            ||                           //
 //      o<-----------------o<========================OO============Oo                           //
 //                                                                                              //
 //                                                                                              //
 //   Legend:                                                                                    //
 //     ------  Direct calls                                                                     //
@@ -842,17 +842,16 @@ void js::GCMarker::markAndScan(T* thing)
 namespace js {
 template <>
 void GCMarker::traverse(JSString* thing) {
   markAndScan(thing);
 }
 template <>
 void GCMarker::traverse(LazyScript* thing) {
   markAndScan(thing);
-  markImplicitEdges(thing);
 }
 template <>
 void GCMarker::traverse(Shape* thing) {
   markAndScan(thing);
 }
 template <>
 void GCMarker::traverse(js::Scope* thing) {
   markAndScan(thing);
@@ -1027,17 +1026,17 @@ void LazyScript::traceChildren(JSTracer*
   }
 
   GCPtrFunction* innerFunctions = this->innerFunctions();
   for (auto i : IntegerRange(numInnerFunctions())) {
     TraceEdge(trc, &innerFunctions[i], "lazyScriptInnerFunction");
   }
 
   if (trc->isMarkingTracer()) {
-    return GCMarker::fromTracer(trc)->markImplicitEdges(this);
+    GCMarker::fromTracer(trc)->markImplicitEdges(this);
   }
 }
 inline void js::GCMarker::eagerlyMarkChildren(LazyScript* thing) {
   if (thing->script_) {
     noteWeakEdge(thing->script_.unsafeUnbarrieredForTracing());
   }
 
   if (thing->function_) {
@@ -1063,16 +1062,18 @@ inline void js::GCMarker::eagerlyMarkChi
       traverseEdge(thing, static_cast<JSString*>(closedOverBindings[i]));
     }
   }
 
   GCPtrFunction* innerFunctions = thing->innerFunctions();
   for (auto i : IntegerRange(thing->numInnerFunctions())) {
     traverseEdge(thing, static_cast<JSObject*>(innerFunctions[i]));
   }
+
+  markImplicitEdges(thing);
 }
 
 void Shape::traceChildren(JSTracer* trc) {
   TraceEdge(trc, &base_, "base");
   TraceEdge(trc, &propidRef(), "propid");
   if (parent) {
     TraceEdge(trc, &parent, "parent");
   }
@@ -1096,17 +1097,17 @@ inline void js::GCMarker::eagerlyMarkChi
     CheckTraversedEdge(shape, base);
     if (mark(base)) {
       MOZ_ASSERT(base->canSkipMarkingShapeTable(shape));
       base->traceChildrenSkipShapeTable(this);
     }
 
     traverseEdge(shape, shape->propidRef().get());
 
-    // When triggered between slices on belhalf of a barrier, these
+    // When triggered between slices on behalf of a barrier, these
     // objects may reside in the nursery, so require an extra check.
     // FIXME: Bug 1157967 - remove the isTenured checks.
     if (shape->hasGetterObject() && shape->getterObject()->isTenured()) {
       traverseEdge(shape, shape->getterObject());
     }
     if (shape->hasSetterObject() && shape->setterObject()->isTenured()) {
       traverseEdge(shape, shape->setterObject());
     }
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/gc/bug-1520778.js
@@ -0,0 +1,18 @@
+// |jit-test| error: ReferenceError
+gczeal(0);
+gcparam("markStackLimit", 1);
+var g = newGlobal({
+    newCompartment: true
+});
+var dbg = new Debugger;
+var gw = dbg.addDebuggee(g);
+dbg.onDebuggerStatement = function(frame) {
+  frame.environment.parent.getVariable('y')
+};
+g.eval(`
+  let y = 1;
+  g = function () { debugger; };
+  g();
+`);
+gczeal(9, 10);
+f4();
--- a/js/src/vm/JSObject.cpp
+++ b/js/src/vm/JSObject.cpp
@@ -4197,16 +4197,20 @@ void JSObject::traceChildren(JSTracer* t
     } while (false);
   }
 
   // Call the trace hook at the end so that during a moving GC the trace hook
   // will see updated fields and slots.
   if (clasp->hasTrace()) {
     clasp->doTrace(trc, this);
   }
+
+  if (trc->isMarkingTracer()) {
+    GCMarker::fromTracer(trc)->markImplicitEdges(this);
+  }
 }
 
 static JSAtom* displayAtomFromObjectGroup(ObjectGroup& group) {
   AutoSweepObjectGroup sweep(&group);
   TypeNewScript* script = group.newScript(sweep);
   if (!script) {
     return nullptr;
   }
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -4406,17 +4406,17 @@ void JSScript::traceChildren(JSTracer* t
 
   if (trc->isMarkingTracer()) {
     realm()->mark();
   }
 
   jit::TraceJitScripts(trc, this);
 
   if (trc->isMarkingTracer()) {
-    return GCMarker::fromTracer(trc)->markImplicitEdges(this);
+    GCMarker::fromTracer(trc)->markImplicitEdges(this);
   }
 }
 
 void LazyScript::finalize(FreeOp* fop) { fop->free_(table_); }
 
 size_t JSScript::calculateLiveFixed(jsbytecode* pc) {
   size_t nlivefixed = numAlwaysLiveFixedSlots();