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
