1876425 - Crash in [@ BaselineStackBuilder::setNextCallee]

Status: RESOLVED FIXED

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

Description

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

Crash report: https://crash-stats.mozilla.org/report/index/1d0af9e5-9b6e-4484-b67e-e50010240124

MOZ_CRASH Reason: MOZ_RELEASE_ASSERT(icScript_->numICEntries() == calleeScript->numICEntries())

Top 9 frames of crashing thread:

0  xul.dll  BaselineStackBuilder::setNextCallee  js/src/jit/BaselineBailouts.cpp:522
0  xul.dll  BaselineStackBuilder::buildStubFrame  js/src/jit/BaselineBailouts.cpp:1089
0  xul.dll  BaselineStackBuilder::prepareForNextFrame  js/src/jit/BaselineBailouts.cpp:926
0  xul.dll  BaselineStackBuilder::buildOneFrame  js/src/jit/BaselineBailouts.cpp:1524
0  xul.dll  js::jit::BailoutIonToBaseline  js/src/jit/BaselineBailouts.cpp:1662
1  xul.dll  js::jit::InvalidationBailout  js/src/jit/Bailouts.cpp:220
2  ?  @0x000003dedd971236  
3  xul.dll  mozilla::dom::binding_detail::GenericGetter<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>  dom/bindings/BindingUtils.cpp:3147
4  ?  @0x00000156b8920a37  

By querying Nightly crashes reported within the last 2 months, here are some insights about the signature:

• First crash report: 2023-12-17
• Process type: Content
• Is startup crash: No
• Has user comments: No
• Is null crash: Yes - 1 out of 9 crashes happened on null or near null memory address

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Core::JavaScript Engine: JIT' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Nicolas B. Pierron [:nbp]]

Matthew, this does not seems to be related to fuses, given that this crashes start in 122, but fuses might use this path more than the rest in the near future.
If you have any idea what else might be responsible for these invalidations feel free to forward this bug.

Looking at the crash reports some URL are notable as a source of issues. Maybe you will find a way to reproduce the crash this way.

Comment 3

[Matthew Gaudet (he/him) [:mgaudet]]

Going to redirect this to Jan, as this assert was added in Bug 1867193

Comment 4

[Jan de Mooij [:jandem]]

I was able to reproduce this with one of the URLs in the crash reports, and I have a shell test case for this.

Comment 5

[Jan de Mooij [:jandem]]

(In reply to Matthew Gaudet (he/him) [:mgaudet] from comment #3)

Going to redirect this to Jan, as this assert was added in Bug 1867193

The bug predates that bug, because I can reproduce it with a debug build of ESR 115 where it's a crash in JIT code. It even repros with a JS shell opt build of FF 113 (from the FTP server).

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1876425 part 1 - Stop using trial inlined ICScripts during bailout if needed. r?iain!

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1876425 part 2 - Add test and comment. r?iain!

Comment 8

[Jan de Mooij [:jandem]]

The most reliable STR I found for the browser:

1. Load https://www.netzclub.net/sim-karte-bestellen/sponsored-surf-basic
2. Wait 30 seconds. The tab will crash at some point.

After the first time you need to restart the browser or use a private window, to not let it skip the initial screen.

Comment 9

[Jan de Mooij [:jandem]]

Comment on attachment 9377977 [details]
Bug 1876425 part 1 - Stop using trial inlined ICScripts during bailout if needed. r?iain!

Security Approval Request

• How easily could an exploit be constructed based on the patch?: It's fairly difficult to write a test for this.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: All
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: Patch should apply or be easy to backport.
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely. Normal Nightly testing and fuzzing should be sufficient.
• Is Android affected?: Yes

Comment 10

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 9377977 [details]
Bug 1876425 part 1 - Stop using trial inlined ICScripts during bailout if needed. r?iain!

Approved to request uplift and land

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/920ea287d7ad
part 1 - Stop using trial inlined ICScripts during bailout if needed. r=iain

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/920ea287d7ad

Comment 13

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

The patch landed in nightly and beta is affected.
:jandem, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox123 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[Jan de Mooij [:jandem]]

Comment on attachment 9377977 [details]
Bug 1876425 part 1 - Stop using trial inlined ICScripts during bailout if needed. r?iain!

Beta/Release Uplift Approval Request

• User impact if declined: Security bugs, crashes.
• 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): Pretty small patch that has been tested on Nightly.
• String changes made/needed:
• Is Android affected?: Yes

Comment 15

[Pascal Chevrel:pascalc]

Comment on attachment 9377977 [details]
Bug 1876425 part 1 - Stop using trial inlined ICScripts during bailout if needed. r?iain!

Approved for 123 beta 9, thanks.

Comment 16

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/8901eb99b191

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9377977 [details]
Bug 1876425 part 1 - Stop using trial inlined ICScripts during bailout if needed. r?iain!

This needs a rebased patch for esr115.

Comment 18

[Jan de Mooij [:jandem]]

Attachment: Bug 1876425 part 1 - Stop using trial inlined ICScripts during bailout if needed. r=iain! (ESR115)

Comment 19

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/427c475a8b7e

Comment 20

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

a month ago, tjr placed a reminder on the bug using the whiteboard tag [reminder-test 2024-04-02] .

jandem, please refer to the original comment to better understand the reason for the reminder.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fb90c48c8fab
part 2 - Add test and comment. r=iain

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/fb90c48c8fab