Closed Bug 822978 (Wimplicit-function-declaration) Opened 12 years ago Closed 4 years ago

Enable -Werror=implicit-function-declaration by default

Categories

(Developer Infrastructure :: Source Code Analysis, defect)

defect
Not set
normal

Tracking

(firefox80 fixed)

RESOLVED FIXED
mozilla80
Tracking Status
firefox80 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

(Regressed 1 open bug)

Details

Attachments

(2 files)

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.
Attached patch Enable -Werro by default (deleted) — Splinter Review
I already know it breaks jemalloc3 configure on linux and mozjemalloc on android.
Attachment #693798 - Flags: review?(ted)
Assignee: nobody → mh+mozilla
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.
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+
No longer blocks: 794240
Depends on: 794240
Does it affect third-party code (nspr, sqlite, etc)?
Depends on: 782111
Depends on: 1033188
Product: Core → Firefox Build System
Alias: Wimplicit-function-declaration
Component: General → Source Code Analysis
Depends on: 1378623
Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/5e0244588a91 Enable -Werror=implicit-function-declaration by default. r=dmajor

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)
Depends on: 1648680
Depends on: 1200075
Flags: needinfo?(mh+mozilla)
Depends on: 1652330
Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/55b614c379d2 Enable -Werror=implicit-function-declaration by default. r=dmajor
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80
Regressions: 1659612

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.

+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)

(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.

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.

(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.

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.

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.

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

Known to fail: 7.4.0, 8.3.0, 9.1.0

https://gcc.gnu.org/PR91172

Regressions: 1666108
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: