Closed
Bug 1264246
Opened 8 years ago
Closed 8 years ago
Uninitialised value use in IsEmpty (gfx/2d/BaseRect.h:61)
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: jseward, Assigned: lsalzman)
Details
Attachments
(3 files)
2.33 KB,
text/plain
|
Details | |
1.02 KB,
patch
|
Details | Diff | Splinter Review | |
1.83 KB,
patch
|
mchang
:
review+
|
Details | Diff | Splinter Review |
DISPLAY=:1.0 ./mach mochitest \ --valgrind-args=--show-mismatched-frees=no,--fullpath-after=REL/ \ --valgrind=/home/sewardj/VgTRUNK/merge/Inst/bin/valgrind \ dom/canvas/test/test_canvas.html 2>&1 | tee spew-05-mc-11 SkCanvas::getClipBounds is inconsistent in its return conventions. It returns a boolean indicating whether or not it succeeded, and it appears that the original plan was for it to set the out-parameter |bounds| to a safe value even on failure, so that callers can ignore the boolean if they want. But the first return point from the function doesn't do that: it returns |false| without changing the out-parameter. The result is that "Rect GetClipBounds(SkCanvas *aCanvas)" in DrawTargetSkia.cpp can pass stack-allocated junk back to its caller: static inline Rect GetClipBounds(SkCanvas *aCanvas) { SkRect clipBounds; aCanvas->getClipBounds(&clipBounds); return SkRectToRect(clipBounds); } The uninitialised values get passed around a bit and Valgrind eventually comments on them (somewhat confusingly) in BaseRect::IsEmpty (BaseRect.h:61).
Reporter | ||
Comment 1•8 years ago
|
||
Reporter | ||
Comment 2•8 years ago
|
||
A possible fix.
Assignee | ||
Comment 3•8 years ago
|
||
(In reply to Julian Seward [:jseward] from comment #2) > Created attachment 8740890 [details] [diff] [review] > bug1264246-IsEmpty-1.cset > > A possible fix. We shouldn't really be modifying Skia for something that is better fixed in our own code.
Assignee | ||
Comment 4•8 years ago
|
||
This makes GetClipBounds check the result of SkCanvas::getClipBounds, and if it fails return an empty rect like it should be. Also this deals with an incidental finding that getClipBounds is actually doing quite expensive math, so only calls it in the case we need to check it against mask bounds.
Updated•8 years ago
|
Attachment #8741033 -
Flags: review?(mchang) → review+
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1442d37b6449
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•