1620125 - crash near null in [@ gfxFont::DrawOneGlyph]

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect, P2)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Reduced with m-c: 20200304-616ea1d7d24d

==9591==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000040 (pc 0x7f9579392372 bp 0x7ffdcb653910 sp 0x7ffdcb653640 T0)
==9591==The signal is caused by a READ memory access.
==9591==Hint: address points to the zero page.
    #0 0x7f9579392371 in void gfxFont::DrawOneGlyph<(gfxFont::FontComplexityT)1>(unsigned int, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float> const&, GlyphBufferAzure&, bool*) const src/gfx/thebes/gfxFont.cpp
    #1 0x7f95792ab68f in bool gfxFont::DrawGlyphs<(gfxFont::FontComplexityT)1, (gfxFont::SpacingT)0>(gfxShapedText const*, unsigned int, unsigned int, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float>*, mozilla::gfx::BaseMatrix<float> const*, GlyphBufferAzure&) src/gfx/thebes/gfxFont.cpp:1850:13
    #2 0x7f95792a888b in gfxFont::Draw(gfxTextRun const*, unsigned int, unsigned int, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float>*, TextRunDrawParams const&, mozilla::gfx::ShapedTextFlags) src/gfx/thebes/gfxFont.cpp:2279:13
    #3 0x7f95793d0440 in gfxTextRun::DrawGlyphs(gfxFont*, gfxTextRun::Range, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float>*, gfxTextRun::PropertyProvider*, gfxTextRun::Range, TextRunDrawParams&, mozilla::gfx::ShapedTextFlags) const src/gfx/thebes/gfxTextRun.cpp:417:10
    #4 0x7f95793d25ed in gfxTextRun::Draw(gfxTextRun::Range, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float>, gfxTextRun::DrawParams const&) const src/gfx/thebes/gfxTextRun.cpp:661:5
    #5 0x7f957892e42e in nsFontMetrics::DrawString(char16_t const*, unsigned int, int, int, gfxContext*, mozilla::gfx::DrawTarget*) src/gfx/src/nsFontMetrics.cpp:343:12
    #6 0x7f957e4271b6 in nsLayoutUtils::DrawUniDirString(char16_t const*, unsigned int, nsPoint const&, nsFontMetrics&, gfxContext&) src/layout/base/nsLayoutUtils.cpp:6172:18
    #7 0x7f957e534349 in BulletRenderer::PaintTextToContext(nsIFrame*, gfxContext*, bool) src/layout/generic/nsBulletFrame.cpp:415:3
    #8 0x7f957e5339fb in BulletRenderer::Paint(gfxContext&, nsPoint, nsRect const&, unsigned int, bool, nsIFrame*) src/layout/generic/nsBulletFrame.cpp:395:5
    #9 0x7f957e537d98 in nsBulletFrame::PaintBullet(gfxContext&, nsPoint, nsRect const&, unsigned int, bool) src/layout/generic/nsBulletFrame.cpp:817:14
    #10 0x7f957e537ab8 in nsDisplayBullet::Paint(nsDisplayListBuilder*, gfxContext*) src/layout/generic/nsBulletFrame.cpp:643:63
    #11 0x7f957eaf6e5b in mozilla::FrameLayerBuilder::PaintItems(std::vector<mozilla::AssignedDisplayItem, std::allocator<mozilla::AssignedDisplayItem> >&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, gfxContext*, nsDisplayListBuilder*, nsPresContext*, mozilla::gfx::IntPointTyped<mozilla::gfx::UnknownUnits> const&, float, float) src/layout/painting/FrameLayerBuilder.cpp:7122:20
    #12 0x7f957eaf91e1 in mozilla::FrameLayerBuilder::DrawPaintedLayer(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*) src/layout/painting/FrameLayerBuilder.cpp:7282:19
    #13 0x7f9578f93bcd in mozilla::layers::ClientPaintedLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor*) src/gfx/layers/client/ClientPaintedLayer.cpp:159:9
    #14 0x7f9578fbfd44 in mozilla::layers::ClientContainerLayer::RenderLayer() src/gfx/layers/client/ClientContainerLayer.h:53:29
    #15 0x7f9578f897e7 in mozilla::layers::ClientLayerManager::EndTransactionInternal(void (*)(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) src/gfx/layers/client/ClientLayerManager.cpp:352:13
    #16 0x7f9578f8b0aa in mozilla::layers::ClientLayerManager::EndTransaction(void (*)(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) src/gfx/layers/client/ClientLayerManager.cpp:415:3
    #17 0x7f957eb75783 in nsDisplayList::PaintRoot(nsDisplayListBuilder*, gfxContext*, unsigned int) src/layout/painting/nsDisplayList.cpp:3144:19
    #18 0x7f957e4186a5 in nsLayoutUtils::PaintFrame(gfxContext*, nsIFrame*, nsRegion const&, unsigned int, nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) src/layout/base/nsLayoutUtils.cpp:4095:13
    #19 0x7f957e3314e4 in mozilla::PresShell::Paint(nsView*, nsRegion const&, mozilla::PaintFlags) src/layout/base/PresShell.cpp:6068:5
    #20 0x7f957dd7693e in nsViewManager::ProcessPendingUpdatesPaint(nsIWidget*) src/view/nsViewManager.cpp:461:18
    #21 0x7f957dd7600d in nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) src/view/nsViewManager.cpp:396:22
    #22 0x7f957dd78b0d in nsViewManager::ProcessPendingUpdates() src/view/nsViewManager.cpp:1019:5
    #23 0x7f957e2acf49 in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:2186:11
    #24 0x7f957e2ba6e6 in TickDriver src/layout/base/nsRefreshDriver.cpp:374:13
    #25 0x7f957e2ba6e6 in mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:351:7
    #26 0x7f957e2ba27c in mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:368:5
    #27 0x7f957e2b9262 in RunRefreshDrivers src/layout/base/nsRefreshDriver.cpp:825:5
    #28 0x7f957e2b9262 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:743:16
    #29 0x7f957e2b8daf in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NormalPriorityNotify() src/layout/base/nsRefreshDriver.cpp:659:9
    #30 0x7f957e2b9704 in applyImpl<mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver, void (mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::*)()> src/obj-firefox/dist/include/nsThreadUtils.h:1158:12
    #31 0x7f957e2b9704 in apply<mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver, void (mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::*)()> src/obj-firefox/dist/include/nsThreadUtils.h:1164:12
    #32 0x7f957e2b9704 in mozilla::detail::RunnableMethodImpl<mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver*, void (mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::*)(), true, (mozilla::RunnableKind)0>::Run() src/obj-firefox/dist/include/nsThreadUtils.h:1210:13
    #33 0x7f9575ad3628 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1220:14
    #34 0x7f9575ade48c in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:481:10
    #35 0x7f9576d65f1f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:87:21
    #36 0x7f9576c56f17 in RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
    #37 0x7f9576c56f17 in RunHandler src/ipc/chromium/src/base/message_loop.cc:308:3
    #38 0x7f9576c56f17 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290:3
    #39 0x7f957de1c848 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
    #40 0x7f958199c606 in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:926:20
    #41 0x7f9576c56f17 in RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
    #42 0x7f9576c56f17 in RunHandler src/ipc/chromium/src/base/message_loop.cc:308:3
    #43 0x7f9576c56f17 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290:3
    #44 0x7f958199bcc8 in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:761:34
    #45 0x55bfd4d60f83 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #46 0x55bfd4d60f83 in main src/browser/app/nsBrowserApp.cpp:303:18

Comment 1

[Aria Beingessner [:Gankra]]

Could you provide an about:support for the config you can reproduce this crash in?

Tentatively marking as high priority since this is a pretty reasonable input, and null pointer accesses are pretty scary security-wise.

Comment 2

[Jonathan Kew [:jfkthame]]

Probably a regression from bug 1616995. Just checking...

Comment 3

[Jonathan Kew [:jfkthame]]

Definitely bug 1616995, and I think I see the problem. Patch coming, after I test it locally.

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1620125 - Don't assume mVerticalMetrics has necessarily been initialized before DrawOneGlyph is called. r=lsalzman

Comment 5

[Aria Beingessner [:Gankra]]

Reproduced on the Toronto Office's testing machine. Doesn't happen with WR, only D3D11.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c8f5a8ec70c4
Don't assume mVerticalMetrics has necessarily been initialized before DrawOneGlyph is called. r=lsalzman

Comment 7

[Jason Kratzer [:jkratzer]]

BugMon: Verified bug as reproducible on 2e1a978b09d7

Comment 8

[Jason Kratzer [:jkratzer]]

BugMon: Reduced build range to...
> Start: fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 (20200228161940)
> End: 4f2f9bda72ac1c91576d2ca18d8323fe4104d99f (20200229040456)
> Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fc9d28ae4655254a6ef8a57ac6577545b2a42ca7&tochange=4f2f9bda72ac1c91576d2ca18d8323fe4104d99f

Comment 9

[Jonathan Kew [:jfkthame]]

Comment on attachment 9131177 [details]
Bug 1620125 - Don't assume mVerticalMetrics has necessarily been initialized before DrawOneGlyph is called. r=lsalzman

Beta/Release Uplift Approval Request

• User impact if declined: Potential crash with vertical text
• 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: Load testcase from bug. (Also included as crashtest with the patch, currently on autoland.)
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Trivial patch to call safe accessor method instead of directly accessing metrics field (that may not be set yet)
• String changes made/needed:

Comment 10

[Jonathan Kew [:jfkthame]]

I know it's very late in the cycle, but the patch is sufficiently trivial that I think we should take it if possible rather than shipping this crash bug.

Comment 11

[Jonathan Kew [:jfkthame]]

Comment on attachment 9131177 [details]
Bug 1620125 - Don't assume mVerticalMetrics has necessarily been initialized before DrawOneGlyph is called. r=lsalzman

Sorry, ignore this... doesn't need uplift as we're currently still on the same version as the regression landed.

Comment 12

[Alexandru Michis [:malexandru]]

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