Closed Bug 1479394 Opened 7 years ago Closed 7 years ago

Crash [@ js::AbstractFramePtr::global] with Debugger and stackTest

Categories

(Core :: JavaScript Engine, defect, P2)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox-esr60 --- unaffected
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- fixed

People

(Reporter: decoder, Assigned: jandem)

References

Details

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

Crash Data

Attachments

(1 file)

The following testcase crashes on mozilla-central revision 0be4463d2915 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --ion-eager --ion-offthread-compile=off): stackTest(new Function(` var evalInFrame = (function evalInFrame(global) { var dbgGlobal = newGlobal(); var dbg = new dbgGlobal.Debugger(); return function evalInFrame(upCount, code) { dbg.addDebuggee(global); var frame = dbg.getNewestFrame().older; for (var get of (yield)) frame = frame.older; }; })(this); function f() { evalInFrame(0, "a.push(1)"); } f(); `)); Backtrace: received signal SIGSEGV, Segmentation fault. js::AbstractFramePtr::global (this=0x7fffffffb0c8) at js/src/vm/Stack-inl.h:694 #0 js::AbstractFramePtr::global (this=0x7fffffffb0c8) at js/src/vm/Stack-inl.h:694 #1 js::Debugger::forEachDebuggerFrame<js::Debugger::inFrameMaps(js::AbstractFramePtr)::<lambda(js::DebuggerFrame*)> >(js::AbstractFramePtr, js::Debugger::<lambda(js::DebuggerFrame*)>) (frame=..., fn=..., fn@entry=...) at js/src/vm/Debugger.cpp:2685 #2 0x0000000000b024b4 in js::Debugger::inFrameMaps (frame=..., frame@entry=...) at js/src/vm/Debugger.cpp:6608 #3 0x000000000081cbff in js::jit::HandleExceptionIon (overrecursed=0x7fffffffb1c7, rfe=0x7fffffffb830, frame=..., cx=0x7ffff5f17000) at js/src/jit/JitFrames.cpp:209 #4 js::jit::HandleException (rfe=<optimized out>) at js/src/jit/JitFrames.cpp:702 #5 0x00002712bf80d3d6 in ?? () [...] #23 0x0000000000000000 in ?? () rax 0xe5e5e5e5e5e5e5e5 -1880844493789993499 rbx 0x7ffff4a5e660 140737297901152 rcx 0x0 0 rdx 0x3 3 rsi 0x7fffffffb137 140737488335159 rdi 0x7fffffffb0c8 140737488335048 rbp 0x7fffffffb120 140737488335136 rsp 0x7fffffffb0c0 140737488335040 r8 0x0 0 r9 0x10 16 r10 0x1d 29 r11 0x7ffff3e66040 140737285349440 r12 0x7fffffffb0c8 140737488335048 r13 0x7ffff5f17000 140737319628800 r14 0x7fffffffb2e0 140737488335584 r15 0x7fffffffbac0 140737488337600 rip 0xb022f4 <js::Debugger::forEachDebuggerFrame<js::Debugger::inFrameMaps(js::AbstractFramePtr)::<lambda(js::DebuggerFrame*)> >(js::AbstractFramePtr, js::Debugger::<lambda(js::DebuggerFrame*)>)+68> => 0xb022f4 <js::Debugger::forEachDebuggerFrame<js::Debugger::inFrameMaps(js::AbstractFramePtr)::<lambda(js::DebuggerFrame*)> >(js::AbstractFramePtr, js::Debugger::<lambda(js::DebuggerFrame*)>)+68>: mov 0x20(%rax),%rdi 0xb022f8 <js::Debugger::forEachDebuggerFrame<js::Debugger::inFrameMaps(js::AbstractFramePtr)::<lambda(js::DebuggerFrame*)> >(js::AbstractFramePtr, js::Debugger::<lambda(js::DebuggerFrame*)>)+72>: callq 0x5b6130 <JS::Realm::maybeGlobal() const>
How important is this?
Flags: needinfo?(sdetar)
Jandem do you think you could help answer David's question about how important this is?
Flags: needinfo?(sdetar) → needinfo?(jdemooij)
decoder, can we get JSBugmon to bisect this?
Flags: needinfo?(choller)
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
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/66ee3a9fe9ca user: Nicolas B. Pierron date: Mon May 28 15:41:23 2018 +0000 summary: Bug 1418971 - Remove rematerialized frames after bailouts and exceptions. r=jandem This iteration took 286.958 seconds to run.
Flags: needinfo?(choller)
Flags: needinfo?(jdemooij) → needinfo?(nicolas.b.pierron)
Slightly simplified: var dbgGlobal = newGlobal(); var dbg = new dbgGlobal.Debugger(this); function f1() { dbg.getNewestFrame().older; throw new Error(); } function f2() { f1(); } function f3() { f2(); } stackTest(f3); ---- The code in HandleExceptionIon is like this: if (shouldBail) { ... ExceptionBailoutInfo propagateInfo; uint32_t retval = ExceptionHandlerBailout(cx, frame, rfe, propagateInfo, overrecursed); if (retval == BAILOUT_RETURN_OK) return; } MOZ_ASSERT_IF(rematFrame, !Debugger::inFrameMaps(rematFrame)); The call to ExceptionHandlerBailout calls BailoutIonToBaseline, which destroys rematFrame from this ScopeExit: // Ion bailout can fail due to overrecursion and OOM. In such cases we // cannot honor any further Debugger hooks on the frame, and need to // ensure that its Debugger.Frame entry is cleaned up. auto guardRemoveRematerializedFramesFromDebugger = mozilla::MakeScopeExit([&] { activation->removeRematerializedFramesFromDebugger(cx, iter.fp()); });
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision cbeaa2d94304).
autobisectjs shows this is probably related to the following changeset: The first good revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/9697472e6ab7 user: Jan de Mooij date: Tue Oct 23 01:18:22 2018 +0000 summary: Bug 1493627 part 2 - Unify OOM testing state variables for different kinds to make AutoEnterOOMUnsafeRegion work for stack checks. r=jonco Not sure if this may be related, but is bug 1493627 a likely fix?
Flags: needinfo?(jdemooij)
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #7) > Not sure if this may be related, but is bug 1493627 a likely fix? Ugh, it's not a fix but it hides the bug. This code uses AutoEnterOOMUnsafeRegion and that now also works for stack-"OOM" testing: https://searchfox.org/mozilla-central/rev/c0b26c40769a1e5607a1ae8be37fe64df64fc55e/js/src/jit/Bailouts.cpp#217-221 I'll post a patch to clean up the bailout code.
Flags: needinfo?(nicolas.b.pierron)
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Pushed by jdemooij@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b81f4153e1a8 Clean up exception handling in bailout code and remove a MOZ_ASSERT_IF that triggered a UAF. r=nbp
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: