Closed Bug 1556033 Opened 11 months ago Closed 5 months ago

[jsdbg2] Forced return from onEnterFrame caused by generator object 'throw' method crashes

Categories

(Core :: JavaScript Engine, defect, P1, critical)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- wontfix
firefox67 --- wontfix
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- wontfix
firefox71 --- wontfix
firefox72 --- fixed

People

(Reporter: decoder, Assigned: loganfsmyth)

References

(Blocks 1 open bug, Regression)

Details

(5 keywords, Whiteboard: [jsbugmon:update])

Attachments

(1 file, 1 obsolete file)

The following testcase crashes on mozilla-central revision 462fc9264901 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off --baseline-eager):

let g = newGlobal({newCompartment: true});
g.eval(`async function f(x) { await x; }`);
let dbg = new Debugger(g);
function test(when) {
  let hits = 0;
  dbg.onEnterFrame = frame => {
    if (hits++ == when)
        return {
          return: "exit"
         };
  };
  g.f(import("foo.js")).then(value => {});
}
test(1);

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  js::GeneratorThrowOrReturn (cx=<optimized out>, cx@entry=0x7ffff5f19000, frame=..., genObj=genObj@entry=..., arg=arg@entry=..., resumeKind=resumeKind@entry=js::GeneratorResumeKind::Return) at js/src/vm/GeneratorObject.cpp:142
#1  0x0000555556141567 in js::jit::GeneratorThrowOrReturn (cx=0x7ffff5f19000, frame=0x7fffffffc478, genObj=..., arg=..., resumeKindArg=<optimized out>) at js/src/jit/VMFunctions.cpp:1039
#2  0x0000012f10d0a481 in ?? ()
[...]
#6  0x0000000000000000 in ?? ()
rax	0x555557d37000	93825034055680
rbx	0x19ee34e00ff8	28510880010232
rcx	0x555556be4b90	93825015892880
rdx	0x0	0
rsi	0x7ffff6eeb770	140737336227696
rdi	0x7ffff6eea540	140737336223040
rbp	0x7fffffffc380	140737488339840
rsp	0x7fffffffc320	140737488339744
r8	0x7ffff6eeb770	140737336227696
r9	0x7ffff7fe6cc0	140737354034368
r10	0x58	88
r11	0x7ffff6b927a0	140737332717472
r12	0x7fffffffc450	140737488340048
r13	0x2	2
r14	0x7fffffffc458	140737488340056
r15	0x2	2
rip	0x555555b0a6f9 <js::GeneratorThrowOrReturn(JSContext*, js::AbstractFramePtr, JS::Handle<js::AbstractGeneratorObject*>, JS::Handle<JS::Value>, js::GeneratorResumeKind)+473>
=> 0x555555b0a6f9 <js::GeneratorThrowOrReturn(JSContext*, js::AbstractFramePtr, JS::Handle<js::AbstractGeneratorObject*>, JS::Handle<JS::Value>, js::GeneratorResumeKind)+473>:	movl   $0x0,0x0
   0x555555b0a704 <js::GeneratorThrowOrReturn(JSContext*, js::AbstractFramePtr, JS::Handle<js::AbstractGeneratorObject*>, JS::Handle<JS::Value>, js::GeneratorResumeKind)+484>:	ud2

Jim, I wonder if this is from your recent changes.

Flags: needinfo?(jimb)
Priority: -- → P1

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/dfcdd2084fea
user: André Bargull
date: Tue Feb 26 08:33:30 2019 -0800
summary: Bug 1530324 - Part 7: Remove initial-yield for async functions. r=arai

Andre, is bug 1530324 a likely regressor?

Flags: needinfo?(andrebargull)
Regressed by: 1530324

Bug 1530324 removed the wrapper function for async functions, which likely changed how the test case from comment #0 interacts with async functions. But the actual issue here is also reproducible with plain generator functions, so I think this is an older issue.

Is it possible to run autobisectjs against the following test case?

Run with js --baseline-eager /tmp/test.js:

let g = newGlobal({newCompartment: true});
g.eval(`function* f(x) { yield; }`);
let dbg = new Debugger(g);
function test(when) {
  let hits = 0;
  dbg.onEnterFrame = frame => {
    if (hits++ === when) {
      return {
        return: "exit"
      };
    }
  };
  let it = g.f();
  it.next();
  it.throw();
}
test(2);
Flags: needinfo?(andrebargull) → needinfo?(nth10sd)

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/87509a363c9e
user: Jason Orendorff
date: Wed Aug 15 15:09:30 2018 -0500
summary: Bug 1475417 - Part 2: Fire onEnterFrame when resuming a generator or async function. r=jandem, r=jimb

I used Andre's testcase in comment 3. Jason, is bug 1475417 a likely regressor?

Flags: needinfo?(nth10sd) → needinfo?(jorendorff)
Regressed by: 1475417
No longer regressed by: 1530324

It would definitely affect the behavior of the code, even if it isn't the exact regressor. I'll have to take a look at this.

Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]

I can reproduce this in 8aa8bbbf0bee (2019-10-15) with the test case from comment 0. Taking.

Assignee: nobody → jimb
Flags: needinfo?(jimb)

Further trimmed down:

let g = newGlobal({newCompartment: true});
g.eval(`function* f(x) { }`);
let dbg = new Debugger(g);

let it = g.f();

dbg.onEnterFrame = () => ({ return: "exit" });
it.throw();
Assignee: jimb → loganfsmyth
Summary: Assertion failure: genObj->isRunning(), at js/src/vm/GeneratorObject.cpp:142 or Crash [@ JSObject::is<js::EnvironmentObject>] → [jsdbg2] Forced return from onEnterFrame caused by generator object 'throw' method crashes
Blocks: dbg-72
Attachment #9107772 - Attachment is obsolete: true

The debugger allows its hooks to return explicit completion values that should
take the place of any completion value of the function beig executed. It also
handles tearing down any in-progress generators and such.

In this case, we were attempting to treat the return completion from the
debugger as if it were a real inline 'return' statement, but that attempts to
evaluate the frame and run finally blocks and such, which we do not want
because debugger completions are more like function termination with a result.

Pushed by loganfsmyth@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/41919e4ed34d
Properly skip frame execution if the debugger overrides it. r=jandem
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
Flags: needinfo?(jorendorff) → in-testsuite+
You need to log in before you can comment on or make changes to this bug.