Assertion failure: debuggers, at vm/Debugger.cpp

RESOLVED FIXED in Firefox 36

Status

()

defect
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gkw, Assigned: shu)

Tracking

(Blocks 2 bugs, {assertion, regression, testcase})

Trunk
mozilla37
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox36 fixed, firefox37 fixed)

Details

(Whiteboard: [jsbugmon:update,testComment=10,origRev=29d086b32a26])

Attachments

(1 attachment, 1 obsolete attachment)

// 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.
Flags: needinfo?(shu)
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unknown exception (check manually)
Flags: needinfo?(shu)
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??
Attachment #8532758 - Flags: review?(jimb) → review+
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.
Attachment #8532758 - Attachment is obsolete: true
Attachment #8533391 - Flags: review?(jimb)
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!
Attachment #8533391 - Flags: review?(jimb) → review+
Assigning to Shu-yu since he has a patch.
Assignee: nobody → shu
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/263322550d0a
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Blocks: 1114757
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.
Whiteboard: [jsbugmon:] → [jsbugmon:update,testComment=10,origRev=29d086b32a26]
You need to log in before you can comment on or make changes to this bug.