[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170112042325.GC1533@brightrain.aerifal.cx>
Date: Wed, 11 Jan 2017 23:23:25 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Clang warning silencing patch
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@...ovin.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@...ovin.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
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
