Bug 1034463 - Ensure that PCLocationMap only ever stores scripts from its own compartment. r=shu
authorNick Fitzgerald <fitzgen@mozilla.com>
Sat, 12 Jul 2014 12:54:00 -0400
changeset 215728 f555ff5c59b8450a6fb626ec8f313be35bdefaaf
parent 215727 7c7e14a34c49bdc18c684a6306cd0b4b3f84f671
child 215729 c274ab1b4086c248d1e0a21f33b38043c4f1f184
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu
bugs1034463
milestone33.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 1034463 - Ensure that PCLocationMap only ever stores scripts from its own compartment. r=shu
js/src/jscntxtinlines.h
js/src/vm/SavedStacks.cpp
js/src/vm/SavedStacks.h
--- a/js/src/jscntxtinlines.h
+++ b/js/src/jscntxtinlines.h
@@ -130,16 +130,17 @@ class CompartmentChecker
 
     void check(JSScript *script) {
         if (script)
             check(script->compartment());
     }
 
     void check(InterpreterFrame *fp);
     void check(AbstractFramePtr frame);
+    void check(SavedStacks *stacks);
 };
 #endif /* JS_CRASH_DIAGNOSTICS */
 
 /*
  * Don't perform these checks when called from a finalizer. The checking
  * depends on other objects not having been swept yet.
  */
 #define START_ASSERT_SAME_COMPARTMENT()                                       \
--- a/js/src/vm/SavedStacks.cpp
+++ b/js/src/vm/SavedStacks.cpp
@@ -11,16 +11,17 @@
 #include "jscompartment.h"
 #include "jsfriendapi.h"
 #include "jsnum.h"
 
 #include "gc/Marking.h"
 #include "vm/GlobalObject.h"
 #include "vm/StringBuffer.h"
 
+#include "jscntxtinlines.h"
 #include "jsobjinlines.h"
 
 using mozilla::AddToHash;
 using mozilla::HashString;
 
 namespace js {
 
 struct SavedFrame::Lookup {
@@ -394,17 +395,17 @@ SavedStacks::init()
 
     return frames.init();
 }
 
 bool
 SavedStacks::saveCurrentStack(JSContext *cx, MutableHandleSavedFrame frame, unsigned maxFrameCount)
 {
     JS_ASSERT(initialized());
-    JS_ASSERT(&cx->compartment()->savedStacks() == this);
+    assertSameCompartment(cx, this);
 
     FrameIter iter(cx, FrameIter::ALL_CONTEXTS, FrameIter::GO_THROUGH_SAVED);
     return insertFrames(cx, iter, frame, maxFrameCount);
 }
 
 void
 SavedStacks::sweep(JSRuntime *rt)
 {
@@ -499,18 +500,21 @@ SavedStacks::insertFrames(JSContext *cx,
     // potentially memoized location result and copy it into |location|. When we
     // do not have a |JSScript| for this frame (asm.js frames), we take a slow
     // path that doesn't employ memoization, and update |location|'s slots
     // directly.
     AutoLocationValueRooter location(cx);
     if (iter.hasScript()) {
         JSScript *script = iter.script();
         jsbytecode *pc = iter.pc();
-        if (!getLocation(cx, script, pc, &location))
-            return false;
+        {
+            AutoCompartment ac(cx, iter.compartment());
+            if (!cx->compartment()->savedStacks().getLocation(cx, script, pc, &location))
+                return false;
+        }
     } else {
         const char *filename = iter.scriptFilename();
         if (!filename)
             filename = "";
         location.get().source = Atomize(cx, filename, strlen(filename));
         if (!location.get().source)
             return false;
         uint32_t column;
@@ -590,23 +594,23 @@ SavedStacks::getOrCreateSavedFrameProtot
 
 SavedFrame *
 SavedStacks::createFrameFromLookup(JSContext *cx, const SavedFrame::Lookup &lookup)
 {
     RootedObject proto(cx, getOrCreateSavedFramePrototype(cx));
     if (!proto)
         return nullptr;
 
-    JS_ASSERT(proto->compartment() == cx->compartment());
+    assertSameCompartment(cx, proto);
 
     RootedObject global(cx, cx->compartment()->maybeGlobal());
     if (!global)
         return nullptr;
 
-    JS_ASSERT(global->compartment() == cx->compartment());
+    assertSameCompartment(cx, global);
 
     RootedObject frameObj(cx, NewObjectWithGivenProto(cx, &SavedFrame::class_, proto, global));
     if (!frameObj)
         return nullptr;
 
     SavedFrame &f = frameObj->as<SavedFrame>();
     f.initFromLookup(lookup);
 
@@ -630,16 +634,22 @@ SavedStacks::sweepPCLocationMap()
         }
     }
 }
 
 bool
 SavedStacks::getLocation(JSContext *cx, JSScript *script, jsbytecode *pc,
                          MutableHandleLocationValue locationp)
 {
+    // We should only ever be caching location values for scripts in this
+    // compartment. Otherwise, we would get dead cross-compartment scripts in
+    // the cache because our compartment's sweep method isn't called when their
+    // compartment gets collected.
+    assertSameCompartment(cx, this, script);
+
     PCKey key(script, pc);
     PCLocationMap::AddPtr p = pcLocationMap.lookupForAdd(key);
 
     if (!p) {
         const char *filename = script->filename() ? script->filename() : "";
         RootedAtom source(cx, Atomize(cx, filename, strlen(filename)));
         if (!source)
             return false;
@@ -661,9 +671,21 @@ SavedStacksMetadataCallback(JSContext *c
 {
     RootedSavedFrame frame(cx);
     if (!cx->compartment()->savedStacks().saveCurrentStack(cx, &frame))
         return false;
     *pmetadata = frame;
     return true;
 }
 
+#ifdef JS_CRASH_DIAGNOSTICS
+void
+CompartmentChecker::check(SavedStacks *stacks)
+{
+    if (&compartment->savedStacks() != stacks) {
+        printf("*** Compartment SavedStacks mismatch: %p vs. %p\n",
+               (void *) &compartment->savedStacks(), stacks);
+        MOZ_CRASH();
+    }
+}
+#endif /* JS_CRASH_DIAGNOSTICS */
+
 } /* namespace js */
--- a/js/src/vm/SavedStacks.h
+++ b/js/src/vm/SavedStacks.h
@@ -108,18 +108,18 @@ class SavedStacks {
     void     sweep(JSRuntime *rt);
     void     trace(JSTracer *trc);
     uint32_t count();
     void     clear();
 
     size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf);
 
   private:
-    SavedFrame::Set          frames;
-    JSObject                 *savedFrameProto;
+    SavedFrame::Set frames;
+    JSObject        *savedFrameProto;
 
     bool       insertFrames(JSContext *cx, FrameIter &iter, MutableHandleSavedFrame frame,
                             unsigned maxFrameCount = 0);
     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, const SavedFrame::Lookup &lookup);