Hi!
When looking at the PR, I wrote a cleanup patch with various things I've
noticed, with latest Martin's changes half of them aren't valid anymore, but
I found further ones.
So, besides formatting fixes, this patch tries to make sure the rng1 ranges
are meaningful even in some corner cases. The first hunk and half is about
what rng1 will be for single argument attribute where to the corresponding
argument INTEGER_CST is passed, vanilla trunk can stuck into rng1[0] ==
rng1[1] e.g. negative value with whatever precision the argument has
(say for int argument and -23 passed to it will return (size_t) -23,
but range will be [-23, -23]) or e.g. for arguments wider than size_t like
__int128 could return numbers above SIZE_MAX. Whatever the argument type
is, the attribute expects that value to be passed over to malloc/calloc or
similar functions and so it will be promoted or demoted to size_t. The
rng1[0] = wi::zero (rng1[1].get_precision ());
line is to avoid weird ranges and (conservatively) cover the whole range
of sizes the allocator function can return. E.g. if the ranges for the two
arguments are [2, SIZE_MAX] * [2, SIZE_MAX], the upper bound overflows and
gimple_call_alloc_size would return SIZE_MAX with [4, SIZE_MAX] in the rng1,
but that is not accurate, because due to the overflow also [0, 3] would be
possible. Or for [SIZE_MAX - 2, SIZE_MAX] * [SIZE_MAX - 2, SIZE_MAX] where
the overflow is both on the low and upper bounds, the function would return
SIZE_MAX and set rng1 to [((__int128) SIZE_MAX - 2) * (SIZE_MAX - 2), SIZE_MAX]
i.e. range where low bound is much higher than upper bound (== invalid
range). The patch will just use [0, SIZE_MAX] range in those cases.
And the second hunk in tree-ssa-strlen.c is a fix for pointer comparison of
INTEGER_CSTs, while we cache INTEGER_CSTs and INTEGER_CSTs of the same type
with the same value and no overflow will compare equal, in that function, it
is actually quite unlikely they will have the same type, because destsize is
most likely a sizetype integer, while len most likely size_t (aka
size_type_node), but could be anything else that passes
useless_type_conversion_p).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-12-17 Jakub Jelinek <ja...@redhat.com>
PR tree-optimization/92868
* builtins.c (gimple_call_alloc_size): If there is only one size
argument
and INTEGER_CST is passed to it, call get_range only on the converted
value.
For overflow, set rng1[0] to zero. Formatting fix.
(compute_objsize): Formatting fix.
* tree-ssa-strlen.c (maybe_warn_overflow): Remove spurious ; after }.
Use operand_equal_p instead of pointer comparison to compare
INTEGER_CSTs.
Formatting fixes.
--- gcc/builtins.c.jj 2019-12-14 23:19:40.861879033 +0100
+++ gcc/builtins.c 2019-12-16 10:00:27.858664110 +0100
@@ -3749,6 +3749,13 @@ gimple_call_alloc_size (gimple *stmt, wi
}
tree size = gimple_call_arg (stmt, argidx1);
+ if (argidx2 > nargs && TREE_CODE (size) == INTEGER_CST)
+ {
+ size = fold_convert (sizetype, size);
+ if (rng1)
+ get_range (size, rng1, rvals);
+ return size;
+ }
wide_int rng1_buf[2];
/* If RNG1 is not set, use the buffer. */
@@ -3758,12 +3765,10 @@ gimple_call_alloc_size (gimple *stmt, wi
if (!get_range (size, rng1, rvals))
return NULL_TREE;
- if (argidx2 > nargs && TREE_CODE (size) == INTEGER_CST)
- return fold_convert (sizetype, size);
-
/* To handle ranges do the math in wide_int and return the product
of the upper bounds as a constant. Ignore anti-ranges. */
- tree n = argidx2 < nargs ? gimple_call_arg (stmt, argidx2) :
integer_one_node;
+ tree n
+ = argidx2 < nargs ? gimple_call_arg (stmt, argidx2) : integer_one_node;
wide_int rng2[2];
if (!get_range (n, rng2, rvals))
return NULL_TREE;
@@ -3783,6 +3788,7 @@ gimple_call_alloc_size (gimple *stmt, wi
if (wi::gtu_p (rng1[1], wi::to_wide (size_max, prec)))
{
rng1[1] = wi::to_wide (size_max);
+ rng1[0] = wi::zero (rng1[1].get_precision ());
return size_max;
}
@@ -3962,8 +3968,7 @@ compute_objsize (tree dest, int ostype,
if (!ostype)
return NULL_TREE;
- if (TREE_CODE (dest) == ARRAY_REF
- || TREE_CODE (dest) == MEM_REF)
+ if (TREE_CODE (dest) == ARRAY_REF || TREE_CODE (dest) == MEM_REF)
{
tree ref = TREE_OPERAND (dest, 0);
tree off = TREE_OPERAND (dest, 1);
--- gcc/tree-ssa-strlen.c.jj 2019-12-14 23:19:40.860879048 +0100
+++ gcc/tree-ssa-strlen.c 2019-12-16 10:14:51.061654913 +0100
@@ -2039,7 +2039,7 @@ maybe_warn_overflow (gimple *stmt, tree
{
sizrng[0] = wi::zero (siz_prec);
sizrng[1] = wi::to_wide (TYPE_MAX_VALUE (sizetype));
- };
+ }
sizrng[0] = sizrng[0].from (sizrng[0], siz_prec, UNSIGNED);
sizrng[1] = sizrng[1].from (sizrng[1], siz_prec, UNSIGNED);
@@ -2048,7 +2048,11 @@ maybe_warn_overflow (gimple *stmt, tree
and the offset into the destination is zero. This might happen
in the case of a pair of malloc and memset calls to allocate
an object and clear it as if by calloc. */
- if (destsize == len && !plus_one && offrng[0] == 0 && offrng[0] == offrng[1])
+ if (destsize
+ && operand_equal_p (destsize, len, 0)
+ && !plus_one
+ && offrng[0] == 0
+ && offrng[0] == offrng[1])
return;
wide_int lenrng[2];
@@ -2096,13 +2100,11 @@ maybe_warn_overflow (gimple *stmt, tree
spcrng[1] -= wi::ltu_p (offrng[0], spcrng[1]) ? offrng[0] : spcrng[1];
}
- if (wi::leu_p (lenrng[0], spcrng[0])
- && wi::leu_p (lenrng[1], spcrng[1]))
+ if (wi::leu_p (lenrng[0], spcrng[0]) && wi::leu_p (lenrng[1], spcrng[1]))
return;
if (lenrng[0] == spcrng[1]
- && (len != destsize
- || !si || !is_strlen_related_p (si->ptr, len)))
+ && (len != destsize || !si || !is_strlen_related_p (si->ptr, len)))
return;
location_t loc = gimple_nonartificial_location (stmt);
@@ -2113,8 +2115,7 @@ maybe_warn_overflow (gimple *stmt, tree
bool warned = false;
if (wi::leu_p (lenrng[0], spcrng[1]))
{
- if (len != destsize
- && (!si || !is_strlen_related_p (si->ptr, len)))
+ if (len != destsize && (!si || !is_strlen_related_p (si->ptr, len)))
return;
warned = (writefn
Jakub
