1891662 - Assertion failure: !hasBaselineScript(), at jit/JitScript.h:480

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: debug stack

var n = [
  96, 0, 0, 3, 2, 1, 253, [], 1, 0, 0, 11, 0, 14, 4, 110, 97, 109, 101, 1, 7,
  1, 0, 4, 109, 97, 105, 110,
];
function f(_, y) {
  var x = Object.getPrototypeOf(y);
  Object.setPrototypeOf(
    y,
    new Proxy(x, {
      get(a, b, c) {
        return Reflect.get(a, b, c);
      },
    })
  );
}
function g(_, y) {
  f(_, y);
}
g(0, n);
h = newGlobal({ newCompartment: true });
h.parent = this;
h.eval(
  "(" +
    function () {
      Debugger(parent).onExceptionUnwind = function (frame) {
        frame.older;
      };
    } +
    ")()"
);
try {
  throw 0;
} catch (e) {}
new Uint8Array(n);
oomTest(function () {
  g(0, (function () {})());
  function f() {}
});

(gdb) bt
#0  js::jit::JitScript::setBaselineScript (this=0x7ffff5684bb0, script=script@entry=0x2424d64100, baselineScript=<optimized out>)
    at /home/genxps15/trees/mozilla-central/js/src/jit/JitScript.h:480
#1  0x0000555557e3c8cc in UndoRecompileBaselineScriptsForDebugMode (cx=0x7ffff6740600, entries=...)
    at /home/genxps15/trees/mozilla-central/js/src/jit/BaselineDebugModeOSR.cpp:480
#2  js::jit::RecompileOnStackBaselineScriptsForDebugMode (cx=cx@entry=0x7ffff6740600, obs=..., observing=observing@entry=js::DebugAPI::Observing)
    at /home/genxps15/trees/mozilla-central/js/src/jit/BaselineDebugModeOSR.cpp:534
#3  0x00005555579ba4f5 in js::Debugger::updateExecutionObservabilityOfFrames (cx=cx@entry=0x7ffff6740600, obs=...,
    observing=observing@entry=js::DebugAPI::Observing) at /home/genxps15/trees/mozilla-central/js/src/debugger/Debugger.cpp:3168
#4  0x00005555579a2c4e in js::Debugger::ensureExecutionObservabilityOfFrame (cx=cx@entry=0x7ffff6740600, frame=...)
    at /home/genxps15/trees/mozilla-central/js/src/debugger/Debugger.cpp:3439
#5  0x00005555579a2004 in js::Debugger::getFrame (this=this@entry=0x7ffff5681200, cx=cx@entry=0x7ffff6740600, iter=..., result=result@entry=...)
    at /home/genxps15/trees/mozilla-central/js/src/debugger/Debugger.cpp:696
/snip

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/bd4da5d6dfe0
user:        Doug Thayer
date:        Wed Apr 05 05:57:06 2023 +0000
summary:     Bug 1819722 - Monomorphic inlining r=iain

Run with --fuzzing-safe --fast-warmup --baseline-warmup-threshold=0, compile with AR=ar sh ../configure --enable-debug --enable-debug-symbols --with-ccache --enable-nspr-build --enable-ctypes --enable-gczeal --enable-rust-simd --disable-tests, tested on m-c rev fcfbb607fde2.

Setting s-s as a start and because a similar (but not identical assert) bug 1851599 (related to the regressor) was marked sec-high.

Iain, is bug 1819722 a likely regressor?

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1819722

Comment 2

[Iain Ireland [:iain]]

This is touching a bunch of complicated code, but is fundamentally a pretty simple bug.

In bug 1505689 (part 1) we rewrote setBaselineScript to assert that we were not clobbering/leaking an existing baseline script. However, it looks like we missed this code in UndoRecompileBaselineScriptsForDebugMode, which restores the original version of scripts in case we OOM while recompiling. Anything that hits that path will crash. The fix is trivial.

I think the only thing that actually goes wrong here is that our memory accounting gets confused, so this is not security-sensitive.

Comment 3

[Iain Ireland [:iain]]

Attachment: Bug 1891662: Clear existing baseline script in UndoRecompileBaselineScriptsForDebugMode r=jandem

One alternative approach would be to use AutoEnterOOMUnsafeRegion and crash if we OOM while recompiling an on-stack baseline script. I'm tempted, but this patch is simple enough that we can let this code live for now.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e9cdeeb2f957
Clear existing baseline script in UndoRecompileBaselineScriptsForDebugMode r=jandem

Comment 5

[Cristian Tuns]

Backed out for causing bc failures in browser_errorpage.js

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | toolkit/components/httpsonlyerror/tests/browser/browser_errorpage.js | Test timed out -

Comment 6

[Iain Ireland [:iain]]

I don't see any realistic way that my change could affect that testcase. The code I modified only executes if we run out of memory at a very specific moment while enabling or disabling the debugger. Before adding my testcase, we had no code coverage of this function (at least according to Searchfox), which I think implies that browser_errorpage.js doesn't even run the changed code. Even if it did, the change is small, well-understood, and clearly more correct. Any debug build that reached this point prior to my patch would immediately crash.

It looks like this test case has been flaky for a while. This comment (posted before I landed my patch) says that it failed 29 times on autoland in the last week.

I've kicked off a few more runs of this testcase for my build and the preceding build. We'll see what they say.

Comment 7

[Cristian Tuns]

It looks like the retriggers turned green, bad coincidence, all my retriggers were yellow. I can reland it if you want.

Comment 8

[Iain Ireland [:iain]]

That would be great, thanks!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/660a39c9cdc7
Clear existing baseline script in UndoRecompileBaselineScriptsForDebugMode r=jandem

Comment 10

[tszentpeteri]

https://hg.mozilla.org/mozilla-central/rev/660a39c9cdc7