1726515 - Crash in [@ Servo_FontFamily_Generic]

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Gabriele Svelto [:gsvelto] (PTO)]

Crash report: https://crash-stats.mozilla.org/report/index/6a3b971b-296c-4d8c-9800-fb74c0210819

Reason: SIGSEGV /SEGV_MAPERR

Top 10 frames of crashing thread:

0 libxul.so Servo_FontFamily_Generic /usr/src/debug/firefox-91.0-1.fc34.x86_64/servo/ports/geckolib/glue.rs:7098
1 libxul.so mozilla::dom::CanvasRenderingContext2D::GetCurrentFontStyle /usr/src/debug/firefox-91.0-1.fc34.x86_64/dom/canvas/CanvasRenderingContext2D.cpp:4088
2 libxul.so mozilla::dom::CanvasRenderingContext2D::GetFont /usr/src/debug/firefox-91.0-1.fc34.x86_64/objdir/dist/include/mozilla/dom/CanvasRenderingContext2D.h:280
3 libxul.so mozilla::dom::CanvasRenderingContext2D_Binding::get_font /usr/src/debug/firefox-91.0-1.fc34.x86_64/objdir/dom/bindings/CanvasRenderingContext2DBinding.cpp:6546
4 libxul.so bool mozilla::dom::binding_detail::GenericGetter<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions> /usr/src/debug/firefox-91.0-1.fc34.x86_64/dom/bindings/BindingUtils.cpp:3179
5 libxul.so js::InternalCallOrConstruct 
6 libxul.so js::CallGetter 
7 libxul.so js::NativeGetProperty 
8 libxul.so js::GetProperty 
9 libxul.so Interpret 

This seems to be specific to Fedora (all crash reports are coming from Fedora 34 to be precise) and it happens when navigating eBay according to the comments. Martin can you have a look?

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Martin, Fedora does compile with GCC, right?

This smells like similar ABI issues we've seen with GCC-calling-into-LLVM in the past (see https://bugs.llvm.org/show_bug.cgi?id=44228 / bug 1600735). Same case of passing a single u8 enum value by value in a function with a single argument, and that getting compiled to some indexing operation causing the issue.

Should be very easy to work around.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1726515 - Workaround GCC-calling-into-LLVM ABI issue by making GenericFontFamily larger. r=stransky,#layout-reviewers

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd6b1ac432af
Workaround GCC-calling-into-LLVM ABI issue by making GenericFontFamily larger. r=hiro

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Martin, you probably want to cherry-pick this change in Fedora.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9237096 [details]
Bug 1726515 - Workaround GCC-calling-into-LLVM ABI issue by making GenericFontFamily larger. r=stransky,#layout-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Crashes on builds compiled with GCC.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• 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): Changes the size of the enum to avoid an ABI issue between Rust and C++.
• String changes made/needed: none

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: See above
• User impact if declined: See above
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): See above
• String or UUID changes made by this patch: none

Comment 7

[Emilio Cobos Álvarez (:emilio)]

This doesn't affect mozilla builds because clang always zero-extends in the caller. But we might want to get this in the release repo anyways.

Comment 8

[Martin Stránský [:stransky] (ni? me)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)

Martin, you probably want to cherry-pick this change in Fedora.

Thanks for the info!

Comment 9

[Sandor Molnar[:smolnar]]

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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9237096 [details]
Bug 1726515 - Workaround GCC-calling-into-LLVM ABI issue by making GenericFontFamily larger. r=stransky,#layout-reviewers

Approved for 92.0b8 and 91.1esr. I think we can live with it on release for another couple weeks, though.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1face96c4d3c

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/65fd2287abed