Assertion failure: target.hasCachedSavedFrame(), at js/src/vm/SavedStacks.cpp:1690 with Debugger
Categories
(Core :: JavaScript Engine, defect, P3)
Tracking
()
People
(Reporter: decoder, Assigned: jimb)
References
Details
(4 keywords, Whiteboard: [jsbugmon:testComment=9,origRev=8ec327de0ba7])
Attachments
(1 file)
The following testcase crashes on mozilla-central revision d2963b5a2897 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off --ion-eager): var evalInFrame = (function (global) { var dbgGlobal = newGlobal(); var dbg = new dbgGlobal.Debugger(); return function evalInFrame(upCount, code) { dbg.addDebuggee(global); var frame = dbg.getNewestFrame().older; frame = frame.older; var completion = frame.eval(code); }; })(this); function isObject(obj) {} function assertStackIs(x) { return new Promise(resolve => {}); } f = isObject; var obj2 = {}; obj2['b' + {}] = 0; for (var k in obj2) (function g() { evalInFrame(1, "assertStackIs(['eval-code', f, 'bound(f)', 'global-code'])", true); })(); Backtrace: received signal SIGSEGV, Segmentation fault. 0x0000555555b23006 in js::SavedStacks::checkForEvalInFramePrev (this=this@entry=0x7ffff5f821e8, cx=0x7ffff5f18000, lookup=lookup@entry=...) at js/src/vm/SavedStacks.cpp:1690 #0 0x0000555555b23006 in js::SavedStacks::checkForEvalInFramePrev (this=this@entry=0x7ffff5f821e8, cx=0x7ffff5f18000, lookup=lookup@entry=...) at js/src/vm/SavedStacks.cpp:1690 #1 0x0000555555b376a4 in js::SavedStacks::insertFrames(JSContext*, JS::MutableHandle<js::SavedFrame*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (this=this@entry=0x7ffff5f821e8, cx=<optimized out>, cx@entry=0x7ffff5f18000, frame=..., capture=capture@entry=<unknown type>) at js/src/vm/SavedStacks.cpp:1572 #2 0x0000555555b381ee in js::SavedStacks::saveCurrentStack(JSContext*, JS::MutableHandle<js::SavedFrame*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (this=0x7ffff5f821e8, cx=cx@entry=0x7ffff5f18000, frame=..., capture=capture@entry=<unknown type>) at js/src/vm/SavedStacks.cpp:1289 #3 0x0000555555d23645 in JS::CaptureCurrentStack(JSContext*, JS::MutableHandle<JSObject*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (cx=cx@entry=0x7ffff5f18000, stackp=stackp@entry=..., capture=capture@entry=<unknown type>) at js/src/jsapi.cpp:6781 #4 0x0000555555942740 in PromiseDebugInfo::create (cx=cx@entry=0x7ffff5f18000, promise=promise@entry=..., maybeNow=...) at js/src/builtin/Promise.cpp:295 #5 0x0000555555922f2a in CreatePromiseObjectInternal (cx=0x7ffff5f18000, proto=..., proto@entry=..., protoIsWrapped=protoIsWrapped@entry=false, informDebugger=informDebugger@entry=false) at js/src/builtin/Promise.cpp:1982 #6 0x00005555559258bb in js::PromiseObject::create (cx=<optimized out>, executor=executor@entry=..., proto=..., proto@entry=..., needsWrapping=<optimized out>) at js/src/builtin/Promise.cpp:2118 #7 0x0000555555926082 in PromiseConstructor (cx=<optimized out>, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/Promise.cpp:2082 #8 0x00005555558ac175 in CallJSNative (cx=0x7ffff5f18000, native=native@entry=0x555555925ec0 <PromiseConstructor(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:468 #9 0x00005555558ac369 in CallJSNativeConstructor (cx=<optimized out>, native=0x555555925ec0 <PromiseConstructor(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:485 #10 0x00005555558a06d4 in InternalConstruct (cx=<optimized out>, args=...) at js/src/vm/Interpreter.cpp:661 #11 0x00005555558a084d in js::ConstructFromStack (cx=<optimized out>, args=...) at js/src/vm/Interpreter.cpp:707 #12 0x0000555555f8f72e in js::jit::DoCallFallback (cx=<optimized out>, frame=0x7fffffff8cc8, stub_=<optimized out>, argc=<optimized out>, vp=0x7fffffff8c68, res=...) at js/src/jit/BaselineIC.cpp:3666 #13 0x00002e2c3c54ff03 in ?? () [...] #37 0x0000000000000000 in ?? () rax 0x0 0 rbx 0x7fffffff78d0 140737488320720 rcx 0x7ffff6c1c2dd 140737333281501 rdx 0x0 0 rsi 0x7ffff6eeb770 140737336227696 rdi 0x7ffff6eea540 140737336223040 rbp 0x7fffffff7670 140737488320112 rsp 0x7fffffff75e0 140737488319968 r8 0x7ffff6eeb770 140737336227696 r9 0x7ffff7fe6cc0 140737354034368 r10 0x58 88 r11 0x7ffff6b927a0 140737332717472 r12 0x0 0 r13 0x7ffff5f18000 140737319632896 r14 0x7fffffff7620 140737488320032 r15 0x7fffffff78d0 140737488320720 rip 0x555555b23006 <js::SavedStacks::checkForEvalInFramePrev(JSContext*, js::SavedFrame::HandleLookup)+822> => 0x555555b23006 <js::SavedStacks::checkForEvalInFramePrev(JSContext*, js::SavedFrame::HandleLookup)+822>: movl $0x0,0x0 0x555555b23011 <js::SavedStacks::checkForEvalInFramePrev(JSContext*, js::SavedFrame::HandleLookup)+833>: ud2
autobisectjs shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/1a7cb8857de4 user: Jim Blandy date: Wed Mar 14 12:54:04 2018 -0700 summary: Bug 1444604: Part 7: Reconcile LiveSavedFrameCache with evalInFramePrev by tweaking SavedFrame parent links. r=jorendorff Jim, is bug 1444604 a likely regressor?
Updated•5 years ago
|
Assignee | ||
Comment 2•5 years ago
|
||
I can reproduce this. Taking.
Assignee | ||
Comment 3•5 years ago
|
||
Somewhat reduced: var evalInFrame = (function (global) { var dbgGlobal = newGlobal(); var dbg = new dbgGlobal.Debugger(); return function evalInFrame(code) { dbg.addDebuggee(global); var frame = dbg.getNewestFrame().older; frame = frame.older; var completion = frame.eval(code); }; })(this); while (true) { (function g() { evalInFrame("saveStack()"); })(); }
Assignee | ||
Comment 4•5 years ago
|
||
We're calling saveStack from a Debugger 'eval' frame evaluating in the test script's top-level frame. Thus, the Debugger 'eval' frame's actual parent is the call to evalInFrame, but its evalInFramePrev link refers to the top-level frame. This is fine. However, there's something interesting. The stack as seen by SavedStacks::insertFrame's FrameIter consists of jit::CommonFrameLayout frames, which I think are Ion frames (the --ion-eager flag is required, to reproduce), yet the Debugger 'eval' frame's 'evalInFramePrev' link refers to a jit::RematerializedFrame. This upsets SavedStacks::checkForEvalInFramePrev, which expects that, since it is walking frames from oldest to youngest, bringing the LiveSavedFrameCache up to date as it goes, 'evalInFramePrev' must refer to something in the cache. A RematerializedFrame is built from an Ion frame, to give Debugger something it can work with. An Ion frame may represent several inlined JavaScript frames, and omit variables that one would expect to find, based on the source code. Unpacking an Ion frame to produce one or more RematerializedFrames uses the same information an Ion bailout would use to build a separate RematerializedFrames for each inlined call, with all variables present and usable. SpiderMonkey puts off unpacking Ion frames into RematerializedFrames until Debugger asks for something the Ion frame can't easily support - like an eval. Since Debugger can assign to variables in a RematerializedFrame, they should be considered the canonical source of truth once they are created. It seems that FrameIter should not be producing the old jit::CommonFrameLayout frames once the RematerializedFrames exist.
Assignee | ||
Comment 5•5 years ago
|
||
Well, that's not quite right - if one needs variable values, then yes, the RematerializedFrames must be used. But for simply producing stack traces, anything that produces inline frames (as FrameIter does) should be fine.
Assignee | ||
Comment 6•5 years ago
|
||
I'm kind of excited about this, because it seems like we should trigger this assertion any time we try to save a stack in a Debugger.Frame eval. The following asserts, too: var evalInFrame = (function (global) { var dbgGlobal = newGlobal(); var dbg = new dbgGlobal.Debugger(global); return function evalInFrame(code) { let frame = dbg.getNewestFrame().older.older; print(frame.script.callee); frame.eval(code); }; })(this); while (true) { (function g() { evalInFrame("new Error"); })(); }
Assignee | ||
Comment 7•5 years ago
|
||
The symptoms of hitting this bug in production (without the debug-only asserts) would simply be that a captured stack (for an error or promise) would end at the eval frame - it wouldn't have the frame in whose environment we're carrying out the evaluation as a parent. So it doesn't seem terribly consequential.
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment hidden (obsolete) |
var evalInFrame = (function (global) {
var dbgGlobal = newGlobal();
var dbg = new dbgGlobal.Debugger();
return function evalInFrame(code) {
dbg.addDebuggee(global);
var frame = dbg.getNewestFrame().older;
frame = frame.older;
var completion = frame.eval(code);
};
})(this);
while (true) {
(function g() { evalInFrame("saveStack()"); })();
}
asserts js shell compiled with --enable-debug on m-c rev 8ec327de0ba7 using --fuzzing-safe --no-threads --ion-eager --more-compartments at Assertion failure: target.hasCachedSavedFrame(), at js/src/vm/SavedStacks.cpp:1550
Updated•5 years ago
|
Comment 10•5 years ago
|
||
JSBugMon: Cannot process bug: Error: Failed to isolate test from comment
Updated•5 years ago
|
Assignee | ||
Comment 11•5 years ago
|
||
I haven't been working on this, since it's P3 (per comment 7).
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Updated•4 years ago
|
Updated•4 years ago
|
Comment 14•4 years ago
|
||
I'm hitting this with WarpBuilder enabled on debug/bug-1444604-reduced.js.
The fix for this isn't trivial. I think I'll add --no-warp to that test but it would be great if we could fix this bug.
Comment 15•4 years ago
|
||
This reproduction can be trimmed down to
var dbgGlobal = newGlobal({ newCompartment: true });
var dbg = new dbgGlobal.Debugger(globalThis);
for (var i = 0; i < 1; i++) {
(() => {
dbg.getNewestFrame().older.eval("saveStack()");
})();
}
run with --fuzzing-safe --no-threads --ion-eager
.
I was looking into a related bug and decided to check out this one too. I have a potential fix but I'll need review before I'm particularly confident that it's the right fix.
Comment 16•4 years ago
|
||
This bug is caused because creating a LiveSavedFrameCache::FramePtr from a
FrameIter special-cases RematerializedFrames that are also PhysicalJitFrames.
This creates a FramePtr for a CommonFrameLayout, which is reasonable, but it
conflicts with the fact that ExecuteKernel is only passed an AbstractFramePtr
which means that when we go to compare the two FramePtr values, one is a
RematerializedFrame and one is a CommonFrameLayout even though they both
refer to the same frame.
This patch uses the FrameIter construction method for all FramePtr instances
so we avoid this two-constructor approach and can be more easily sure that
all code paths can properly construct the same FramePtr.
Comment 17•4 years ago
|
||
Pushed by loganfsmyth@gmail.com: https://hg.mozilla.org/integration/autoland/rev/07f09d4d22f7 Consistently use a FrameIter to produce consistent cache pointers. r=jorendorff
Comment 18•4 years ago
|
||
bugherder |
Updated•4 years ago
|
Description
•