Patchwork PR tree-optimization/55350: invalid pointer operand to PLUS_EXPR

login
register
mail settings
Submitter Aldy Hernandez
Date Nov. 20, 2012, 5:46 p.m.
Message ID <50ABC1DE.6080001@redhat.com>
Download mbox | patch
Permalink /patch/200492/
State New
Headers show

Comments

Aldy Hernandez - Nov. 20, 2012, 5:46 p.m.
The problem here is that the SLSR pass is promoting a POINTER_PLUS_EXPR 
to a PLUS_EXPR.  Since pointer arithmetic is invalid in 
{PLUS,MINUS}_EXPR's, the gimple verifier chokes on the invalid statement.

Fixed by maintaining the POINTER_PLUS_EXPR when appropriate.

OK for trunk?
commit ae7b615a11be6a0c5cc00f7ea0bf29c142931490
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Tue Nov 20 11:20:58 2012 -0600

    	PR tree-optimization/55350
    	* gimple-ssa-strength-reduction.c (replace_dependent): Handle
    	POINTER_{PLUS,MINUS}_EXPR correctly.
Jakub Jelinek - Nov. 20, 2012, 5:52 p.m.
On Tue, Nov 20, 2012 at 11:46:06AM -0600, Aldy Hernandez wrote:
> The problem here is that the SLSR pass is promoting a
> POINTER_PLUS_EXPR to a PLUS_EXPR.  Since pointer arithmetic is
> invalid in {PLUS,MINUS}_EXPR's, the gimple verifier chokes on the
> invalid statement.
> 
> Fixed by maintaining the POINTER_PLUS_EXPR when appropriate.
> 
> OK for trunk?

Ok, thanks.

> -  if (bump.is_negative ())
> +  if (bump.is_negative ()
> +      && cand_code != POINTER_PLUS_EXPR)
>      {
>        code = MINUS_EXPR;
>        bump = -bump;

I wonder if this isn't unsafe even when bump is the shwi minimum
(say LONG_MIN for LP64 target/64-bit HWI).  But sure, it is preexisting
code, so perhaps something for Bill to look at later.

	Jakub
Eric Botcazou - Nov. 20, 2012, 10:23 p.m.
> The problem here is that the SLSR pass is promoting a POINTER_PLUS_EXPR
> to a PLUS_EXPR.  Since pointer arithmetic is invalid in
> {PLUS,MINUS}_EXPR's, the gimple verifier chokes on the invalid statement.
> 
> Fixed by maintaining the POINTER_PLUS_EXPR when appropriate.
> 
> OK for trunk?

The ChangeLog mentions POINTER_MINUS_EXPR, which doesn't exist.
Aldy Hernandez - Nov. 20, 2012, 10:34 p.m.
On 11/20/12 16:23, Eric Botcazou wrote:
>> The problem here is that the SLSR pass is promoting a POINTER_PLUS_EXPR
>> to a PLUS_EXPR.  Since pointer arithmetic is invalid in
>> {PLUS,MINUS}_EXPR's, the gimple verifier chokes on the invalid statement.
>>
>> Fixed by maintaining the POINTER_PLUS_EXPR when appropriate.
>>
>> OK for trunk?
>
> The ChangeLog mentions POINTER_MINUS_EXPR, which doesn't exist.
>

Actually the ChangeLog is correct, what was incorrect was the svn commit 
message.  Is there a way to change the commit message retroactively?
Eric Botcazou - Nov. 20, 2012, 10:59 p.m.
> Actually the ChangeLog is correct, what was incorrect was the svn commit
> message.  Is there a way to change the commit message retroactively?

Probably, but I wouldn't bother about that.

Patch

diff --git a/gcc/gimple-ssa-strength-reduction.c b/gcc/gimple-ssa-strength-reduction.c
index 8e2a247..65fc6b1 100644
--- a/gcc/gimple-ssa-strength-reduction.c
+++ b/gcc/gimple-ssa-strength-reduction.c
@@ -1643,10 +1643,19 @@  replace_dependent (slsr_cand_t c, enum tree_code cand_code)
 
   basis = lookup_cand (c->basis);
   basis_name = gimple_assign_lhs (basis->cand_stmt);
-  incr_type = TREE_TYPE (gimple_assign_rhs1 (c->cand_stmt));
-  code = PLUS_EXPR;
+  if (cand_code == POINTER_PLUS_EXPR)
+    {
+      incr_type = sizetype;
+      code = cand_code;
+    }
+  else
+    {
+      incr_type = TREE_TYPE (gimple_assign_rhs1 (c->cand_stmt));
+      code = PLUS_EXPR;
+    }
 
-  if (bump.is_negative ())
+  if (bump.is_negative ()
+      && cand_code != POINTER_PLUS_EXPR)
     {
       code = MINUS_EXPR;
       bump = -bump;
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr55350.c b/gcc/testsuite/gcc.c-torture/compile/pr55350.c
new file mode 100644
index 0000000..f10daea
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr55350.c
@@ -0,0 +1,8 @@ 
+void
+foo (__INTPTR_TYPE__ x, __INTPTR_TYPE__ y)
+{
+  int i;
+  void **a = (void *)  (8UL * (x / 8UL));
+  for (i = 0; i < x; i++)
+    a[i] = (void *) y;
+}