On 01/11/2017 02:05 AM, Christophe Lyon wrote:
Hi Martin,
On 9 January 2017 at 04:14, Jeff Law <l...@redhat.com> wrote:
On 01/08/2017 02:04 PM, Martin Sebor wrote:
On 01/06/2017 09:45 AM, Jeff Law wrote:
On 01/05/2017 08:52 PM, Martin Sebor wrote:
So Richi asked for removal of the VR_ANTI_RANGE handling, which would
imply removal of operand_signed_p.
What are the implications if we do that?
I just got back to this yesterday. The implications of the removal
of the anti-range handling are a number of false negatives in the
test suite:
I was thinking more at a higher level. ie, are the warnings still
useful if we don't have the anti-range handling? I suspect so, but
would like to hear your opinion.
...
n = ~[-4, MAX]; (I.e., n is either negative or too big.)
p = malloc (n);
Understood. The low level question is do we get these kinds of ranges
often enough in computations leading to allocation sizes?
My intuition tells me that they are likely common enough not to
disregard but I don't have a lot of data to back it up with. In
a Bash build a full 23% of all checked calls are of this kind (24
out of 106). In a Binutils build only 4% are (9 out of 228). In
Glibc, a little under 3%. My guess is that the number will be
inversely proportional to the quality of the code.
So I think you've made a case that we do want to handle this case. So
what's left is how best to avoid the infinite recursion and mitigate the
pathological cases.
What you're computing seems to be "this object may have been derived
from a signed type". Right? It's a property we can compute for any
given SSA_NAME and it's not context/path specific beyond the
context/path information encoded by the SSA graph.
So just thinking out load here, could we walk the IL once to identify
call sites and build a worklist of SSA_NAMEs we care about. Then we
iterate on the worklist much like Aldy's code he's working on for the
unswitching vs uninitialized variable issue?
Thanks for the suggestion. It occurred to me while working on the fix
for 78973 (the non-bug) that size ranges should be handled the same by
-Wstringop-overflow as by -Walloc-size-larger-than, and that both have
the same problem: missing or incomplete support for anti-ranges. The
attached patch moves get_size_range() from builtins.c to calls.c and
adds better support for anti-ranges. That solves the problems also
lets it get rid of the objectionable operand_positive_p function.
Martin
PS The change to the alloc_max_size function is only needed to make
it possible to specify any argument to the -Walloc-size-larger-than
option, including 0 and -1, so that allocations of any size, including
zero can be flagged.
gcc-78775.diff
PR tree-optimization/78775 - [7 Regression] ICE in
maybe_warn_alloc_args_overflow
gcc/ChangeLog:
PR tree-optimization/78775
* builtins.c (get_size_range): Move...
* calls.c: ...to here.
(alloc_max_size): Accept zero argument.
(operand_signed_p): Remove.
(maybe_warn_alloc_args_overflow): Call get_size_range.
* calls.h (get_size_range): Declare.
gcc/testsuite/ChangeLog:
PR tree-optimization/78775
* gcc.dg/attr-alloc_size-4.c: Add test cases.
* gcc.dg/pr78775.c: New test.
* gcc.dg/pr78973-2.c: New test.
* gcc.dg/pr78973.c: New test.
The new test (gcc.dg/pr78973.c) fails on arm targets (there's no warning).
In addition, I have noticed a new failure:
gcc.dg/attr-alloc_size-4.c (test for warnings, line 140)
on target arm-none-linux-gnueabihf --with-cpu=cortex-a5
(works fine --with-cpu=cortex-a9)
Thanks. I'm tracking the test failure on powerpc64* in bug 79051.
It's caused by a VRP bug/limitation described in bug 79054. Let
me add arm and m68k to the list of targets.
Martin
