Crash in mozilla::dom::FontFace::Entry::GetUserFontSets
Categories
(Core :: Layout: Text and Fonts, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox64 | --- | unaffected |
firefox65 | --- | unaffected |
firefox66 | --- | fixed |
firefox67 | --- | fixed |
People
(Reporter: philipp, Unassigned)
References
Details
(Keywords: crash, regression)
Crash Data
This bug is for crash report bp-684c5f60-faf8-4b41-afb5-e64360190124.
Top 7 frames of crashing thread:
0 libxul.so mozilla::dom::FontFace::Entry::GetUserFontSets mfbt/RefPtr.h:267
1 libxul.so nsFontFaceLoader::LoadTimerCallback layout/style/nsFontFaceLoader.cpp:171
2 libxul.so nsTimerEvent::Run xpcom/threads/nsTimerImpl.cpp:559
3 libxul.so nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1160
4 libxul.so NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:468
5 libxul.so nsThreadManager::SpinEventLoopUntilInternal xpcom/threads/nsThreadUtils.h:335
6 libxul.so _NS_InvokeByIndex
crashes with this signature are starting to show up since 66.0a1 build 20190122094123 and they are more common on fennec.cthe changes of bug 1519918 are in the pushlog for that build, could they be related?
Comment 1•5 years ago
|
||
Hmm, I don't understand how this crash can happen... The only way GetUserFontSets can crash on a null deref is with a null this
pointer, which would make sense, except in LoadTimerCallback we've already dereferenced it a bunch of times...
Tyson, do you or somebody from your team happen to have any test-case that could resemble this signature?
Comment 2•5 years ago
|
||
I don't see anything that looks similar at the moment, neither does jkratzer. We will be sure to update this bug if we find something.
Comment 3•5 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #1)
Hmm, I don't understand how this crash can happen... The only way GetUserFontSets can crash on a null deref is with a null
this
pointer, which would make sense, except in LoadTimerCallback we've already dereferenced it a bunch of times...
If I had to guess, I think it may actually be at the loader->mUserFontEntry.get() call[1] that we're crashing, but optimization reordering somehow ends up causing the stack to blame the later GetUserFontSets call. The loader's RefPtr<gfxUserFontEntry> mUserFontEntry member is the thing that seems to involve RefPtr in the execution of this code.
Very possibly unrelated, but we have also recently had a couple of nsFontFaceLoader crashes reported recently[2].
Tyson: any testcases with crashes where nsFontFaceLoader members such as the member are null?
Comment 4•5 years ago
|
||
I don't see any that match that description either. I have checked our code coverage reports and only our css variable-font fuzzer hits this code path atm. I have added some basic support to another more general Layout/DOM fuzzer we have as well. We'll see if that shakes anything out.
:jwatt, :emilio do you have any suggested code patterns or example test cases that might help exercise this code in different ways? I might be able to add those ideas to the fuzzer as well.
:jkratzer Is this something you can add to Dominode?
Comment 5•5 years ago
|
||
Adding valid / invalid / slow @font-face rules should trigger different related codepaths. It's a bit trickier than that because in order to trigger the load you need to have some text on the page using that font, so you need to make sure that happens. Alternatively / additionally using the document.fonts API should also help here, with the same caveat (IIRC).
Comment 6•5 years ago
|
||
I'm working on implementing this into Dominode. I should have it running within the next week.
Updated•5 years ago
|
Reporter | ||
Comment 7•5 years ago
|
||
the issue looks fixed in 66.0b6.
Updated•5 years ago
|
Description
•