1323837 - Crash at null [@mozilla::gfx::DrawTargetD2D1::FinalizeDrawing]

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: test_case.html

==10324==ERROR: AddressSanitizer: access-violation on unknown address 0x00000000
0010 (pc 0x7ff810193764 bp 0x000000000000 sp 0x00e8269f7fb0 T0)
==10324==The signal is caused by a READ memory access.
==10324==Hint: address points to the zero page.
    #0 0x7ff810193763 in ⌂gdiplus_NULL_THUNK_DATA_DLA (C:\windows\SYSTEM32\d2d1.dll+0x180043763)
    #1 0x7fffd3021bc5 in mozilla::gfx::DrawTargetD2D1::FinalizeDrawing(enum mozilla::gfx::CompositionOp,class mozilla::gfx::Pattern const &) c:\m-c\gfx\2d\DrawTargetD2D1.cpp:1392
    #2 0x7fffd304197d in mozilla::gfx::DrawTargetD2D1::Fill(class mozilla::gfx::Path const *,class mozilla::gfx::Pattern const &,struct mozilla::gfx::DrawOptions const &) c:\m-c\gfx\2d\DrawTargetD2D1.cpp:529
    #3 0x7fffd8a77671 in mozilla::dom::CanvasRenderingContext2D::Fill(class mozilla::dom::CanvasPath const &,enum mozilla::dom::CanvasWindingRule const &) c:\m-c\dom\canvas\CanvasRenderingContext2D.cpp:3195
    #4 0x7fffd6c883e8 in mozilla::dom::CanvasRenderingContext2DBinding::fill C:\m-c\obj64-clang-cl-optimized\dom\bindings\CanvasRenderingContext2DBinding.cpp:3451
    #5 0x7fffd88b76c6 in mozilla::dom::GenericBindingMethod(struct JSContext *,unsigned int,class JS::Value *) c:\m-c\dom\bindings\BindingUtils.cpp:2886
...
see log.txt

Comment 1

[Tyson Smith [:tsmith]]

Attachment: log.txt

Comment 2

[Peter Chang[:pchang]]

Attachment: Bug 1323837 - Draw nothing if there are no color stops for gradient effect,

Review commit: https://reviewboard.mozilla.org/r/99038/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/99038/

Comment 3

[Peter Chang[:pchang]]

Comment on attachment 8819183 [details]
Bug 1323837 - Draw nothing if there are no color stops for gradient effect,

https://reviewboard.mozilla.org/r/99038/#review99286

::: gfx/2d/DrawTargetD2D1.cpp:1387
(Diff revision 1)
>  
> +  if (!pat->mStops.get()) {
> +    // Draw nothing because of no stops
> +    return;
> +  }
> +

I guess early return here is fine because there is another StopCollection validation before rendering.

http://searchfox.org/mozilla-central/source/gfx/2d/RadialGradientEffectD2D1.cpp#115

Comment 4

[Bas Schouten (:bas.schouten)]

Comment on attachment 8819183 [details]
Bug 1323837 - Draw nothing if there are no color stops for gradient effect,

https://reviewboard.mozilla.org/r/99038/#review99798

::: gfx/2d/DrawTargetD2D1.cpp:1383
(Diff revision 1)
>    if (pat->mCenter1 == pat->mCenter2 && pat->mRadius1 == pat->mRadius2) {
>      // Draw nothing!
>      return;
>    }
>  
> +  if (!pat->mStops.get()) {

nit: I don't think the .get() is required, is it?

Comment 5

[Peter Chang[:pchang]]

Comment on attachment 8819183 [details]
Bug 1323837 - Draw nothing if there are no color stops for gradient effect,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/99038/diff/1-2/

Comment 6

[Peter Chang[:pchang]]

Comment on attachment 8819183 [details]
Bug 1323837 - Draw nothing if there are no color stops for gradient effect,

https://reviewboard.mozilla.org/r/99038/#review99798

> nit: I don't think the .get() is required, is it?

yes, I just updated the patch

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/981f3d75def4
Draw nothing if there are no color stops for gradient effect, r=bas

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/981f3d75def4

Comment 10

[Peter Chang[:pchang]]

Comment on attachment 8819183 [details]
Bug 1323837 - Draw nothing if there are no color stops for gradient effect,

Approval Request Comment
[Feature/Bug causing the regression]:None
[User impact if declined]: might have chance to hit crash for windows user
[Is this code covered by automated tests?]:no
[Has the fix been verified in Nightly?]: It was landed for two weeks without problem
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:no
[Is the change risky?]:no
[Why is the change risky/not risky?]: it's a pointer checking
[String changes made/needed]:none

Comment 11

[Gerry Chang [:gchang]]

Comment on attachment 8819183 [details]
Bug 1323837 - Draw nothing if there are no color stops for gradient effect,

Fix a crash. Beta51+ & Aurora52+. Should be in 51 RC.

Comment 12

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/29164ffa1889

Comment 13

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/releases/mozilla-beta/rev/331456c584bf