666646 - Fix toolkit/crashreporter/google-breakpad compiler warnings

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Joey Armstrong [:joey]]

% uname -a
Darwin banshee.local 10.7.4 Darwin Kernel Version 10.7.4: Mon Apr 18 21:24:17 PDT 2011; root:xnu-1504.14.12~3/RELEASE_X86_64 x86_64


/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/common/mac/MachIPC.mm:190: warning: 'bootstrap_register' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/servers/bootstrap.h:272)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/common/mac/MachIPC.mm:192: warning: 'bootstrap_register' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/servers/bootstrap.h:272)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/common/mac/HTTPMultipartUpload.m:157: warning: ISO C90 forbids mixed declarations and code
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/common/mac/HTTPMultipartUpload.m:168: warning: ISO C90 forbids mixed declarations and code
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/common/mac/HTTPMultipartUpload.m:191: warning: ISO C90 forbids mixed declarations and code
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:193: warning: 'NXSwapBigLongToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:187)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:201: warning: 'NXSwapBigLongToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:187)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:225: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:246: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:248: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:250: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:252: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:254: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:191: warning: 'NXSwapBigLongToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:187)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:201: warning: 'NXSwapBigLongToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:187)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:225: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:245: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:247: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:249: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:251: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:253: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:191: warning: 'NXSwapBigLongToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:187)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:201: warning: 'NXSwapBigLongToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:187)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:225: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:245: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:247: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:249: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:251: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)
/mozilla/sandbox/gml/toolkit/crashreporter/google-breakpad/src/client/mac/handler/breakpad_nlist_64.cc:253: warning: 'NXSwapBigIntToHost' is deprecated (declared at /Developer/SDKs/MacOSX10.6.sdk/usr/include/architecture/byte_order.h:178)

Comment 1

[:Cykesiopka]

FWIW,
Bug 791775 updated the snapshot to Breakpad SVN r1047:

'NXSwapBigIntToHost' is deprecated
  -> Last usages removed

'NXSwapBigLongToHost' is deprecated
  -> Last usages removed

'bootstrap_register' is deprecated
  -> Upstream introduced "Wrapper for bootstrap_register to avoid deprecation warnings"

Comment 2

[Chris Peterson [:cpeterson]]

Attachment: 666646_fix-crashreporter-warnings.patch

* Workaround a clang warning (on 32-bit OS X builds) by casting uint32_t to time_t. This change is a not ideal because the eventloopNestingLevel variable is a counter and does not actually represent a time value, but this code has been coopting XX_TTOA(), a helper macro to sprintf time_t values, to sprintf a uint32_t.

toolkit/crashreporter/nsExceptionHandler.cpp:776:17: error: format specifies type 'long' but the argument has type 'uint32_t' (aka 'unsigned int') [-Wformat]

* Suppress warnings in third-party Breakpad code that we don't want to fix locally in mozilla-central:

toolkit/crashreporter/google-breakpad/src/common/dwarf_cu_to_module.h:224:10 [-Wredeclared-class-member] class member cannot be redeclared
toolkit/crashreporter/google-breakpad/src/common/md5.cc:169:3 [-Wdeprecated-register] 'register' storage class specifier is deprecated
toolkit/crashreporter/google-breakpad/src/processor/tokenize.cc:65:7 [-Wlogical-not-parentheses] logical not is only applied to the left hand side of this comparison

c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\dist\include\mozilla/MSIntTypes.h(133) : warning C4005: 'PRIx64' : macro redefinition
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\toolkit\crashreporter\google-breakpad\src\google_breakpad/common/breakpad_types.h(83) : see previous definition of 'PRIx64'

* Then mark toolkit/crashreporter as FAIL_ON_WARNINGS. Because some crashreporter files are third-party code, one could argue that we should not compile it with warnings-as-errors because updating our in-tree copy of Breakpad would break the Firefox build if Google's code introduced new warnings (that we have not suppressed in toolkit/crashreporter/moz.build).

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 8421512 [details] [diff] [review]
666646_fix-crashreporter-warnings.patch

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

If the Breakpad warnings are easily fixable we could take upstream patches for them (and cherrypick them locally), assuming they're not already fixed upstream.

Comment 4

[Chris Peterson [:cpeterson]]

Attachment: fix-crashreporter-warnings-v2.patch

In fact, all three gcc warnings I suppressed have been fixed upstream:

https://code.google.com/p/google-breakpad/source/detail?r=1163
https://code.google.com/p/google-breakpad/source/detail?r=1196
https://code.google.com/p/google-breakpad/source/detail?r=1282

This patch removes my gcc warning suppressions from moz.build and adds local patches from upstream. Note that two of the upstream patches (r1163 and r1282) contained more changes than we would want, so my local patches are trivial excerpts of the upstream patches. Is that OK?

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 8422212 [details] [diff] [review]
fix-crashreporter-warnings-v2.patch

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

That's fine, thanks! You don't actually have to add local patches for things that you're cherry-picking from upstream, since we'll just pick them up with the next sync. Local patches are for things that haven't landed upstream, so we don't lose them with the next sync.

Comment 6

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/111c883ca3bf

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/111c883ca3bf

Comment 8

[Michal Novotny [:michal]]

I cannot build Firefox after setting FAIL_ON_WARNINGS to true.

$ c++ --version
c++ (GCC) 4.8.2 20131212 (Red Hat 4.8.2-7)


c++ -o Unified_cpp_crashreporter0.o -c -I../../dist/stl_wrappers -I../../dist/system_wrappers -include /opt/moz/hg-inbound/config/gcc_hidden.h -DOS_POSIX=1 -DOS_LINUX=1 -DUNICODE -D_UNICODE -DMOZ_GLUE_IN_PROGRAM -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL  -DSTATIC_EXPORTABLE_JS_API -DNO_NSPR_10_SUPPORT -I/opt/moz/hg-inbound/toolkit/crashreporter -I. -I/opt/moz/hg-inbound/ipc/chromium/src -I/opt/moz/hg-inbound/ipc/glue -I/opt/moz/hg-inbound/toolkit/crashreporter/google-breakpad/src -I/opt/moz/hg-inbound/_obj-browser-release-tb-fp-dbg/ipc/ipdl/_ipdlheaders -I../../dist/include  -I/opt/moz/hg-inbound/_obj-browser-release-tb-fp-dbg/dist/include/nspr -I/opt/moz/hg-inbound/_obj-browser-release-tb-fp-dbg/dist/include/nss  -I/opt/moz/hg-inbound/_obj-browser-release-tb-fp-dbg/dist/include -I/opt/moz/hg-inbound/modules/zlib/src    -fPIC   -DMOZILLA_CLIENT -include ../../mozilla-config.h -MD -MP -MF .deps/Unified_cpp_crashreporter0.o.pp  -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Werror=int-to-pointer-cast -Wtype-limits -Wempty-body -Wsign-compare -Wno-invalid-offsetof -Wcast-align -Wno-error=uninitialized -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -gdwarf-2 -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -std=gnu++0x -pthread -pipe  -DDEBUG -D_DEBUG -DTRACING -gdwarf-2 -fno-omit-frame-pointer  -Werror     /opt/moz/hg-inbound/_obj-browser-release-tb-fp-dbg/toolkit/crashreporter/Unified_cpp_crashreporter0.cpp
In file included from /opt/moz/hg-inbound/toolkit/crashreporter/nsExceptionHandler.cpp:50:0,
                 from /opt/moz/hg-inbound/_obj-browser-release-tb-fp-dbg/toolkit/crashreporter/Unified_cpp_crashreporter0.cpp:2:
/opt/moz/hg-inbound/toolkit/crashreporter/google-breakpad/src/third_party/lss/linux_syscall_support.h:1474:20: error: invalid suffix on literal; C++11 requires a space between literal and identifier [-Werror=literal-suffix]
                    ".globl "SYS_SYSCALL_ENTRYPOINT"\n"
                    ^
/opt/moz/hg-inbound/toolkit/crashreporter/google-breakpad/src/third_party/lss/linux_syscall_support.h:1475:20: error: invalid suffix on literal; C++11 requires a space between literal and identifier [-Werror=literal-suffix]
                    ".common "SYS_SYSCALL_ENTRYPOINT",8,8\n"
                    ^
/opt/moz/hg-inbound/toolkit/crashreporter/google-breakpad/src/third_party/lss/linux_syscall_support.h:1481:20: error: invalid suffix on literal; C++11 requires a space between literal and identifier [-Werror=literal-suffix]
                    "mov  "SYS_SYSCALL_ENTRYPOINT"@GOT(%0), %0\n"
                    ^
/opt/moz/hg-inbound/toolkit/crashreporter/google-breakpad/src/third_party/lss/linux_syscall_support.h:1488:28: error: invalid suffix on literal; C++11 requires a space between literal and identifier [-Werror=literal-suffix]
                            ".globl "SYS_SYSCALL_ENTRYPOINT"\n"                \
                            ^
/opt/moz/hg-inbound/toolkit/crashreporter/google-breakpad/src/third_party/lss/linux_syscall_support.h:1489:28: error: invalid suffix on literal; C++11 requires a space between literal and identifier [-Werror=literal-suffix]
                            ".common "SYS_SYSCALL_ENTRYPOINT",8,8\n"           \
                            ^
/opt/moz/hg-inbound/toolkit/crashreporter/google-breakpad/src/third_party/lss/linux_syscall_support.h:1496:28: error: invalid suffix on literal; C++11 requires a space between literal and identifier [-Werror=literal-suffix]
                            "mov  "SYS_SYSCALL_ENTRYPOINT"@GOT(%%eax), %%eax\n"\
                            ^
/opt/moz/hg-inbound/toolkit/crashreporter/google-breakpad/src/third_party/lss/linux_syscall_support.h:1768:20: error: invalid suffix on literal; C++11 requires a space between literal and identifier [-Werror=literal-suffix]
                    ".globl "SYS_SYSCALL_ENTRYPOINT"\n"
                    ^
/opt/moz/hg-inbound/toolkit/crashreporter/google-breakpad/src/third_party/lss/linux_syscall_support.h:1769:20: error: invalid suffix on literal; C++11 requires a space between literal and identifier [-Werror=literal-suffix]
                    ".common "SYS_SYSCALL_ENTRYPOINT",8,8\n"
                    ^
/opt/moz/hg-inbound/toolkit/crashreporter/google-breakpad/src/third_party/lss/linux_syscall_support.h:1771:20: error: invalid suffix on literal; C++11 requires a space between literal and identifier [-Werror=literal-suffix]
                    "mov "SYS_SYSCALL_ENTRYPOINT"@GOTPCREL(%%rip), %0\n"
                    ^
/opt/moz/hg-inbound/toolkit/crashreporter/google-breakpad/src/third_party/lss/linux_syscall_support.h:1779:15: error: invalid suffix on literal; C++11 requires a space between literal and identifier [-Werror=literal-suffix]
               ".globl "SYS_SYSCALL_ENTRYPOINT"\n"                             \
               ^
/opt/moz/hg-inbound/toolkit/crashreporter/google-breakpad/src/third_party/lss/linux_syscall_support.h:1780:15: error: invalid suffix on literal; C++11 requires a space between literal and identifier [-Werror=literal-suffix]
               ".common "SYS_SYSCALL_ENTRYPOINT",8,8\n"                        \
               ^
/opt/moz/hg-inbound/toolkit/crashreporter/google-breakpad/src/third_party/lss/linux_syscall_support.h:1782:15: error: invalid suffix on literal; C++11 requires a space between literal and identifier [-Werror=literal-suffix]
               "mov "SYS_SYSCALL_ENTRYPOINT"@GOTPCREL(%%rip), %%rcx\n"         \
               ^
cc1plus: all warnings being treated as errors

Comment 9

[Chris Peterson [:cpeterson]]

Michal: the gcc 4.8 issue is being tracked in bug 1011680. As a workaround, you can remove "ac_add_options --enable-warnings-as-errors" from your mozconfig file (so FAIL_ON_WARNINGS is ignored).

Comment 10

[(not currently active) Ted Mielczarek]

Do note that --enable-warnings-as-errors is only officially supported with the exact toolchains we use on TBPL builds. Everything else is best-effort.

Comment 11

[Chris Peterson [:cpeterson]]

Attachment: part-2-cherrypick-md5-fix.patch

In my previous patch, I forgot to cherrypick md5.cc's -Wdeprecated-register warning fix from upstream google-breakpad:

https://code.google.com/p/google-breakpad/source/detail?r=1282

Comment 12

[Chris Peterson [:cpeterson]]

Part 2:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c303feddaea2

Comment 13

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c303feddaea2