1325159 - Null-deref in [@ ShouldBuildLayerEvenIfInvisible]

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect, P1)

Description

[Jesse Schwartzentruber (:truber)]

Attachment: testcase.html

The attached testcase crashes in mozilla-central rev c36fbe84042d

==23952==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f9c5cd28fb4 bp 0x7fff935ba170 sp 0x7fff935ba170 T0)
    #0 0x7f9c5cd28fb3 in nsDisplayPerspective::ShouldBuildLayerEvenIfInvisible(nsDisplayListBuilder*) /home/worker/workspace/build/src/layout/base/nsDisplayList.h:4400:12
    #1 0x7f9c5ca656f3 in mozilla::ContainerState::ProcessDisplayItems(nsDisplayList*) /home/worker/workspace/build/src/layout/base/FrameLayerBuilder.cpp:4207:12
    #2 0x7f9c5ca767d5 in mozilla::FrameLayerBuilder::BuildContainerLayerFor(nsDisplayListBuilder*, mozilla::layers::LayerManager*, nsIFrame*, nsDisplayItem*, nsDisplayList*, mozilla::ContainerLayerParameters const&, mozilla::gfx::Matrix4x4Typed<mozilla::gfx::UnknownUnits, mozilla::gfx::UnknownUnits> const*, unsigned int) /home/worker/workspace/build/src/layout/base/FrameLayerBuilder.cpp:5537:5
    #3 0x7f9c5cc047be in nsDisplayOwnLayer::BuildLayer(nsDisplayListBuilder*, mozilla::layers::LayerManager*, mozilla::ContainerLayerParameters const&) /home/worker/workspace/build/src/layout/base/nsDisplayList.cpp:4938:34
    #4 0x7f9c5cc05373 in nsDisplaySubDocument::BuildLayer(nsDisplayListBuilder*, mozilla::layers::LayerManager*, mozilla::ContainerLayerParameters const&) /home/worker/workspace/build/src/layout/base/nsDisplayList.cpp:4989:25
    #5 0x7f9c5ca66d79 in mozilla::ContainerState::ProcessDisplayItems(nsDisplayList*) /home/worker/workspace/build/src/layout/base/FrameLayerBuilder.cpp:4309:32
    #6 0x7f9c5ca767d5 in mozilla::FrameLayerBuilder::BuildContainerLayerFor(nsDisplayListBuilder*, mozilla::layers::LayerManager*, nsIFrame*, nsDisplayItem*, nsDisplayList*, mozilla::ContainerLayerParameters const&, mozilla::gfx::Matrix4x4Typed<mozilla::gfx::UnknownUnits, mozilla::gfx::UnknownUnits> const*, unsigned int) /home/worker/workspace/build/src/layout/base/FrameLayerBuilder.cpp:5537:5
    #7 0x7f9c5cbc995c in nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, unsigned int) /home/worker/workspace/build/src/layout/base/nsDisplayList.cpp:1861:12
    #8 0x7f9c5cc7edfc in nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) /home/worker/workspace/build/src/layout/base/nsLayoutUtils.cpp:3650:7
    #9 0x7f9c5ccfd1c5 in PresShell::Paint(nsView*, nsRegion const&, unsigned int) /home/worker/workspace/build/src/layout/base/nsPresShell.cpp:6387:5

Comment 1

[Ethan Lin[:ethlin]]

Attachment: nullptr check

Add nullptr check for GetTop() in ShouldBuildLayerEvenIfInvisible/DoUpdateBoundsPreserves3D.
Please feel free to take this bug if final fix is not so simple.

Comment 2

[Ethan Lin[:ethlin]]

Attachment: Add crashtest

Add the testcase.html to crashtest.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/47dd6b1e1dbd
Add nullptr check for GetTop(). r=mattwoodrow
https://hg.mozilla.org/integration/mozilla-inbound/rev/59ff490ac047
Add crash test for bug 1325159. r=pchang

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/47dd6b1e1dbd
https://hg.mozilla.org/mozilla-central/rev/59ff490ac047

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Aurora approval when you get a chance.

Comment 6

[Ethan Lin[:ethlin]]

Comment on attachment 8821068 [details] [diff] [review]
nullptr check

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: may crash when user visit certain website 
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: no
[Why is the change risky/not risky?]: This is just adding some null pointer checks.
[String changes made/needed]: none

Comment 7

[Ethan Lin[:ethlin]]

Comment on attachment 8821068 [details] [diff] [review]
nullptr check

Looks like there are some conflicts when applying the patch to aurora.

Comment 8

[Ethan Lin[:ethlin]]

Attachment: patch for aurora

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: may crash when user visit certain website 
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: no
[Why is the change risky/not risky?]: This is just adding some null pointer checks.
[String changes made/needed]: none

Comment 9

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 8829026 [details] [diff] [review]
patch for aurora

crash fix, beta52+

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a9a756031446
https://hg.mozilla.org/releases/mozilla-beta/rev/89e4f1010091