Closed
Bug 1418971
Opened 3 years ago
Closed 3 years ago
Assertion failure: rematFrame->script() == frame->script(), at js/src/jit/BaselineBailouts.cpp:1816 or Assertion failure: containsPC(pc), at js/src/jsscript.cpp:4060 with Debugger
Categories
(Core :: JavaScript Engine, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla63
People
(Reporter: decoder, Assigned: nbp)
References
(Blocks 1 open bug)
Details
(4 keywords, Whiteboard: [jsbugmon:update,bisect])
Attachments
(1 file)
5.86 KB,
patch
|
jandem
:
review+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision fc194660762d+ (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu --enable-simulator=arm, run with --fuzzing-safe --cpu-count=2): try { Object.defineProperty(this, "fuzzutils", { value:{} }); } catch(exc) {} gcPreserveCode() try { interruptTest(new Function("")); oomTest(new Function(` 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; for (var i = 0; i < upCount; i++) { if (!frame.older) break; frame = frame.older; } var completion = frame.eval(code); if (completion.return) { var v = completion.return; if (typeof v === "object") v = v.unsafeDereference(); return v; } if (completion.throw) { var v = completion.throw; if (typeof v === "object") v = v.unsafeDereference(); throw v; } if (completion === null) terminate(); }; })(this); function f() evalInFrame(1, "print(a)"); function g() evalInFrame(1, "a = 42"); function testGet() (function ( v1, ... get) { f(); })(); function testSet() (function () { g(); })(); try { testGet(); } catch (e) {} try { testSet(); } catch (e) {} `)); Array.prototype.set = function(index, value) {} } catch(exc) {} Backtrace: received signal SIGSEGV, Segmentation fault. 0x08ab1b7f in CopyFromRematerializedFrame (frame=0xf5fffdc8, inlineDepth=2, fp=0xf5fffeb0 "\324L\232AB\020", act=0xffffbbe0, cx=0xf6e1d800) at js/src/jit/BaselineBailouts.cpp:1816 #0 0x08ab1b7f in CopyFromRematerializedFrame (frame=0xf5fffdc8, inlineDepth=2, fp=0xf5fffeb0 "\324L\232AB\020", act=0xffffbbe0, cx=0xf6e1d800) at js/src/jit/BaselineBailouts.cpp:1816 #1 js::jit::FinishBailoutToBaseline (bailoutInfo=0x0) at js/src/jit/BaselineBailouts.cpp:1974 #2 0x08544e50 in js::jit::Simulator::softwareInterrupt (this=0xf6e58000, instr=0xf5a60514) at js/src/jit/arm/Simulator-arm.cpp:2587 [...] #14 0x08581769 in JS_CallFunction (cx=0xf6e1d800, obj=..., fun=..., args=..., rval=...) at js/src/jsapi.cpp:2991 #15 0x084994e2 in OOMTest (cx=0xf6e1d800, argc=1, vp=0xf5a4c060) at js/src/builtin/TestingFunctions.cpp:1650 #16 0x0818ea29 in js::CallJSNative (cx=0xf6e1d800, native=0x84991c0 <OOMTest(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:291 [...] #30 main (argc=4, argv=0xffffce24, envp=0xffffce38) at js/src/shell/js.cpp:8987 eax 0x0 0 ebx 0xf5d723d0 -170449968 ecx 0xf7da4864 -136689564 edx 0x0 0 esi 0xf5fffdc8 -167772728 edi 0x8d73ff4 148324340 ebp 0xffffb958 4294949208 esp 0xffffb870 4294948976 eip 0x8ab1b7f <js::jit::FinishBailoutToBaseline(js::jit::BaselineBailoutInfo*)+4015> => 0x8ab1b7f <js::jit::FinishBailoutToBaseline(js::jit::BaselineBailoutInfo*)+4015>: movl $0x0,0x0 0x8ab1b89 <js::jit::FinishBailoutToBaseline(js::jit::BaselineBailoutInfo*)+4025>: ud2
Comment 1•3 years ago
|
||
Nicolas, can you take a look?
Flags: needinfo?(nicolas.b.pierron)
Priority: -- → P1
Assignee | ||
Comment 2•3 years ago
|
||
I can reproduce this issue on Mozilla central, after modifying the test case to add braces to single-statement functions. The test case does print a lot of 42 before stalling and asserting.
Assignee | ||
Comment 3•3 years ago
|
||
I can reproduce this issue on x64 as well.
Assignee: nobody → nicolas.b.pierron
Status: NEW → ASSIGNED
status-firefox60:
--- → affected
status-firefox61:
--- → affected
status-firefox62:
--- → affected
Hardware: ARM → Unspecified
Updated•3 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:bisect]
Comment 4•3 years ago
|
||
JSBugMon: Cannot process bug: Error: Unsupported architecture "Unspecified" required by bug
Updated•3 years ago
|
Whiteboard: [jsbugmon:bisect] → [jsbugmon:]
Comment 5•3 years ago
|
||
JSBugMon: Bisection requested, failed due to error: Error: Unsupported architecture "Unspecified" required by bug
Assignee | ||
Comment 6•3 years ago
|
||
The following testcase crashes on mozilla-central revision fc194660762d+ (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --without-intl-api --enable-optimize, run with --fuzzing-safe --cpu-count=2): try { Object.defineProperty(this, "fuzzutils", { value:{} }); } catch(exc) {} gcPreserveCode() try { interruptTest(new Function("")); oomTest(new Function(` 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; for (var i = 0; i < upCount; i++) { if (!frame.older) break; frame = frame.older; } var completion = frame.eval(code); if (completion.return) { var v = completion.return; if (typeof v === "object") v = v.unsafeDereference(); return v; } if (completion.throw) { var v = completion.throw; if (typeof v === "object") v = v.unsafeDereference(); throw v; } if (completion === null) terminate(); }; })(this); function f() { evalInFrame(1, "print(a)"); } function g() { evalInFrame(1, "a = 42"); } function testGet() { (function ( v1, ... get) { f(); })(); } function testSet() { (function () { g(); })(); } try { testGet(); } catch (e) {} try { testSet(); } catch (e) {} `)); Array.prototype.set = function(index, value) {} } catch(exc) {}
Hardware: Unspecified → x86_64
Whiteboard: [jsbugmon:] → [jsbugmon:update,bisect]
Assignee | ||
Comment 7•3 years ago
|
||
The problem comes from an OOM after the creation of the rematerialized frame under js::Debugger::getNewestFrame. The OOM error is propagated and unwind the caller frame. Therefore we should catch these errors and clean-up the previously rematerialized frame of the activation such that the top pointer cannot alias a newly created frame. I should be able to come-up with a patch soonish.
Assignee | ||
Comment 8•3 years ago
|
||
I have a patch to fix this assertion, but now I am investigating another assertion which is preventing me from landing this patch.
Assignee | ||
Comment 9•3 years ago
|
||
This patch ensure that rematerialized frames used by the Debugger are properly removed to ensure that no stale data is used during bailouts.
Attachment #8981184 -
Flags: review?(jdemooij)
Comment 10•3 years ago
|
||
Comment on attachment 8981184 [details] [diff] [review] Remove rematerialized frames after bailouts and exceptions. Review of attachment 8981184 [details] [diff] [review]: ----------------------------------------------------------------- Good find. Please add a jit-test for this. ::: js/src/jit/BaselineBailouts.cpp @@ +1950,5 @@ > // even if earlier ones failed, to invoke the proper frame > // cleanup in the Debugger. > ok = CopyFromRematerializedFrame(cx, act, outerFp, --inlineDepth, > + iter.baselineFrame()) > + && ok; Nit: I think this would be clearer as: if (!CopyFromRematerializedFrame(...)) ok = false;
Attachment #8981184 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 11•3 years ago
|
||
(waiting on the second patch from Bug 1464829 to land)
Comment 12•3 years ago
|
||
Pushed by npierron@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/66ee3a9fe9ca Remove rematerialized frames after bailouts and exceptions. r=jandem
Comment 13•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/66ee3a9fe9ca
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Flags: needinfo?(nicolas.b.pierron)
Marking this tracking 62 as a reminder that we may want to uplift for beta 10 or 11.
tracking-firefox62:
--- → +
Updated•3 years ago
|
status-firefox-esr52:
--- → unaffected
Assignee | ||
Comment 15•3 years ago
|
||
Comment on attachment 8981184 [details] [diff] [review] Remove rematerialized frames after bailouts and exceptions. Approval Request Comment [Feature/Bug causing the regression]: …? [User impact if declined]: bad debugger output. [Is this code covered by automated tests?]: test case not included as it takes too long with the oomTest function, and hard to synthesized one. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: no. [List of other uplifts needed for the feature/fix]: Bug 1464829 patch*es* [Is the change risky?]: I do not think so. [Why is the change risky/not risky?]: Add extra clean-up code to avoid stale values. [String changes made/needed]: none.
Attachment #8981184 -
Flags: approval-mozilla-beta?
Updated•3 years ago
|
Keywords: regression
We missed the 2nd patch in the other bug initially, so this will need to wait a day or two for uplift. But I haven't forgotten it :)
Comment on attachment 8981184 [details] [diff] [review] Remove rematerialized frames after bailouts and exceptions. Fix for assertion, the dependent work has all landed on beta so this is ready for uplift (for beta 13)
Attachment #8981184 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 19•3 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/c2571cc8a086
Comment 20•2 years ago
|
||
Pushed by npierron@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1554e5028374 Add Bug 1363233 test case. r=me
Comment 21•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1554e5028374
You need to log in
before you can comment on or make changes to this bug.
Description
•