Closed Bug 1026998 Opened 6 years ago Closed 5 years ago

Uninitialised value use in CanvasRenderingContext2D::EnsureTarget

Categories

(Core :: Canvas: 2D, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: jseward, Assigned: jrmuizel)

References

Details

Attachments

(1 file, 1 obsolete file)

TEST_PATH=content/base/test/test_fileapi_slice.html

produces a whole avalanche of uninitialised value errors, the first of
which is

Conditional jump or move depends on uninitialised value(s)
   at 0x6AC0452: mozilla::dom::CanvasRenderingContext2D::EnsureTarget() (content/canvas/src/CanvasRenderingContext2D.cpp:916)
   by 0x6ACDFF6: mozilla::dom::CanvasRenderingContext2D::SetIsOpaque(bool) (content/canvas/src/CanvasRenderingContext2D.cpp:1073)
   by 0x6B07D3A: mozilla::dom::HTMLCanvasElement::UpdateContext(JSContext*, JS::Handle<JS::Value>) (content/html/content/src/HTMLCanvasElement.cpp:806)
   by 0x6B080F1: mozilla::dom::HTMLCanvasElement::GetContext(JSContext*, nsAString_internal const&, JS::Handle<JS::Value>, mozilla::ErrorResult&) (content/html/content/src/HTMLCanvasElement.cpp:732)
   by 0x62631E2: mozilla::dom::HTMLCanvasElementBinding::getContext(JSContext*, JS::Handle<JSObject*>, mozilla::dom::HTMLCanvasElement*, JSJitMethodCallArgs const&) (ff-O-linux64/dom/bindings/HTMLCanvasElementBinding.cpp:218)
   by 0x65B9026: mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) (dom/bindings/BindingUtils.cpp:2347)
   by 0x7C3ABC8: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (js/src/jscntxtinlines.h:241)
   by 0x7C35902: Interpret(JSContext*, js::RunState&) (js/src/vm/Interpreter.cpp:2566)
   by 0x7C39033: js::RunScript(JSContext*, js::RunState&) (js/src/vm/Interpreter.cpp:402)
   by 0x7C39209: js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) (js/src/vm/Interpreter.cpp:610)
   by 0x7C39331: js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) (js/src/vm/Interpreter.cpp:647)
   by 0x7B8AB18: Evaluate(JSContext*, JS::Handle<JSObject*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::Value*) (js/src/jsapi.cpp:4988)

 Uninitialised value was created by a heap allocation
   at 0x4809064: malloc (/home/sewardj/VgTRUNK/merge/coregrind/m_replacemalloc/vg_replace_malloc.c:292)
   by 0x481D86B: moz_xmalloc (memory/mozalloc/mozalloc.cpp:52)
   by 0x6B079DC: mozilla::dom::HTMLCanvasElement::GetContextHelper(nsAString_internal const&, nsICanvasRenderingContextInternal**) (ff-O-linux64/content/html/content/src/../../../../dist/include/mozilla/mozalloc.h:201)
   by 0x6B0806D: mozilla::dom::HTMLCanvasElement::GetContext(JSContext*, nsAString_internal const&, JS::Handle<JS::Value>, mozilla::ErrorResult&) (content/html/content/src/HTMLCanvasElement.cpp:717)
   by 0x62631E2: mozilla::dom::HTMLCanvasElementBinding::getContext(JSContext*, JS::Handle<JSObject*>, mozilla::dom::HTMLCanvasElement*, JSJitMethodCallArgs const&) (ff-O-linux64/dom/bindings/HTMLCanvasElementBinding.cpp:218)
   by 0x65B9026: mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) (dom/bindings/BindingUtils.cpp:2347)
   by 0x7C3ABC8: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (js/src/jscntxtinlines.h:241)
   by 0x7C35902: Interpret(JSContext*, js::RunState&) (js/src/vm/Interpreter.cpp:2566)
   by 0x7C39033: js::RunScript(JSContext*, js::RunState&) (js/src/vm/Interpreter.cpp:402)
   by 0x7C39209: js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) (js/src/vm/Interpreter.cpp:610)
   by 0x7C39331: js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) (js/src/vm/Interpreter.cpp:647)
   by 0x7B8AB18: Evaluate(JSContext*, JS::Handle<JSObject*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::Value*) (js/src/jsapi.cpp:4988)
Peering a bit at the code, the complaints start here

CanvasRenderingContext2D.cpp, CanvasRenderingContext2D::EnsureTarget()

915   IntSize size(mWidth, mHeight);
916   if (size.width <= 0xFFFF && size.height <= 0xFFFF &&
917       size.width >= 0 && size.height >= 0) {

It complains about all 4 comparisons, which implies both mWidth
and mHeight are undefined.  

CanvasRenderingContext2D::CanvasRenderingContext2D() at line 561
doesn't appear to initialise them.  I tried the following hack

 CanvasRenderingContext2D::CanvasRenderingContext2D()
   : mForceSoftware(false), mZero(false), mOpaque(false), mResetLayer(true)
   , mIPC(false)
   , mStream(nullptr)
   , mIsEntireFrameInvalid(false)
   , mPredictManyRedrawCalls(false), mPathTransformWillUpdate(false)
   , mInvalidateCount(0)
+,mWidth(0),mHeight(0)
 {
   sNumLivingContexts++;
   SetIsDOMBinding();
 }

and that seems to get rid of all of the Memcheck complaints.

Anyone familiar with this code know if that's a valid fix, or if it
merely papers over some deeper problem?
Jeff, is this class one of those that magically gets all the fields zero'd out on construction, without having to do this explicitly, and Memcheck is just "confused"?  I can never remember where to look.
Flags: needinfo?(jmuizelaar)
This looks like a bug to me. Here's a possible fix.
Assignee: nobody → jmuizelaar
Attachment #8447460 - Flags: review?(bas)
Flags: needinfo?(jmuizelaar)
Comment on attachment 8447460 [details] [diff] [review]
Intialize the canvas width and height members to the default values

Review of attachment 8447460 [details] [diff] [review]:
-----------------------------------------------------------------

I'm fine with this, but don't we technically expect the size to be set by the time EnsureTarget is called? It sort of sounds to me like there might also be some other unexpected behavior. Although this patch by itself is a good idea.
Attachment #8447460 - Flags: review?(bas) → review+
Removes extraneous comma.  No other changes.

I verified that this gets rid of the V complaints I saw.
Attachment #8447460 - Attachment is obsolete: true
Attachment #8448759 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/2b00d9897dc6
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Depends on: 1035244
You need to log in before you can comment on or make changes to this bug.