[PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Markus Trippelsdorf]

On 2016.12.15 at 19:27 -0700, Martin Sebor wrote:
> On 12/14/2016 09:19 PM, Jeff Law wrote:
> > On 12/14/2016 03:56 PM, Martin Sebor wrote:
> > > The -Wnonnull warning improvement (PR c/17308 - nonnull attribute
> > > not as useful as it could be) causes a couple of false positives
> > > in a powerpc64le bootstrap.  The attached fix suppresses them.
> > > It passes bootstrap on powerpc64le but tests are still running.
> > > 
> > > I couldn't reproduce the bootstrap comparison failure that Segher
> > > mentioned in his comment on the bug.  I've gone over the patch
> > > again to see if I could spot what could possibly be behind it
> > > but couldn't really see anything.  Jeff, you mentioned attribute
> > > nonnull is exploited by the null pointer optimization.  Do you
> > > think it could possibly have this effect in GCC?
> > It's certainly possible.  It would be an indicator that something in GCC
> > is passing a NULL pointer into a routine where it shouldn't at runtime
> > and that GCC is using its new knowledge to optimize the code assuming
> > that can't happen.
> > 
> > ie, it's an indicator of a latent bug in GCC.  Depending on the
> > difficulty in tracking down, you may need to revert the change.  This is
> > exactly the kind of scenario I was concerned about in that approval
> > message.
> 
> I couldn't reproduce the comparison error either on powerpc64-linux
> or on powerpc64le-linux.
> 
> > The change to the vec<T, va_heap, vl_ptr> is OK.  Can you please verify
> > that if you install just that change that ppc bootstraps are restored
> > and if so, please install.
> 
> Done.
> 
> A profiledbootstrap exposed a few more instances of the enhanced
> -Wnonnull warning.  I spent some time analyzing them and decided
> that three of them are justified (those in gengtype-parse.c and
> gengtype.c) and the other three false positives.
> 
> The attached patch silences them.
> 
> The gengtype code alternates between checking for null pointers
> and assuming that the same pointers are not null (and passing nulls
> to nonnull functions like libiberty's lbasename).  It could probably
> benefit from some more cleanup but I'm out of cycles for that.
> 
> There are two outstanding instances of -Wnonnull in the profiled-
> bootstrap log that both point to the same function that I haven't
> figured out yet:
> 
> In function ‘void check_function_format(tree, int, tree_node**)’:
> cc1plus: warning: argument 1 null where non-null expected [-Wnonnull]
> /usr/include/string.h:398:15: note: in a call to function ‘size_t
> strlen(const char*)’ declared here
> 
> I'll deal with them next but I want to get this patch reviewed
> and approved so bootstrap can be restored on the targets affected
> by the vec.h warning.
> 
> While waiting for builds to finish I also built Binutils, Busybox,
> and the Linux kernel to see if the warning shows up there.  It does
> not.

It does for me with an allmodconf. At -O2 I get three warnings, and at
-O3 I get two additional warnings. Now these additional ones happen way
too deep into the pipeline to be reliable. (For a reduced testcase see:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78817#c8)

When you as the author of the warning have difficulties in figuring out
what causes these warnings, what about the average user?

Therefore -fsanitize=undefined should be preferred. It gives you better
info and a backtrace that makes diagnosis easy.

So in my opinion -Wnonnull should warn only for trivial cases.

-- 
Markus