mailing list of musl libc
 help / color / mirror / code / Atom feed
* Clang warning silencing patch
@ 2017-01-10  9:58 Dmitry Golovin
  2017-01-10 10:34 ` Dmitry Golovin
  0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Golovin @ 2017-01-10  9:58 UTC (permalink / raw)
  To: musl

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

The attached patch will reduce the amount of warnings produced when using clang compiler.

It does two things:

  1. Tests for excess-precision=standard and rounding-math are disabled for clang. The problem is that those tests pass, but do no good: every compilation commands produce two warnings about unsupported optimization flags.

  2. LDFLAGS (added to --help) variable is only used for linking, CFLAGS is only used for compiling, none of them are used for assembly. This suppresses all unused argument warnings.

Regards,
Dmitry

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ldflags-musl.patch --]
[-- Type: text/x-diff; name="ldflags-musl.patch", Size: 1976 bytes --]

With this patch CFLAGS and LDFLAGS are separated, so CFLAGS only used when compiling and LDFLAGS only used when linking.
This would silence many clang's warnings about unused arguments.

diff --git a/Makefile b/Makefile
index 8246b78..508878c 100644
--- a/Makefile
+++ b/Makefile
@@ -142,7 +142,7 @@ CC_CMD = $(CC) $(CFLAGS_ALL) -c -o $@ $<
 ifeq ($(ADD_CFI),yes)
 	AS_CMD = LC_ALL=C awk -f $(srcdir)/tools/add-cfi.common.awk -f $(srcdir)/tools/add-cfi.$(ARCH).awk $< | $(CC) $(CFLAGS_ALL) -x assembler -c -o $@ -
 else
-	AS_CMD = $(CC_CMD)
+	AS_CMD = $(CC) -c -o $@ $<
 endif
 
 obj/%.o: $(srcdir)/%.s
@@ -164,7 +164,7 @@ obj/%.lo: $(srcdir)/%.c $(GENH) $(IMPH)
 	$(CC_CMD)
 
 lib/libc.so: $(LOBJS) $(LDSO_OBJS)
-	$(CC) $(CFLAGS_ALL) $(LDFLAGS_ALL) -nostdlib -shared \
+	$(CC) $(LDFLAGS_ALL) -nostdlib -shared \
 	-Wl,-e,_dlstart -o $@ $(LOBJS) $(LDSO_OBJS) $(LIBCC)
 
 lib/libc.a: $(AOBJS)
diff --git a/configure b/configure
index c2db298..9f01ae5 100755
--- a/configure
+++ b/configure
@@ -39,6 +39,7 @@ Optional features:
 Some influential environment variables:
   CC                      C compiler command [detected]
   CFLAGS                  C compiler flags [-Os -pipe ...]
+  LDFLAGS                 linker flags [none]
   CROSS_COMPILE           prefix for cross compiler and tools [none]
   LIBCC                   compiler runtime library [detected]
 
@@ -337,9 +338,17 @@ tryflag CFLAGS_C99FSE -std=c99
 tryflag CFLAGS_C99FSE -nostdinc
 tryflag CFLAGS_C99FSE -ffreestanding \
 || tryflag CFLAGS_C99FSE -fno-builtin
+
+#
+# Do not check for excess-precision=standard and rounding-math on clang
+# because the test doesn't fail, but clang shows them as
+# unsupported optimization flags
+#
+if test "$cc_family" != clang ; then
 tryflag CFLAGS_C99FSE -fexcess-precision=standard \
 || { test "$ARCH" = i386 && tryflag CFLAGS_C99FSE -ffloat-store ; }
 tryflag CFLAGS_C99FSE -frounding-math
+fi
 
 #
 # We may use the may_alias attribute if __GNUC__ is defined, so

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: Clang warning silencing patch
  2017-01-10  9:58 Clang warning silencing patch Dmitry Golovin
@ 2017-01-10 10:34 ` Dmitry Golovin
  2017-01-12  4:23   ` Rich Felker
  0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Golovin @ 2017-01-10 10:34 UTC (permalink / raw)
  To: musl

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

Sorry, I probably have misformatted the patch. Here it is hopefully in correct format. Please tell me if it is wrong.

Regards,
Dmitry

10.01.2017, 11:59, "Dmitry Golovin" <dima@golovin.in>:
> The attached patch will reduce the amount of warnings produced when using clang compiler.
>
> It does two things:
>
>   1. Tests for excess-precision=standard and rounding-math are disabled for clang. The problem is that those tests pass, but do no good: every compilation commands produce two warnings about unsupported optimization flags.
>
>   2. LDFLAGS (added to --help) variable is only used for linking, CFLAGS is only used for compiling, none of them are used for assembly. This suppresses all unused argument warnings.
>
> Regards,
> Dmitry

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 3464-improvements-for-building-with-clang.patch --]
[-- Type: text/x-diff; name="3464-improvements-for-building-with-clang.patch", Size: 2546 bytes --]

From 2b3f03c46211ad3699e2a72f9054861ba7933d52 Mon Sep 17 00:00:00 2001
From: Dmitry Golovin <dima@golovin.in>
Date: Tue, 10 Jan 2017 12:26:27 +0200
Subject: [PATCH 3464/3464] improvements for building with clang

  1. Tests for excess-precision=standard and rounding-math are disabled
     for clang. The problem is that those tests pass, but do no good:
     every compilation commands produce two warnings about
     unsupported optimization flags.

  2. LDFLAGS (added to --help) variable is only used for linking,
     CFLAGS is only used for compiling, none of them are used for assembly.
     This suppresses all unused argument warnings.
---
 Makefile  | 4 ++--
 configure | 9 +++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 8246b78..508878c 100644
--- a/Makefile
+++ b/Makefile
@@ -142,7 +142,7 @@ CC_CMD = $(CC) $(CFLAGS_ALL) -c -o $@ $<
 ifeq ($(ADD_CFI),yes)
 	AS_CMD = LC_ALL=C awk -f $(srcdir)/tools/add-cfi.common.awk -f $(srcdir)/tools/add-cfi.$(ARCH).awk $< | $(CC) $(CFLAGS_ALL) -x assembler -c -o $@ -
 else
-	AS_CMD = $(CC_CMD)
+	AS_CMD = $(CC) -c -o $@ $<
 endif
 
 obj/%.o: $(srcdir)/%.s
@@ -164,7 +164,7 @@ obj/%.lo: $(srcdir)/%.c $(GENH) $(IMPH)
 	$(CC_CMD)
 
 lib/libc.so: $(LOBJS) $(LDSO_OBJS)
-	$(CC) $(CFLAGS_ALL) $(LDFLAGS_ALL) -nostdlib -shared \
+	$(CC) $(LDFLAGS_ALL) -nostdlib -shared \
 	-Wl,-e,_dlstart -o $@ $(LOBJS) $(LDSO_OBJS) $(LIBCC)
 
 lib/libc.a: $(AOBJS)
diff --git a/configure b/configure
index c2db298..9f01ae5 100755
--- a/configure
+++ b/configure
@@ -39,6 +39,7 @@ Optional features:
 Some influential environment variables:
   CC                      C compiler command [detected]
   CFLAGS                  C compiler flags [-Os -pipe ...]
+  LDFLAGS                 linker flags [none]
   CROSS_COMPILE           prefix for cross compiler and tools [none]
   LIBCC                   compiler runtime library [detected]
 
@@ -337,9 +338,17 @@ tryflag CFLAGS_C99FSE -std=c99
 tryflag CFLAGS_C99FSE -nostdinc
 tryflag CFLAGS_C99FSE -ffreestanding \
 || tryflag CFLAGS_C99FSE -fno-builtin
+
+#
+# Do not check for excess-precision=standard and rounding-math on clang
+# because the test doesn't fail, but clang shows them as
+# unsupported optimization flags
+#
+if test "$cc_family" != clang ; then
 tryflag CFLAGS_C99FSE -fexcess-precision=standard \
 || { test "$ARCH" = i386 && tryflag CFLAGS_C99FSE -ffloat-store ; }
 tryflag CFLAGS_C99FSE -frounding-math
+fi
 
 #
 # We may use the may_alias attribute if __GNUC__ is defined, so
-- 
2.7.4


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: Clang warning silencing patch
  2017-01-10 10:34 ` Dmitry Golovin
@ 2017-01-12  4:23   ` Rich Felker
  2017-01-12 11:29     ` Dmitry Golovin
  0 siblings, 1 reply; 4+ messages in thread
From: Rich Felker @ 2017-01-12  4:23 UTC (permalink / raw)
  To: musl

On Tue, Jan 10, 2017 at 12:34:07PM +0200, Dmitry Golovin wrote:
> Sorry, I probably have misformatted the patch. Here it is hopefully in correct format. Please tell me if it is wrong.
> 
> Regards,
> Dmitry
> 
> 10.01.2017, 11:59, "Dmitry Golovin" <dima@golovin.in>:
> > The attached patch will reduce the amount of warnings produced when using clang compiler.
> >
> > It does two things:
> >
> >   1. Tests for excess-precision=standard and rounding-math are
> > disabled for clang. The problem is that those tests pass, but do
> > no good: every compilation commands produce two warnings about
> > unsupported optimization flags.

We attempt to catch issues like that with:

tryflag   CFLAGS_TRY  -Werror=unknown-warning-option
tryflag   CFLAGS_TRY  -Werror=unused-command-line-argument
tryldflag LDFLAGS_TRY -Werror=unknown-warning-option
tryldflag LDFLAGS_TRY -Werror=unused-command-line-argument

If it's not working, can you figure out why? Hard-coding them disabled
for a particular compiler is not reasonable. If they ever are
supported and necessary to get the correct behavior, we'll have
hard-coded wrong values, which is a lot worse than a spurious warning.

> >   2. LDFLAGS (added to --help) variable is only used for linking,
> > CFLAGS is only used for compiling, none of them are used for
> > assembly. This suppresses all unused argument warnings.

At the expense of wrong behavior; see below:

> From 2b3f03c46211ad3699e2a72f9054861ba7933d52 Mon Sep 17 00:00:00 2001
> From: Dmitry Golovin <dima@golovin.in>
> Date: Tue, 10 Jan 2017 12:26:27 +0200
> Subject: [PATCH 3464/3464] improvements for building with clang
> 
>   1. Tests for excess-precision=standard and rounding-math are disabled
>      for clang. The problem is that those tests pass, but do no good:
>      every compilation commands produce two warnings about
>      unsupported optimization flags.
> 
>   2. LDFLAGS (added to --help) variable is only used for linking,
>      CFLAGS is only used for compiling, none of them are used for assembly.
>      This suppresses all unused argument warnings.
> ---
>  Makefile  | 4 ++--
>  configure | 9 +++++++++
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 8246b78..508878c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -142,7 +142,7 @@ CC_CMD = $(CC) $(CFLAGS_ALL) -c -o $@ $<
>  ifeq ($(ADD_CFI),yes)
>  	AS_CMD = LC_ALL=C awk -f $(srcdir)/tools/add-cfi.common.awk -f $(srcdir)/tools/add-cfi.$(ARCH).awk $< | $(CC) $(CFLAGS_ALL) -x assembler -c -o $@ -
>  else
> -	AS_CMD = $(CC_CMD)
> +	AS_CMD = $(CC) -c -o $@ $<
>  endif

Thi is definitely not acceptable as-is; it drops all of the CFLAGS
intended to affect the assembler, including critical ones like
-Wa,--noexecstack.

Ultimately I consider what clang is doing here a bug. The conventional
compiler driver behavior has always been for the driver to accept all
options and only apply the ones relevant to what it's currently doing.
The clang folks have a habit of gratuitously breaking things like this
(see how their arm assembler is pedantic and violates the official arm
documentation in regards to which mnemonic forms it accepts for which
-march settings) and I really don't want to play whack-a-mole with
them.

>  obj/%.o: $(srcdir)/%.s
> @@ -164,7 +164,7 @@ obj/%.lo: $(srcdir)/%.c $(GENH) $(IMPH)
>  	$(CC_CMD)
>  
>  lib/libc.so: $(LOBJS) $(LDSO_OBJS)
> -	$(CC) $(CFLAGS_ALL) $(LDFLAGS_ALL) -nostdlib -shared \
> +	$(CC) $(LDFLAGS_ALL) -nostdlib -shared \
>  	-Wl,-e,_dlstart -o $@ $(LOBJS) $(LDSO_OBJS) $(LIBCC)

This probably doesn't currently break anything, but I can't say for
sure. In principle there can be CFLAGS that should also be specified
at link like (like stack protector, sanitizer, etc. stuff) that
probably don't affect musl, but omitting CFLAGS when linking is not in
general a good practice.

>  lib/libc.a: $(AOBJS)
> diff --git a/configure b/configure
> index c2db298..9f01ae5 100755
> --- a/configure
> +++ b/configure
> @@ -39,6 +39,7 @@ Optional features:
>  Some influential environment variables:
>    CC                      C compiler command [detected]
>    CFLAGS                  C compiler flags [-Os -pipe ...]
> +  LDFLAGS                 linker flags [none]
>    CROSS_COMPILE           prefix for cross compiler and tools [none]
>    LIBCC                   compiler runtime library [detected]

This hunk looks ok.

Rich


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: Clang warning silencing patch
  2017-01-12  4:23   ` Rich Felker
@ 2017-01-12 11:29     ` Dmitry Golovin
  0 siblings, 0 replies; 4+ messages in thread
From: Dmitry Golovin @ 2017-01-12 11:29 UTC (permalink / raw)
  To: musl

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

I just figured out the proper test for whether the option is supported (patch attached, it just adds one line).

I understand why you don't want to separate CFLAGS from LDFLAGS. People tend to put flags for compiler, assembler and linker into a single variable which makes life much easier.

All the unused arguments warnings can possibly be silenced by adding `-Qunused-arguments`, but I don't think that this is the right method to be used.

With my patch assembler flags (like --noexecstack) are omitted. Maybe the proper thing to do is to introduce ASFLAGS variable, then parse CFLAGS and put all '-Wl,*' to LDFLAGS and '-Wa,*' to ASFLAGS, removing them from CFLAGS, but it looks like nobody is really doing such thing, so probably just leaving it as it is now and getting some harmless unused arguments warnings is okay.

Regards,
Dmitry

12.01.2017, 06:23, "Rich Felker" <dalias@libc.org>:
> On Tue, Jan 10, 2017 at 12:34:07PM +0200, Dmitry Golovin wrote:
>>  Sorry, I probably have misformatted the patch. Here it is hopefully in correct format. Please tell me if it is wrong.
>>
>>  Regards,
>>  Dmitry
>>
>>  10.01.2017, 11:59, "Dmitry Golovin" <dima@golovin.in>:
>>  > The attached patch will reduce the amount of warnings produced when using clang compiler.
>>  >
>>  > It does two things:
>>  >
>>  >   1. Tests for excess-precision=standard and rounding-math are
>>  > disabled for clang. The problem is that those tests pass, but do
>>  > no good: every compilation commands produce two warnings about
>>  > unsupported optimization flags.
>
> We attempt to catch issues like that with:
>
> tryflag CFLAGS_TRY -Werror=unknown-warning-option
> tryflag CFLAGS_TRY -Werror=unused-command-line-argument
> tryldflag LDFLAGS_TRY -Werror=unknown-warning-option
> tryldflag LDFLAGS_TRY -Werror=unused-command-line-argument
>
> If it's not working, can you figure out why? Hard-coding them disabled
> for a particular compiler is not reasonable. If they ever are
> supported and necessary to get the correct behavior, we'll have
> hard-coded wrong values, which is a lot worse than a spurious warning.
>
>>  >   2. LDFLAGS (added to --help) variable is only used for linking,
>>  > CFLAGS is only used for compiling, none of them are used for
>>  > assembly. This suppresses all unused argument warnings.
>
> At the expense of wrong behavior; see below:
>
>>  From 2b3f03c46211ad3699e2a72f9054861ba7933d52 Mon Sep 17 00:00:00 2001
>>  From: Dmitry Golovin <dima@golovin.in>
>>  Date: Tue, 10 Jan 2017 12:26:27 +0200
>>  Subject: [PATCH 3464/3464] improvements for building with clang
>>
>>    1. Tests for excess-precision=standard and rounding-math are disabled
>>       for clang. The problem is that those tests pass, but do no good:
>>       every compilation commands produce two warnings about
>>       unsupported optimization flags.
>>
>>    2. LDFLAGS (added to --help) variable is only used for linking,
>>       CFLAGS is only used for compiling, none of them are used for assembly.
>>       This suppresses all unused argument warnings.
>>  ---
>>   Makefile | 4 ++--
>>   configure | 9 +++++++++
>>   2 files changed, 11 insertions(+), 2 deletions(-)
>>
>>  diff --git a/Makefile b/Makefile
>>  index 8246b78..508878c 100644
>>  --- a/Makefile
>>  +++ b/Makefile
>>  @@ -142,7 +142,7 @@ CC_CMD = $(CC) $(CFLAGS_ALL) -c -o $@ $<
>>   ifeq ($(ADD_CFI),yes)
>>           AS_CMD = LC_ALL=C awk -f $(srcdir)/tools/add-cfi.common.awk -f $(srcdir)/tools/add-cfi.$(ARCH).awk $< | $(CC) $(CFLAGS_ALL) -x assembler -c -o $@ -
>>   else
>>  - AS_CMD = $(CC_CMD)
>>  + AS_CMD = $(CC) -c -o $@ $<
>>   endif
>
> Thi is definitely not acceptable as-is; it drops all of the CFLAGS
> intended to affect the assembler, including critical ones like
> -Wa,--noexecstack.
>
> Ultimately I consider what clang is doing here a bug. The conventional
> compiler driver behavior has always been for the driver to accept all
> options and only apply the ones relevant to what it's currently doing.
> The clang folks have a habit of gratuitously breaking things like this
> (see how their arm assembler is pedantic and violates the official arm
> documentation in regards to which mnemonic forms it accepts for which
> -march settings) and I really don't want to play whack-a-mole with
> them.
>
>>   obj/%.o: $(srcdir)/%.s
>>  @@ -164,7 +164,7 @@ obj/%.lo: $(srcdir)/%.c $(GENH) $(IMPH)
>>           $(CC_CMD)
>>
>>   lib/libc.so: $(LOBJS) $(LDSO_OBJS)
>>  - $(CC) $(CFLAGS_ALL) $(LDFLAGS_ALL) -nostdlib -shared \
>>  + $(CC) $(LDFLAGS_ALL) -nostdlib -shared \
>>           -Wl,-e,_dlstart -o $@ $(LOBJS) $(LDSO_OBJS) $(LIBCC)
>
> This probably doesn't currently break anything, but I can't say for
> sure. In principle there can be CFLAGS that should also be specified
> at link like (like stack protector, sanitizer, etc. stuff) that
> probably don't affect musl, but omitting CFLAGS when linking is not in
> general a good practice.
>
>>   lib/libc.a: $(AOBJS)
>>  diff --git a/configure b/configure
>>  index c2db298..9f01ae5 100755
>>  --- a/configure
>>  +++ b/configure
>>  @@ -39,6 +39,7 @@ Optional features:
>>   Some influential environment variables:
>>     CC C compiler command [detected]
>>     CFLAGS C compiler flags [-Os -pipe ...]
>>  + LDFLAGS linker flags [none]
>>     CROSS_COMPILE prefix for cross compiler and tools [none]
>>     LIBCC compiler runtime library [detected]
>
> This hunk looks ok.
>
> Rich

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-properly-test-for-unsupported-optimization-flags-wit.patch --]
[-- Type: text/x-diff; name="0001-properly-test-for-unsupported-optimization-flags-wit.patch", Size: 710 bytes --]

From e8d9cc1bddf204d83957d2c73bd287f3c3c1360b Mon Sep 17 00:00:00 2001
From: Dmitry Golovin <dima@golovin.in>
Date: Thu, 12 Jan 2017 13:00:58 +0200
Subject: [PATCH 1/1] properly test for unsupported optimization flags with
 clang

---
 configure | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index c2db298..73d0b04 100755
--- a/configure
+++ b/configure
@@ -249,6 +249,7 @@ fi
 #
 tryflag   CFLAGS_TRY  -Werror=unknown-warning-option
 tryflag   CFLAGS_TRY  -Werror=unused-command-line-argument
+tryflag   CFLAGS_TRY  -Werror=ignored-optimization-argument
 tryldflag LDFLAGS_TRY -Werror=unknown-warning-option
 tryldflag LDFLAGS_TRY -Werror=unused-command-line-argument
 
-- 
2.7.4


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-01-12 11:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-10  9:58 Clang warning silencing patch Dmitry Golovin
2017-01-10 10:34 ` Dmitry Golovin
2017-01-12  4:23   ` Rich Felker
2017-01-12 11:29     ` Dmitry Golovin

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).