Closed
Bug 1437537
Opened 7 years ago
Closed 7 years ago
Assertion failure: !cx->isExceptionPending(), at js/src/vm/Debugger.cpp:1460
Categories
(Core :: JavaScript Engine, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox59 | --- | wontfix |
firefox60 | --- | wontfix |
firefox61 | --- | fixed |
People
(Reporter: decoder, Assigned: jorendorff)
Details
(4 keywords, Whiteboard: [jsbugmon:update])
Attachments
(1 file)
4.28 KB,
patch
|
jimb
:
review+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 2b7d42d527af (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off):
var g = newGlobal();
var dbg = new Debugger(g);
dbg.onEnterFrame = function(frame) {
var step = 0;
frame.onStep = function() {
++step;
if (step == 3) return {
return: undefined
}
}
}
setJitCompilerOption("ion.warmup.trigger", 30);
b: g.eval("try { g(); } catch (e) { }") + (this) << (this) << 1
Backtrace:
received signal SIGSEGV, Segmentation fault.
0x0000000000b176f0 in js::Debugger::resultToCompletion (cx=0x7ffff5f16000, ok=<optimized out>, rv=..., status=0x7fffffffb558, value=...) at js/src/vm/Debugger.cpp:1460
#0 0x0000000000b176f0 in js::Debugger::resultToCompletion (cx=0x7ffff5f16000, ok=<optimized out>, rv=..., status=0x7fffffffb558, value=...) at js/src/vm/Debugger.cpp:1460
#1 0x0000000000b51686 in js::Debugger::slowPathOnLeaveFrame (cx=cx@entry=0x7ffff5f16000, frame=..., pc=pc@entry=0x7ffff49f985c "\346\001\230\216\213", frameOk=frameOk@entry=true) at js/src/vm/Debugger.cpp:957
#2 0x0000000000574e03 in js::Debugger::onLeaveFrame (cx=cx@entry=0x7ffff5f16000, frame=..., pc=pc@entry=0x7ffff49f985c "\346\001\230\216\213", ok=ok@entry=true) at js/src/vm/Debugger-inl.h:24
#3 0x00000000005579f7 in ForcedReturn (frameOk=true, ei=..., regs=..., cx=0x7ffff5f16000) at js/src/vm/Interpreter.cpp:1156
#4 ForcedReturn (cx=0x7ffff5f16000, regs=...) at js/src/vm/Interpreter.cpp:1167
#5 0x000000000056bcdf in Interpret (cx=0x7ffff5f16000, state=...) at js/src/vm/Interpreter.cpp:1966
#6 0x000000000056c965 in js::RunScript (cx=0x7ffff5f16000, state=...) at js/src/vm/Interpreter.cpp:423
#7 0x000000000056fb8d in js::ExecuteKernel (cx=0x7ffff5f16000, script=..., script@entry=..., envChainArg=..., newTargetValue=..., evalInFrame=..., evalInFrame@entry=..., result=<optimized out>) at js/src/vm/Interpreter.cpp:706
#8 0x00000000005a28b4 in EvalKernel (cx=0x7ffff5f16000, v=..., evalType=evalType@entry=INDIRECT_EVAL, caller=..., env=..., pc=pc@entry=0x0, vp=...) at js/src/builtin/Eval.cpp:324
#9 0x00000000005a2ca0 in js::IndirectEval (cx=0x7ffff5f16000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/Eval.cpp:417
#10 0x0000000000578861 in js::CallJSNative (cx=0x7ffff5f16000, native=0x5a2bb0 <js::IndirectEval(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:291
#11 0x000000000056ce57 in js::InternalCallOrConstruct (cx=cx@entry=0x7ffff5f16000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:473
#12 0x000000000056d22d in InternalCall (cx=cx@entry=0x7ffff5f16000, args=...) at js/src/vm/Interpreter.cpp:522
#13 0x000000000056d3b0 in js::Call (cx=cx@entry=0x7ffff5f16000, fval=..., fval@entry=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:541
#14 0x0000000000aecd27 in js::ForwardingProxyHandler::call (this=this@entry=0x2044ea0 <js::CrossCompartmentWrapper::singleton>, cx=0x7ffff5f16000, proxy=..., proxy@entry=..., args=...) at js/src/proxy/Wrapper.cpp:176
#15 0x0000000000ac8f7b in js::CrossCompartmentWrapper::call (this=0x2044ea0 <js::CrossCompartmentWrapper::singleton>, cx=0x7ffff5f16000, wrapper=..., args=...) at js/src/proxy/CrossCompartmentWrapper.cpp:359
#16 0x0000000000ac3855 in js::Proxy::call (cx=0x7ffff5f16000, proxy=proxy@entry=..., args=...) at js/src/proxy/Proxy.cpp:511
#17 0x0000000000ac391d in js::proxy_Call (cx=0x7ffff5f16000, argc=<optimized out>, vp=<optimized out>) at js/src/proxy/Proxy.cpp:770
#18 0x0000000000578861 in js::CallJSNative (cx=0x7ffff5f16000, native=0xac38a0 <js::proxy_Call(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:291
[...]
#32 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:9317
rax 0x0 0
rbx 0x7ffff5f16000 140737319624704
rcx 0x7ffff6c282ad 140737333330605
rdx 0x0 0
rsi 0x7ffff6ef7770 140737336276848
rdi 0x7ffff6ef6540 140737336272192
rbp 0x7fffffffb4c0 140737488336064
rsp 0x7fffffffb4a0 140737488336032
r8 0x7ffff6ef7770 140737336276848
r9 0x7ffff7fe4780 140737354024832
r10 0x58 88
r11 0x7ffff6b9e7a0 140737332766624
r12 0x7fffffffb558 140737488336216
r13 0x0 0
r14 0x1 1
r15 0x7fffffffb5a0 140737488336288
rip 0xb176f0 <js::Debugger::resultToCompletion(JSContext*, bool, JS::Value const&, JSTrapStatus*, JS::MutableHandle<JS::Value>)+256>
=> 0xb176f0 <js::Debugger::resultToCompletion(JSContext*, bool, JS::Value const&, JSTrapStatus*, JS::MutableHandle<JS::Value>)+256>: movl $0x0,0x0
0xb176fb <js::Debugger::resultToCompletion(JSContext*, bool, JS::Value const&, JSTrapStatus*, JS::MutableHandle<JS::Value>)+267>: ud2
Updated•7 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Comment 1•7 years ago
|
||
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:
The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/5b77f711e2b4
user: Shu-yu Guo
date: Wed Jun 28 14:22:00 2017 -0400
summary: Bug 1375447 - Save exception state in Debugger::onTrap. r=jandem
This iteration took 256.398 seconds to run.
Jan, not sure who should own this now? Or :jimb?
Flags: needinfo?(jdemooij)
Comment 3•7 years ago
|
||
I'll take a look tomorrow.
Comment 4•7 years ago
|
||
This is a debugger issue. What's happening is:
(1) The interpreter throws an exception under GetNameOperation.
(2) We enter a catch block, then we end up here:
https://searchfox.org/mozilla-central/rev/769222fadff46164f8cc0dc7a0bae5a60dc2f335/js/src/vm/Interpreter.cpp#1954
(3) We call the onStep handler and do a forced return.
(4) We end up in Debugger::slowPathOnLeaveFrame. We assert in Debugger::resultToCompletion, because we have ok == true but we still have the pending exception on the context.
We should probably clear the exception somewhere.
Flags: needinfo?(jdemooij) → needinfo?(jimb)
Comment 5•7 years ago
|
||
I agree: if Debugger forces a return from a catch handler, then the exception is considered caught, so it should have been cleared.
Flags: needinfo?(jimb)
Comment 6•7 years ago
|
||
Is the source location of the frame passed to the onStep handler (as its `this` value) in the catch handler? Comment 4 suggests this is so; I think that means that the exception should have been cleared before step (3).
Comment 7•7 years ago
|
||
No, it shouldn't. Below is what the bytecode for a try-catch block looks like; I'm going to bet the frame's offset is pointing at something like the instructions 20-29, since it's only the JSOP_EXCEPTION bytecode that actually transfers the exception from the context (clearing it) onto the stack.
We were clearly aware that this case could arise when we wrote the onstep handler, because we carefully save the exception across handler calls, and explain why:
https://searchfox.org/mozilla-central/rev/769222fadff46164f8cc0dc7a0bae5a60dc2f335/js/src/vm/Debugger.cpp#2064-2070
But perhaps we didn't try doing a forced return?
--- bytecode for a try/catch block:
js> dis(() => { try { throw new Error(); } catch (e) { print(e); } })
flags: LAMBDA ARROW
loc op
----- --
main:
00000: try 20 (+20) #
00001: getgname "Error" # Error
00006: is-constructing # Error JS_IS_CONSTRUCTING
00007: dupat 1 # Error JS_IS_CONSTRUCTING Error
00011: new 0 # (new Error(...))
00014: throw #
00015: goto 55 (+40) # !!! UNREACHABLE !!!
# try-catch from try @ 00000
00020: jumptarget #
00021: undefined # undefined
00022: setrval #
00023: uninitialized # UNINITIALIZED
00024: initlexical 0 # UNINITIALIZED
00028: pop #
00029: exception # EXCEPTION
00030: initlexical 0 # EXCEPTION
00034: pop #
00035: getgname "print" # print
00040: gimplicitthis "print" # print THIS
00045: getlocal 0 # print THIS e
00049: call-ignores-rv 1 # print(...)
00052: pop #
00053: debugleavelexicalenv #
00054: nop #
00055: jumptarget #
00056: retrval #
Comment 8•7 years ago
|
||
Slightly reduced test case, unfortunately still with magic numbers:
var g = newGlobal();
var dbg = new Debugger(g);
dbg.onEnterFrame = function(frame) {
frame.onStep = function() {
if (this.offset == 20)
return { return: undefined };
}
}
let f = g.Function(`try { throw new Error; } catch (e) { print("caught: " + e); }`);
f();
Comment 9•7 years ago
|
||
No magic numbers, fuzzer-like behavior:
var g = newGlobal();
var dbg = new Debugger(g);
let f = g.Function(`try { throw new Error; } catch (e) { return 'natural'; }`);
let limit = -1;
for(;;) {
dbg.onEnterFrame = function(frame) {
frame.onStep = function() {
if (this.offset > limit) {
limit = this.offset;
return { return: 'forced' };
}
}
}
if (f() == 'natural')
break;
}
Assignee | ||
Updated•7 years ago
|
Priority: -- → P1
Assignee | ||
Updated•7 years ago
|
Priority: P1 → P2
Assignee | ||
Comment 10•7 years ago
|
||
Attachment #8957036 -
Flags: review?(jimb)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → jorendorff
Status: NEW → ASSIGNED
Comment 11•7 years ago
|
||
Comment on attachment 8957036 [details] [diff] [review]
Fix assertion when forcing a return while paused on JSOP_EXCEPTION
Review of attachment 8957036 [details] [diff] [review]:
-----------------------------------------------------------------
Fantastic.
Attachment #8957036 -
Flags: review?(jimb) → review+
Assignee | ||
Comment 12•7 years ago
|
||
Assignee | ||
Comment 13•7 years ago
|
||
Assignee | ||
Comment 14•7 years ago
|
||
Assignee | ||
Comment 15•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2d7e417c6fbf4fe6d5584be183f6e0f10c0a524
Bug 1437537 - Fix assertion when forcing a return while paused on JSOP_EXCEPTION. r=jimb.
Updated•7 years ago
|
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
Comment 16•7 years ago
|
||
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 6d0f84afc194).
![]() |
||
Comment 17•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Comment 18•7 years ago
|
||
Is there a user impact here which justifies Beta uplift consideration, or can this ride the trains?
status-firefox59:
--- → wontfix
status-firefox-esr52:
--- → unaffected
Flags: needinfo?(jorendorff)
Flags: in-testsuite+
![]() |
||
Updated•7 years ago
|
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:update]
Assignee | ||
Comment 19•7 years ago
|
||
Re: comment 18, I don't think so. Forwarding ni? to Jim in case he disagrees.
Flags: needinfo?(jorendorff) → needinfo?(jimb)
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•