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

RESOLVED FIXED in Firefox 62

Status

()

defect
P1
critical
RESOLVED FIXED
2 years ago
11 months ago

People

(Reporter: decoder, Assigned: nbp)

Tracking

(Blocks 2 bugs, 4 keywords)

Trunk
mozilla63
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox-esr60 wontfix, firefox59 wontfix, firefox60 wontfix, firefox61 wontfix, firefox62+ fixed, firefox63 fixed)

Details

(Whiteboard: [jsbugmon:update,bisect])

Attachments

(1 attachment)

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
Nicolas, can you take a look?
Flags: needinfo?(nicolas.b.pierron)
Priority: -- → P1
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.
I can reproduce this issue on x64 as well.
Assignee: nobody → nicolas.b.pierron
Status: NEW → ASSIGNED
Hardware: ARM → Unspecified
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:bisect]
JSBugMon: Cannot process bug: Error: Unsupported architecture "Unspecified" required by bug
Whiteboard: [jsbugmon:bisect] → [jsbugmon:]
JSBugMon: Bisection requested, failed due to error: Error: Unsupported architecture "Unspecified" required by bug
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]
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.
I have a patch to fix this assertion, but now I am investigating another assertion which is preventing me from landing this patch.
Depends on: 1464829
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 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+
(waiting on the second patch from Bug 1464829 to land)
Pushed by npierron@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/66ee3a9fe9ca
Remove rematerialized frames after bailouts and exceptions. r=jandem
https://hg.mozilla.org/mozilla-central/rev/66ee3a9fe9ca
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Flags: needinfo?(nicolas.b.pierron)
Marking this tracking 62 as a reminder that we may want to uplift for beta 10 or 11.
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?
Duplicate of this bug: 1363233
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+
You need to log in before you can comment on or make changes to this bug.