Re: [PATCH] Make assert in gimple_phi_arg more strict

[Richard Biener]

On Thu, 20 Jul 2017, Tom de Vries wrote:

> Hi,
> 
> this patch checks that gimple_phi_arg accesses args only in the inclusive
> 0..(nargs-1) region.
> 
> There are a couple of functions that manipulate args in the inclusive
> nargs..(capacity-1) region, so these have been updated to temporarily increase
> nargs to capacity while doing the manipulation.
> 
> Using this patch, I found PR81489 - "invalid phi argument used in
> find_implicit_erroneous_behavior".
> 
> Bootstrapped and reg-tested on x86_64.
> 
> OK for trunk?

I've used the following to only init the PHI node up to nargs, not
capacity (but still consistently zero things).  Currently 
bootstrapping/testing on x86_64-unknown-linux-gnu (plus the adjusted
assert of course).

I think it's slightly cleaner than adjusting nargs if it works out well.

Sorry for duplicating work here.

Thanks,
Richard.

Index: gcc/tree-phinodes.c
===================================================================
--- gcc/tree-phinodes.c (revision 250379)
+++ gcc/tree-phinodes.c (working copy)
@@ -190,7 +190,7 @@ make_phi_node (tree var, int len)
   else
     gimple_phi_set_result (phi, make_ssa_name (var, phi));
 
-  for (i = 0; i < capacity; i++)
+  for (i = 0; i < len; i++)
     {
       use_operand_p  imm;
 
@@ -248,6 +248,10 @@ resize_phi_node (gphi *phi, size_t len)
   new_phi = allocate_phi_node (len);
 
   memcpy (new_phi, phi, old_size);
+  memset ((char *)new_phi + old_size, 0,
+         (sizeof (struct gphi)
+          - sizeof (struct phi_arg_d)
+          + sizeof (struct phi_arg_d) * len) - old_size);
 
   for (i = 0; i < gimple_phi_num_args (new_phi); i++)
     {
@@ -260,18 +264,6 @@ resize_phi_node (gphi *phi, size_t len)
 
   new_phi->capacity = len;
 
-  for (i = gimple_phi_num_args (new_phi); i < len; i++)
-    {
-      use_operand_p imm;
-
-      gimple_phi_arg_set_location (new_phi, i, UNKNOWN_LOCATION);
-      imm = gimple_phi_arg_imm_use_ptr (new_phi, i);
-      imm->use = gimple_phi_arg_def_ptr (new_phi, i);
-      imm->prev = NULL;
-      imm->next = NULL;
-      imm->loc.stmt = new_phi;
-    }
-
   return new_phi;
 }
 
@@ -300,6 +292,8 @@ reserve_phi_args_for_new_edge (basic_blo
          stmt = new_phi;
        }
 
+      stmt->nargs++;
+
       /* We represent a "missing PHI argument" by placing NULL_TREE in
         the corresponding slot.  If PHI arguments were added
         immediately after an edge is created, this zeroing would not
@@ -307,10 +301,13 @@ reserve_phi_args_for_new_edge (basic_blo
         example, the loop optimizer duplicates several basic blocks,
         redirects edges, and then fixes up PHI arguments later in
         batch.  */
+      use_operand_p imm = gimple_phi_arg_imm_use_ptr (stmt, len - 1);
+      imm->use = gimple_phi_arg_def_ptr (stmt, len - 1);
+      imm->prev = NULL;
+      imm->next = NULL;
+      imm->loc.stmt = stmt;
       SET_PHI_ARG_DEF (stmt, len - 1, NULL_TREE);
       gimple_phi_arg_set_location (stmt, len - 1, UNKNOWN_LOCATION);
-
-      stmt->nargs++;
     }
 }