1901664 - Assertion failure: v.isSymbol() || v.isBigInt(), at jsnum.cpp:2058

Status: RESOLVED FIXED

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

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: testcase

Testcase is attached.

2058      MOZ_ASSERT(v.isSymbol() || v.isBigInt());
(gdb) bt
#0  js::ToNumberSlow (cx=0x7ffff6635100, v_=v_@entry=..., out=out@entry=0x7fffffffb0c0) at /home/ubumain/trees/mozilla-central/js/src/jsnum.cpp:2058
#1  0x000055555795b4cd in js::ToNumber (cx=0x7ffff7a1ca60 <_IO_stdfile_2_lock>, vp=...) at /home/ubumain/trees/mozilla-central/js/src/jsnum.h:244
#2  js::ToNumericSlow (cx=0x7ffff7a1ca60 <_IO_stdfile_2_lock>, vp=vp@entry=...) at /home/ubumain/trees/mozilla-central/js/src/jsnum.cpp:2084
#3  0x00005555572b5458 in js::ToNumeric (cx=0x7ffff6635100, vp=...) at /home/ubumain/trees/mozilla-central/js/src/jsnum.h:260
#4  js::PowOperation (cx=cx@entry=0x7ffff6635100, lhs=..., rhs=..., res=...) at /home/ubumain/trees/mozilla-central/js/src/vm/Interpreter-inl.h:801
#5  0x00005555572b8eaa in js::PowValues (cx=0x7ffff7a1ca60 <_IO_stdfile_2_lock>, cx@entry=0x7ffff6635100, lhs=..., rhs=..., res=..., res@entry=...) at /home/ubumain/trees/mozilla-central/js/src/vm/Interpreter.cpp:4807
#6  0x0000555557e92b8c in js::jit::DoBinaryArithFallback (cx=0x7ffff6635100, frame=0x7fffffffb458, stub=0x7ffff6532ee8, lhs=..., rhs=..., ret=...) at /home/ubumain/trees/mozilla-central/js/src/jit/BaselineIC.cpp:2295
#7  0x00000a1c8a24072f in ?? ()
#8  0xfff9800000000000 in ?? ()
#9  0x0000000000000065 in ?? ()
#10 0x00007fffffffb4a0 in ?? ()
#11 0x00000a1c8a268f3f in ?? ()
#12 0x0000000000000001 in ?? ()
#13 0x00007fffffffb458 in ?? ()
#14 0x00007ffff6532ee8 in ?? ()
#15 0xfff8800000000000 in ?? ()
#16 0xfffa800000000009 in ?? ()
#17 0xfffa800000000009 in ?? ()
#18 0xfff8800000000000 in ?? ()
#19 0xfff9800000000000 in ?? ()
#20 0xfff9800000000000 in ?? ()
#21 0x00000bddd8765510 in ?? ()
#22 0x00007ffff6644975 in ?? ()
#23 0x00007ffff6532ea8 in ?? ()
#24 0x00000bddd873f088 in ?? ()
#25 0x00007ffff6532de0 in ?? ()
#26 0x0000000000000000 in ?? ()
(gdb)

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/7dc6122ebd68
user:        Denis Palmeiro
date:        Wed Nov 22 01:53:40 2023 +0000
summary:     Bug 1847258: Use the warmup counter when the last IC is attached as the Ion hint threshold, and adjust inlining heuristics when the hint is used. r=iain

Run with --fuzzing-safe --no-threads --baseline-eager, compile with AR=ar sh ../configure --enable-debug --enable-debug-symbols --with-ccache --enable-nspr-build --enable-ctypes --enable-gczeal --enable-rust-simd --disable-tests, tested on m-c rev 9e49a1b86e40.

I did a quick search of previous similar assertion failures and most (if not all) were s-s, so setting this one as well.

Jan, do you mind taking a look as a start? Or perhaps you might wish to forward to Denis/Iain as necessary.

Comment 1

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

Set release status flags based on info from the regressing bug 1847258

Comment 2

[Jan de Mooij [:jandem]]

I think the problem is the optimization for MPow here where we unwrap MToDouble(Int32) for the power operand. We later replace the ToDouble with MagicValue(JS_OPTIMIZED_OUT) and crash in Baseline after a bailout. Calling setImplicitlyUsedUnchecked on the ToDouble fixes this.

I'll see if I can get a better test case for this.

Comment 3

[Jan de Mooij [:jandem]]

I don't think this is security-sensitive because in debug builds we fail the assertion here but non-debug builds will throw a "can't convert symbol to number" exception when we see a MagicValue there.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1901664 - Mark MPow's ToDouble operand as implicitly used when we replace it. r?iain!

The problem was that the ToDouble was captured in a resume point and later replaced
with the optimized-out magic value in EliminateDeadResumePointOperands. After a bailout,
the Pow code in Baseline could then see this magic value.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/29e82479760d
Mark MPow's ToDouble operand as implicitly used when we replace it. r=iain

Comment 6

[Aron Cseh]

https://hg.mozilla.org/mozilla-central/rev/29e82479760d

Comment 7

[Jan de Mooij [:jandem]]

Comment on attachment 9406961 [details]
Bug 1901664 - Mark MPow's ToDouble operand as implicitly used when we replace it. r?iain!

Beta/Release Uplift Approval Request

• User impact if declined: Potential crashes. This is a trivial fix that would be nice to have in ESR 128.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• 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): Just sets a flag on a MIR instruction to disable certain optimizations later.
• String changes made/needed:
• Is Android affected?: Yes

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9406961 [details]
Bug 1901664 - Mark MPow's ToDouble operand as implicitly used when we replace it. r?iain!

Approved for 128.0b4.

Comment 9

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/919095027a24