Closed
Bug 1339637
Opened 7 years ago
Closed 7 years ago
skia: signed integer overflow in SkClampRange::init()
Categories
(Core :: Graphics, defect, P1)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: tsmith, Assigned: lsalzman)
Details
(Keywords: csectype-intoverflow, Whiteboard: [gfx-noted])
Attachments
(1 file)
2.25 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
This seems to fix a few assertions that I was coming across as well. Logged as: https://bugs.chromium.org/p/skia/issues/detail?id=6219 Fixed upstream by: https://chromium.googlesource.com/skia/+/d9569664fe76068adcc87d7a029350f5dbdbc4a6 src/effects/gradients/SkClampRange.cpp:98:21: runtime error: signed integer overflow: -9223372036854775808 + -3368429952742457344 cannot be represented in type 'long' #0 0x28528c6 in SkClampRange::init(long, long, int, int, int) src/effects/gradients/SkClampRange.cpp:98:21 #1 0x13f5066 in (anonymous namespace)::shadeSpan_linear_clamp(int (*)(int), long, long, unsigned int*, unsigned int const*, int, int) src/effects/gradients/SkLinearGradient.cpp:280:5 #2 0x13f1ea9 in SkLinearGradient::LinearGradientContext::shadeSpan(int, int, unsigned int*, int) src/effects/gradients/SkLinearGradient.cpp:383:9 #3 0x239829a in SkARGB32_Shader_Blitter::blitRect(int, int, int, int) src/core/SkBlitter_ARGB32.cpp:460:17 #4 0x10391bb in SkScan::FillIRect(SkIRect const&, SkRegion const*, SkBlitter*) src/core/SkScan.cpp:50:13 #5 0x103a276 in SkScan::FillIRect(SkIRect const&, SkRasterClip const&, SkBlitter*) src/core/SkScan.cpp:80:9 #6 0xcc266f in SkDraw::drawPaint(SkPaint const&) const src/core/SkDraw.cpp:294:5 #7 0x900fde in SkCanvas::internalDrawPaint(SkPaint const&) src/core/SkCanvas.cpp:2098:9 #8 0x90086c in SkCanvas::onDrawPaint(SkPaint const&) src/core/SkCanvas.cpp:2091:5 #9 0x5a123d in fuzzLinearGradient(Fuzz*) fuzz/FuzzGradients.cpp:83:9 #10 0x5a5832 in fuzz_Gradients(Fuzz*) fuzz/FuzzGradients.cpp:205:13 #11 0x5b1136 in fuzz_api(sk_sp<SkData>) fuzz/fuzz.cpp:147:13 #12 0x5b1136 in fuzz_file(char const*) fuzz/fuzz.cpp:88 #13 0x5af60e in main fuzz/fuzz.cpp:60:19 #14 0x7fa4cbb6d82f in __libc_start_main /build/glibc-t3gR2i/glibc-2.23/csu/../csu/libc-start.c:291 #15 0x49fd48 in _start (fuzz_ubsan+0x49fd48)
Could cherry pick this change
Assignee: nobody → lsalzman
Assignee | ||
Comment 2•7 years ago
|
||
Just a straight backport of upstream Skia fix: https://chromium.googlesource.com/skia/+/d9569664fe76068adcc87d7a029350f5dbdbc4a6
Attachment #8839574 -
Flags: review?(jmuizelaar)
Assignee | ||
Updated•7 years ago
|
Priority: -- → P1
Whiteboard: [gfx-noted]
Updated•7 years ago
|
Attachment #8839574 -
Flags: review?(jmuizelaar) → review+
Pushed by lsalzman@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8568a8c74f30 fix overflow in SkClampRange::init(). r=jrmuizel
Comment 4•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8568a8c74f30
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Assignee | ||
Comment 5•7 years ago
|
||
Can you show some of the testcases that caused the assertion so we can assess quickly whether this would benefit from uplift?
Flags: needinfo?(twsmith)
Reporter | ||
Comment 6•7 years ago
|
||
The only test cases that I have are attached to the upstream bug report and the fix. The addition to tests/GradientTest.cpp (diff[1]) will likely be the most helpful. [1] https://chromium.googlesource.com/skia/+/d9569664fe76068adcc87d7a029350f5dbdbc4a6%5E%21/#F1
Flags: needinfo?(twsmith)
Assignee | ||
Comment 7•7 years ago
|
||
(In reply to Tyson Smith [:tsmith] from comment #6) > The only test cases that I have are attached to the upstream bug report and > the fix. The addition to tests/GradientTest.cpp (diff[1]) will likely be the > most helpful. > > [1] > https://chromium.googlesource.com/skia/+/ > d9569664fe76068adcc87d7a029350f5dbdbc4a6%5E%21/#F1 Unless we can prove this can actually be done via an html or javascript testcase, I am not sure this warrants an uplift at all.
Flags: needinfo?(ryanvm)
Comment 8•7 years ago
|
||
(In reply to Lee Salzman [:lsalzman] from comment #7) > Unless we can prove this can actually be done via an html or javascript > testcase, I am not sure this warrants an uplift at all. Sounds like a reasonable position to me.
Flags: needinfo?(ryanvm)
You need to log in
before you can comment on or make changes to this bug.
Description
•