1375447 - Assertion failure: !cx->isExceptionPending(), at js/src/jscntxtinlines.h:409 with Debugger

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision e49151136658 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe):

var g = newGlobal();
var dbg = new Debugger;
var gw = dbg.addDebuggee(g);
g.eval(`
var line0 = Error().lineNumber;
function f() {
    try {
	throw 4;
    } catch(e) {}
}
`);
var script = gw.getOwnPropertyDescriptor("f").value.script;
var handler = {
    hit: function() {}
};
var offs = script.getLineOffsets(g.line0 + 4);
for (var i = 0; i < offs.length; i++) script.setBreakpoint(offs[i], handler);
assertEq(g.f(), true);



Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x0000000000502cb0 in js::CheckForInterrupt (cx=0x7ffff6924000) at js/src/jscntxtinlines.h:409
#0  0x0000000000502cb0 in js::CheckForInterrupt (cx=0x7ffff6924000) at js/src/jscntxtinlines.h:409
#1  0x000000000052b01d in Interpret (cx=0x7ffff6924000, state=...) at js/src/vm/Interpreter.cpp:2099
#2  0x0000000000536c32 in js::RunScript (cx=0x7ffff6924000, state=...) at js/src/vm/Interpreter.cpp:410
#3  0x00000000005371b7 in js::InternalCallOrConstruct (cx=cx@entry=0x7ffff6924000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:488
#4  0x0000000000537498 in InternalCall (cx=cx@entry=0x7ffff6924000, args=...) at js/src/vm/Interpreter.cpp:515
#5  0x00000000005375cd in js::Call (cx=cx@entry=0x7ffff6924000, fval=..., fval@entry=..., thisv=..., thisv@entry=..., args=..., rval=..., rval@entry=...) at js/src/vm/Interpreter.cpp:534
#6  0x0000000000b1eb70 in CallMethodIfPresent (name=0x1147c90 "hit", argc=1, rval=..., argv=0x7fffffffba00, obj=..., cx=0x7ffff6924000) at js/src/vm/Debugger.cpp:1734
#7  js::Debugger::onTrap (cx=0x7ffff6924000, vp=...) at js/src/vm/Debugger.cpp:2018
#8  0x00000000005345fd in Interpret (cx=0x7ffff6924000, state=...) at js/src/vm/Interpreter.cpp:1949
#9  0x0000000000536c32 in js::RunScript (cx=0x7ffff6924000, state=...) at js/src/vm/Interpreter.cpp:410
#10 0x00000000005371b7 in js::InternalCallOrConstruct (cx=cx@entry=0x7ffff6924000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:488
#11 0x0000000000537498 in InternalCall (cx=cx@entry=0x7ffff6924000, args=...) at js/src/vm/Interpreter.cpp:515
#12 0x00000000005375cd in js::Call (cx=cx@entry=0x7ffff6924000, fval=..., fval@entry=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:534
#13 0x0000000000a7224d in js::Wrapper::call (this=this@entry=0x1edb7a0 <js::CrossCompartmentWrapper::singleton>, cx=cx@entry=0x7ffff6924000, proxy=..., proxy@entry=..., args=...) at js/src/proxy/Wrapper.cpp:166
#14 0x0000000000a5fce2 in js::CrossCompartmentWrapper::call (this=0x1edb7a0 <js::CrossCompartmentWrapper::singleton>, cx=0x7ffff6924000, wrapper=..., args=...) at js/src/proxy/CrossCompartmentWrapper.cpp:353
#15 0x0000000000a69dda in js::Proxy::call (cx=cx@entry=0x7ffff6924000, proxy=proxy@entry=..., args=...) at js/src/proxy/Proxy.cpp:479
#16 0x0000000000a69ecc in js::proxy_Call (cx=0x7ffff6924000, argc=<optimized out>, vp=<optimized out>) at js/src/proxy/Proxy.cpp:739
#17 0x000000000054167f in js::CallJSNative (cx=cx@entry=0x7ffff6924000, native=0xa69e50 <js::proxy_Call(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:293
[...]
#30 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:8515
rax	0x0	0
rbx	0x7ffff6924000	140737330167808
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffb110	140737488335120
rsp	0x7fffffffb100	140737488335104
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x1	1
r13	0x7ffff6924020	140737330167840
r14	0x7fffffffb600	140737488336384
r15	0x7fffffffb8a0	140737488337056
rip	0x502cb0 <js::CheckForInterrupt(JSContext*)+96>
=> 0x502cb0 <js::CheckForInterrupt(JSContext*)+96>:	movl   $0x0,0x0
   0x502cbb <js::CheckForInterrupt(JSContext*)+107>:	ud2

Comment 1

[Fuzzing Team]

JSBugMon: Bisection requested, result:
=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20160302090939" and the hash "09bb9469a14d4587e44027e648438a5f23526cd7".
The "bad" changeset has the timestamp "20160302093919" and the hash "9de2c10a1cc34fdeade8523b53f03d567e7f190b".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=09bb9469a14d4587e44027e648438a5f23526cd7&tochange=9de2c10a1cc34fdeade8523b53f03d567e7f190b

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/432cce28b337
user:        Boris Zbarsky
date:        Wed Mar 02 12:38:23 2016 -0500
summary:     Bug 1252905.  Don't invoke the interrupt callback while an exception is pending.  r=jandem

Boris, is bug 1252905 a likely regressor?

Comment 3

[Boris Zbarsky [:bzbarsky]]

> Boris, is bug 1252905 a likely regressor?

That bug added the assertion that's failing.  Like the label says, the thing it's asserting doesn't happen is not supposed to happen.

As usual, some part of the JS engine screwed up and left an exception sitting around.  

From a quick look, the g.f() call is throwing 4, as expected.  Then we land in HandleError in js::Interpret, which calls debugger throw hooks (e.g. js::Debugger::onTrap in this case) which runs random script, all while the exception is pending on the JSContext.  This last bit is bogus and needs to be fixed.

Comment 4

[Boris Zbarsky [:bzbarsky]]

I guess it's not the throw hooks that land in onTrap but just the EnableInterruptsPseudoOpcode bits.  Point is, they run after the throw but before the catch.

Comment 5

[Shu-yu Guo [:shu]]

Attachment: Save exception state in Debugger::onTrap.

Comment 6

[Shu-yu Guo [:shu]]

Comment on attachment 8881959 [details] [diff] [review]
Save exception state in Debugger::onTrap.

Review of attachment 8881959 [details] [diff] [review]:
-----------------------------------------------------------------

ISTM both Debugger::onTrap and Debugger::onSingleStep need to save and restore the exception state, as they both can be called while isExceptionPending in a catch block's "prologue".

Also refactors the manual exception saving code in onSingleStep to use AutoSaveExceptionState.

Comment 7

[Jan de Mooij [:jandem]]

Comment on attachment 8881959 [details] [diff] [review]
Save exception state in Debugger::onTrap.

Review of attachment 8881959 [details] [diff] [review]:
-----------------------------------------------------------------

Makes sense.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b77f711e2b4
Save exception state in Debugger::onTrap. r=jandem

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5b77f711e2b4

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Is there enough user impact here to warrant consideration for backport to Beta or can it ride the 56 train?

Comment 11

[Jim Blandy :jimb]

I think it can ride the train.