Issue 1832 in webm: Build: When building with GCC, C++ warning flag -Wextra-semi is added to CFLAGS

[ferd… via monorail]

Status: Unconfirmed
Owner: ----

New issue 1832 by ferd...@gmail.com: Build: When building with GCC, C++ warning flag -Wextra-semi is added to CFLAGS
https://bugs.chromium.org/p/webm/issues/detail?id=1832

In the configure script, process_toolchain() checks if the compiler is gcc, and if so adds several flags to the CFLAGS, including:

check_add_cflags -Wextra-semi
check_add_cflags -Wextra-semi-stmt

But, as the GCC documentation notes[1][2], -Wextra-semi is only valid for C++ and Objective-C++ code. (-Wextra-semi-stmt doesn't appear to be documented at all, at least in recent GCC versions. Looks like it fails the check_add_cflags call and gets omitted.)

This leads to a slew of warnings when compiling C code, e.g.:

ccache gcc -m64 -DNDEBUG -O3 -fPIC -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -Wdisabled-optimization -Wextra-semi -Wfloat-conversion -Wformat=2 -Wpointer-arith -Wtype-limits -Wcast-qual -Wvla -Wimplicit-function-declaration -Wmissing-declarations -Wmissing-prototypes -Wshadow -Wuninitialized -Wunused -Wextra -Wundef -Wframe-larger-than=52000 -std=gnu99 -I. -I".." -M ../vpx_dsp/skin_detection.c | sed -e 's;^\([a-zA-Z0-9_]*\)\.o;vpx_dsp/skin_detection.c.o vpx_dsp/skin_detection.c.d;' > vpx_dsp/skin_detection.c.d
cc1: warning: command-line option ‘-Wextra-semi’ is valid for C++/ObjC++ but not for C

Shouldn't -Wextra-semi be added to the CXXFLAGS, rather than CFLAGS?

[1]: GCC 13.2 version: https://gcc.gnu.org/onlinedocs/gcc-13.2.0/gcc/C_002b_002b-Dialect-Options.html#index-Wextra-semi
[2]: GCC 9.5 version: https://gcc.gnu.org/onlinedocs/gcc-9.5.0/gcc/Warning-Options.html#index-Wextra-sem

--
You received this message because:
1. The project was configured to send all issue notifications to this address

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

[jz… via monorail]

Updates:
Components: libvpx

Comment #1 on issue 1832 by jz...@google.com: Build: When building with GCC, C++ warning flag -Wextra-semi is added to CFLAGS
https://bugs.chromium.org/p/webm/issues/detail?id=1832#c1

Thanks for the report. check_add_cflags() checks for C and C++ separately, so should only add the flag when it's supported [1]. The test is done with -Werror.

I can adjust the configuration, but note clang accepts this flag. What version of gcc are you building with?

[1]: https://chromium.googlesource.com/webm/libvpx/+/refs/tags/v1.13.1/build/make/configure.sh#368

[ferd… via monorail]

Comment #2 on issue 1832 by ferd...@gmail.com: Build: When building with GCC, C++ warning flag -Wextra-semi is added to CFLAGS
https://bugs.chromium.org/p/webm/issues/detail?id=1832#c2

Seems the issue is that gcc's -Werror doesn't turn warnings from the preprocessor's command-line parsing into errors, only warnings produced while parsing the code itself. To make preprocessor warnings an error, you need -Wp,-Werror. It's possible this is a gcc bug/regression, but at least with 13.2.1 that's how things seem to be.

Using a vpx-conf-... test source file generated by configure.sh:

# Unrecognized options are an error, even without -Werror

$ gcc -Wextra-semi-stmt -c -o /tmp/vpx-conf-214180-26678.o /tmp/vpx-conf-214180-26678.c
gcc: error: unrecognized command-line option ‘-Wextra-semi-stmt’; did you mean ‘-Wextra-semi’?

# Misused command line options are only warnings, even with -Werror

$ gcc -Werror -Wextra-semi -c -o /tmp/vpx-conf-214180-26678.o /tmp/vpx-conf-214180-26678.c

cc1: warning: command-line option ‘-Wextra-semi’ is valid for C++/ObjC++ but not for C

# Using -Wp,-Werror turns the warning into an error

$ gcc -Wp,-Werror -Wextra-semi -c -o /tmp/vpx-conf-214180-26678.o /tmp/vpx-conf-214180-26678.c
cc1: error: command-line option ‘-Wextra-semi’ is valid for C++/ObjC++ but not for C [-Werror]
cc1: all warnings being treated as errors

[ferd… via monorail]

Comment #3 on issue 1832 by ferd...@gmail.com: Build: When building with GCC, C++ warning flag -Wextra-semi is added to CFLAGS
https://bugs.chromium.org/p/webm/issues/detail?id=1832#c3

Regarding Clang, I saw those flags were added inside an 'if enabled gcc' so I assumed that was the only toolchain they were relevant for. But if they apply to Clang as well, then obviously -Wextra-semi-stmt makes sense to check. (Like I said, it's correctly excluded by check_add_cflags anyway, when building with GCC.)

And if Clang accepts -Wextra-semi for C as well as C++/ObjC++, then I guess leaving it under check_add_cflags and fixing the error-promotion of GCC's option warnings is the way to go.

[ferd… via monorail]

Comment #4 on issue 1832 by ferd...@gmail.com: Build: When building with GCC, C++ warning flag -Wextra-semi is added to CFLAGS
https://bugs.chromium.org/p/webm/issues/detail?id=1832#c4

> # Unrecognized options are an error, even without -Werror
>
> $ gcc -Wextra-semi-stmt -c -o /tmp/vpx-conf-214180-26678.o /tmp/vpx-conf-214180-26678.c
> gcc: error: unrecognized command-line option ‘-Wextra-semi-stmt’; did you mean ‘-Wextra-semi’?

Interestingly, Clang is the opposite, and only warns on unrecognized options unless -Werror is also supplied. (Heck, it doesn't even warn, if the default-enabled option for warnings is turned off via -Wno-unknown-warning-option.)

[jz… via monorail]

Comment #5 on issue 1832 by jz...@google.com: Build: When building with GCC, C++ warning flag -Wextra-semi is added to CFLAGS
https://bugs.chromium.org/p/webm/issues/detail?id=1832#c5

Thanks for investigating this further. To confirm, are you seeing the issue with gcc 13.2.1? 13.2.0 doesn't have an issue for me.

`-Wp,-Werror` seems reasonable, but I'd like to have a configuration that reproduces the issue to simplify testing.

[ferd… via monorail]

Comment #6 on issue 1832 by ferd...@gmail.com: Build: When building with GCC, C++ warning flag -Wextra-semi is added to CFLAGS
https://bugs.chromium.org/p/webm/issues/detail?id=1832#c6

That's correct, my tests were run with this build of gcc:

$ gcc --version
gcc (GCC) 13.2.1 20231011 (Red Hat 13.2.1-4)

...from the official packages on Fedora 39.

Oh! Now, this is interesting. I decided to experimentally remove /usr/lib64/ccache/ from my path, to disable the automatic compiler wrappers ccache provides — just in case it made a difference. And it DOES!

configuring with the gcc and g++ on my path being /usr/bin/{gcc,g++}, with or without --enable-ccache, gcc -Werror -Wextra-semi results in an error:

check_cflags -Wextra-semi
check_cc -Werror -Wextra-semi
BEGIN /tmp/vpx-conf-1569474-1291.c
1 int x;
END /tmp/vpx-conf-1569474-1291.c
gcc -m64 -DNDEBUG -O3 -fPIC -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -Wdisabled-optimization -Werror -Wextra-semi -c -o /tmp/vpx-conf-1569474-1291.o /tmp/vpx-conf-1569474-1291.c

cc1: error: command-line option ‘-Wextra-semi’ is valid for C++/ObjC++ but not for C [-Werror]
cc1: all warnings being treated as errors

But if gcc is /usr/lib64/ccache/gcc (a symlink to ../../bin/ccache, or /usr/bin/ccache), then gcc -Werror -Wextra-semi only results in a warning:

check_cflags -Wextra-semi
check_cc -Werror -Wextra-semi
BEGIN /tmp/vpx-conf-1571280-17190.c
1 int x;
END /tmp/vpx-conf-1571280-17190.c
gcc -m64 -DNDEBUG -O3 -fPIC -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -Wdisabled-optimization -Werror -Wextra-semi -c -o /tmp/vpx-conf-1571280-17190.o /tmp/vpx-conf-1571280-17190.c

cc1: warning: command-line option ‘-Wextra-semi’ is valid for C++/ObjC++ but not for C

...Well, that's annoying! The ccache wrappers so rarely cause any sort of problems, I half forgot I'd even had them enabled.

I'm not 100% sure of the exact issue, but presumably it's ccache deciding not to re-run the preprocessor because it's already cached the object file (or at least the preprocessor output) for that code. I even tried adding logic to configure.sh so that each input file type would get a unique filename and would write object files based on that name, so that

gcc ... -o vpx-conf-1234-9876.o vpx-conf-1234-9876.c

would be different from

g++ ... -o vpx-conf-1234-6666.o vpx-conf-1234-6666.cc

...but it didn't make any difference. I guess the lesson is, make REALLY sure ccache is disabled when running compiler feature detection. Apologies for wasting your time with this!

[jz… via monorail]

Comment #7 on issue 1832 by jz...@google.com: Build: When building with GCC, C++ warning flag -Wextra-semi is added to CFLAGS
https://bugs.chromium.org/p/webm/issues/detail?id=1832#c7

> I'm not 100% sure of the exact issue, but presumably it's ccache deciding not to re-run the preprocessor because it's already cached the object file (or at least the preprocessor output) for that code.

Thanks for looking into it. In the past setting the CCACHE_CPP2 environment variable would help some cases, but I think it's the default now.

[ferd… via monorail]

Comment #8 on issue 1832 by ferd...@gmail.com: Build: When building with GCC, C++ warning flag -Wextra-semi is added to CFLAGS
https://bugs.chromium.org/p/webm/issues/detail?id=1832#c8

> In the past setting the CCACHE_CPP2 environment variable would help some cases, but I think it's the default now.

Yeah, I tried that and no dice. And it seems I was wrong about the cache itself being the issue — even with a _completely_ empty cache, on the very first run `/usr/lib64/ccache/gcc -Werror -Wextra-semi -o foo.o foo.c` still only triggers a warning instead of an error. It seems ccache just executes the preprocessor slightly differently than gcc itself does. Probably a bug on their end, at least arguably.

[ferd… via monorail]

Comment #9 on issue 1832 by ferd...@gmail.com: Build: When building with GCC, C++ warning flag -Wextra-semi is added to CFLAGS
https://bugs.chromium.org/p/webm/issues/detail?id=1832#c9

Huh. Already reported way back in 2020:

https://github.com/ccache/ccache/issues/738

(still open)

[jz… via monorail]

Updates:
Status: WontFix

Comment #10 on issue 1832 by jz...@google.com: Build: When building with GCC, C++ warning flag -Wextra-semi is added to CFLAGS
https://bugs.chromium.org/p/webm/issues/detail?id=1832#c10

Thanks for linking the ccache issue. An alternative workaround in configure would be to use the -Werror=<warning> form, but that would require further parsing of the flags. The -Wp,error workaround might be simpler though its use is discouraged in the gcc manual.

For now I don't think we'll take action on this. If you feel strongly about the issue, we could explore -Wp,error with detection of ccache to limit any potential side effects.

[ferd… via monorail]

Comment #11 on issue 1832 by ferd...@gmail.com: Build: When building with GCC, C++ warning flag -Wextra-semi is added to CFLAGS
https://bugs.chromium.org/p/webm/issues/detail?id=1832#c11

No, I don't think there's any action needed, it was my configuration error that caused the issue.