procyberian/gcc
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

tree-ssa-strlen: optimization skips clobbering store [PR111519]

Browse source 
The following testcase is miscompiled, because count_nonzero_bytes incorrectly
uses get_strinfo information on a pointer from which an earlier instruction
loads SSA_NAME stored at the current instruction.  get_strinfo shows a state
right before the current store though, so if there are some stores in between
the current store and the load, the string length information might have
changed.

The patch passes around gimple_vuse from the store and punts instead of using
strinfo on loads from MEM_REF which have different gimple_vuse from that.

2023-10-11  Richard Biener  <rguenther@suse.de>
	    Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/111519
	* tree-ssa-strlen.cc (strlen_pass::count_nonzero_bytes): Add vuse
	argument and pass it through to recursive calls and
	count_nonzero_bytes_addr calls.  Don't shadow the stmt argument, but
	change stmt for gimple_assign_single_p statements for which we don't
	immediately punt.
	(strlen_pass::count_nonzero_bytes_addr): Add vuse argument and pass
	it through to recursive calls and count_nonzero_bytes calls.  Don't
	use get_strinfo if gimple_vuse (stmt) is different from vuse.  Don't
	shadow the stmt argument.

	* gcc.dg/torture/pr111519.c: New testcase.
This commit is contained in:
Jakub Jelinek 2023-10-11 08:58:29 +02:00
parent c414924231
commit e75bf1985f
2 changed files with 79 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

48
gcc/testsuite/gcc.dg/torture/pr111519.c Normal file
View file

@ -0,0 +1,48 @@
/* PR tree-optimization/111519 */
/* { dg-do run } */
int a, o;
char b, f, i;
long c;
static signed char d;
static char g;
unsigned *h;
signed char *e = &f;
static signed char **j = &e;
static long k[2];
unsigned **l = &h;
short m;
volatile int z;
__attribute__((noipa)) void
foo (char *p)
{
(void) p;
}
int
main ()
{
int p = z;
signed char *n = &d;
*n = 0;
while (c)
for (; i; i--)
;
for (g = 0; g <= 1; g++)
{
*n = **j;
k[g] = 0 != &m;
*e = l && k[0];
}
if (p)
foo (&b);
for (; o < 4; o++)
{
a = d;
if (p)
foo (&b);
}
if (a != 1)
__builtin_abort ();
}

53
gcc/tree-ssa-strlen.cc
View file

@ -281,14 +281,14 @@ public:
gimple *stmt,
unsigned lenrange[3], bool *nulterm,
bool *allnul, bool *allnonnul);
bool count_nonzero_bytes (tree exp,
bool count_nonzero_bytes (tree exp, tree vuse,
gimple *stmt,
unsigned HOST_WIDE_INT offset,
unsigned HOST_WIDE_INT nbytes,
unsigned lenrange[3], bool *nulterm,
bool *allnul, bool *allnonnul,
ssa_name_limit_t &snlim);
bool count_nonzero_bytes_addr (tree exp,
bool count_nonzero_bytes_addr (tree exp, tree vuse,
gimple *stmt,
unsigned HOST_WIDE_INT offset,
unsigned HOST_WIDE_INT nbytes,
@ -4531,8 +4531,8 @@ nonzero_bytes_for_type (tree type, unsigned lenrange[3],
}
/* Recursively determine the minimum and maximum number of leading nonzero
bytes in the representation of EXP and set LENRANGE[0] and LENRANGE[1]
to each.
bytes in the representation of EXP at memory state VUSE and set
LENRANGE[0] and LENRANGE[1] to each.
Sets LENRANGE[2] to the total size of the access (which may be less
than LENRANGE[1] when what's being referenced by EXP is a pointer
rather than an array).
@ -4546,7 +4546,7 @@ nonzero_bytes_for_type (tree type, unsigned lenrange[3],
Returns true on success and false otherwise. */
bool
strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt,
strlen_pass::count_nonzero_bytes (tree exp, tree vuse, gimple *stmt,
unsigned HOST_WIDE_INT offset,
unsigned HOST_WIDE_INT nbytes,
unsigned lenrange[3], bool *nulterm,
@ -4566,22 +4566,23 @@ strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt,
exact value is not known) recurse once to set the range
for an arbitrary constant. */
exp = build_int_cst (type, 1);
return count_nonzero_bytes (exp, stmt,
return count_nonzero_bytes (exp, vuse, stmt,
offset, 1, lenrange,
nulterm, allnul, allnonnul, snlim);
}
gimple *stmt = SSA_NAME_DEF_STMT (exp);
if (gimple_assign_single_p (stmt))
gimple *g = SSA_NAME_DEF_STMT (exp);
if (gimple_assign_single_p (g))
{
exp = gimple_assign_rhs1 (stmt);
exp = gimple_assign_rhs1 (g);
if (!DECL_P (exp)
&& TREE_CODE (exp) != CONSTRUCTOR
&& TREE_CODE (exp) != MEM_REF)
return false;
/* Handle DECLs, CONSTRUCTOR and MEM_REF below. */
stmt = g;
}
else if (gimple_code (stmt) == GIMPLE_PHI)
else if (gimple_code (g) == GIMPLE_PHI)
{
/* Avoid processing an SSA_NAME that has already been visited
or if an SSA_NAME limit has been reached. Indicate success
@ -4590,11 +4591,11 @@ strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt,
return res > 0;
/* Determine the minimum and maximum from the PHI arguments. */
unsigned int n = gimple_phi_num_args (stmt);
unsigned int n = gimple_phi_num_args (g);
for (unsigned i = 0; i != n; i++)
{
tree def = gimple_phi_arg_def (stmt, i);
if (!count_nonzero_bytes (def, stmt,
tree def = gimple_phi_arg_def (g, i);
if (!count_nonzero_bytes (def, vuse, g,
offset, nbytes, lenrange, nulterm,
allnul, allnonnul, snlim))
return false;
@ -4652,7 +4653,7 @@ strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt,
return false;
/* Handle MEM_REF = SSA_NAME types of assignments. */
return count_nonzero_bytes_addr (arg, stmt,
return count_nonzero_bytes_addr (arg, vuse, stmt,
offset, nbytes, lenrange, nulterm,
allnul, allnonnul, snlim);
}
@ -4765,7 +4766,7 @@ strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt,
bytes that are pointed to by EXP, which should be a pointer. */
bool
strlen_pass::count_nonzero_bytes_addr (tree exp, gimple *stmt,
strlen_pass::count_nonzero_bytes_addr (tree exp, tree vuse, gimple *stmt,
unsigned HOST_WIDE_INT offset,
unsigned HOST_WIDE_INT nbytes,
unsigned lenrange[3], bool *nulterm,
@ -4775,6 +4776,14 @@ strlen_pass::count_nonzero_bytes_addr (tree exp, gimple *stmt,
int idx = get_stridx (exp, stmt);
if (idx > 0)
{
/* get_strinfo reflects string lengths before the current statement,
where the current statement is the outermost count_nonzero_bytes
stmt. If there are any stores in between stmt and that
current statement, the string length information might describe
something significantly different. */
if (gimple_vuse (stmt) != vuse)
return false;
strinfo *si = get_strinfo (idx);
if (!si)
return false;
@ -4835,14 +4844,14 @@ strlen_pass::count_nonzero_bytes_addr (tree exp, gimple *stmt,
}
if (TREE_CODE (exp) == ADDR_EXPR)
return count_nonzero_bytes (TREE_OPERAND (exp, 0), stmt,
return count_nonzero_bytes (TREE_OPERAND (exp, 0), vuse, stmt,
offset, nbytes,
lenrange, nulterm, allnul, allnonnul, snlim);
if (TREE_CODE (exp) == SSA_NAME)
{
gimple *stmt = SSA_NAME_DEF_STMT (exp);
if (gimple_code (stmt) == GIMPLE_PHI)
gimple *g = SSA_NAME_DEF_STMT (exp);
if (gimple_code (g) == GIMPLE_PHI)
{
/* Avoid processing an SSA_NAME that has already been visited
or if an SSA_NAME limit has been reached. Indicate success
@ -4851,11 +4860,11 @@ strlen_pass::count_nonzero_bytes_addr (tree exp, gimple *stmt,
return res > 0;
/* Determine the minimum and maximum from the PHI arguments. */
unsigned int n = gimple_phi_num_args (stmt);
unsigned int n = gimple_phi_num_args (g);
for (unsigned i = 0; i != n; i++)
{
tree def = gimple_phi_arg_def (stmt, i);
if (!count_nonzero_bytes_addr (def, stmt,
tree def = gimple_phi_arg_def (g, i);
if (!count_nonzero_bytes_addr (def, vuse, g,
offset, nbytes, lenrange,
nulterm, allnul, allnonnul,
snlim))
@ -4903,7 +4912,7 @@ strlen_pass::count_nonzero_bytes (tree expr_or_type, gimple *stmt,
ssa_name_limit_t snlim;
tree expr = expr_or_type;
return count_nonzero_bytes (expr, stmt,
return count_nonzero_bytes (expr, gimple_vuse (stmt), stmt,
0, 0, lenrange, nulterm, allnul, allnonnul,
snlim);
}