[PATCH] config: Pass the compiler -Werror during warning detection

[PATCH] config: Pass the compiler -Werror during warning detection

[Filipe Brandenburger]

[-- Attachment #1: Type: text/plain, Size: 2388 bytes --]

Otherwise the compilation prints a warning but exits with 0 return code.
However, later during the compilation, when -Werror is enforced to about
every file, the unsupported options start breaking the builds.

Tested: Ran configure with clang-3.5 and built libelf/ tree with it.

  $ autoreconf -i
  $ ./configure CC=clang-3.5 ...
  $ make -C libelf

Signed-off-by: Filipe Brandenburger <filbranden@google.com>
---
 ChangeLog    | 3 +++
 configure.ac | 6 +++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index f2b5c8fa17e0..1ec202ac9ae4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,6 +1,9 @@
 2016-05-02  Filipe Brandenburger  <filbranden@google.com>
 
 	* configure.ac (argp check): Pass pass &argv.
+	* configure.ac (-W<...> checks): Pass -Werror to the warning checks,
+	to ensure unsupported warning options are noticed during ./configure
+	time and not only later during build.
 
 2016-03-31  Mark Wielaard  <mjw@redhat.com>
 
diff --git a/configure.ac b/configure.ac
index 72cb22e82d8a..07c04637adaa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -298,7 +298,7 @@ AS_IF([test "x$enable_symbol_versioning" = "xno"],
 
 AC_CACHE_CHECK([whether gcc accepts -Wstack-usage], ac_cv_stack_usage, [dnl
 old_CFLAGS="$CFLAGS"
-CFLAGS="$CFLAGS -Wstack-usage=262144"
+CFLAGS="$CFLAGS -Wstack-usage=262144 -Werror"
 AC_COMPILE_IFELSE([AC_LANG_SOURCE([])],
 		  ac_cv_stack_usage=yes, ac_cv_stack_usage=no)
 CFLAGS="$old_CFLAGS"])
@@ -319,7 +319,7 @@ AM_CONDITIONAL(SANE_LOGICAL_OP_WARNING,
 # -Wduplicated-cond was added by GCC6
 AC_CACHE_CHECK([whether gcc accepts -Wduplicated-cond], ac_cv_duplicated_cond, [dnl
 old_CFLAGS="$CFLAGS"
-CFLAGS="$CFLAGS -Wduplicated-cond"
+CFLAGS="$CFLAGS -Wduplicated-cond -Werror"
 AC_COMPILE_IFELSE([AC_LANG_SOURCE([])],
 		  ac_cv_duplicated_cond=yes, ac_cv_duplicated_cond=no)
 CFLAGS="$old_CFLAGS"])
@@ -329,7 +329,7 @@ AM_CONDITIONAL(HAVE_DUPLICATED_COND_WARNING,
 # -Wnull-dereference was added by GCC6
 AC_CACHE_CHECK([whether gcc accepts -Wnull-dereference], ac_cv_null_dereference, [dnl
 old_CFLAGS="$CFLAGS"
-CFLAGS="$CFLAGS -Wnull-dereference"
+CFLAGS="$CFLAGS -Wnull-dereference -Werror"
 AC_COMPILE_IFELSE([AC_LANG_SOURCE([])],
 		  ac_cv_null_dereference=yes, ac_cv_null_dereference=no)
 CFLAGS="$old_CFLAGS"])
-- 
2.8.0.rc3.226.g39d4020

Re: [PATCH] config: Pass the compiler -Werror during warning detection

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 1403 bytes --]

On Mon, 2016-05-02 at 14:46 -0700, Filipe Brandenburger wrote:
> On Mon, May 2, 2016 at 2:21 PM, Mark Wielaard <mjw@redhat.com> wrote:
> > I am not against this patch if it makes our compiler feature detection
> > better.
> 
> I'd say it makes configure closer to build, considering during build
> time the Makefile is already adding -Werror to most targets.
> Furthermore, the check for -Wlogical-op was already using -Werror as
> well, so I don't see why not have the three others be consistent with
> it (granted, the -Wlogical-op one actually uses non-trivial code,
> while the others just compile a trivial file.)

That makes sense. The -Wlogical-op one was special because we didn't
just want to check that the warning flag was supported, but also that it
didn't produce false positives, which happened with older GCC releases.

> Right now I have to "trick" configure into skipping that test, with:
> 
>   $ ./configure CC=clang-3.5 ac_cv_c99=yes
> 
> And then the build works, at least of libelf/ subdir.
> 
> I hope you agree with my arguments on consistency (-Werror at
> configure vs. build time), but if you feel strongly against this patch
> we do have some possible workarounds (such as passing -Werror in
> CFLAGS of ./configure).

The patch is fine. Pushed it to master.
Just wanted to double check the changes/improvements were deliberate.

Thanks,

Mark

Re: [PATCH] config: Pass the compiler -Werror during warning detection

[Filipe Brandenburger]

[-- Attachment #1: Type: text/plain, Size: 2784 bytes --]

Hi Mark,

On Mon, May 2, 2016 at 2:21 PM, Mark Wielaard <mjw@redhat.com> wrote:
> On Mon, May 02, 2016 at 01:26:13PM -0700, Filipe Brandenburger wrote:
>> Otherwise the compilation prints a warning but exits with 0 return code.
>> However, later during the compilation, when -Werror is enforced to about
>> every file, the unsupported options start breaking the builds.
>
> Odd. Normally bad command line options should always cause a non-zero
> exit code. Do you know if that was done deliberate or if it was fixed
> in a later version of the compiler?

This seems to be deliberately done in clang (perhaps for easier
compatibility with other compilers?)... An invalid option produces a
-Wunknown-warning-option warning which is by default enabled as a
warning and not an error... -Werror=unknown-warning-option turns those
into errors and -Werror itself will turn that one on as part of all
the others...

>> Tested: Ran configure with clang-3.5 and built libelf/ tree with it.
>>
>>   $ autoreconf -i
>>   $ ./configure CC=clang-3.5 ...
>>   $ make -C libelf
>
> I am not against this patch if it makes our compiler feature detection
> better.

I'd say it makes configure closer to build, considering during build
time the Makefile is already adding -Werror to most targets.
Furthermore, the check for -Wlogical-op was already using -Werror as
well, so I don't see why not have the three others be consistent with
it (granted, the -Wlogical-op one actually uses non-trivial code,
while the others just compile a trivial file.)

> But clang really isn't supported.

Yes this is understood :-)

> There have been many patches to try to make elfutils build with it.

Yes we're aware. What we need right now is only libelf/ and that one
seems to build fine with clang only.

> But it seems there are just too many missing features.

I'm not sure I'd call those "features" :-) But I digress...

> In fact configure should check all basic language
> constructs we are using ("gcc with GNU99 support"), so people don't
> accidentially try building with a broken compiler. Does clang now
> implement all gnu99 extensions or is our feature check broken?

No it does not implement them and for instance it will never implement
nested functions. But, as I mentioned, libelf/ tree is sane enough to
build with it.

Right now I have to "trick" configure into skipping that test, with:

  $ ./configure CC=clang-3.5 ac_cv_c99=yes

And then the build works, at least of libelf/ subdir.

I hope you agree with my arguments on consistency (-Werror at
configure vs. build time), but if you feel strongly against this patch
we do have some possible workarounds (such as passing -Werror in
CFLAGS of ./configure).

Thank you!
Filipe

Re: [PATCH] config: Pass the compiler -Werror during warning detection

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 1136 bytes --]

On Mon, May 02, 2016 at 01:26:13PM -0700, Filipe Brandenburger wrote:
> Otherwise the compilation prints a warning but exits with 0 return code.
> However, later during the compilation, when -Werror is enforced to about
> every file, the unsupported options start breaking the builds.

Odd. Normally bad command line options should always cause a non-zero
exit code. Do you know if that was done deliberate or if it was fixed
in a later version of the compiler?

> Tested: Ran configure with clang-3.5 and built libelf/ tree with it.
> 
>   $ autoreconf -i
>   $ ./configure CC=clang-3.5 ...
>   $ make -C libelf

I am not against this patch if it makes our compiler feature detection
better. But clang really isn't supported. There have been many patches
to try to make elfutils build with it. But it seems there are just too
many missing features. In fact configure should check all basic language
constructs we are using ("gcc with GNU99 support"), so people don't
accidentially try building with a broken compiler. Does clang now
implement all gnu99 extensions or is our feature check broken?

Thanks,

Mark