Fix a breakpoint GC bug found by billm. See bug 677386 comment 8, first paragraph. r=billm on IRC.
authorJason Orendorff <jorendorff@mozilla.com>
Wed, 24 Aug 2011 18:42:19 -0500
changeset 75861 6daedd1baec4267352979c3e42d4e1ea64206d19
parent 75860 29527b07008dd2dfce9ecc355e9c46dd8df433c6
child 75862 db8de6cd0712f9651b39c46992fe07963f9a12a1
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewersbillm
bugs677386
milestone9.0a1
Fix a breakpoint GC bug found by billm. See bug 677386 comment 8, first paragraph. r=billm on IRC.
js/src/jit-test/tests/debug/breakpoint-gc-03.js
js/src/jscompartment.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/breakpoint-gc-03.js
@@ -0,0 +1,25 @@
+// |jit-test| debug
+// The GC can cope with old and new breakpoints at the same position.
+
+// This is a regression test for a bug Bill McCloskey found just by looking at
+// the source code. See bug 677386 comment 8. Here we're testing that the trap
+// string is correctly marked. (The silly expression for the trap string is to
+// ensure that it isn't constant-folded; it's harder to get a compile-time
+// constant to be GC'd.)
+
+var g = newGlobal('new-compartment');
+g.eval("var d = 0;\n" +
+       "function f() { return 'ok'; }\n" +
+       "trap(f, 0, Array(17).join('\\n') + 'd++;');\n");
+
+var dbg = new Debugger;
+var gw = dbg.addDebuggee(g);
+var fw = gw.getOwnPropertyDescriptor("f").value;
+var bp = {hits: 0, hit: function (frame) { this.hits++; }};
+fw.script.setBreakpoint(0, bp);
+
+gc();
+
+g.f();
+assertEq(g.d, 1);
+assertEq(bp.hits, 1);
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -809,17 +809,17 @@ JSCompartment::markBreakpointsIterativel
     bool markedAny = false;
     JSContext *cx = trc->context;
     for (BreakpointSiteMap::Range r = breakpointSites.all(); !r.empty(); r.popFront()) {
         BreakpointSite *site = r.front().value;
 
         // Mark jsdbgapi state if any. But if we know the scriptObject, put off
         // marking trap state until we know the scriptObject is live.
         if (site->trapHandler &&
-            (!site->scriptObject || IsAboutToBeFinalized(cx, site->scriptObject)))
+            (!site->scriptObject || !IsAboutToBeFinalized(cx, site->scriptObject)))
         {
             if (site->trapClosure.isMarkable() &&
                 IsAboutToBeFinalized(cx, site->trapClosure.toGCThing()))
             {
                 markedAny = true;
             }
             MarkValue(trc, site->trapClosure, "trap closure");
         }