dom/bindings/ErrorResult.h: [-Wuninitialized] 'ErrorResult::mMessage' may be use uninitialized in this function

RESOLVED FIXED in Firefox 29

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cpeterson, Assigned: cpeterson)

Tracking

(Blocks: 1 bug)

Trunk
mozilla29
All
Android
Points:
---

Firefox Tracking Flags

(firefox28 wontfix, firefox29 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 8355748 [details] [diff] [review]
Wuninitialized-ErrorResult.patch

Android's gcc 4.6 warns that ErrorResult::mMessage may be uninitialized when it is referenced in its destructor. This is a false positive because, even though the mMessage union is not initialized in the constructor, the ErrorResult's IsTypeError() checks avoid using an uninitialized mMessage.

Note that initializing mMessage to nullptr in the constructor only initializes half of the union bits mMessage shares with the JS::Value `mJSException`.

This is the last -Wuninitialized warning on Android using gcc 4.6 or 4.8 (because gcc now categorizes most uninitialized false positives as -Wmaybe-uninitialized).

https://tbpl.mozilla.org/?tree=Try&rev=7b90fbe4cef6
Attachment #8355748 - Flags: review?(bzbarsky)
> This is a false positive

Right.  And this is extremely performance-sensitive code, where literally every machine instruction matters...

Can we just explicitly tell gcc this use is ok via some pragma or something?
(Assignee)

Comment 2

5 years ago
Created attachment 8355812 [details] [diff] [review]
ErrorResult-v2.patch

What if the ErrorResult constructor only initializes mMessage #ifdef DEBUG? The warning is triggered by #ifdef DEBUG code in the destructor.

This warning is from gcc 4.6, which is not our default gcc version for Android or Linux. So please feel free to r- if you don't want this. :)
Attachment #8355748 - Attachment is obsolete: true
Attachment #8355748 - Flags: review?(bzbarsky)
Attachment #8355812 - Flags: review?(bzbarsky)
Comment on attachment 8355812 [details] [diff] [review]
ErrorResult-v2.patch

r=me if you add a comment saying that this is only done to silence a warning from debug-only code.
Attachment #8355812 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 4

5 years ago
Landed with comment about this warning only affecting debug code:
https://hg.mozilla.org/integration/mozilla-inbound/rev/04f5a7e8f14c
status-firefox28: --- → wontfix
status-firefox29: --- → fixed
https://hg.mozilla.org/mozilla-central/rev/04f5a7e8f14c
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.