1762769 - Assertion failure: graph().osrBlock(), at js/src/jit/WarpBuilder.cpp:298 or Assertion failure: osrEntryOffset_.isSome(), at jit/shared/CodeGenerator-shared.h:126 or Crash [@ js::jit::CodeGenerator::link]

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect, P2)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20220403-b9165a6769de (debug build, run with --fuzzing-safe --ion-offthread-compile=off):

try {
  try {} finally {
    throw a;
  }
} catch {}
for (;;);

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x0000555557769a09 in js::jit::WarpBuilder::build() ()
#1  0x0000555557a5ba7e in js::jit::CompileBackEnd(js::jit::MIRGenerator*, js::jit::WarpSnapshot*) ()
#2  0x0000555557a5d1bc in js::jit::Compile(JSContext*, JS::Handle<JSScript*>, js::jit::BaselineFrame*, unsigned char*) ()
#3  0x0000555557a5dc48 in IonCompileScriptForBaseline(JSContext*, js::jit::BaselineFrame*, unsigned char*) ()
#4  0x0000555557a5e37a in js::jit::IonCompileScriptForBaselineOSR(JSContext*, js::jit::BaselineFrame*, unsigned int, unsigned char*, js::jit::IonOsrTempData**) ()
#5  0x00002873171f2dd7 in ?? ()
#6  0x0000000000000000 in ?? ()
rax	0x555555842841	93824995305537
rbx	0x7fffffffb620	140737488336416
rcx	0x5555581b3898	93825038760088
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7fffffffb600	140737488336384
rsp	0x7fffffffb5f0	140737488336368
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f99840	140737353717824
r10	0x0	0
r11	0x0	0
r12	0x7ffff4ead178	140737302417784
r13	0x7ffff4ead2f8	140737302418168
r14	0x7ffff4ead2f8	140737302418168
r15	0x7fffffffb998	140737488337304
rip	0x555557769a09 <js::jit::WarpBuilder::build()+265>
=> 0x555557769a09 <_ZN2js3jit11WarpBuilder5buildEv+265>:	movl   $0x12a,0x0
   0x555557769a14 <_ZN2js3jit11WarpBuilder5buildEv+276>:	callq  0x555556b76777 <abort>

S-s until triaged because this is a JIT assert.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as reproducible on mozilla-central 20220403215202-2d8724cbbddd.
The bug appears to have been introduced in the following build range:

Start: 60080026a143c4f4515557c1704b0b3775607001 (20220330234750)
End: 3358bc3f9c0891fd1e14c7b9903085be32818646 (20220330235837)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=60080026a143c4f4515557c1704b0b3775607001&tochange=3358bc3f9c0891fd1e14c7b9903085be32818646

Comment 4

[Jan de Mooij [:jandem]]

We try to trigger OSR for the infinite loop, but Warp must think the loop is unreachable and doesn't add the OSR block.

(The CodeGenerator release assertion probably saves us here, security-wise..)

Comment 5

[Iain Ireland [:iain]]

Yeah, the problem here is that BytecodeAnalysis assumes that if a JSOp::ResumeIndex (which pushes the resume address for a finally) is normally reachable, then so is that resume address. This is false if the finally block unconditionally throws, because we won't reach the end of the finally block.

I'll have to think a bit about the best way to fix this.

Comment 6

[Daniel Veditz [:dveditz]]

Going to tag this sec-moderate for now because of the uncertainty that the release assert "probably" saves us

Comment 7

[Iain Ireland [:iain]]

Attachment: Bug 1762769: Don't mark ops only reachable via retsub as normally reachable r=jandem

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Don't mark ops only reachable via retsub as normally reachable r=jandem
https://hg.mozilla.org/integration/autoland/rev/0ba054ab33393a437711d2746a2111206b08e0db
https://hg.mozilla.org/mozilla-central/rev/0ba054ab3339

Comment 9

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as fixed on rev mozilla-central 20220412035701-0bcad14b3c3a.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 11

[lukas.bernhard]

The patch also fixes another assertion found via fuzzing: Assertion failure: !returns.empty(), at js/src/jit/WarpBuilder.cpp:3401
There the sample, just in case you're interested:

function v5() {
    try {
        if (v4) {
            throw RegExp;
        } else {
            try {
            } finally {
                throw 1;
            }
        }
    } catch(v8) {
    }
}

function main() {
    for (let v4 = 0; v4 < 40; v4++) {
        const v9 = v5();
    }
}
main();

invoked as: obj-x86_64-pc-linux-gnu/dist/bin/js --fast-warmup --no-threads --cpu-count=1 --ion-offthread-compile=off --fuzzing-safe --differential-testing

Comment 12

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

The patch landed in nightly and beta is affected.
:iain, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 13

[Iain Ireland [:iain]]

Comment on attachment 9271163 [details]
Bug 1762769: Don't mark ops only reachable via retsub as normally reachable r=jandem

Beta/Release Uplift Approval Request

• User impact if declined: Easily triggered crashes from JS
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a well-understood one-line fix to a recently introduced bug.

Crash stats isn't showing any crashes in the wild yet, so it might also be reasonable to let this ride the trains.

• String changes made/needed:

Comment 14

[Dianna Smith [:diannaS]]

Comment on attachment 9271163 [details]
Bug 1762769: Don't mark ops only reachable via retsub as normally reachable r=jandem

Approved for 100.0b7

Comment 15

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7f9e531acb24