Bug 1028863 - Root SavedFrame::Lookup while creating new SavedFrame r=terrence

js/src/gc/Marking.cpp

@@ -591,6 +591,7 @@ DeclMarkerImpl(Object, GlobalObject)
 DeclMarkerImpl(Object, JSObject)
 DeclMarkerImpl(Object, JSFunction)
 DeclMarkerImpl(Object, ObjectImpl)
+DeclMarkerImpl(Object, SavedFrame)
 DeclMarkerImpl(Object, ScopeObject)
 DeclMarkerImpl(Script, JSScript)
 DeclMarkerImpl(LazyScript, LazyScript)

js/src/gc/Marking.h

@@ -23,6 +23,7 @@ class DebugScopeObject;
 class GCMarker;
 class GlobalObject;
 class LazyScript;
+class SavedFrame;
 class ScopeObject;
 class Shape;
 class UnownedBaseShape;
@@ -111,6 +112,7 @@ DeclMarker(Object, DebugScopeObject)
 DeclMarker(Object, GlobalObject)
 DeclMarker(Object, JSObject)
 DeclMarker(Object, JSFunction)
+DeclMarker(Object, SavedFrame)
 DeclMarker(Object, ScopeObject)
 DeclMarker(Script, JSScript)
 DeclMarker(LazyScript, LazyScript)

js/src/jit-test/tests/gc/bug-1028863.js

@@ -0,0 +1,3 @@
+function writeTestCaseResult( expect, actual, string ) {}
+schedulegc(10);
+if (saveStack() == 3) done = true;

js/src/jsgc.h

@@ -41,6 +41,7 @@ class GlobalObject;
 class LazyScript;
 class Nursery;
 class PropertyName;
+class SavedFrame;
 class ScopeObject;
 class Shape;
 class UnownedBaseShape;
@@ -143,6 +144,7 @@ template <> struct MapTypeToTraceKind<SharedArrayBufferObject>{ static const JSG
 template <> struct MapTypeToTraceKind<DebugScopeObject> { static const JSGCTraceKind kind = JSTRACE_OBJECT; };
 template <> struct MapTypeToTraceKind<GlobalObject>     { static const JSGCTraceKind kind = JSTRACE_OBJECT; };
 template <> struct MapTypeToTraceKind<ScopeObject>      { static const JSGCTraceKind kind = JSTRACE_OBJECT; };
+template <> struct MapTypeToTraceKind<SavedFrame>       { static const JSGCTraceKind kind = JSTRACE_OBJECT; };
 template <> struct MapTypeToTraceKind<JSScript>         { static const JSGCTraceKind kind = JSTRACE_SCRIPT; };
 template <> struct MapTypeToTraceKind<LazyScript>       { static const JSGCTraceKind kind = JSTRACE_LAZY_SCRIPT; };
 template <> struct MapTypeToTraceKind<Shape>            { static const JSGCTraceKind kind = JSTRACE_SHAPE; };

js/src/vm/SavedStacks.cpp

@@ -22,6 +22,51 @@ using mozilla::HashString;
 
 namespace js {
 
+struct SavedFrame::Lookup {
+    Lookup(JSAtom *source, size_t line, size_t column, JSAtom *functionDisplayName,
+           SavedFrame *parent, JSPrincipals *principals)
+        : source(source),
+          line(line),
+          column(column),
+          functionDisplayName(functionDisplayName),
+          parent(parent),
+          principals(principals)
+    {
+        JS_ASSERT(source);
+    }
+
+    JSAtom       *source;
+    size_t       line;
+    size_t       column;
+    JSAtom       *functionDisplayName;
+    SavedFrame   *parent;
+    JSPrincipals *principals;
+};
+
+class SavedFrame::AutoLookupRooter : public JS::CustomAutoRooter
+{
+  public:
+    AutoLookupRooter(JSContext *cx, JSAtom *source, size_t line, size_t column,
+                     JSAtom *functionDisplayName, SavedFrame *parent, JSPrincipals *principals)
+      : JS::CustomAutoRooter(cx),
+        value(source, line, column, functionDisplayName, parent, principals) {}
+
+    operator const SavedFrame::Lookup&() const { return value; }
+
+  private:
+    virtual void trace(JSTracer *trc) {
+        gc::MarkStringUnbarriered(trc, &value.source, "SavedFrame::Lookup::source");
+        if (value.functionDisplayName) {
+            gc::MarkStringUnbarriered(trc, &value.functionDisplayName,
+                                      "SavedFrame::Lookup::functionDisplayName");
+        }
+        if (value.parent)
+            gc::MarkObjectUnbarriered(trc, &value.parent, "SavedFrame::Lookup::parent");
+    }
+
+    SavedFrame::Lookup value;
+};
+
 /* static */ HashNumber
 SavedFrame::HashPolicy::hash(const Lookup &lookup)
 {
@@ -155,7 +200,7 @@ SavedFrame::getPrincipals()
 }
 
 void
-SavedFrame::initFromLookup(Lookup &lookup)
+SavedFrame::initFromLookup(const Lookup &lookup)
 {
     JS_ASSERT(lookup.source);
     JS_ASSERT(getReservedSlot(JSSLOT_SOURCE).isUndefined());
@@ -247,7 +292,7 @@ SavedFrame::checkThis(JSContext *cx, CallArgs &args, const char *fnName)
 //   - Rooted<SavedFrame *> frame (will be non-null)
 #define THIS_SAVEDFRAME(cx, argc, vp, fnName, args, frame)         \
     CallArgs args = CallArgsFromVp(argc, vp);                      \
-    Rooted<SavedFrame *> frame(cx, checkThis(cx, args, fnName));   \
+    RootedSavedFrame frame(cx, checkThis(cx, args, fnName));   \
     if (!frame)                                                    \
         return false
 
@@ -361,7 +406,7 @@ SavedStacks::init()
 }
 
 bool
-SavedStacks::saveCurrentStack(JSContext *cx, MutableHandle<SavedFrame*> frame)
+SavedStacks::saveCurrentStack(JSContext *cx, MutableHandleSavedFrame frame)
 {
     JS_ASSERT(initialized());
     JS_ASSERT(&cx->compartment()->savedStacks() == this);
@@ -389,12 +434,11 @@ SavedStacks::sweep(JSRuntime *rt)
                 }
 
                 if (obj != temp || parentMoved) {
-                    Rooted<SavedFrame*> parent(rt, frame->getParent());
                     e.rekeyFront(SavedFrame::Lookup(frame->getSource(),
                                                     frame->getLine(),
                                                     frame->getColumn(),
                                                     frame->getFunctionDisplayName(),
-                                                    parent,
+                                                    frame->getParent(),
                                                     frame->getPrincipals()),
                                  ReadBarriered<SavedFrame *>(frame));
                 }
@@ -429,7 +473,7 @@ SavedStacks::sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf)
 }
 
 bool
-SavedStacks::insertFrames(JSContext *cx, ScriptFrameIter &iter, MutableHandle<SavedFrame*> frame)
+SavedStacks::insertFrames(JSContext *cx, ScriptFrameIter &iter, MutableHandleSavedFrame frame)
 {
     if (iter.done()) {
         frame.set(nullptr);
@@ -445,7 +489,7 @@ SavedStacks::insertFrames(JSContext *cx, ScriptFrameIter &iter, MutableHandle<Sa
     JS_CHECK_RECURSION_DONT_REPORT(cx, return false);
 
     ScriptFrameIter thisFrame(iter);
-    Rooted<SavedFrame*> parentFrame(cx);
+    RootedSavedFrame parentFrame(cx);
     if (!insertFrames(cx, ++iter, &parentFrame))
         return false;
 
@@ -454,7 +498,8 @@ SavedStacks::insertFrames(JSContext *cx, ScriptFrameIter &iter, MutableHandle<Sa
         return false;
 
     JSFunction *callee = thisFrame.maybeCallee();
-    SavedFrame::Lookup lookup(location.source,
+    SavedFrame::AutoLookupRooter lookup(cx,
+                                        location.source,
                                         location.line,
                                         location.column,
                                         callee ? callee->displayAtom() : nullptr,
@@ -466,13 +511,13 @@ SavedStacks::insertFrames(JSContext *cx, ScriptFrameIter &iter, MutableHandle<Sa
 }
 
 SavedFrame *
-SavedStacks::getOrCreateSavedFrame(JSContext *cx, SavedFrame::Lookup &lookup)
+SavedStacks::getOrCreateSavedFrame(JSContext *cx, const SavedFrame::Lookup &lookup)
 {
     SavedFrame::Set::AddPtr p = frames.lookupForAdd(lookup);
     if (p)
         return *p;
 
-    Rooted<SavedFrame *> frame(cx, createFrameFromLookup(cx, lookup));
+    RootedSavedFrame frame(cx, createFrameFromLookup(cx, lookup));
     if (!frame)
         return nullptr;
 
@@ -508,7 +553,7 @@ SavedStacks::getOrCreateSavedFramePrototype(JSContext *cx)
 }
 
 SavedFrame *
-SavedStacks::createFrameFromLookup(JSContext *cx, SavedFrame::Lookup &lookup)
+SavedStacks::createFrameFromLookup(JSContext *cx, const SavedFrame::Lookup &lookup)
 {
     RootedObject proto(cx, getOrCreateSavedFramePrototype(cx));
     if (!proto)
@@ -578,7 +623,7 @@ SavedStacks::getLocation(JSContext *cx, JSScript *script, jsbytecode *pc,
 bool
 SavedStacksMetadataCallback(JSContext *cx, JSObject **pmetadata)
 {
-    Rooted<SavedFrame *> frame(cx);
+    RootedSavedFrame frame(cx);
     if (!cx->compartment()->savedStacks().saveCurrentStack(cx, &frame))
         return false;
     *pmetadata = frame;

js/src/vm/SavedStacks.h

@@ -48,8 +48,10 @@ class SavedFrame : public JSObject {
                     HashPolicy,
                     SystemAllocPolicy> Set;
 
+    class AutoLookupRooter;
+
   private:
-    void initFromLookup(Lookup &lookup);
+    void initFromLookup(const Lookup &lookup);
 
     enum {
         // The reserved slots in the SavedFrame class.
@@ -79,26 +81,9 @@ class SavedFrame : public JSObject {
     static SavedFrame *checkThis(JSContext *cx, CallArgs &args, const char *fnName);
 };
 
-struct SavedFrame::Lookup {
+typedef JS::Handle<SavedFrame*> HandleSavedFrame;
-    Lookup(JSAtom *source, size_t line, size_t column, JSAtom *functionDisplayName,
+typedef JS::MutableHandle<SavedFrame*> MutableHandleSavedFrame;
-           SavedFrame *parent, JSPrincipals *principals)
+typedef JS::Rooted<SavedFrame*> RootedSavedFrame;
-        : source(source),
-          line(line),
-          column(column),
-          functionDisplayName(functionDisplayName),
-          parent(parent),
-          principals(principals)
-    {
-        JS_ASSERT(source);
-    }
-
-    JSAtom       *source;
-    size_t       line;
-    size_t       column;
-    JSAtom       *functionDisplayName;
-    SavedFrame   *parent;
-    JSPrincipals *principals;
-};
 
 struct SavedFrame::HashPolicy
 {
@@ -119,7 +104,7 @@ class SavedStacks {
 
     bool     init();
     bool     initialized() const { return frames.initialized(); }
-    bool     saveCurrentStack(JSContext *cx, MutableHandle<SavedFrame*> frame);
+    bool     saveCurrentStack(JSContext *cx, MutableHandleSavedFrame frame);
     void     sweep(JSRuntime *rt);
     uint32_t count();
     void     clear();
@@ -130,12 +115,12 @@ class SavedStacks {
     SavedFrame::Set          frames;
     JSObject                 *savedFrameProto;
 
-    bool       insertFrames(JSContext *cx, ScriptFrameIter &iter, MutableHandle<SavedFrame*> frame);
+    bool       insertFrames(JSContext *cx, ScriptFrameIter &iter, MutableHandleSavedFrame frame);
-    SavedFrame *getOrCreateSavedFrame(JSContext *cx, SavedFrame::Lookup &lookup);
+    SavedFrame *getOrCreateSavedFrame(JSContext *cx, const SavedFrame::Lookup &lookup);
     // |SavedFrame.prototype| is created lazily and held weakly. It should only
     // be accessed through this method.
     JSObject   *getOrCreateSavedFramePrototype(JSContext *cx);
-    SavedFrame *createFrameFromLookup(JSContext *cx, SavedFrame::Lookup &lookup);
+    SavedFrame *createFrameFromLookup(JSContext *cx, const SavedFrame::Lookup &lookup);
 
     // Cache for memoizing PCToLineNumber lookups.