On 11/30/2016 09:09 PM, Martin Sebor wrote:
What I think this tells us is that we're not at a place where we're
clean. But we can incrementally get there. The warning is only
catching a fairly small subset of the cases AFAICT. That's not unusual
and analyzing why it didn't trigger on those cases might be useful as
well.
The warning has no smarts. It relies on constant propagation and
won't find a call unless it sees it's being made with a constant
zero. Looking at the top two on the list the calls are in extern
functions not called from the same source file, so it probably just
doesn't see that the functions are being called from another file
with a zero. Building GCC with LTO might perhaps help.
I should also add that for GCC abd provided the main concern is
non-unique pointers, the warning find just the right subset of
calls, (other concerns are portability to non-GCC compilers or
to library implementations).
GCC makes sure the size is a multiple of stack alignment. When
the argument is constant and a multiple of stack size GCC does
nothing, and so when the size is zero it just returns the top
of stack, resulting in non-unique pointers. When it's not
constant, GCC emits code to round up the size to a multiple
of stack alignment, which makes each pointer unique.
So where does this leave us for gcc-7? I'm wondering if we drop the
warning in, but not enable it by default anywhere. We fix the cases we
can (such as reg-stack,c tree-ssa-threadedge.c, maybe others) before
stage3 closes, and shoot for the rest in gcc-8, including improvign the
warning (if there's something we can clearly improve), and enabling the
warning in -Wall or -Wextra.
I'm fine with deferring the GCC fixes and working on the cleanup
over time but I don't think that needs to gate enabling the option
with -Wextra. The warnings can be suppressed or prevented from
causing errors during a GCC build either via a command line option
or by pragma in the code. AFAICT, from the other warnings I see
go by, this is what has been done for -Wno-implicit-fallthrough
while those warnings are being cleaned up. Why not take the same
approach here?
As much as I would like to improve the warning itself I'm also not
sure I see much of an opportunity for it. It's not prone to high
rates of false positives (hardly any in fact) and the cases it
misses are those where it simply doesn't see the argument value
because it's not been made available by constant propagation.
That said, I consider the -Walloc-size-larger-than warning to be
the more important part of the patch by far. I'd hate a lack of
consensus on how to deal with GCC's handful of instances of
alloca(0) to stall the rest of the patch.
Thanks
Martin
