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

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: 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

Comment 1

[Boris Zbarsky [: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?

Comment 2

[Chris Peterson [:cpeterson]]

Attachment: 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. :)

Comment 3

[Boris Zbarsky [: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.

Comment 4

[Chris Peterson [:cpeterson]]

Landed with comment about this warning only affecting debug code:
https://hg.mozilla.org/integration/mozilla-inbound/rev/04f5a7e8f14c

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/04f5a7e8f14c