On 02/20/2018 12:03 PM, Martin Sebor wrote:
On 02/20/2018 10:17 AM, Jakub Jelinek wrote:
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
It sounds like not setting *minlen is the problem then. Attached
is a slightly smaller patch that fixes the bug with no testsuite
regressions on x86_64-linux. How does it look to you?
A safer and even more conservative alternative that should be
equivalent to your approach while avoiding the sprintf regressions
is to add another mode to the function and have it clear *minlen
as an option. This lets the strlen code obtain the conservative
lower bound without compromising the sprintf warnings.
Martin
PR tree-optimization/84478 - [8 Regression] pdftex miscompilation on i386
gcc/ChangeLog:
PR tree-optimization/84478
* gimple-fold.h (get_range_strlen): Add argument.
* gimple-fold.c (get_range_strlen): Set *MINLEN to zero.
(get_range_strlen): Reset range on failure.
* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Add third argument
to the call to get_range_strlen.
gcc/testsuite/ChangeLog:
PR tree-optimization/84478
* gcc.c-torture/execute/pr84478.c: New test.
Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c (revision 257850)
+++ gcc/gimple-fold.c (working copy)
@@ -1290,6 +1290,9 @@ gimple_fold_builtin_memset (gimple_stmt_iterator *
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.
+ When STRICT_MIN is set the function will clear MINMAXLEN[0] if
+ the length of any of the referenced strings cannot be determined
+ (and thus can be zero).
Set *FLEXP to 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
@@ -1297,7 +1300,7 @@ gimple_fold_builtin_memset (gimple_stmt_iterator *
static bool
get_range_strlen (tree arg, tree length[2], bitmap *visited, int type,
- bool fuzzy, bool *flexp)
+ bool fuzzy, bool strict_min, bool *flexp)
{
tree var, val = NULL_TREE;
gimple *def_stmt;
@@ -1320,7 +1323,8 @@ get_range_strlen (tree arg, tree length[2], bitmap
if (TREE_CODE (aop0) == INDIRECT_REF
&& TREE_CODE (TREE_OPERAND (aop0, 0)) == SSA_NAME)
return get_range_strlen (TREE_OPERAND (aop0, 0),
- length, visited, type, fuzzy, flexp);
+ length, visited, type, fuzzy,
+ strict_min, flexp);
}
else if (TREE_CODE (TREE_OPERAND (op, 0)) == COMPONENT_REF && fuzzy)
{
@@ -1354,7 +1358,7 @@ get_range_strlen (tree arg, tree length[2], bitmap
{
if (TREE_CODE (arg) == ADDR_EXPR)
return get_range_strlen (TREE_OPERAND (arg, 0), length,
- visited, type, fuzzy, flexp);
+ visited, type, fuzzy, strict_min, flexp);
if (TREE_CODE (arg) == ARRAY_REF)
{
@@ -1497,14 +1501,17 @@ get_range_strlen (tree arg, tree length[2], bitmap
|| gimple_assign_unary_nop_p (def_stmt))
{
tree rhs = gimple_assign_rhs1 (def_stmt);
- return get_range_strlen (rhs, length, visited, type, fuzzy, flexp);
+ return get_range_strlen (rhs, length, visited, type, fuzzy,
+ strict_min, flexp);
}
else if (gimple_assign_rhs_code (def_stmt) == COND_EXPR)
{
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,
+ strict_min, flexp)
+ && get_range_strlen (op3, length, visited, type, fuzzy,
+ strict_min, flexp));
}
return false;
@@ -1527,10 +1534,15 @@ get_range_strlen (tree arg, tree length[2], bitmap
if (arg == gimple_phi_result (def_stmt))
continue;
- if (!get_range_strlen (arg, length, visited, type, fuzzy, flexp))
+ if (!get_range_strlen (arg, length, visited, type, fuzzy,
+ strict_min, flexp))
{
if (fuzzy)
- *maxlen = build_all_ones_cst (size_type_node);
+ {
+ if (strict_min)
+ *minlen = ssize_int (0);
+ *maxlen = build_all_ones_cst (size_type_node);
+ }
else
return false;
}
@@ -1551,6 +1563,9 @@ get_range_strlen (tree arg, tree length[2], bitmap
and array declared as 'char array[8]', MINMAXLEN[0] will be set
to 3 and MINMAXLEN[1] to 7, the longest string that could be
stored in array.
+ When STRICT_MIN is set the function will clear MINMAXLEN[0] if
+ the length of any of the referenced strings cannot be determined
+ (and thus can be zero).
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
@@ -1557,7 +1572,7 @@ get_range_strlen (tree arg, tree length[2], bitmap
due to it being used as a poor-man's flexible array member. */
bool
-get_range_strlen (tree arg, tree minmaxlen[2])
+get_range_strlen (tree arg, tree minmaxlen[2], bool strict_min /* = true */)
{
bitmap visited = NULL;
@@ -1565,7 +1580,12 @@ bool
minmaxlen[1] = NULL_TREE;
bool flexarray = false;
- get_range_strlen (arg, minmaxlen, &visited, 1, true, &flexarray);
+ if (!get_range_strlen (arg, minmaxlen, &visited, 1, true, strict_min,
+ &flexarray))
+ {
+ minmaxlen[0] = NULL_TREE;
+ minmaxlen[1] = NULL_TREE;
+ }
if (visited)
BITMAP_FREE (visited);
@@ -1580,7 +1600,7 @@ get_maxval_strlen (tree arg, int type)
tree len[2] = { NULL_TREE, NULL_TREE };
bool dummy;
- if (!get_range_strlen (arg, len, &visited, type, false, &dummy))
+ if (!get_range_strlen (arg, len, &visited, type, false, true, &dummy))
len[1] = NULL_TREE;
if (visited)
BITMAP_FREE (visited);
Index: gcc/gimple-fold.h
===================================================================
--- gcc/gimple-fold.h (revision 257850)
+++ gcc/gimple-fold.h (working copy)
@@ -25,7 +25,7 @@ along with GCC; see the file COPYING3. If not see
extern tree create_tmp_reg_or_ssa_name (tree, gimple *stmt = NULL);
extern tree canonicalize_constructor_val (tree, tree);
extern tree get_symbol_constant_value (tree);
-extern bool get_range_strlen (tree, tree[2]);
+extern bool get_range_strlen (tree, tree[2], bool = true);
extern tree get_maxval_strlen (tree, int);
extern void gimplify_and_update_call_from_tree (gimple_stmt_iterator *, tree);
extern bool fold_stmt (gimple_stmt_iterator *);
Index: gcc/testsuite/gcc.c-torture/execute/pr84478.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/pr84478.c (nonexistent)
+++ gcc/testsuite/gcc.c-torture/execute/pr84478.c (working copy)
@@ -0,0 +1,48 @@
+/* PR tree-optimization/84478 - pdftex miscompilation on i386 */
+
+long poolptr;
+unsigned char *strpool;
+static const char *poolfilearr[] = {
+ "mu",
+#define A "",
+#define B A A A A A 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 A
+ ((void *)0)
+};
+
+__attribute__((noipa)) long
+makestring (void)
+{
+ return 0;
+}
+
+__attribute__((noipa))
+int
+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 ()
+{
+ poolptr = 0;
+ strpool = __builtin_malloc (1000);
+ asm volatile ("" : : : "memory");
+ volatile int r = loadpoolstrings (1000);
+ __builtin_free (strpool);
+ return 0;
+}
