Re: Add -Werror to build_warnings

Re: Add -Werror to build_warnings

[Paul Schlie]

> From: Alan Modra <amodra@bigpond.net.au>
>> - and/or recommend that GCC-4.0 eliminate that frivolous warning by default.
> I'm not going to do any such thing.  I rather like the fact that gcc-4.0
> has more warnings, despite it causing some work to make our code warning
> free.  Case in point: Better analysis of uninitialized variables found
> two places in binutils where we really could use an uninitialized
> variable in corner cases.  It also warned about 4 or 5 other places that
> were not real problems.  That's a very good ratio.  I'll take 100 false
> positives if a compiler finds me one real error..

- Fully understand your position in general, however honestly question the
  utility of the specific warning: of a comparison between two pointers to
  otherwise equivalent rank data types which only in sign-ness; as it's not
  clear that such a comparison could ever lead to unintended consequences,
  as even arguably evidenced by the apparent intent to disable it, rather
  than be alerted of such comparisons by default (presumably as it's not
  perceived as likely helpful)?  However would very much appreciate an
  example to the contrary, as I'm obviously confused by it's purpose, as
  the sign-ness of a data type seems irrelevant when attempting to compare
  the addresses two otherwise compatible data types?

  (but not a big deal, as the warning may easily be disabled as desired)

Re: Add -Werror to build_warnings

[Zack Weinberg]

Paul Schlie <schlie@comcast.net> writes:

Would you please write complete sentences?  Your telegraphic style is
extremely difficult to read.

> - Fully understand your position in general, however honestly question the
>   utility of the specific warning: of a comparison between two pointers to
>   otherwise equivalent rank data types which only in sign-ness; as it's not
>   clear that such a comparison could ever lead to unintended consequences,

You are mistaken.  We flushed all these warnings out of GCC about a
year ago, and we found real bugs.  Mostly, if I recall correctly, they
were cases where the two pointers should never have been being
compared in the first place, and this was the only diagnostic you got.

zw

Re: Add -Werror to build_warnings

[Paul Schlie]

> From: Zack Weinberg <zack@codesourcery.com>
> Would you please write complete sentences?  Your telegraphic style is
> extremely difficult to read.
>> Paul Schlie <schlie@comcast.net> writes:
>> - Fully understand your position in general, however honestly question the
>>   utility of the specific warning: of a comparison between two pointers to
>>   otherwise equivalent rank data types which only in sign-ness; as it's not
>>   clear that such a comparison could ever lead to unintended consequences,
> 
> You are mistaken.  We flushed all these warnings out of GCC about a
> year ago, and we found real bugs.  Mostly, if I recall correctly, they
> were cases where the two pointers should never have been being
> compared in the first place, and this was the only diagnostic you got.

- OK, I accept that it basically forces you to at least look; at which time
  you may discover something that wasn't intended somewhere. Which one could
  argue is better than nothing.

Thanks,

Re: Add -Werror to build_warnings

[Alan Modra]

On Thu, Feb 17, 2005 at 11:34:02PM -0500, Paul Schlie wrote:
>   as even arguably evidenced by the apparent intent to disable it, rather

Temporarily.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: Add -Werror to build_warnings

[Alan Modra]

On Thu, Feb 17, 2005 at 09:18:27PM -0500, Paul Schlie wrote:
> - and/or recommend that GCC-4.0 eliminate that frivolous warning by default.

I'm not going to do any such thing.  I rather like the fact that gcc-4.0
has more warnings, despite it causing some work to make our code warning
free.  Case in point: Better analysis of uninitialized variables found
two places in binutils where we really could use an uninitialized
variable in corner cases.  It also warned about 4 or 5 other places that
were not real problems.  That's a very good ratio.  I'll take 100 false
positives if a compiler finds me one real error..

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: Add -Werror to build_warnings

[Paul Schlie]

(sorry, wrong subject in earlier message got a way from me)

> Alan Modra <amodra at bigpond dot net dot au>
> I think we could enable -Werror once someone approves my cgen patch.
> About all we're left with is a zillion gcc-4.0 warnings about
> "pointer targets ... differ in signedness".  So if Ben can figure out
> how to add -Wno-pointer-sign for compilers that support it, then -Werror
> should soon smoke out the last few compile warnings.

- and/or recommend that GCC-4.0 eliminate that frivolous warning by default.

Re: Add -Werror to build_warnings

[Alan Modra]

I think we could enable -Werror once someone approves my cgen patch.
About all we're left with is a zillion gcc-4.0 warnings about
"pointer targets ... differ in signedness".  So if Ben can figure out
how to add -Wno-pointer-sign for compilers that support it, then -Werror
should soon smoke out the last few compile warnings.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: Add -Werror to build_warnings

[Nick Clifton]

Hi Ben,

> Oh, I misread the shell code in configure.in.  OK, it's on by default.
> In that case, I withdraw my patch.  We're not ready yet.


My plan is to introduce your patch after the 2.16 branch has been taken.

Cheers
   Nick

Re: Add -Werror to build_warnings

[Ben Elliston]

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

> > These flags are only used when the user
> > explicitly passes --enable-build-warnings
> 
> Huh?  I seem to get them by default.

Oh, I misread the shell code in configure.in.  OK, it's on by default.
In that case, I withdraw my patch.  We're not ready yet.

Cheers, Ben


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Add -Werror to build_warnings

[Alan Modra]

On Wed, Feb 16, 2005 at 11:21:03AM +1100, Ben Elliston wrote:
> These flags are only used when the user
> explicitly passes --enable-build-warnings

Huh?  I seem to get them by default.

> 	* configure.in (build_warnings): Add -Werror.
> 	* configure: Rebuild.

I like the idea, but I don't think we are quite ready yet.  I still see
quite a few warnings with --enable-targets=all.  You need to compile
with at least gcc-3.4 and gcc-4.0 on 32-bit and 64-bit targets, with
--enable-targets=all.  On the 32-bit target, compile with
--enable-targets=all --enable-64-bit-bfd too, so that you test both
64-bit and 32-bit bfd_vma.  Once you've done that, I'd say we are
probably ready for -Werror in all but gas/.  For gas, you need to
compile for a number of targets, since gas isn't multi-target.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Add -Werror to build_warnings

[Ben Elliston]

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

Now that Nick has finished off the last of the cleanups (hooray!), now
is likely the time to add -Werror to the build_warnings variable in
the various configure.ins.  These flags are only used when the user
explicitly passes --enable-build-warnings, so we can do this now that
we build cleanly with GCC with minimal disruption.

Okay for mainline?

bfd,binutils,gas,gprof,ld,opcodes/ChangeLog:

2005-02-16  Ben Elliston  <bje@au.ibm.com>

	* configure.in (build_warnings): Add -Werror.
	* configure: Rebuild.

The diffs for {bfd,binutils,gas,gprof,ld,opcodes}/configure.in are all
the same, so I'll paste in just one:

Index: configure.in
===================================================================
RCS file: /cvs/src/src/bfd/configure.in,v
retrieving revision 1.166
diff -u -p -c -r1.166 configure.in
*** configure.in	6 Jan 2005 16:18:20 -0000	1.166
--- configure.in	16 Feb 2005 00:14:02 -0000
*************** AC_ARG_WITH(mmap,
*** 51,57 ****
    *)    AC_MSG_ERROR(bad value ${withval} for BFD with-mmap option) ;;
  esac],[want_mmap=false])dnl
  
! build_warnings="-W -Wall -Wstrict-prototypes -Wmissing-prototypes"
  AC_ARG_ENABLE(build-warnings,
  [  --enable-build-warnings Enable build-time compiler warnings if gcc is used],
  [case "${enableval}" in
--- 51,57 ----
    *)    AC_MSG_ERROR(bad value ${withval} for BFD with-mmap option) ;;
  esac],[want_mmap=false])dnl
  
! build_warnings="-W -Wall -Wstrict-prototypes -Wmissing-prototypes -Werror"
  AC_ARG_ENABLE(build-warnings,
  [  --enable-build-warnings Enable build-time compiler warnings if gcc is used],
  [case "${enableval}" in

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]