822978 - (Wimplicit-function-declaration) Enable -Werror=implicit-function-declaration by default

Assignee

Description

•

12 years ago

See bug 794240 comment 16 for an example of a subtle build failure due to implicit function declarations. More vicious cases could show up as runtime crashes or erratic behavior.

Mike Hommey [:glandium]

Assignee

Comment 1

•

12 years ago

Attached patch Enable -Werro by default — Details — Splinter Review

I already know it breaks jemalloc3 configure on linux and mozjemalloc on android.

Attachment #693798 - Flags: review?(ted)

Mike Hommey [:glandium]

Assignee

Updated

•

12 years ago

Assignee: nobody → mh+mozilla

Mike Hommey [:glandium]

Assignee

Comment 2

•

12 years ago

Main contenders: ffsl and pthread_atfork not defined in any header in the Android NDK. The jemalloc3 configure problem on linux is a missing include in a configure compile test.

(not currently active) Ted Mielczarek

Comment 3

•

12 years ago

Comment on attachment 693798 [details] [diff] [review]
Enable -Werro by default

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

::: configure.in
@@ +1462,2 @@
>      MOZ_C_SUPPORTS_WARNING(-W, type-limits, ac_c_has_wtype_limits)
>      MOZ_C_SUPPORTS_WARNING(-W, empty-body, ac_c_has_wempty_body)

Starting to wonder if we should just move this whole list into a macro in compiler-opts.m4.

Attachment #693798 - Flags: review?(ted) → review+

Mike Hommey [:glandium]

Assignee

Updated

•

12 years ago

No longer blocks: 794240

Depends on: 794240

Jan Beich

Comment 4

•

11 years ago

Does it affect third-party code (nspr, sqlite, etc)?

Depends on: 782111

Chris Peterson [:cpeterson]

Updated

•

10 years ago

Depends on: 1033188

BMO Automation

Updated

•

6 years ago

Product: Core → Firefox Build System

Chris Peterson [:cpeterson]

Updated

•

6 years ago

Alias: Wimplicit-function-declaration

Component: General → Source Code Analysis

Depends on: 1378623

Mike Hommey [:glandium]

Assignee

Comment 5

•

4 years ago

Attached file Bug 822978 - Enable -Werror=implicit-function-declaration by default. — Details

Pulsebot

Comment 6

•

4 years ago

Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/5e0244588a91
Enable -Werror=implicit-function-declaration by default. r=dmajor

Cosmin Sabou [:CosminS]

Comment 7

•

4 years ago

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&revision=5e0244588a9158f6570b58bde8fc90abc14dedfd&selectedTaskRun=H0UhyNXOR0KwiMg0EWzdqg.0

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=307617763&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/2c80937b7a6b49a1572f9e2593724d6851d68849

[task 2020-06-26T03:03:38.714Z] 03:03:38     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/third_party/sqlite3/src'
[task 2020-06-26T03:03:38.714Z] 03:03:38     INFO -  third_party/sqlite3/src/sqlite3.o
[task 2020-06-26T03:03:38.714Z] 03:03:38     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/i686-w64-mingw32-clang -std=gnu99 -o sqlite3.o -c  -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -ftrivial-auto-var-init=pattern -DDEBUG=1 -DSQLITE_SECURE_DELETE=1 -DSQLITE_THREADSAFE=1 -DSQLITE_CORE=1 -DSQLITE_ENABLE_FTS3=1 -DSQLITE_ENABLE_UNLOCK_NOTIFY=1 -DSQLITE_ENABLE_DBSTAT_VTAB=1 -DSQLITE_DEFAULT_PAGE_SIZE=32768 -DSQLITE_MAX_DEFAULT_PAGE_SIZE=32768 -DSQLITE_WIN32_GETVERSIONEX=0 -DSQLITE_ALLOW_URI_AUTHORITY=1 -DSQLITE_DEBUG=1 -DSQLITE_ENABLE_API_ARMOR -DHAVE_MALLOC_USABLE_SIZE -DSQLITE_WITHOUT_MSIZE -DSQLITE_OMIT_DEPRECATED -DSQLITE_OMIT_BUILTIN_TEST '-DSQLITE_TEMP_FILE_PREFIX="mz_etilqs_"' -DMOZ_HAS_MOZGLUE -I/builds/worker/checkouts/gecko/third_party/sqlite3/src -I/builds/worker/workspace/obj-build/third_party/sqlite3/src -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -include /builds/worker/workspace/obj-build/mozilla-config.h -DMOZILLA_CLIENT -Qunused-arguments -D_HAS_EXCEPTIONS=0 -fno-strict-aliasing -mstackrealign -ffunction-sections -fdata-sections -fno-math-errno -pipe -g -gcodeview -O2 -fno-omit-frame-pointer -funwind-tables -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wno-unknown-pragmas -Wno-unused-function -Wno-conversion-null -Wno-switch -Wno-enum-compare -Wno-gnu-zero-variadic-macro-arguments -Werror=implicit-function-declaration -Wno-sign-compare -Wno-type-limits -fexperimental-new-pass-manager  -MD -MP -MF .deps/sqlite3.o.pp   /builds/worker/checkouts/gecko/third_party/sqlite3/src/sqlite3.c
[task 2020-06-26T03:03:38.714Z] 03:03:38    ERROR -  /builds/worker/checkouts/gecko/third_party/sqlite3/src/sqlite3.c:23687:15: error: implicit declaration of function 'malloc_usable_size' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
[task 2020-06-26T03:03:38.714Z] 03:03:38     INFO -    return (int)SQLITE_MALLOCSIZE(pPrior);
[task 2020-06-26T03:03:38.714Z] 03:03:38     INFO -                ^
[task 2020-06-26T03:03:38.714Z] 03:03:38     INFO -  /builds/worker/checkouts/gecko/third_party/sqlite3/src/sqlite3.c:23616:38: note: expanded from macro 'SQLITE_MALLOCSIZE'
[task 2020-06-26T03:03:38.714Z] 03:03:38     INFO -  #      define SQLITE_MALLOCSIZE(x)   malloc_usable_size(x)
[task 2020-06-26T03:03:38.714Z] 03:03:38     INFO -                                       ^
[task 2020-06-26T03:03:38.714Z] 03:03:38    ERROR -  /builds/worker/checkouts/gecko/third_party/sqlite3/src/sqlite3.c:23714:7: error: implicit declaration of function 'malloc_usable_size' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
[task 2020-06-26T03:03:38.714Z] 03:03:38     INFO -        SQLITE_MALLOCSIZE(pPrior), nByte);
[task 2020-06-26T03:03:38.714Z] 03:03:38     INFO -        ^
[task 2020-06-26T03:03:38.714Z] 03:03:38     INFO -  /builds/worker/checkouts/gecko/third_party/sqlite3/src/sqlite3.c:23616:38: note: expanded from macro 'SQLITE_MALLOCSIZE'
[task 2020-06-26T03:03:38.714Z] 03:03:38     INFO -  #      define SQLITE_MALLOCSIZE(x)   malloc_usable_size(x)
[task 2020-06-26T03:03:38.714Z] 03:03:38     INFO -                                       ^
[task 2020-06-26T03:03:38.751Z] 03:03:38  WARNING -  /builds/worker/checkouts/gecko/third_party/sqlite3/src/sqlite3.c:30469:10: warning: 'return' will never be executed [-Wunreachable-code-return]
[task 2020-06-26T03:03:38.751Z] 03:03:38     INFO -    return 0; /* NOT REACHED */
[task 2020-06-26T03:03:38.752Z] 03:03:38     INFO -           ^
[task 2020-06-26T03:03:38.790Z] 03:03:38  WARNING -  /builds/worker/checkouts/gecko/third_party/sqlite3/src/sqlite3.c:44768:5: warning: code will never be executed [-Wunreachable-code]
[task 2020-06-26T03:03:38.791Z] 03:03:38     INFO -      int lk;
[task 2020-06-26T03:03:38.791Z] 03:03:38     INFO -      ^~~~~~~
[task 2020-06-26T03:03:38.818Z] 03:03:38  WARNING -  /builds/worker/checkouts/gecko/third_party/sqlite3/src/sqlite3.c:44795:11: warning: code will never be executed [-Wunreachable-code]
[task 2020-06-26T03:03:38.819Z] 03:03:38     INFO -      res = winUnlockFile(&pFile->h, SHARED_FIRST+pFile->sharedLockByte, 0, 1, 0);
[task 2020-06-26T03:03:38.819Z] 03:03:38     INFO -            ^~~~~~~~~~~~~
[task 2020-06-26T03:03:38.857Z] 03:03:38  WARNING -  /builds/worker/checkouts/gecko/third_party/sqlite3/src/sqlite3.c:46285:18: warning: code will never be executed [-Wunreachable-code]
[task 2020-06-26T03:03:38.857Z] 03:03:38     INFO -      zConverted = winUtf8ToMbcs(zFilename, osAreFileApisANSI());
[task 2020-06-26T03:03:38.857Z] 03:03:38     INFO -                   ^~~~~~~~~~~~~
[task 2020-06-26T03:03:38.891Z] 03:03:38  WARNING -  /builds/worker/checkouts/gecko/third_party/sqlite3/src/sqlite3.c:46473:5: warning: code will never be executed [-Wunreachable-code]
[task 2020-06-26T03:03:38.892Z] 03:03:38     INFO -      char *zUtf8;
[task 2020-06-26T03:03:38.892Z] 03:03:38     INFO -      ^~~~~~~~~~~~
[task 2020-06-26T03:03:38.928Z] 03:03:38  WARNING -  /builds/worker/checkouts/gecko/third_party/sqlite3/src/sqlite3.c:47076:15: warning: implicit conversion turns string literal into bool: 'char [23]' to '_Bool' [-Wstring-conversion]
[task 2020-06-26T03:03:38.929Z] 03:03:38     INFO -        assert(!"Invalid flags argument");
[task 2020-06-26T03:03:38.930Z] 03:03:38     INFO -               ~^~~~~~~~~~~~~~~~~~~~~~~~
[task 2020-06-26T03:03:38.931Z] 03:03:38     INFO -  /builds/worker/fetches/clang/bin/../i686-w64-mingw32/include/assert.h:73:7: note: expanded from macro 'assert'
[task 2020-06-26T03:03:38.932Z] 03:03:38     INFO -   ((!!(_Expression)) || \
[task 2020-06-26T03:03:38.932Z] 03:03:38     INFO -        ^~~~~~~~~~~
[task 2020-06-26T03:03:38.967Z] 03:03:38  WARNING -  /builds/worker/checkouts/gecko/third_party/sqlite3/src/sqlite3.c:47292:5: warning: code will never be executed [-Wunreachable-code]
[task 2020-06-26T03:03:38.968Z] 03:03:38     INFO -      char *zTemp;
[task 2020-06-26T03:03:38.968Z] 03:03:38     INFO -      ^~~~~~~~~~~~
[task 2020-06-26T03:03:39.005Z] 03:03:39  WARNING -  /builds/worker/checkouts/gecko/third_party/sqlite3/src/sqlite3.c:109354:12: warning: code will never be executed [-Wunreachable-code]
[task 2020-06-26T03:03:39.006Z] 03:03:39     INFO -      pVfs = sqlite3_vfs_find("memdb");
[task 2020-06-26T03:03:39.007Z] 03:03:39     INFO -             ^~~~~~~~~~~~~~~~
[task 2020-06-26T03:03:39.046Z] 03:03:39  WARNING -  /builds/worker/checkouts/gecko/third_party/sqlite3/src/sqlite3.c:145969:39: warning: code will never be executed [-Wunreachable-code]
[task 2020-06-26T03:03:39.047Z] 03:03:39     INFO -      if( pTerm->wtFlags & TERM_VNULL ) continue;
[task 2020-06-26T03:03:39.047Z] 03:03:39     INFO -                                        ^~~~~~~~
[task 2020-06-26T03:03:39.084Z] 03:03:39  WARNING -  /builds/worker/checkouts/gecko/third_party/sqlite3/src/sqlite3.c:145918:39: warning: code will never be executed [-Wunreachable-code]
[task 2020-06-26T03:03:39.085Z] 03:03:39     INFO -      if( pTerm->wtFlags & TERM_VNULL ) continue;
[task 2020-06-26T03:03:39.086Z] 03:03:39     INFO -                                        ^~~~~~~~
[task 2020-06-26T03:03:39.121Z] 03:03:39  WARNING -  /builds/worker/checkouts/gecko/third_party/sqlite3/src/sqlite3.c:156872:9: warning: code will never be executed [-Wunreachable-code]
[task 2020-06-26T03:03:39.121Z] 03:03:39     INFO -          YYMINORTYPE yylhsminor;
[task 2020-06-26T03:03:39.122Z] 03:03:39     INFO -          ^~~~~~~~~~~~~~~~~~~~~~~
[task 2020-06-26T03:03:39.122Z] 03:03:39     INFO -  11 warnings and 2 errors generated.
[task 2020-06-26T03:03:39.123Z] 03:03:39    ERROR -  make[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:669: sqlite3.o] Error 1
[task 2020-06-26T03:03:39.123Z] 03:03:39     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/third_party/sqlite3/src'
[task 2020-06-26T03:03:39.124Z] 03:03:39    ERROR -  make[3]: *** [/builds/worker/checkouts/gecko/config/recurse.mk:72: third_party/sqlite3/src/target-objects] Error 2
[task 2020-06-26T03:03:39.124Z] 03:03:39     INFO -  make[3]: *** Waiting for unfinished jobs....

Flags: needinfo?(mh+mozilla)

Mike Hommey [:glandium]

Assignee

Updated

•

4 years ago

Depends on: 1648680

Mike Hommey [:glandium]

Assignee

Updated

•

4 years ago

Depends on: 1200075

Mike Hommey [:glandium]

Assignee

Updated

•

4 years ago

Flags: needinfo?(mh+mozilla)

Kai Engert (:KaiE:)

Updated

•

4 years ago

Depends on: 1652330

Pulsebot

Comment 8

•

4 years ago

Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/55b614c379d2
Enable -Werror=implicit-function-declaration by default. r=dmajor

Andreea Pavel [:apavel]

Comment 9

•

4 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/55b614c379d2

Status: NEW → RESOLVED

Closed: 4 years ago

status-firefox80: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla80

Jan Beich

Updated

•

4 years ago

Regressions: 1659612

Thomas Deutschmann

Comment 10

•

4 years ago

firefox-80 build log is now full of

6:54.06 cc1plus: warning: '-Werror=' argument '-Werror=implicit-function-declaration' is not valid for C++

messages.

I am also wondering if we cannot do

--- a/build/moz.configure/warnings.configure
+++ b/build/moz.configure/warnings.configure
@@ -216,7 +216,8 @@ with only_when(depends(target)(lambda t: t.kernel == 'WINNT')):
 check_and_add_gcc_warning('-Wno-gnu-zero-variadic-macro-arguments')

 # Make it an error to be missing function declarations.
-check_and_add_gcc_warning('-Werror=implicit-function-declaration')
+check_and_add_gcc_warning('-Werror=implicit-function-declaration',
+                         when='--enable-warnings-as-errors')

 # Disable a warning with gcc 7. See bug 1320656
 # We are far from using C++17 and the impact of the warning will be

instead.

Chris Peterson [:cpeterson]

Comment 11

•

4 years ago

+check_and_add_gcc_warning('-Werror=implicit-function-declaration',
+                         when='--enable-warnings-as-errors')

I don't think we want to limit -Werror=implicit-function-declaration to when we're compiling with --enable-warnings-as-errors. That would be redundant because -Wimplicit-function-declaration is already enabled by -Wall. The goal of this bug was to enable -Werror=implicit-function-declaration even when not compiling with --enable-warnings-as-errors.

Can we instead only add -Werror=implicit-function-declaration for c_compiler?

-check_and_add_gcc_warning('-Werror=implicit-function-declaration')
+check_and_add_gcc_warning('-Werror=implicit-function-declaration', c_compiler)

Nathan Froyd [:froydnj]

Comment 12

•

4 years ago

(In reply to Chris Peterson [:cpeterson] from comment #11)

+check_and_add_gcc_warning('-Werror=implicit-function-declaration',
+                         when='--enable-warnings-as-errors')
I don't think we want to limit -Werror=implicit-function-declaration to when we're compiling with --enable-warnings-as-errors. That would be redundant because -Wimplicit-function-declaration is already enabled by -Wall. The goal of this bug was to enable -Werror=implicit-function-declaration even when not compiling with --enable-warnings-as-errors.

Can we instead only add -Werror=implicit-function-declaration for c_compiler?
-check_and_add_gcc_warning('-Werror=implicit-function-declaration')
+check_and_add_gcc_warning('-Werror=implicit-function-declaration', c_compiler)

This is the right thing to do.

Mike Hommey [:glandium]

Assignee

Comment 13

•

4 years ago

Except that would make it essentially useless. We don't have that many C files compiled, and the original problem this was set to prevent was on C++ files.

Nathan Froyd [:froydnj]

Comment 14

•

4 years ago

(In reply to Mike Hommey [:glandium] from comment #13)

Except that would make it essentially useless. We don't have that many C files compiled, and the original problem this was set to prevent was on C++ files.

I guess we could just enable it if we're using clang? Comment 10 suggests (cc1plus) that this warning shows up as a GCC-only thing.

Mike Hommey [:glandium]

Assignee

Comment 15

•

4 years ago

Oh, I missed that:

6:54.06 cc1plus: warning: '-Werror=' argument '-Werror=implicit-function-declaration' is not valid for C++

This is interesting... is that a new warning? I don't reproduce with gcc 8.3.

Mike Hommey [:glandium]

Assignee

Comment 16

•

4 years ago

Now, something else that is interesting is that neither gcc nor clang do implicit function declaration in C++ (as in, they emit a different error when using an undeclared function). That could be something that was fixed in the recent years, making -Werror=implicit-function-declaration no as necessary as it would have been when the bug was filed.

Thomas Deutschmann

Comment 17

•

4 years ago

I cannot answer myself but a helpful person from Gentoo toolchain project showed me

$ for cxx in /usr/bin/g++-*; do echo $cxx; $cxx -c -x c++ - </dev/null -Werror=implicit-function-declaration; echo $?; done
/usr/bin/g++-10.2.0
cc1plus: warning: '-Werror=' argument '-Werror=implicit-function-declaration' is not valid for C++
0
/usr/bin/g++-11.0.0
cc1plus: warning: '-Werror=' argument '-Werror=implicit-function-declaration' is not valid for C++
0
/usr/bin/g++-4.9.4
0
/usr/bin/g++-5.5.0
0
/usr/bin/g++-6.5.0
0
/usr/bin/g++-7.5.0
cc1plus: warning: '-Werror=' argument '-Werror=implicit-function-declaration' is not valid for C++
0
/usr/bin/g++-8.4.0
cc1plus: warning: '-Werror=' argument '-Werror=implicit-function-declaration' is not valid for C++
0
/usr/bin/g++-9.3.0
cc1plus: warning: '-Werror=' argument '-Werror=implicit-function-declaration' is not valid for C++
0

Thomas Deutschmann

Comment 18

•

4 years ago

Known to fail: 7.4.0, 8.3.0, 9.1.0

https://gcc.gnu.org/PR91172

Sylvestre Ledru [:Sylvestre]

Updated

•

4 years ago

Regressions: 1666108

BMO Automation

Updated

•

2 years ago

Product: Firefox Build System → Developer Infrastructure

Enable -Werro by default 12 years ago Mike Hommey [:glandium] 5.92 KB, patch	ted : review+	Details \| Diff \| Splinter Review
Bug 822978 - Enable -Werror=implicit-function-declaration by default. 4 years ago Mike Hommey [:glandium] 47 bytes, text/x-phabricator-request		Details \| Review