Closed Bug 1106719 Opened 5 years ago Closed 5 years ago

Crash [@ JSFunction::nonLazyScript] or Assertion failure: nativeOffset >= entry.nativeOffset, at jit/BaselineJIT.cpp or Assertion failure: hasScript(), at jsfun.h

Categories

(Core :: JavaScript Engine: JIT, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla37
Tracking Status
firefox36 --- fixed
firefox37 --- fixed

People

(Reporter: gkw, Assigned: shu)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update])

Crash Data

Attachments

(3 files, 2 obsolete files)

// Random chosen test: js/src/jit-test/tests/gc/bug-880776.js
// -- reduced away --
// Random chosen test: js/src/jit-test/tests/debug/onExceptionUnwind-06.js
g = newGlobal()
g.parent = this
g.eval("Debugger(parent).onExceptionUnwind=(function(){})")
// Random chosen test: js/src/jit-test/tests/baseline/bug847425.js
gcparam("maxBytes", gcparam("gcBytes"))
function f() {
    f()
    y(arguments)
}
f()
// jsfunfuzz
// -- reduced away --

asserts js debug shell on m-c changeset 3a1dcd04ea02 with --fuzzing-safe --ion-eager --no-threads at Assertion failure: nativeOffset >= entry.nativeOffset, at jit/BaselineJIT.cpp.

Debug configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

This was found by combining random jit-tests together with jsfunfuzz, the specific file(s) is/are:

http://hg.mozilla.org/mozilla-central/file/3a1dcd04ea02/js/src/jit-test/tests/gc/bug-880776.js (later reduced away)
http://hg.mozilla.org/mozilla-central/file/3a1dcd04ea02/js/src/jit-test/tests/debug/onExceptionUnwind-06.js
http://hg.mozilla.org/mozilla-central/file/3a1dcd04ea02/js/src/jit-test/tests/baseline/bug847425.js

Due to skipped revisions, the first bad revision could be any of:
changeset:   https://hg.mozilla.org/mozilla-central/rev/b160657339f8
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)

changeset:   https://hg.mozilla.org/mozilla-central/rev/bb2f13ba7b1c
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)

changeset:   https://hg.mozilla.org/mozilla-central/rev/1176cc3c3b34
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)

changeset:   https://hg.mozilla.org/mozilla-central/rev/f8e316fa65bb
user:        Shu-yu Guo
date:        Thu Nov 13 14:39:40 2014 -0800
summary:     Bug 1063330 - Remove the JS shell's evalInFrame. (r=jimb)

changeset:   https://hg.mozilla.org/mozilla-central/rev/96a2f59f6ce4
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)

changeset:   https://hg.mozilla.org/mozilla-central/rev/06d07689a043
user:        Shu-yu Guo
date:        Thu Nov 13 14:39:41 2014 -0800
summary:     Bug 1032869 - Part 4: Add an auto-updated DebugModeOSRVolatileJitFrameIterator. (r=jandem)

Shu-yu, are any of these bugs likely regressors?
Flags: needinfo?(shu)
Whiteboard: [jsbugmon:update]
Attached file stack
(lldb) bt 5
* thread #1: tid = 0x1ee44d, 0x0000000100249332 js-dbg-opt-64-dm-nsprBuild-darwin-3a1dcd04ea02`js::jit::BaselineScript::pcForNativeOffset(this=<unavailable>, script=<unavailable>, nativeOffset=<unavailable>, isReturn=<unavailable>) + 962 at BaselineJIT.cpp:788, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x0000000100249332 js-dbg-opt-64-dm-nsprBuild-darwin-3a1dcd04ea02`js::jit::BaselineScript::pcForNativeOffset(this=<unavailable>, script=<unavailable>, nativeOffset=<unavailable>, isReturn=<unavailable>) + 962 at BaselineJIT.cpp:788
    frame #1: 0x000000010020581b js-dbg-opt-64-dm-nsprBuild-darwin-3a1dcd04ea02`js::jit::RecompileOnStackBaselineScriptsForDebugMode(JSContext*, js::Debugger::ExecutionObservableSet const&, js::Debugger::IsObserving) [inlined] CollectJitStackScripts(cx=<unavailable>, obs=<unavailable>, activation=0x00007fff5fbfcc48, entries=0x0000000101e01cf0) + 642 at BaselineDebugModeOSR.cpp:220
    frame #2: 0x0000000100205599 js-dbg-opt-64-dm-nsprBuild-darwin-3a1dcd04ea02`js::jit::RecompileOnStackBaselineScriptsForDebugMode(cx=0x0000000101e01cf0, obs=0x00007fff5fbfbd48, observing=Observing) + 505 at BaselineDebugModeOSR.cpp:811
    frame #3: 0x0000000100636249 js-dbg-opt-64-dm-nsprBuild-darwin-3a1dcd04ea02`js::Debugger::updateExecutionObservabilityOfFrames(cx=0x0000000101e01cf0, obs=0x00007fff5fbfbd48, observing=Observing) + 89 at Debugger.cpp:1825
    frame #4: 0x000000010062bf8a js-dbg-opt-64-dm-nsprBuild-darwin-3a1dcd04ea02`js::Debugger::ensureExecutionObservabilityOfFrame(cx=0x0000000101e01cf0, frame=(ptr_ = 140734799793250)) + 330 at Debugger.cpp:1990
(lldb)
Prior to reduction, the assert was:

Assertion failure: hasScript(), at jsfun.h

Sometimes, the assert does not show and the stack is just:

(gdb) bt
#0  0x0000000000405d6c in JSFunction::nonLazyScript (this=<optimized out>) at /home/fuzz2lin/trees/mozilla-central/js/src/shell/../jsfun.h:313
#1  0x0000000000467d7a in JSFunction::nonLazyScript (this=<optimized out>) at /home/fuzz2lin/trees/mozilla-central/js/src/shell/../jsfun.h:316
#2  0x00000000009d52a3 in js::Debugger::removeDebuggeeGlobal (this=0x385f5f0, fop=fop@entry=0x7fffa56fe190, global=global@entry=0x7fa1ae55e060, debugEnum=0x0)
    at /home/fuzz2lin/trees/mozilla-central/js/src/vm/Debugger.cpp:3033
#3  0x00000000009d55e9 in js::Debugger::detachAllDebuggersFromGlobal (fop=fop@entry=0x7fffa56fe190, global=0x7fa1ae55e060) at /home/fuzz2lin/trees/mozilla-central/js/src/vm/Debugger.cpp:2309
#4  0x000000000086f1b1 in JSCompartment::sweepGlobalObject (this=0x3921800, fop=fop@entry=0x7fffa56fe190) at /home/fuzz2lin/trees/mozilla-central/js/src/jscompartment.cpp:573
#5  0x00000000009031ca in js::gc::GCRuntime::beginSweepingZoneGroup (this=this@entry=0x381c438) at /home/fuzz2lin/trees/mozilla-central/js/src/jsgc.cpp:5087
#6  0x00000000009043f8 in js::gc::GCRuntime::beginSweepPhase (this=this@entry=0x381c438, lastGC=lastGC@entry=false) at /home/fuzz2lin/trees/mozilla-central/js/src/jsgc.cpp:5256
#7  0x0000000000905314 in js::gc::GCRuntime::incrementalCollectSlice (this=this@entry=0x381c438, budget=..., reason=reason@entry=JS::gcreason::DESTROY_CONTEXT)
    at /home/fuzz2lin/trees/mozilla-central/js/src/jsgc.cpp:5959
#8  0x0000000000905d28 in js::gc::GCRuntime::gcCycle (this=this@entry=0x381c438, incremental=incremental@entry=false, budget=..., gckind=gckind@entry=js::GC_NORMAL,
    reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at /home/fuzz2lin/trees/mozilla-central/js/src/jsgc.cpp:6169
#9  0x00000000009060b3 in js::gc::GCRuntime::collect (this=this@entry=0x381c438, incremental=incremental@entry=false, budget=..., gckind=gckind@entry=js::GC_NORMAL,
    reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at /home/fuzz2lin/trees/mozilla-central/js/src/jsgc.cpp:6296
#10 0x00000000009064d7 in js::gc::GCRuntime::gc (this=this@entry=0x381c438, gckind=gckind@entry=js::GC_NORMAL, reason=reason@entry=JS::gcreason::DESTROY_CONTEXT)
    at /home/fuzz2lin/trees/mozilla-central/js/src/jsgc.cpp:6342
#11 0x0000000000865f91 in js::DestroyContext (cx=0x383a4f0, mode=js::DCM_FORCE_GC) at /home/fuzz2lin/trees/mozilla-central/js/src/jscntxt.cpp:261
#12 0x00000000008663de in JS_DestroyContext (cx=<optimized out>) at /home/fuzz2lin/trees/mozilla-central/js/src/jsapi.cpp:754
#13 0x000000000044d25a in DestroyContext (withGC=true, cx=0x383a4f0) at /home/fuzz2lin/trees/mozilla-central/js/src/shell/js.cpp:5293
#14 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at /home/fuzz2lin/trees/mozilla-central/js/src/shell/js.cpp:6022
(gdb)
Crash Signature: [@ JSFunction::nonLazyScript]
Summary: Assertion failure: nativeOffset >= entry.nativeOffset, at jit/BaselineJIT.cpp → Crash [@ JSFunction::nonLazyScript] or Assertion failure: nativeOffset >= entry.nativeOffset, at jit/BaselineJIT.cpp or Assertion failure: hasScript(), at jsfun.h
CC'ing GC folks in case they find anything special in the stack of comment 2.
Similar to bug 1100320. Debug mode OSR wasn't able to handle OOM exceptions,
which could get thrown from prologue jitcode.
Attachment #8531256 - Flags: review?(jimb)
Flags: needinfo?(shu)
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #3)
> CC'ing GC folks in case they find anything special in the stack of comment 2.

Do you have the non-reduced test case and can you confirm this patch fixes that as well? Could be a separate bug.
Forgot to add actual patch.
Attachment #8531256 - Attachment is obsolete: true
Attachment #8531256 - Flags: review?(jimb)
Attachment #8531291 - Flags: review?(jimb)
Comment on attachment 8531291 [details] [diff] [review]
Don't call onExceptionUnwind and onPop debugger hooks on OOM.

(In reply to Shu-yu Guo [:shu] from comment #5)
> Do you have the non-reduced test case and can you confirm this patch fixes
> that as well? Could be a separate bug.

I have tested the non-reduced testcase on my side and yes I can confirm that the patch fixes that as well. \o/
Attachment #8531291 - Flags: feedback+
Updated docs.
Attachment #8531291 - Attachment is obsolete: true
Attachment #8531291 - Flags: review?(jimb)
Attachment #8531614 - Flags: review?(jimb)
Comment on attachment 8531614 [details] [diff] [review]
Don't call onExceptionUnwind and onPop debugger hooks on OOM.

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

Looks good to me - but I'd like these comment and doc changes.

::: js/src/doc/Debugger/Debugger.Frame.md
@@ +258,5 @@
>      reported to the next handler.
>  
> +    This property is ignored on `"debugger"` frames. This property is also
> +    ignored when unwinding a frame due to an over-recursion or out-of-memory
> +    exception.

Pre-existing, but: could we refer to it as a 'handler', not a 'property'? Also, I think "is not called" would be a bit more explicit than "is ignored".

::: js/src/doc/Debugger/Debugger.md
@@ +151,5 @@
>      `continue`, or `break` statement, or a new exception. In those cases the
>      old exception does not continue to propagate; it is discarded.)
>  
> +    This property is ignored when unwinding a frame due to an over-recursion
> +    or out-of-memory exception.

Same comments here.

::: js/src/jit-test/tests/debug/bug1106719.js
@@ +3,5 @@
> +g = newGlobal()
> +g.parent = this
> +g.eval("Debugger(parent).onExceptionUnwind=(function(){})")
> +// Random chosen test: js/src/jit-test/tests/baseline/bug847425.js
> +gcparam("maxBytes", gcparam("gcBytes"))

Would it be as effective to call TestingFunctions.cpp's 'reportOutOfMemory' function? I think that would make for a clearer test.

::: js/src/vm/Debugger.cpp
@@ +597,5 @@
>  
>      // This path can be hit via unwinding the stack due to
>      // over-recursion. Only fire the frames' onPop handlers if we haven't
>      // over-recursed, because invoking more JS will only result in more
> +    // over-recursion errors. See slowPathOnExceptionUnwind. Ditto for OOM.

Pity your poor reader. People are only going to see the "Ditto for OOM" if they read the whole comment carefully. We should put the comment in the form we would have chosen if we were writing it afresh. So:

This patch can be hit via unwinding the stack due to over-recursion or OOM. In those cases, don't fire the frames' onPop handlers, because invoking JS will only trigger the same condition. See slowPathOnExceptionUnwind.

@@ +724,5 @@
>  /* static */ JSTrapStatus
>  Debugger::slowPathOnExceptionUnwind(JSContext *cx, AbstractFramePtr frame)
>  {
>      // Invoking more JS on an over-recursed stack is only going to result in
> +    // more over-recursion errors. Ditto for OOM.

Here:

Invoking more JS on an over-recursed stack or after OOM is only going to result in more of the same error.
Attachment #8531614 - Flags: review?(jimb) → review+
Excellent points about the comments.
(In reply to Jim Blandy :jimb from comment #9)
> Comment on attachment 8531614 [details] [diff] [review]
> Don't call onExceptionUnwind and onPop debugger hooks on OOM.
> 
> Review of attachment 8531614 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/jit-test/tests/debug/bug1106719.js
> @@ +3,5 @@
> > +g = newGlobal()
> > +g.parent = this
> > +g.eval("Debugger(parent).onExceptionUnwind=(function(){})")
> > +// Random chosen test: js/src/jit-test/tests/baseline/bug847425.js
> > +gcparam("maxBytes", gcparam("gcBytes"))
> 
> Would it be as effective to call TestingFunctions.cpp's 'reportOutOfMemory'
> function? I think that would make for a clearer test.
> 

No, because this needs to throw an OOM in the JIT prologue of a function, not via a VM call.
https://hg.mozilla.org/mozilla-central/rev/144c58a03316
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Blocks: 1114757
Fixed for Fx36 by the roll-up in bug 1114757.
Assignee: nobody → shu
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.