Re: [PATCH 1/1] MINOR: build: force CC to set a return code when probing options

2021-03-09 Thread Willy Tarreau
Hi Bertrand,

On Sat, Mar 06, 2021 at 08:25:46PM +, Bertrand Jacquin wrote:
> gcc returns non zero code if an option is not supported (tested
> from 6.5 to 10.2).
> 
>   $ gcc -Wfoobar -E -xc - -o /dev/null < /dev/null > /dev/null 2>&1 ; echo $?
>   1
> 
> clang always return 0 if an option in not recognized unless
> -Werror is also passed, preventing a correct probing of options
> supported by the compiler (tested with clang 6.0.1 to 11.1.0).
> 
>   $ clang -Wfoobar -E -xc - -o /dev/null < /dev/null > /dev/null 2>&1 ; echo 
> $?
>   0
>   $ clang -Werror -Wfoobar -E -xc - -o /dev/null < /dev/null > /dev/null 2>&1 
> ; echo $?
>   1

That's interesting, because we've constantly been complaining about gcc
not accepting random -Wfoobar and requiring runtime discovery, but on the
other hand, not having any info about support is a problem as well. I can
indeed imagine that we could enable -Werror=some-option-about-commandline
but that would require to detect it, so I think that -Werror indeed remains
an acceptable solution here.

> Please note today this is not visible since clang 11 exit with SIGABRT
> or with return code 1 on older version due to bad file descriptor from
> file descriptor handling
> 
>   $ clang -Wfoobar -E -xc - -o /dev/null < /dev/null 2>&0 ; echo $?
>   Aborted (core dumped)
>   134

This is particularly ugly, I can imagine how annoying it can become if
a tens of cores are produced upon every single haproxy build. Let's hope
they quickly fix it.

Thanks for the explanation, I'm taking your patch!
Willy



Re: [PATCH 1/1] MINOR: build: force CC to set a return code when probing options

2021-03-06 Thread Lukas Tribus
Hello Bertrand,

On Sun, 7 Mar 2021 at 00:53, Bertrand Jacquin  wrote:
> I am not proposing haproxy build-system to use -Werror here, I'm only
> proposing to use -Werror when probing for options supported by the
> compiler, as effectively clang return a code if 0 even if an option is
> not supported. The fact haproxy does not use -Wno-clobbered today is
> with clang build because of an internal bug in clang with how haproxy is
> using stdin/stderr indirection.
>
> With the proposal above, Werror is only use to probe options, it is not
> reflected in SPEC_CFLAGS.

Got it, thanks for clarifying.


Lukas



Re: [PATCH 1/1] MINOR: build: force CC to set a return code when probing options

2021-03-06 Thread Bertrand Jacquin
Hi Lukas,

On Saturday, March 06 2021 at 23:48:52 +0100, Lukas Tribus wrote:
> Hello,
> 
> On Sat, 6 Mar 2021 at 21:25, Bertrand Jacquin  wrote:
> >
> > gcc returns non zero code if an option is not supported (tested
> > from 6.5 to 10.2).
> >
> >   $ gcc -Wfoobar -E -xc - -o /dev/null < /dev/null > /dev/null 2>&1 ; echo 
> > $?
> >   1
> >
> > clang always return 0 if an option in not recognized unless
> > -Werror is also passed, preventing a correct probing of options
> > supported by the compiler (tested with clang 6.0.1 to 11.1.0)
> 
> -Werror is more than just "-Wunknown-warning-option" on clang.
> -Werror/ERR is specifically optional in the Makefile.
> 
> If we want to change the haproxy build-system to do -Werror from now
> on we need a) discuss it and b) fix it all up. We can't hardcode
> -Werror and at the same time claim that it's an optional parameter.

I am not proposing haproxy build-system to use -Werror here, I'm only
proposing to use -Werror when probing for options supported by the
compiler, as effectively clang return a code if 0 even if an option is
not supported. The fact haproxy does not use -Wno-clobbered today is
with clang build because of an internal bug in clang with how haproxy is
using stdin/stderr indirection.

With the proposal above, Werror is only use to probe options, it is not
reflected in SPEC_CFLAGS.

-- 
Bertrand



Re: [PATCH 1/1] MINOR: build: force CC to set a return code when probing options

2021-03-06 Thread Lukas Tribus
Hello,

On Sat, 6 Mar 2021 at 21:25, Bertrand Jacquin  wrote:
>
> gcc returns non zero code if an option is not supported (tested
> from 6.5 to 10.2).
>
>   $ gcc -Wfoobar -E -xc - -o /dev/null < /dev/null > /dev/null 2>&1 ; echo $?
>   1
>
> clang always return 0 if an option in not recognized unless
> -Werror is also passed, preventing a correct probing of options
> supported by the compiler (tested with clang 6.0.1 to 11.1.0)

-Werror is more than just "-Wunknown-warning-option" on clang.
-Werror/ERR is specifically optional in the Makefile.

If we want to change the haproxy build-system to do -Werror from now
on we need a) discuss it and b) fix it all up. We can't hardcode
-Werror and at the same time claim that it's an optional parameter.




Lukas



[PATCH 1/1] MINOR: build: force CC to set a return code when probing options

2021-03-06 Thread Bertrand Jacquin
gcc returns non zero code if an option is not supported (tested
from 6.5 to 10.2).

  $ gcc -Wfoobar -E -xc - -o /dev/null < /dev/null > /dev/null 2>&1 ; echo $?
  1

clang always return 0 if an option in not recognized unless
-Werror is also passed, preventing a correct probing of options
supported by the compiler (tested with clang 6.0.1 to 11.1.0).

  $ clang -Wfoobar -E -xc - -o /dev/null < /dev/null > /dev/null 2>&1 ; echo $?
  0
  $ clang -Werror -Wfoobar -E -xc - -o /dev/null < /dev/null > /dev/null 2>&1 ; 
echo $?
  1

Please note today this is not visible since clang 11 exit with SIGABRT
or with return code 1 on older version due to bad file descriptor from
file descriptor handling

  $ clang -Wfoobar -E -xc - -o /dev/null < /dev/null 2>&0 ; echo $?
  Aborted (core dumped)
  134
  $ clang -Wfoobar -E -xc - -o /dev/null < /dev/null ; echo $?
  warning: unknown warning option '-Wfoobar'; did you mean '-Wformat'? 
[-Wunknown-warning-option]
  1 warning generated.
  0
  $ clang-11 -Werror -Wfoobar -E -xc - -o /dev/null < /dev/null ; echo $?
  error: unknown warning option '-Wfoobar'; did you mean '-Wformat'? 
[-Werror,-Wunknown-warning-option]
  1

This specific issue is being tracked with clang upstream in 
https://bugs.llvm.org/show_bug.cgi?id=49463
---
 Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 77960ba4cac5..0867047fdc69 100644
--- a/Makefile
+++ b/Makefile
@@ -126,16 +126,16 @@ endif
 # Usage: CFLAGS += $(call cc-opt,option). Eg: $(call cc-opt,-fwrapv)
 # Note: ensure the referencing variable is assigned using ":=" and not "=" to
 #   call it only once.
-cc-opt = $(shell set -e; if $(CC) $(1) -E -xc - -o /dev/null &0 
2>&0; then echo "$(1)"; fi;)
+cc-opt = $(shell set -e; if $(CC) -Werror $(1) -E -xc - -o /dev/null 
&0 2>&0; then echo "$(1)"; fi;)
 
 # same but emits $2 if $1 is not supported
-cc-opt-alt = $(shell set -e; if $(CC) $(1) -E -xc - -o /dev/null &0 2>&0; then echo "$(1)"; else echo "$(2)"; fi;)
+cc-opt-alt = $(shell set -e; if $(CC) -Werror $(1) -E -xc - -o /dev/null 
&0 2>&0; then echo "$(1)"; else echo "$(2)"; fi;)
 
 # Disable a warning when supported by the compiler. Don't put spaces around the
 # warning! And don't use cc-opt which doesn't always report an error until
 # another one is also returned.
 # Usage: CFLAGS += $(call cc-nowarn,warning). Eg: $(call 
cc-opt,format-truncation)
-cc-nowarn = $(shell set -e; if $(CC) -W$(1) -E -xc - -o /dev/null &0 2>&0; then echo "-Wno-$(1)"; fi;)
+cc-nowarn = $(shell set -e; if $(CC) -Werror -W$(1) -E -xc - -o /dev/null 
&0 2>&0; then echo "-Wno-$(1)"; fi;)
 
  Installation options.
 DESTDIR =