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.
Martin
gcc-78817.diff
PR bootstrap/78817 - stage2 bootstrap failure in vec.h:1613:5: error: argument
1 null where non-null expected after r243661
gcc/ChangeLog:
PR bootstrap/78817
* vec.h (vec<T, A, vl_embed>::quick_grow_cleared): Assert postcondition.
( vec<T, va_heap, vl_ptr>::safe_grow_cleared): Ditto.
I'm pretty sure the vec<T, A, vl_embed> change is not needed. The
address method for that type can't ever return NULL AFAICT. So there
should be no way to see a NULL flowing into the memset call for that case.
In the vec<T, va_heap, vl_ptr> case the address method can clearly
return NULL:
const T *address (void) const
{ return m_vec ? m_vec->m_vecdata : NULL; }
I've only lightly tried to follow things down through the vec
implementation. But it seems that if create a vec with no elements we
can start with m_vec NULL.
We can call safe_grow_cleared on such a vector. If we did and LEN was
zero, then ISTM that m_vec will continue to be NULL if I follow the
callgraph maze correctly. So there is a potential path to the address()
call where m_vec is NULL and thus address() could return NULL.
We happen to know that can't happen at runtime due to the if (sz != 0)
check. That's a good indicator that jump threading has missed an
opportunity. I'd probably need to look at the .i file as well as the
dumps to be sure, but as is usually the case, there appears to be a
missed optimization here leading to the false positive warning.
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.
Thanks,
Jeff
