diff mbox

[PR,middle-end/59119] Avoid referencing released SSA_NAMEs

Message ID 52840706.9040403@redhat.com
State New
Headers show

Commit Message

Jeff Law Nov. 13, 2013, 11:11 p.m. UTC
We have a known API wart with the SSA_NAME manager.  Specifically, once 
you start releasing names, you can't have dangling references (say in 
unreachable blocks) and allococate new names.

The erroneous path optimization could run afoul of that requirement as 
it removed statements after an explicit erroneous statement, 
particularly in the code to adjust debug statements.  That resulted in a 
segfault after we tried to us a NULL TREE_TYPE of a released SSA_NAME.

I think I know how we can fix the name manager, but that's future work.

For the erroneous path optimizer, two changes avoid these problems. 
First on a local basis, when we remove statements we do so from the end 
of the block up to, but not including the builtin_trap.

Second, on a global basis, we first find all the implicit erroneous 
paths that have to be isolated.  That process creates SSA_NAMEs, but 
does not remove edges in the CFG (it redirects edges).  So it should be 
safe.

After we find all the implicit erroneous paths in the current function, 
we then look for any explicit erroneous statements in each block and 
optimize those (which can create unreachable code and dangling references).

Bootstrapped and regression tested on x86_64-unknown-linux-gnu. 
Installed on the trunk.
* PR middle-end/59119
	* gimple-ssa-isolate-paths.c (find_implicit_erroneous_behaviour): New
	function, extracted from gimple_ssa_isolate_erroneous_paths.
	(find_explicit_erroneous_behaviour): Similarly.
	(insert_trap_and_remove_trailing_statements): Remove statements
	in reverse order.

	* PR middle-end/59119
	* gcc.c-torture/compile/pr59119.c: New test.
diff mbox

Patch

diff --git a/gcc/gimple-ssa-isolate-paths.c b/gcc/gimple-ssa-isolate-paths.c
index 2d8f176..0051da2 100644
--- a/gcc/gimple-ssa-isolate-paths.c
+++ b/gcc/gimple-ssa-isolate-paths.c
@@ -100,14 +100,16 @@  insert_trap_and_remove_trailing_statements (gimple_stmt_iterator *si_p, tree op)
   else
     gsi_insert_before (si_p, seq, GSI_NEW_STMT);
 
-  /* The iterator points to the __builtin_trap.  Advance the iterator
-     and delete everything else in the block.  */
-  gsi_next (si_p);
-  for (; !gsi_end_p (*si_p);)
+  /* We must remove statements from the end of the block so that we
+     never reference a released SSA_NAME.  */
+  basic_block bb = gimple_bb (gsi_stmt (*si_p));
+  for (gimple_stmt_iterator si = gsi_last_bb (bb);
+       gsi_stmt (si) != gsi_stmt (*si_p);
+       si = gsi_last_bb (bb))
     {
-      stmt = gsi_stmt (*si_p);
+      stmt = gsi_stmt (si);
       unlink_stmt_vdef (stmt);
-      gsi_remove (si_p, true);
+      gsi_remove (&si, true);
       release_defs (stmt);
     }
 }
@@ -192,40 +194,19 @@  isolate_path (basic_block bb, basic_block duplicate,
   return duplicate;
 }
 
-/* Search the function for statements which, if executed, would cause
-   the program to fault such as a dereference of a NULL pointer.
-
-   Such a program can't be valid if such a statement was to execute
-   according to ISO standards.
-
-   We detect explicit NULL pointer dereferences as well as those implied
-   by a PHI argument having a NULL value which unconditionally flows into
-   a dereference in the same block as the PHI.
-
-   In the former case we replace the offending statement with an
-   unconditional trap and eliminate the outgoing edges from the statement's
-   basic block.  This may expose secondary optimization opportunities.
-
-   In the latter case, we isolate the path(s) with the NULL PHI 
-   feeding the dereference.  We can then replace the offending statement
-   and eliminate the outgoing edges in the duplicate.  Again, this may
-   expose secondary optimization opportunities.
+/* Look for PHI nodes which feed statements in the same block where
+   the value of the PHI node implies the statement is erroneous.
 
-   A warning for both cases may be advisable as well.
+   For example, a NULL PHI arg value which then feeds a pointer
+   dereference.
 
-   Other statically detectable violations of the ISO standard could be
-   handled in a similar way, such as out-of-bounds array indexing.  */
-
-static unsigned int
-gimple_ssa_isolate_erroneous_paths (void)
+   When found isolate and optimize the path associated with the PHI
+   argument feeding the erroneous statement.  */
+static void
+find_implicit_erroneous_behaviour (void)
 {
   basic_block bb;
 
-  initialize_original_copy_tables ();
-
-  /* Search all the blocks for edges which, if traversed, will
-     result in undefined behaviour.  */
-  cfg_altered = false;
   FOR_EACH_BB (bb)
     {
       gimple_stmt_iterator si;
@@ -288,6 +269,21 @@  gimple_ssa_isolate_erroneous_paths (void)
 		}
 	    }
 	}
+    }
+}
+
+/* Look for statements which exhibit erroneous behaviour.  For example
+   a NULL pointer dereference. 
+
+   When found, optimize the block containing the erroneous behaviour.  */
+static void
+find_explicit_erroneous_behaviour (void)
+{
+  basic_block bb;
+
+  FOR_EACH_BB (bb)
+    {
+      gimple_stmt_iterator si;
 
       /* Now look at the statements in the block and see if any of
 	 them explicitly dereference a NULL pointer.  This happens
@@ -318,6 +314,56 @@  gimple_ssa_isolate_erroneous_paths (void)
 	    }
 	}
     }
+}
+/* Search the function for statements which, if executed, would cause
+   the program to fault such as a dereference of a NULL pointer.
+
+   Such a program can't be valid if such a statement was to execute
+   according to ISO standards.
+
+   We detect explicit NULL pointer dereferences as well as those implied
+   by a PHI argument having a NULL value which unconditionally flows into
+   a dereference in the same block as the PHI.
+
+   In the former case we replace the offending statement with an
+   unconditional trap and eliminate the outgoing edges from the statement's
+   basic block.  This may expose secondary optimization opportunities.
+
+   In the latter case, we isolate the path(s) with the NULL PHI 
+   feeding the dereference.  We can then replace the offending statement
+   and eliminate the outgoing edges in the duplicate.  Again, this may
+   expose secondary optimization opportunities.
+
+   A warning for both cases may be advisable as well.
+
+   Other statically detectable violations of the ISO standard could be
+   handled in a similar way, such as out-of-bounds array indexing.  */
+
+static unsigned int
+gimple_ssa_isolate_erroneous_paths (void)
+{
+  initialize_original_copy_tables ();
+
+  /* Search all the blocks for edges which, if traversed, will
+     result in undefined behaviour.  */
+  cfg_altered = false;
+
+  /* First handle cases where traversal of a particular edge
+     triggers undefined behaviour.  These cases require creating
+     duplicate blocks and thus new SSA_NAMEs.
+
+     We want that process complete prior to the phase where we start
+     removing edges from the CFG.  Edge removal may ultimately result in
+     removal of PHI nodes and thus releasing SSA_NAMEs back to the
+     name manager.
+
+     If the two processes run in parallel we could release an SSA_NAME
+     back to the manager but we could still have dangling references
+     to the released SSA_NAME in unreachable blocks.
+     that any released names not have dangling references in the IL.  */
+  find_implicit_erroneous_behaviour ();
+  find_explicit_erroneous_behaviour ();
+
   free_original_copy_tables ();
 
   /* We scramble the CFG and loop structures a bit, clean up 
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr59119.c b/gcc/testsuite/gcc.c-torture/compile/pr59119.c
new file mode 100644
index 0000000..b026ba5
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr59119.c
@@ -0,0 +1,23 @@ 
+extern void *memmove (void *, const void *, __SIZE_TYPE__);
+extern void *memset (void *, int, __SIZE_TYPE__);
+
+typedef struct {
+    long n_prefix;
+    long n_spadding;
+} NumberFieldWidths;
+
+void
+fill_number(char *buf, const NumberFieldWidths *spec)
+{
+    if (spec->n_prefix) {
+        memmove(buf,
+                (char *) 0,
+                spec->n_prefix * sizeof(char));
+        buf += spec->n_prefix;
+    }
+    if (spec->n_spadding) {
+        memset(buf, 0, spec->n_spadding);
+        buf += spec->n_spadding;
+    }
+}
+