Hi!
The following testcase distilled from pdftex is miscompiled on i?86,
because gimple_fold_builtin_strlen sets incorrect value range on
strlen call on SSA_NAME with def_stmt of PHI <"mu", something> where
we can't determine anything about string length of something, so the
right value range is [0, maximum_possible_strlen], but we actually used
[2, INF].
get_range_strlen has many modes, for get_maxval_strlen we check return
value of get_range_strlen, don't set fuzzy and everything seems correct.
Otherwise we have the fuzzy mode which is used in 2 arg get_range_strlen
overload, which is used for warnings and recently also for
gimple_fold_builtin_strlen which sets value ranges from it. Unlike
warnings, which can perhaps afford some heuristics and guessing, for
gimple_fold_builtin_strlen we need to be 100% conservative.
The thing that isn't handled conservatively is PHIs and COND_EXPR.
The current code, if we can't figure one of the args out, for PHIs in
fuzzy mode increases the *maxval value to +INF, but doesn't touch
*minval, for COND_EXPR doesn't adjust the *minval/*maxval at all and just
returns false. Unfortunately, changing that breaks
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for warnings, line 165)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for warnings, line 166)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for warnings, line 167)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for warnings, line 168)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for warnings, line 309)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for warnings, line 310)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for warnings, line 311)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for warnings, line 312)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for excess errors)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for warnings, line 62)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for warnings, line 63)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for warnings, line 402)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for warnings, line 403)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for warnings, line 430)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for warnings, line 431)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for warnings, line 458)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for warnings, line 459)
so this patch instead stops using the 2 argument get_range_strlen
in gimple_fold_builtin_strlen and makes sure we handle that case
conservatively, i.e. PHI of something known + unknown is unknown, likewise
COND_EXPR.
Bootstrapped on x86_64-linux and i686-linux, regtest pending, ok for trunk
if it passes regtest on both?
For stage1 I guess Martin can tweak the warning code paths and if they will
become the same as the conservatively correct ones, what is in this patch
can be adjusted again.
2018-02-20 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/84478
* gimple-fold.c (get_range_strlen): Make minlen const and assume it
can't be NULL. Add type 3 support which is conservatively correct
in PHIs. Formatting and comment capitalization fixes. Add warning
that the 2 argument get_range_strlen is only usable for warnings.
(gimple_fold_builtin_strlen): Use the 6 arg get_range_strlen overload
rather than 2 arg, use it only if it returns true and flexarray is
false, pass 3 as type to it.
* gcc.c-torture/execute/pr84478.c: New test.
--- gcc/gimple-fold.c.jj 2018-02-19 19:57:03.424279589 +0100
+++ gcc/gimple-fold.c 2018-02-20 16:21:34.583020305 +0100
@@ -1283,10 +1283,11 @@ gimple_fold_builtin_memset (gimple_stmt_
value of ARG in LENGTH[0] and LENGTH[1], respectively.
If ARG is an SSA name variable, follow its use-def chains. When
TYPE == 0, if LENGTH[1] is not equal to the length we determine or
- if we are unable to determine the length or value, return False.
+ if we are unable to determine the length or value, return false.
VISITED is a bitmap of visited variables.
TYPE is 0 if string length should be obtained, 1 for maximum string
- length and 2 for maximum value ARG can have.
+ length and 2 for maximum value ARG can have, 3 is like 1, but provide
+ conservatively correct rather than optimistic answer.
When FUZZY is set and the length of a string cannot be determined,
the function instead considers as the maximum possible length the
size of a character array it may refer to.
@@ -1302,9 +1303,8 @@ get_range_strlen (tree arg, tree length[
tree var, val = NULL_TREE;
gimple *def_stmt;
- /* The minimum and maximum length. The MAXLEN pointer stays unchanged
- but MINLEN may be cleared during the execution of the function. */
- tree *minlen = length;
+ /* The minimum and maximum length. */
+ tree *const minlen = length;
tree *const maxlen = length + 1;
if (TREE_CODE (arg) != SSA_NAME)
@@ -1445,12 +1445,11 @@ get_range_strlen (tree arg, tree length[
if (!val)
return false;
- if (minlen
- && (!*minlen
- || (type > 0
- && TREE_CODE (*minlen) == INTEGER_CST
- && TREE_CODE (val) == INTEGER_CST
- && tree_int_cst_lt (val, *minlen))))
+ if (!*minlen
+ || (type > 0
+ && TREE_CODE (*minlen) == INTEGER_CST
+ && TREE_CODE (val) == INTEGER_CST
+ && tree_int_cst_lt (val, *minlen)))
*minlen = val;
if (*maxlen)
@@ -1503,18 +1502,16 @@ get_range_strlen (tree arg, tree length[
{
tree op2 = gimple_assign_rhs2 (def_stmt);
tree op3 = gimple_assign_rhs3 (def_stmt);
- return get_range_strlen (op2, length, visited, type, fuzzy, flexp)
- && get_range_strlen (op3, length, visited, type, fuzzy, flexp);
+ return (get_range_strlen (op2, length, visited, type, fuzzy, flexp)
+ && get_range_strlen (op3, length, visited, type, fuzzy,
+ flexp));
}
return false;
case GIMPLE_PHI:
- {
- /* All the arguments of the PHI node must have the same constant
- length. */
- unsigned i;
-
- for (i = 0; i < gimple_phi_num_args (def_stmt); i++)
+ /* All the arguments of the PHI node must have the same constant
+ length. */
+ for (unsigned i = 0; i < gimple_phi_num_args (def_stmt); i++)
{
tree arg = gimple_phi_arg (def_stmt, i)->def;
@@ -1529,13 +1526,12 @@ get_range_strlen (tree arg, tree length[
if (!get_range_strlen (arg, length, visited, type, fuzzy, flexp))
{
- if (fuzzy)
+ if (fuzzy && type != 3)
*maxlen = build_all_ones_cst (size_type_node);
else
return false;
}
}
- }
return true;
default:
@@ -1554,7 +1550,10 @@ get_range_strlen (tree arg, tree length[
Return true if the range of the string lengths has been obtained
from the upper bound of an array at the end of a struct. Such
an array may hold a string that's longer than its upper bound
- due to it being used as a poor-man's flexible array member. */
+ due to it being used as a poor-man's flexible array member.
+
+ This function should be only used for warning code, as it doesn't
+ handle PHIs in a conservatively correct way. */
bool
get_range_strlen (tree arg, tree minmaxlen[2])
@@ -3533,8 +3532,12 @@ gimple_fold_builtin_strlen (gimple_stmt_
wide_int minlen;
wide_int maxlen;
- tree lenrange[2];
- if (!get_range_strlen (gimple_call_arg (stmt, 0), lenrange)
+ tree lenrange[2] = { NULL_TREE, NULL_TREE };
+ bitmap visited = NULL;
+ bool flexarray = false;
+ if (get_range_strlen (gimple_call_arg (stmt, 0), lenrange, &visited,
+ 3, true, &flexarray)
+ && !flexarray
&& lenrange[0] && TREE_CODE (lenrange[0]) == INTEGER_CST
&& lenrange[1] && TREE_CODE (lenrange[1]) == INTEGER_CST)
{
@@ -3554,6 +3557,9 @@ gimple_fold_builtin_strlen (gimple_stmt_
maxlen = wi::to_wide (max_object_size (), prec) - 2;
}
+ if (visited)
+ BITMAP_FREE (visited);
+
if (minlen == maxlen)
{
lenrange[0] = force_gimple_operand_gsi (gsi, lenrange[0], true, NULL,
--- gcc/testsuite/gcc.c-torture/execute/pr84478.c.jj 2018-02-20 16:32:00.683086212 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr84478.c 2018-02-20 16:31:33.497081640 +0100
@@ -0,0 +1,49 @@
+/* PR tree-optimization/84478 */
+
+long poolptr;
+unsigned char *strpool;
+static const char *poolfilearr[] = {
+ "mu",
+ "",
+#define A "x",
+#define B A "xx", A A "xxx", A A A A A
+#define C B B B B B B B B B B
+#define D C C C C C C C C C C
+ D C C C C C C C B B B
+ ((void *)0)
+};
+
+__attribute__((noipa)) long
+makestring (void)
+{
+ return 1;
+}
+
+__attribute__((noipa)) long
+loadpoolstrings (long spare_size)
+{
+ const char *s;
+ long g = 0;
+ int i = 0, j = 0;
+ while ((s = poolfilearr[j++]))
+ {
+ int l = __builtin_strlen (s);
+ i += l;
+ if (i >= spare_size) return 0;
+ while (l-- > 0) strpool[poolptr++] = *s++;
+ g = makestring ();
+ }
+ return g;
+}
+
+int
+main ()
+{
+ strpool = __builtin_malloc (4000);
+ if (!strpool)
+ return 0;
+ asm volatile ("" : : : "memory");
+ volatile int r = loadpoolstrings (4000);
+ __builtin_free (strpool);
+ return 0;
+}
Jakub