Closed
Bug 1264246
Opened 9 years ago
Closed 9 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•9 years ago
|
||
Reporter | ||
Comment 2•9 years ago
|
||
A possible fix.
Assignee | ||
Comment 3•9 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•9 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•9 years ago
|
Attachment #8741033 -
Flags: review?(mchang) → review+
Comment 6•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 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
•