Closed Bug 1100320 Opened 5 years ago Closed 5 years ago

Assertion failure: result, at jit/BaselineJIT.cpp:500 or Crash [@ pcOffset]


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
firefox36 --- affected


(Reporter: decoder, Unassigned)


(Blocks 1 open bug)


(4 keywords, Whiteboard: [jsbugmon:update])


(1 file)

The following testcase crashes on mozilla-central revision a52bf59965a0 (build with --disable-debug --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --target=i686-pc-linux-gnu --enable-gczeal, run with --fuzzing-safe --no-threads):

var g = newGlobal();
g.parent = this;
g.eval("new Debugger(parent).onExceptionUnwind = function () {};");
function f() {
    var x = f();


Program received signal SIGSEGV, Segmentation fault.
CollectJitStackScripts (entries=..., activation=..., obs=..., cx=0x9316158) at js/src/jit/BaselineDebugModeOSR.cpp:201
201                     if (!entries.append(DebugModeOSREntry(script, entry)))
#0  CollectJitStackScripts (entries=..., activation=..., obs=..., cx=0x9316158) at js/src/jit/BaselineDebugModeOSR.cpp:201
#1  js::jit::RecompileOnStackBaselineScriptsForDebugMode (cx=0x9316158, obs=..., observing=js::Debugger::Observing) at js/src/jit/BaselineDebugModeOSR.cpp:758
#2  0x084b9459 in js::Debugger::updateExecutionObservabilityOfFrames (cx=0x9316158, obs=..., observing=js::Debugger::Observing) at js/src/vm/Debugger.cpp:1728
#3  0x084d6c37 in ensureExecutionObservabilityOfFrame (frame=..., cx=0x9316158) at js/src/vm/Debugger.cpp:1889
#4  js::Debugger::getScriptFrameWithIter (this=0x93e4178, cx=0x9316158, frame=..., maybeIter=0xfff7da04, vp=$jsval(-nan(0xfff8200000000))) at js/src/vm/Debugger.cpp:462
#5  0x084df145 in getScriptFrame (vp=..., iter=..., cx=0x9316158, this=0x93e4178) at js/src/vm/Debugger.h:653
#6  js::Debugger::fireExceptionUnwind (this=0x93e4178, cx=0x9316158, vp=$jsval(-nan(0xfff8200000000))) at js/src/vm/Debugger.cpp:1147
#7  0x084df63d in js::Debugger::dispatchHook (cx=0x9316158, vp=$jsval(-nan(0xfff8200000000)), which=js::Debugger::OnExceptionUnwind) at js/src/vm/Debugger.cpp:1233
eax     0x0     0
ebx     0x92f7ff4       154107892
ecx     0x0     0
edx     0x0     0
esi     0xfff7d948      -534200
edi     0xfff7d444      -535484
ebp     0xf6848160      4135879008
esp     0xfff7d370      4294431600
eip     0x8222406 <js::jit::RecompileOnStackBaselineScriptsForDebugMode(JSContext*, js::Debugger::ExecutionObservableSet const&, js::Debugger::IsObserving)+1174>
=> 0x8222406 <js::jit::RecompileOnStackBaselineScriptsForDebugMode(JSContext*, js::Debugger::ExecutionObservableSet const&, js::Debugger::IsObserving)+1174>:   mov    0x8(%eax),%edx
   0x8222409 <js::jit::RecompileOnStackBaselineScriptsForDebugMode(JSContext*, js::Debugger::ExecutionObservableSet const&, js::Debugger::IsObserving)+1177>:   and    $0x1fffffff,%edx
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result:
Due to skipped revisions, the first bad revision could be any of:
user:        Shu-yu Guo
date:        Thu Nov 13 14:39:39 2014 -0800
summary:     Bug 1032869 - Part 2: Move debuggee-ness to frames and selectively deoptimize when Debugger needs to observe execution. (r=jimb)

user:        Shu-yu Guo
date:        Thu Nov 13 14:39:40 2014 -0800
summary:     Bug 1062629 - Off-thread compartment debug mode should match main thread compartment debug mode. (r=jimb)

user:        Shu-yu Guo
date:        Thu Nov 13 14:39:40 2014 -0800
summary:     Bug 1063328 - Fix on-stack live iterator handling when bailing out in-place due to debug mode OSR. (r=jandem)

user:        Shu-yu Guo
date:        Thu Nov 13 14:39:40 2014 -0800
summary:     Bug 1063330 - Remove the JS shell's evalInFrame. (r=jimb)

user:        Shu-yu Guo
date:        Thu Nov 13 14:39:40 2014 -0800
summary:     Bug 1032869 - Part 3: Don't consider onExceptionUnwind an all-execution-observing hook. (r=jandem)

This iteration took 480.552 seconds to run.
Since onExceptionUnwind is in the test, putting a needinfo for shu here, but also Ccing jimb and jandem for help :)
Flags: needinfo?(shu)
If we try to handle over-recursion with Debugger, then we'll just be piling more JS frames on top of a stack that's already too deep. If over-recursion pops up the JS debugger UI, then the nested event loop will attempt to let other tabs' event handlers run, which will all be immediately killed; it'll be chaos. However, since over-recursion appears to the Debugger to be the same as a slow script dialog, that's indeed what we'll try to do at present: onExceptionUnwind and onPop handlers will run --- or try to.

So, in addition to fixing whatever crashes there might be here, we probably ought to adjust the way we report over-recursion in Debugger.

My ideal solution would be this:

1) Over-recursion should set some kind of flag that js::Debugger::onLeaveFrame and js::Debugger::onExceptionUnwind can see.

2) Those functions should skip calling Debugger handlers until we're down to half our stack quota (or a quarter?). It seems like returning true from onLeaveFrame and JSTRAP_CONTINUE from onExceptionUnwind will keep the ball rolling in this case. (However, note that js::Debugger::slowPathOnLeaveFrame has cleanup code that must be called, even if we skip calling handlers.)

That way, we won't try to spin our nested event loop until we have at least some headroom for other tabs to run --- and neither will we skip reporting the problem altogether. The user will at least see the older part of the over-recursed stack.
The bug was that over-recursion was causing onExceptionUnwind to fire, which in
turned tried to do debug mode OSR. When over-recursion fails, the frame that
tripped the assert is settled on right before the prologue, which debug mode
OSR didn't know how to deal with.

We can either let debug mode OSR handle patching prologue entry, or blacklist
over-recursion fixes this. Based on comment 3 and discussion with Jim, this
patch implements blacklisting over-recursion.
Attachment #8524240 - Flags: review?(jimb)
Flags: needinfo?(shu)
Comment on attachment 8524240 [details] [diff] [review]
Don't call onExceptionUnwind and onPop debugger hooks on over-recursion.

Review of attachment 8524240 [details] [diff] [review]:

I'd like to see a clean try push for this patch. For all I know, the debugger already tests its over-recursion behavior, and we might be breaking that.
Attachment #8524240 - Flags: review?(jimb) → review+
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.