Closed Bug 1013065 Opened 6 years ago Closed 6 years ago

Bug 65664's NSCAP_DONT_PROVIDE_NONCONST_OPEQ autoconf test fails to compile with -Werror=uninitialized, yielding incorrect result and further compilation errors

Categories

(Core :: XPCOM, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33
Tracking Status
firefox32 --- wontfix
firefox33 --- fixed

People

(Reporter: cpeterson, Assigned: cpeterson)

References

Details

Attachments

(1 file)

In bug 1001975, I'm trying to remove configure.in's -Wno-error=uninitialized (bug 716541) so clang and gcc will treat -Wuinitialized as warnings-as-errors in FAIL_ON_WARNINGS directories. clang currently reports no -Wuninitialized warnings and gcc reports only one (bug 1007708), so I may advocate for forcing -Werror=uninitialized across all mozilla-central, not just FAIL_ON_WARNINGS directories.

Unfortunately, when I enable -Werror=uninitialized (in configure.in or my mozconfig) instead of just -Wuninitialized, clang and gcc report hundreds of compilation errors about ambiguous comparisons of nsCOMPtrs and raw pointers:

xpcom/ds/nsObserverList.h:44:54: error: use of overloaded operator '==' is ambiguous (with operand types 'const nsCOMPtr<nsISupports>' and 'nsISupports *')
xpcom/ds/nsSupportsArray.cpp:268:27: error: use of overloaded operator '!=' is ambiguous (with operand types 'nsISupports *' and 'nsCOMPtr<nsISupports>')
xpcom/io/nsDirectoryService.cpp:512:14: error: use of overloaded operator '==' is ambiguous (with operand types 'nsCOMPtr<nsIAtom>' and 'nsIAtom *')
xpcom/threads/TimerThread.h:53:20: error: use of overloaded operator '==' is ambiguous (with operand types 'const nsCOMPtr<nsIThread>' and 'nsIThread *')
xpcom/threads/nsThread.cpp:886:42: error: use of overloaded operator '!=' is ambiguous (with operand types 'nsRefPtr<nsThread::nsNestedEventTarget>' and 'nsIEventTarget *')
../../dist/include/nsTArray.h:546:14: error: use of overloaded operator '==' is ambiguous (with operand types 'const nsCOMPtr<nsIThreadObserver>' and 'nsIThreadObserver *const')
...

Reverting your NSCAP_DONT_PROVIDE_NONCONST_OPEQ workaround from bug 65664 makes these errors go away. I have no idea why -Werror=uninitialized causes these seemingly unrelated errors when -Wuninitialized does not. I would assume this is a gcc bug except clang and gcc both report the same errors and on multiple platforms.

* -Wuninitialized is green:
https://tbpl.mozilla.org/?tree=Try&rev=76c4d06c12fa

* -Werror=uninitialized is red:
https://tbpl.mozilla.org/?tree=Try&rev=66398394f261

* -Werror=uninitialized without NSCAP_DONT_PROVIDE_NONCONST_OPEQ is green:
https://tbpl.mozilla.org/?tree=Try&rev=315356b10719
Attachment #8425244 - Flags: review?(dbaron)
Comment on attachment 8425244 [details] [diff] [review]
remove-NSCAP_DONT_PROVIDE_NONCONST_OPEQ.patch

I'm not sure whether autoconf tests are run with our default compilation flags, but it seems that the most likely theory for the problem here is that they are, so that flipping on -Werror=uninitialized is triggering a bogus failure of this autoconf test by causing a compilation error due to:

error: ‘bar’ is used uninitialized in this function [-Werror=uninitialized]

which was previously:

configure:25700:37: warning: 'bar' is used uninitialized in this function [-Wuninitialized]

(You should find this in config.log in the objdir.)


An easy way to test that would be to see if the problem still occurs if you add:
    Pointer() : myPtr(0) {}
to the definition of clas Pointer, and then also initialize bar to 0.

If that makes the problem go away, then we have an understanding of why it happened.

And I'm fine with removing the autoconf test itself, since it's probably no longer needed, so r=dbaron, as long as you fix the bug number in the commit message.


It might also be worth going through the autoconf output in config.log and looking for other compiler warnings in autoconf tests.
Attachment #8425244 - Flags: review?(dbaron) → review+
Summary: Bug 65664's NSCAP_DONT_PROVIDE_NONCONST_OPEQ workaround triggers unrelated (?) compilation errors with -Werror=uninitialized → Bug 65664's NSCAP_DONT_PROVIDE_NONCONST_OPEQ autoconf test fails to compile with -Werror=uninitialized, yielding incorrect result and further compilation errors
(In reply to David Baron [:dbaron] (UTC-7) (needinfo? for questions) from comment #1)
> Comment on attachment 8425244 [details] [diff] [review]
> remove-NSCAP_DONT_PROVIDE_NONCONST_OPEQ.patch
> 
> I'm not sure whether autoconf tests are run with our default compilation
> flags, but it seems that the most likely theory for the problem here is that
> they are, so that flipping on -Werror=uninitialized is triggering a bogus
> failure of this autoconf test by causing a compilation error due to:
> 
> error: ‘bar’ is used uninitialized in this function [-Werror=uninitialized]
> 
> which was previously:
> 
> configure:25700:37: warning: 'bar' is used uninitialized in this function
> [-Wuninitialized]

This "'bar' is used uninitialized" warning-as-error caused the autoconf test to return an unexpected result, which caused the original compilation errors.


> And I'm fine with removing the autoconf test itself, since it's probably no
> longer needed, so r=dbaron, as long as you fix the bug number in the commit
> message.

I could just fix the configure.in warning, but I'd prefer to remove unnecessary workaround because it complicates some C++ code. :)


> It might also be worth going through the autoconf output in config.log and
> looking for other compiler warnings in autoconf tests.

config.log reports other warnings, but they are in code generated for autoconf's implicit checks, not code from our configure.in. Fortunately, I don't think any of the warnings will cause configure problems.
https://hg.mozilla.org/mozilla-central/rev/66db03e176d4
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.