Bug 23914 - Add --disable-werror to ./configure support (example trigger: CFLAGS=-Og
Summary: Add --disable-werror to ./configure support (example trigger: CFLAGS=-Og
Status: UNCONFIRMED
Alias: None
Product: elfutils
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-23 16:29 UTC by Sergei Trofimovich
Modified: 2018-12-02 23:41 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sergei Trofimovich 2018-11-23 16:29:08 UTC
Initially I attempted to reproduce https://bugs.gentoo.org/671650 on vanilla master on elfutils as:

    $ autoreconf -i -f &&   ./configure --disable-maintainer-mode CFLAGS=-Og &&   make && make check

The problem is in unconditional -Werror addition in
    config/eu.am:       $(if $($(*F)_no_Werror),,-Werror) \

May I ask to add --disable-werror support to ./configure? That way elfutils will be less dependent on exact toolchain version and compiler flags (specifically warning flags).

Example failure:

  dwarf_child.c: In function '__libdw_find_attr':
  dwarf_child.c:99:10: error: 'readp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
    readp += len;
          ^~

Today Gentoo crudely works it around by sed'ing out -Werror at build time:
    https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-libs/elfutils/elfutils-0.175.ebuild#n34
        sed -i 's:-Werror::' */Makefile.in || die

Thanks!
Comment 1 Mark Wielaard 2018-11-23 20:25:27 UTC
I rather have that people report such issues and we fix them.

In this case it really was just that one gcc warning (plus an almost identical one in dwarf_getattrs):
https://sourceware.org/ml/elfutils-devel/2018-q4/msg00174.html

And that build flag actually showed a very interesting nasty bug that we might not have found otherwise:
https://sourceware.org/ml/elfutils-devel/2018-q4/msg00175.html

Does Gentoo show any other compile errors?

BTW for make check to work correctly you do of course need debuginfo, so you really need CLAGS="-g -Og".
Comment 2 Sergei Trofimovich 2018-11-23 23:42:42 UTC
(In reply to Mark Wielaard from comment #1)
> I rather have that people report such issues and we fix them.

Totally understandable. My intent is to only still be able to build old releases with new toolchains or against exotic yet safe CFLAGS  without resort to manual patching of build system.

I would suggest having --enable-werror enabled by default. That way we would be able to disable it downstream for older packages on first offense.

> In this case it really was just that one gcc warning (plus an almost
> identical one in dwarf_getattrs):
> https://sourceware.org/ml/elfutils-devel/2018-q4/msg00174.html
> 
> And that build flag actually showed a very interesting nasty bug that we
> might not have found otherwise:
> https://sourceware.org/ml/elfutils-devel/2018-q4/msg00175.html
> 
> Does Gentoo show any other compile errors?

Yes. I can trigger a lot of them passing various warning flags (I'll post those below).

Gentoo allows users to control CC and CFLAGS and thus the space for getting a warning is wide. People frequently use things like -Wcast-qual or other high signal-to-noise flags for their purposes.

My favourite example is
    ./configure CFLAGS="-g -Wall" # works today without failures
or even ./configure CC=clang CFLAGS="-g -Weverything" but elfutils does not seem to support clang.

Real-world examples used by people:

1. CFLAGS="-g -Wall -Wcast-qual"

  In file included from gelf_xlate.c:166:
  version_xlate.h: In function 'elf_cvt_Verdef':
  version_xlate.h:74:31: error: cast discards 'const' qualifier from pointer target type [-Werror=cast-qual]
         dsrc = (GElf_Verdef *) ((char *) src + def_offset);
                                 ^

2. CFLAGS="-g -O2 -Wstack-protector"

    CC       readelf.o
  readelf.c: In function 'open_input_section':
  readelf.c:581:1: error: stack protector not protecting local variables: variable length buffer [-Werror=stack-protector]
   open_input_section (int fd)
   ^~~~~~~~~~~~~~~~~~

> BTW for make check to work correctly you do of course need debuginfo, so you
> really need CLAGS="-g -Og".

Yes, sorry. Forgot about the unconditional -g passing (we do it all the time).
Comment 3 Mark Wielaard 2018-11-28 13:40:55 UTC
(In reply to Sergei Trofimovich from comment #2)
> Gentoo allows users to control CC and CFLAGS and thus the space for getting
> a warning is wide. People frequently use things like -Wcast-qual or other
> high signal-to-noise flags for their purposes.

If they do and don't care about the warnings, then why don't they simply add -Wno-error too?

> My favourite example is
>     ./configure CFLAGS="-g -Wall" # works today without failures
> or even ./configure CC=clang CFLAGS="-g -Weverything" but elfutils does not
> seem to support clang.

Yes, -Wall is one of the warnings we explicitly enable. See config/eu.am for the full list. Some have configure checks to make sure the compiler actually supports it. And some sadly have to be disabled for some specific source files. If you have concrete warning flags you would like to see enabled by default please do submit them, but we do like to enable them only once the code base is clean. Then we enable them by default and will always catch whenever new code produces a warning (because of -Werror).

An -Weverything flag seems silly, some warnings don't really mix, some are for style issues. IMHO warnings are only interesting if you can take some action to correct the code.

clang support would be nice, but clang doesn't support various GNU C extensions, there is a configure check for it though, so as soon as clang actually support -std=gnu99 it should work.

> Real-world examples used by people:
> 
> 1. CFLAGS="-g -Wall -Wcast-qual"
> 
>   In file included from gelf_xlate.c:166:
>   version_xlate.h: In function 'elf_cvt_Verdef':
>   version_xlate.h:74:31: error: cast discards 'const' qualifier from pointer
> target type [-Werror=cast-qual]
>          dsrc = (GElf_Verdef *) ((char *) src + def_offset);
>                                  ^

There are a lot of cast-qual warnings. It might make sense to clean them up. But it might be hard since in some cases we support overlapping src/dest buffers which might be marked "wrongly" in the external API.

> 2. CFLAGS="-g -O2 -Wstack-protector"
> 
>     CC       readelf.o
>   readelf.c: In function 'open_input_section':
>   readelf.c:581:1: error: stack protector not protecting local variables:
> variable length buffer [-Werror=stack-protector]
>    open_input_section (int fd)
>    ^~~~~~~~~~~~~~~~~~

That in itself wouldn't warn. I assume you are using -fstack-protector[-all|strong] too.

The warning is correct. We do already support -Wstack-usage. But it is disabled for a couple of files. readelf.c is one of them (see src/Makefile.am).
Comment 4 Sergei Trofimovich 2018-12-02 23:41:09 UTC
(In reply to Mark Wielaard from comment #3)
> (In reply to Sergei Trofimovich from comment #2)
> > Gentoo allows users to control CC and CFLAGS and thus the space for getting
> > a warning is wide. People frequently use things like -Wcast-qual or other
> > high signal-to-noise flags for their purposes.
> 
> If they do and don't care about the warnings, then why don't they simply add
> -Wno-error too?

I'm not sure it works when CFLAGS are set system-wide (instead of per-package basis). A few packages do feature testing with -Werror set and insist on keep doing it.

Gentoo can pass CFLAGS="-Wno-error ${USER_CFLAGS}" to elfutils package unconditionally as part of a build script if it's a supported configuration upstream. It should be good enough alternative to --disable-werror.

> > 2. CFLAGS="-g -O2 -Wstack-protector"
> > 
> >     CC       readelf.o
> >   readelf.c: In function 'open_input_section':
> >   readelf.c:581:1: error: stack protector not protecting local variables:
> > variable length buffer [-Werror=stack-protector]
> >    open_input_section (int fd)
> >    ^~~~~~~~~~~~~~~~~~
> 
> That in itself wouldn't warn. I assume you are using
> -fstack-protector[-all|strong] too.

Ah, right. Gentoo's gcc is configured with --enable-default-ssp (#define __SSP_STRONG__ 3). That's why enabling warning alone is enough.