1728588 - Assertion failure: data->nfixed() <= AbstractGeneratorObject::FixedSlotLimit, at js/src/vm/JSScript-inl.h:176

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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

a = [];
for (i = 0; i < 256; ++i) { a.push("x" + i); }
parseModule(`
  var g68 = newGlobal0;
  let ` + a.join(",") + `
  await Super;
`);

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x0000555556f77810 in JSScript::fullyInitFromStencil(JSContext*, js::frontend::CompilationAtomCache const&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&, JS::Handle<JSScript*>, js::frontend::TypedIndex<js::frontend::ScriptStencil>) ()
#1  0x0000555556f78be2 in JSScript::fromStencil(JSContext*, js::frontend::CompilationAtomCache&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&, js::frontend::TypedIndex<js::frontend::ScriptStencil>) ()
#2  0x0000555557462577 in InstantiateTopLevel(JSContext*, js::frontend::CompilationInput&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&) ()
#3  0x00005555574611a2 in js::frontend::CompilationStencil::instantiateStencilAfterPreparation(JSContext*, js::frontend::CompilationInput&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&) ()
#4  0x00005555573dc661 in js::frontend::InstantiateStencils(JSContext*, js::frontend::CompilationInput&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&) ()
#5  0x000055555741508f in bool ParseModuleToStencilAndMaybeInstantiate<char16_t>(JSContext*, js::frontend::CompilationInput&, JS::SourceText<char16_t>&, mozilla::Variant<mozilla::UniquePtr<js::frontend::ExtensibleCompilationStencil, JS::DeletePolicy<js::frontend::ExtensibleCompilationStencil> >, mozilla::UniquePtr<js::frontend::CompilationStencil, JS::DeletePolicy<js::frontend::CompilationStencil> >, js::frontend::CompilationGCOutput*>&) ()
#6  0x00005555573de82e in js::frontend::CompileModule(JSContext*, JS::ReadOnlyCompileOptions const&, JS::SourceText<char16_t>&) ()
#7  0x0000555556aa7cb0 in ParseModule(JSContext*, unsigned int, JS::Value*) ()
[...]
#13 0x0000000000000000 in ?? ()
rax	0x55555574cb50	93824994298704
rbx	0x3b067b1660b0	64899020906672
rcx	0x555558144a30	93825038305840
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7fffffffa070	140737488330864
rsp	0x7fffffff9fe0	140737488330720
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f99840	140737353717824
r10	0x0	0
r11	0x0	0
r12	0x0	0
r13	0x7ffff6019000	140737320685568
r14	0x7fffffffa0b0	140737488330928
r15	0x7fffffffa488	140737488331912
rip	0x555556f77810 <JSScript::fullyInitFromStencil(JSContext*, js::frontend::CompilationAtomCache const&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&, JS::Handle<JSScript*>, js::frontend::TypedIndex<js::frontend::ScriptStencil>)+2224>
=> 0x555556f77810 <_ZN8JSScript20fullyInitFromStencilEP9JSContextRKN2js8frontend20CompilationAtomCacheERKNS3_18CompilationStencilERNS3_19CompilationGCOutputEN2JS6HandleIPS_EENS3_10TypedIndexINS3_13ScriptStencilEEE+2224>:	movl   $0xb0,0x0
   0x555556f7781b <_ZN8JSScript20fullyInitFromStencilEP9JSContextRKN2js8frontend20CompilationAtomCacheERKNS3_18CompilationStencilERNS3_19CompilationGCOutputEN2JS6HandleIPS_EENS3_10TypedIndexINS3_13ScriptStencilEEE+2235>:	callq  0x555556b0b6fa <abort>

Marking as security bug because this is a range 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 20210901155113-0ee7ea150df4.
The bug appears to have been introduced in the following build range:

Start: fef56f826d6496a73b1235abb1aaeae6dbb27f13 (20210817125524)
End: 0242c80e23928675d6c9d2748c9fe90df80b0aaa (20210817131624)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=fef56f826d6496a73b1235abb1aaeae6dbb27f13&tochange=0242c80e23928675d6c9d2748c9fe90df80b0aaa

Comment 4

[Steven DeTar [:sdetar]]

Jan, could you help to investigate this new bug.

Comment 5

[Iain Ireland [:iain]]

I took a quick look at this. It looks like top-level await breaks if there are more than 256 (AbstractGeneratorObject::FixedSlotLimit) locals. "Simplified" testcase (run with --module):

let a0, a1, a2, a3, a4, a5, a6, a7, a8, a9;
let b0, b1, b2, b3, b4, b5, b6, b7, b8, b9;
let c0, c1, c2, c3, c4, c5, c6, c7, c8, c9;
let d0, d1, d2, d3, d4, d5, d6, d7, d8, d9;
let e0, e1, e2, e3, e4, e5, e6, e7, e8, e9;
let f0, f1, f2, f3, f4, f5, f6, f7, f8, f9;
let g0, g1, g2, g3, g4, g5, g6, g7, g8, g9;
let h0, h1, h2, h3, h4, h5, h6, h7, h8, h9;
let i0, i1, i2, i3, i4, i5, i6, i7, i8, i9;
let j0, j1, j2, j3, j4, j5, j6, j7, j8, j9;
let k0, k1, k2, k3, k4, k5, k6, k7, k8, k9;
let l0, l1, l2, l3, l4, l5, l6, l7, l8, l9;
let m0, m1, m2, m3, m4, m5, m6, m7, m8, m9;
let n0, n1, n2, n3, n4, n5, n6, n7, n8, n9;
let o0, o1, o2, o3, o4, o5, o6, o7, o8, o9;
let p0, p1, p2, p3, p4, p5, p6, p7, p8, p9;
let q0, q1, q2, q3, q4, q5, q6, q7, q8, q9;
let r0, r1, r2, r3, r4, r5, r6, r7, r8, r9;
let s0, s1, s2, s3, s4, s5, s6, s7, s8, s9;
let t0, t1, t2, t3, t4, t5, t6, t7, t8, t9;
let u0, u1, u2, u3, u4, u5, u6, u7, u8, u9;
let v0, v1, v2, v3, v4, v5, v6, v7, v8, v9;
let w0, w1, w2, w3, w4, w5, w6, w7, w8, w9;
let x0, x1, x2, x3, x4, x5, x6, x7, x8, x9;
let y0, y1, y2, y3, y4, y5, y6, y7, y8, y9;
let z0, z1, z2, z3, z4, z5, z6, z7, z8, z9;

await 3;

If we instead put the locals inside an async function and await that, we don't end up in BaseScript::initSharedData. I don't know enough about top-level await and stencils to know whether this is a stencil thing or a top-level await thing.

Ted? Yulia?

Comment 6

[Ted Campbell [:tcampbell]]

That check was introduced as part of the Bug 1412202 optimization. The simple fix is for this to also check scope.tooBigToOptimize(). Only remaining question is if we should make that conditional on await (so that sync modules can have as many vars as they want without deopt).

Comment 7

[Ted Campbell [:tcampbell]]

The simple fix correctly leaves sync modules alone and only de-opts large async modules. Trying to assess security implications still, but since the regression is from current nightly cycle, I guess we just land fix ASAP.

Comment 8

[Ted Campbell [:tcampbell]]

According to this, the bug is only a performance heuristic. I'll ask Jan to confirm in review, but this is probably not a sec bug after all.

Comment 9

[Ted Campbell [:tcampbell]]

Attachment: Bug 1728588 - Fix storage heuristic for async JS modules. r?jandem!

With top-level await now supported, ModuleScopes use the generator machinery. As
a heuristic, we use different storage strategies for generators with few vs many
local variables. This patch applies that condition to module scopes as well for
consistency (and to avoid tripping checks).

Comment 10

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

:tcampbell, since this bug contains a bisection range, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0ca93054811d
Fix storage heuristic for async JS modules. r=jandem

Comment 12

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/0ca93054811d

Comment 13

[Bugmon [:jkratzer for issues]]

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