[Bug general/23914] New: Add --disable-werror to ./configure support (example trigger: CFLAGS=-Og

[Bug general/23914] New: Add --disable-werror to ./configure support (example trigger: CFLAGS=-Og

[slyfox at inbox dot ru]

https://sourceware.org/bugzilla/show_bug.cgi?id=23914

            Bug ID: 23914
           Summary: Add --disable-werror to ./configure support (example
                    trigger:  CFLAGS=-Og
           Product: elfutils
           Version: unspecified
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: general
          Assignee: unassigned at sourceware dot org
          Reporter: slyfox at inbox dot ru
                CC: elfutils-devel at sourceware dot org
  Target Milestone: ---

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!

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug general/23914] Add --disable-werror to ./configure support (example trigger: CFLAGS=-Og

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=23914

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mark at klomp dot org

--- Comment #1 from Mark Wielaard <mark at klomp dot org> ---
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".

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug general/23914] Add --disable-werror to ./configure support (example trigger: CFLAGS=-Og

[slyfox at inbox dot ru]

https://sourceware.org/bugzilla/show_bug.cgi?id=23914

--- Comment #2 from Sergei Trofimovich <slyfox at inbox dot ru> ---
(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).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug general/23914] Add --disable-werror to ./configure support (example trigger: CFLAGS=-Og

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=23914

--- Comment #3 from Mark Wielaard <mark at klomp dot org> ---
(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).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug general/23914] Add --disable-werror to ./configure support (example trigger: CFLAGS=-Og

[slyfox at inbox dot ru]

https://sourceware.org/bugzilla/show_bug.cgi?id=23914

--- Comment #4 from Sergei Trofimovich <slyfox at inbox dot ru> ---
(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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.