1519700 - Hit MOZ_CRASH(Invalid try note) at js/src/vm/Interpreter.cpp:1200 with generator function

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision a44934afe25e (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):

function* wrapNoThrow() {
    let iter = {
        [Symbol.iterator]() {
            return this;
        },
        next() {
            return {};
        },
        return () {}
    };
    for (const i of iter)
        yield i;
}
function foo() {
    l2: for (j of wrapNoThrow()) {
        for (i of [1, 2, 3])
            break l2;
        return false;
    }
}
foo();

Backtrace:

received signal SIGSEGV, Segmentation fault.
ProcessTryNotes (regs=..., ei=..., cx=0x7ffff5f18000) at js/src/vm/Interpreter.cpp:1200
#0 ProcessTryNotes (regs=..., ei=..., cx=0x7ffff5f18000) at js/src/vm/Interpreter.cpp:1200
#1 HandleError (regs=..., cx=<optimized out>) at js/src/vm/Interpreter.cpp:1263
#2 Interpret (cx=0x555556a8d6c7, state=...) at js/src/vm/Interpreter.cpp:4368
#3 0x00005555558f2f95 in js::RunScript (cx=0x7ffff5f18000, state=...) at js/src/vm/Interpreter.cpp:424
#4 0x00005555558f6451 in js::ExecuteKernel (cx=<optimized out>, script=..., script@entry=..., envChainArg=..., newTargetValue=..., evalInFrame=..., evalInFrame@entry=..., result=result@entry=0x0) at js/src/vm/Interpreter.cpp:783
#5 0x00005555558f6861 in js::Execute (cx=<optimized out>, cx@entry=0x7ffff5f18000, script=script@entry=..., envChainArg=..., rval=rval@entry=0x0) at js/src/vm/Interpreter.cpp:817
#6 0x0000555555a07173 in ExecuteScript (cx=0x7ffff5f18000, scope=scope@entry=..., script=..., rval=rval@entry=0x0) at js/src/vm/CompilationAndEvaluation.cpp:438
[...]
#12 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:11302
rax 0x555557bf7280 93825032745600
rbx 0x555556a8d6c7 93825014486727
rcx 0x7ffff6c1c2dd 140737333281501
rdx 0x0 0
rsi 0x7ffff6eeb770 140737336227696
rdi 0x7ffff6eea540 140737336223040
rbp 0x7fffffffcec0 140737488342720
rsp 0x7fffffffc8f0 140737488341232
r8 0x7ffff6eeb770 140737336227696
r9 0x7ffff7fe6c80 140737354034304
r10 0x58 88
r11 0x7ffff6b927a0 140737332717472
r12 0x7fffffffcd00 140737488342272
r13 0x7fffffffcd00 140737488342272
r14 0x7fffffffcdd0 140737488342480
r15 0x7fffffffd090 140737488343184
rip 0x5555558e5639 <Interpret(JSContext*, js::RunState&)+4937>
=> 0x5555558e5639 <Interpret(JSContext*, js::RunState&)+4937>: movl $0x0,0x0
0x5555558e5644 <Interpret(JSContext*, js::RunState&)+4948>: ud2

Comment 1

[Iain Ireland [:iain]]

In retrospect, this bug is obvious. It was introduced as part of my patch for bug 1480390.

The offending code is here, in TryNoteIter::settle():

      if (tn_->kind == JSTRY_FOR_OF_ITERCLOSE) {
        do {
          ++tn_;
          MOZ_ASSERT(tn_ != tnEnd_);
          MOZ_ASSERT_IF(pcInRange(), tn_->kind != JSTRY_FOR_OF_ITERCLOSE);
        } while (!(pcInRange() && tn_->kind == JSTRY_FOR_OF));

        // Advance to trynote following the enclosing for-of.
        ++tn_;
      }
      [...]
      if (tn_ == tnEnd_ || isTryNoteValid_(tn_)) {
        return;
      }

When we see a for-of-iterclose, we have to skip ahead in the list of trynotes until we see the enclosing for-of. The bug occurs when we advance to the trynote following that enclosing for-of. tn_++ will advance to the next trynote, but will not check to see if it is in range of the current pc. If it is at a valid stack depth (which is what isTryNoteValid checks), then we will return the wrong trynote.

The fix is to replace |tn_++;| with |continue;|. This will advance the cursor while ensuring that we hit the pcInRange check at the top of the loop.

Comment 2

[Iain Ireland [:iain]]

Attachment: Bug 1519700: Fix incorrect logic in TryNoteIter::settle r=djvj

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9a76ebcfbe9d
Fix incorrect logic in TryNoteIter::settle r=djvj

Comment 4

[Iain Ireland [:iain]]

Attachment: Bug 1519700: Add testcase to jit-tests r=djvj

Comment 5

[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/025feea5945b
user:        Iain Ireland
date:        Fri Jan 11 18:05:36 2019 +0000
summary:     Bug 1480390: Move ForOfIterClose logic inside TryNoteIter r=tcampbell

This iteration took 534.324 seconds to run.

Comment 6

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/9a76ebcfbe9d

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9882be81c9e4
Add testcase to jit-tests r=djvj

Comment 8

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/9882be81c9e4