1417961 - Crash [@ ??] with Debugger

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Christian Holler (:decoder)]

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 --enable-optimize, run with --fuzzing-safe --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;
    for (var i = 0; i < upCount; i++)
      frame = frame.older;
    var completion = frame.eval(code);
    if (completion.throw)
      throw v;
  };
})(this);
try {
  function f() {
    var x = x;
    for (var i = 0; i < 10; - i)
      g(100 * i + x);
  }
  function h() {
    var z = 5;
    evalInFrame(0, "a.push(z)");
    evalInFrame(1, "a.push(y)");
  }
  function g() {
    var y = 4;
    h();
  }
  f();
} catch (exc) {}
eval(`
var a = [1,2,3,4];
function f() {
  var x = x;
  for (var i = 0; i < 10; -i)
    g(100 * i + x);
}
f();
`);


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x00000b9136c45d13 in ?? ()
#0  0x00000b9136c45d13 in ?? ()
#1  0x00000b9136c9391a in ?? ()
#2  0xfff9000000000000 in ?? ()
#3  0xfffe7ffff44aca40 in ?? ()
#4  0xfff8800000000004 in ?? ()
#5  0x0000000000000000 in ?? ()
rax	0x1	1
rbx	0xfff9000000000000	-1970324836974592
rcx	0xfff9000000000000	-1970324836974592
rdx	0x7fffffffa8e8	140737488333032
rsi	0x1	1
rdi	0x0	0
rbp	0x7fffffffa6e8	140737488332520
rsp	0x7fffffffa698	140737488332440
r8	0x0	0
r9	0x1	1
r10	0x7ffff5f20800	140737319667712
r11	0x20	32
r12	0x8	8
r13	0x7fffffffb500	140737488336128
r14	0x2043	8259
r15	0x7fffffffa8b0	140737488332976
rip	0xb9136c45d13	12718317002003
=> 0xb9136c45d13:	mov    0x20(%rdi),%rdi
   0xb9136c45d17:	jmpq   *(%rdi)

Comment 1

[Fuzzing Team]

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/21fe8fa6b8e6
user:        Jan de Mooij
date:        Mon Nov 06 14:10:23 2017 +0100
summary:     Bug 1414228 - Allocate type monitor fallback stubs lazily instead of allocating them in BaselineCompiler. r=tcampbell

This iteration took 290.279 seconds to run.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Patch

Debug mode OSR bug related to Ion bailouts. We hit the oldStub->isFallback() code here:

https://searchfox.org/mozilla-central/rev/c633ffa4c4611f202ca11270dcddb7b29edddff8/js/src/jit/BaselineDebugModeOSR.cpp#735-742

And in this case we didn't ensure we had a type monitor fallback stub. Then we could crash in the bailout return code here:

https://searchfox.org/mozilla-central/rev/c633ffa4c4611f202ca11270dcddb7b29edddff8/js/src/jit/BaselineIC.cpp#3009-3041

This patch just moves the getFallbackMonitorStub call before the isFallback check.

Comment 3

[Jan de Mooij [:jandem]]

Debug mode only but there's an easy fix so we should just backport this.

Comment 4

[Ted Campbell [:tcampbell]]

Comment on attachment 8930056 [details] [diff] [review]
Patch

Review of attachment 8930056 [details] [diff] [review]:
-----------------------------------------------------------------

Ah, interesting. Thanks fuzzers.

Comment 5

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

Tracking this to make sure it lands and we uplift the fix.

Comment 6

[Mike Taylor [:miketaylr]]

Jan, is there anything blocking from landing the patch?

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f2097108539
Fix Baseline debug mode OSR to delazify type monitor chains correctly. r=tcampbell

Comment 8

[Jan de Mooij [:jandem]]

Comment on attachment 8930056 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1414228.
[User impact if declined]: Crashes when using debugger.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Not yet.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Just moves some code; only affects the debugger.
[String changes made/needed]: None.

Comment 9

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/6f2097108539

Comment 10

[Gerry Chang [:gchang]]

Comment on attachment 8930056 [details] [diff] [review]
Patch

Fix a crash when using the debugger. Beta58+.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/43f74013ae08