// Randomly chosen test: js/src/jit-test/tests/basic/bug674776.js var s = ''; for (var i = 0; i < 70000; i++) { s += 'function x' + i + '() { x' + i + '(); } '; } evaluate(s); // Randomly chosen test: js/src/jit-test/tests/debug/Source-element-03.js var g = newGlobal(); (new Debugger).addDebuggee(g); g.offThreadCompileScript('debugger;',{}); g.runOffThreadScript(); asserts js debug shell on m-c changeset 29d086b32a26 without any CLI flags at Assertion failure: debuggers, at vm/Debugger.cpp. I reproduced this using the build from: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-12-05-mozilla-central-debug/jsshell-linux-x86_64.zip This was found by combining random js tests together with jsfunfuzz, the specific file(s) is/are: http://hg.mozilla.org/mozilla-central/file/29d086b32a26/js/src/jit-test/tests/basic/bug674776.js http://hg.mozilla.org/mozilla-central/file/29d086b32a26/js/src/jit-test/tests/debug/Source-element-03.js === Tinderbox Build Bisection Results by autoBisect === The "good" changeset has the timestamp "20141113141452" and the hash "5922f13f8439". The "bad" changeset has the timestamp "20141113143950" and the hash "d65f01b9e6c6". Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5922f13f8439&tochange=d65f01b9e6c6 Shu-yu mentioned that bug 1062629 is a probable likely regressor.

JSBugMon: Cannot process bug: Unknown exception (check manually)

Comment on attachment 8532758 [details] [diff] [review] Fix debuggers sweeping logic for off-thread "debuggee" compartments. Review of attachment 8532758 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. Just some suggestions. ::: js/src/jscompartment.cpp @@ +570,5 @@ > // implies having at least one Debugger still attached. However, for > // off-thread compartments, which are used in off-thread parsing, they > // may be isDebuggee() without there being any Debuggers to prohibit > // asm.js. > + if (isDebuggee() && !options().invisibleToDebugger()) This is just a cleanup, right? Does this change have any effect? ::: js/src/vm/Debugger.cpp @@ +2145,5 @@ > continue; > > + // See note in JSCompartment::sweepGlobalObject. > + if (c->options().invisibleToDebugger()) > + continue; It seems to me that this is, logically, part of the isDebuggee test ("No, *really*, is it a debuggee?"), and belongs in the 'if' condition. Is there a reason we need to check the global first??

After discussion on IRC, we decided that just disabling asm.js compilation when the host compartment is a debuggee is preferable to having a weird "is debuggee but is also invisible to the debugger" state.

Comment on attachment 8533391 [details] [diff] [review] Fix debuggers sweeping logic for off-thread "debuggee" compartments. Review of attachment 8533391 [details] [diff] [review]: ----------------------------------------------------------------- This is much, much nicer. Thanks!

Assigning to Shu-yu since he has a patch.

Fixed for Fx36 by the roll-up in bug 1114757.

The testcase in comment 0 had an accidental extra line break, so re-setting this: // Randomly chosen test: js/src/jit-test/tests/basic/bug674776.js var s = ''; for (var i = 0; i < 70000; i++) { s += 'function x' + i + '() { x' + i + '(); }'; } evaluate(s); // Randomly chosen test: js/src/jit-test/tests/debug/Source-element-03.js var g = newGlobal(); (new Debugger).addDebuggee(g); g.offThreadCompileScript('debugger;',{}); g.runOffThreadScript(); asserts js debug shell on m-c changeset 29d086b32a26 without any CLI flags at Assertion failure: debuggers, at vm/Debugger.cpp.