Closed
Bug 1290628
Opened 5 years ago
Closed 5 years ago
MOZ_CRASH "d0 < fPathLength" in [@SpecialLineRec::addSegment()]
Categories
(Core :: Canvas: 2D, defect)
Core
Canvas: 2D
Tracking
()
RESOLVED
FIXED
mozilla51
People
(Reporter: tsmith, Assigned: ethlin)
References
(Blocks 1 open bug)
Details
(Keywords: assertion, testcase)
Attachments
(3 files, 3 obsolete files)
12.19 KB,
text/plain
|
Details | |
312 bytes,
text/html
|
Details | |
2.26 KB,
patch
|
ritu
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Found with debug build. /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/skia/skia/src/utils/SkDashPath.cpp:188: fatal error: ""d0 < fPathLength"" Abort from sk_abort Hit MOZ_CRASH() at /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/memory/mozalloc/mozalloc_abort.cpp:33 ASAN:DEADLYSIGNAL ================================================================= ==29173==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x0000004e19d3 bp 0x7ffd2380df00 sp 0x7ffd2380def0 T0) #0 0x4e19d2 in mozalloc_abort(char const*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/memory/mozalloc/mozalloc_abort.cpp:33:5 #1 0x7f7efd883e24 in sk_abort_no_print() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/skia/skia/src/ports/SkMemory_mozalloc.cpp:16:5 #2 0x7f7efd887c81 in SpecialLineRec::addSegment(float, float, SkPath*) const /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/skia/skia/src/utils/SkDashPath.cpp:188:9 #3 0x7f7efd887053 in SkDashPath::InternalFilter(SkPath*, SkPath const&, SkStrokeRec*, SkRect const*, float const*, int, float, int, float) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/skia/skia/src/utils/SkDashPath.cpp:275:21 #4 0x7f7efd60e33e in SkDashPathEffect::filterPath(SkPath*, SkPath const&, SkStrokeRec*, SkRect const*) const /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/skia/skia/src/effects/SkDashPathEffect.cpp:40:12 #5 0x7f7efd952a1d in SkPaint::getFillPath(SkPath const&, SkPath*, SkRect const*, float) const /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/skia/skia/src/core/SkPaint.cpp:1971:24 #6 0x7f7efd7ebf7c in SkDraw::drawPath(SkPath const&, SkPaint const&, SkMatrix const*, bool, bool, SkBlitter*) const /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/skia/skia/src/core/SkDraw.cpp:1091:18 #7 0x7f7efd5a8adb in SkDraw::drawPath(SkPath const&, SkPaint const&, SkMatrix const*, bool) const /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/skia/skia/include/core/SkDraw.h:54:9 #8 0x7f7efd7e9668 in SkDraw::drawPoints(SkCanvas::PointMode, unsigned long, SkPoint const*, SkPaint const&, bool) const /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/skia/skia/src/core/SkDraw.cpp:730:25 #9 0x7f7efd5e3534 in SkCanvas::onDrawPoints(SkCanvas::PointMode, unsigned long, SkPoint const*, SkPaint const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/skia/skia/src/core/SkCanvas.cpp:2082:9 ... see log.txt for full log.
Flags: in-testsuite?
Reporter | ||
Comment 1•5 years ago
|
||
Assignee | ||
Comment 2•5 years ago
|
||
The length check is different between [1] and [2]. One is using Scalar and the other one is using double. So using Scalar to do the check can fix the problem. [1] https://hg.mozilla.org/mozilla-central/annotate/2ea3d51ba1bb9f5c3b6921c43ea63f70b4fdf5d2/gfx/skia/skia/src/utils/SkDashPath.cpp#l267 [2] https://hg.mozilla.org/mozilla-central/annotate/2ea3d51ba1bb9f5c3b6921c43ea63f70b4fdf5d2/gfx/skia/skia/src/utils/SkDashPath.cpp#l189
Attachment #8776274 -
Flags: review?(mchang)
Assignee | ||
Comment 3•5 years ago
|
||
Using Scalar to do the length checking may cause infinite loop problem. I think we should just remove the assertion in 'addSegment' since there is only one place use the function and the loop already checked the length in double.
Attachment #8776274 -
Attachment is obsolete: true
Attachment #8776274 -
Flags: review?(mchang)
Attachment #8776277 -
Flags: review?(mchang)
Assignee | ||
Updated•5 years ago
|
Assignee: nobody → ethlin
Assignee | ||
Updated•5 years ago
|
Attachment #8776277 -
Flags: review?(mchang) → review?(lsalzman)
Comment 4•5 years ago
|
||
(In reply to Ethan Lin[:ethlin] from comment #2) > Created attachment 8776274 [details] [diff] [review] > use Scalar to check distance > > The length check is different between [1] and [2]. One is using Scalar and > the other one is using double. So using Scalar to do the check can fix the > problem. > > [1] > https://hg.mozilla.org/mozilla-central/annotate/ > 2ea3d51ba1bb9f5c3b6921c43ea63f70b4fdf5d2/gfx/skia/skia/src/utils/SkDashPath. > cpp#l267 > [2] > https://hg.mozilla.org/mozilla-central/annotate/ > 2ea3d51ba1bb9f5c3b6921c43ea63f70b4fdf5d2/gfx/skia/skia/src/utils/SkDashPath. > cpp#l189 Rather than get rid of the assert, I think it would be better to change the < to <=
Assignee | ||
Comment 5•5 years ago
|
||
Change the assertion to '<=' to prevent hitting the assertion when large number.
Attachment #8776277 -
Attachment is obsolete: true
Attachment #8776277 -
Flags: review?(lsalzman)
Attachment #8776790 -
Flags: review?(lsalzman)
Updated•5 years ago
|
Attachment #8776790 -
Flags: review?(lsalzman) → review+
Assignee | ||
Comment 6•5 years ago
|
||
Rebase to master.
Attachment #8776790 -
Attachment is obsolete: true
Assignee | ||
Comment 7•5 years ago
|
||
try server: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd036fb5c52e
Assignee | ||
Updated•5 years ago
|
Keywords: checkin-needed
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/67b0bcd172f3 Change the assertion rule to prevent assert with large number. r=lsalzman
Keywords: checkin-needed
Comment 9•5 years ago
|
||
Upstream Skia bug report: https://codereview.chromium.org/2209303004
Comment 10•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/67b0bcd172f3
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Updated•5 years ago
|
Flags: in-testsuite? → in-testsuite+
Comment 11•5 years ago
|
||
Ethan, we probably want to uplift that to aurora. Could you fill the uplift request? Thanks
Flags: needinfo?(ethlin)
Assignee | ||
Comment 12•5 years ago
|
||
Comment on attachment 8777186 [details] [diff] [review] change the assertion (carry r+: lsalzman) Approval Request Comment [Feature/regressing bug #]: none [User impact if declined]: Debug build may crash when using canvas. [Describe test coverage new/current, TreeHerder]: test on locally and tryserver [Risks and why]: low risk [String/UUID change made/needed]: none
Flags: needinfo?(ethlin)
Attachment #8777186 -
Flags: approval-mozilla-aurora?
Comment on attachment 8777186 [details] [diff] [review] change the assertion (carry r+: lsalzman) Has stabilized on Nightly for 2 weeks, Aurora50+
Attachment #8777186 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 14•5 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/1955c0608854
You need to log in
before you can comment on or make changes to this bug.
Description
•