On 11/10/13 20:11, Jeff Law wrote:
OK. It sounds like there's a pretty general consensus that we ought ot
go ahead and leave in a load/store of a NULL pointer. I'll go ahead and
run with that. I'll probably just emit SSA_NAME = *0, unless someone
thinks we ought ot distinguish between loads and stores, emitting
SSA_NAME = *0 and *0 = 0 for the two cases respectively.
[ ... ]
Here's a patch which arranges to leave a null pointer dereferece in the
IL. For a memory load through a null pointer, we just leave the
statement as-is since the LHS is going to be a throw-away SSA_NAME. For
a store through a null pointer, we try to simplify the RHS when it's
trivially easy to do so (when the RHS is an integral type). This allows
DCE to sometimes do a better job if the RHS is something that was
computed on the isolated path. This could be easily extended to
floating types, vectors and structures if someone is interested. We
issue the builtin_trap after the null dereference.
For other cases (invalid NULL argument, invalid NULL return), we
continue to issue the builtin trap in the same manner we did before.
I've adjusted the existing tests and added a new one which verifies the
RHS of a store through a null pointer is simplified.
Bootstrapped and regression tested on x86_64-unknown-linux-gnu.
Installed on the trunk.
* gimple-ssa-isolate-paths.c (check_loadstore): New function.
(insert_trap_and_remove_trailing_statements): New argument OP which
is the NULL pointer. Emit the trap after the load/store through
the NULL pointer. Simplify the RHS of a store through a NULL pointer
when trivial to do so.
(isolate_path): Corresponding changes.
(gimple_ssa_isolate_erroneous_path): Likewise.
* gcc.dg/tree-ssa/isolate-1.c: Update expected output.
* gcc.dg/tree-ssa/isolate-5.c: New test.
diff --git a/gcc/gimple-ssa-isolate-paths.c b/gcc/gimple-ssa-isolate-paths.c
index 983ed4d..1203cfc 100644
--- a/gcc/gimple-ssa-isolate-paths.c
+++ b/gcc/gimple-ssa-isolate-paths.c
@@ -39,17 +39,65 @@ along with GCC; see the file COPYING3. If not see
static bool cfg_altered;
-/* Insert a trap before SI and remove SI and all statements after SI. */
+/* Callback for walk_stmt_load_store_ops.
+
+ Return TRUE if OP will dereference the tree stored in DATA, FALSE
+ otherwise.
+
+ This routine only makes a superficial check for a dereference. Thus,
+ it must only be used if it is safe to return a false negative. */
+static bool
+check_loadstore (gimple stmt ATTRIBUTE_UNUSED, tree op, void *data)
+{
+ if ((TREE_CODE (op) == MEM_REF || TREE_CODE (op) == TARGET_MEM_REF)
+ && operand_equal_p (TREE_OPERAND (op, 0), (tree)data, 0))
+ return true;
+ return false;
+}
+
+/* Insert a trap after SI and remove SI and all statements after the trap. */
static void
-insert_trap_and_remove_trailing_statements (gimple_stmt_iterator *si_p)
+insert_trap_and_remove_trailing_statements (gimple_stmt_iterator *si_p, tree
op)
{
- gimple_seq seq = NULL;
- gimple stmt = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0);
- gimple_seq_add_stmt (&seq, stmt);
- gsi_insert_before (si_p, seq, GSI_SAME_STMT);
+ /* We want the NULL pointer dereference to actually occur so that
+ code that wishes to catch the signal can do so.
- /* Now delete all remaining statements in this block. */
+ If the dereference is a load, then there's nothing to do as the
+ LHS will be a throw-away SSA_NAME and the LHS is the NULL dereference.
+
+ If the dereference is a store and we can easily transform the RHS,
+ then simplify the RHS to enable more DCE. */
+ gimple stmt = gsi_stmt (*si_p);
+ if (walk_stmt_load_store_ops (stmt, (void *)op, NULL, check_loadstore)
+ && INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt))))
+ {
+ /* We just need to turn the RHS into zero converted to the proper
+ type. */
+ tree type = TREE_TYPE (gimple_assign_lhs (stmt));
+ gimple_assign_set_rhs_code (stmt, INTEGER_CST);
+ gimple_assign_set_rhs1 (stmt, fold_convert (type, integer_zero_node));
+ update_stmt (stmt);
+ }
+
+ gimple new_stmt
+ = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0);
+ gimple_seq seq = NULL;
+ gimple_seq_add_stmt (&seq, new_stmt);
+
+ /* If we had a NULL pointer dereference, then we want to insert the
+ __builtin_trap after the statement, for the other cases we want
+ to insert before the statement. */
+ if (walk_stmt_load_store_ops (stmt, (void *)op,
+ check_loadstore,
+ check_loadstore))
+ gsi_insert_after (si_p, seq, GSI_NEW_STMT);
+ 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);)
{
stmt = gsi_stmt (*si_p);
@@ -73,7 +121,8 @@ insert_trap_and_remove_trailing_statements
(gimple_stmt_iterator *si_p)
Return BB'. */
basic_block
-isolate_path (basic_block bb, basic_block duplicate, edge e, gimple stmt)
+isolate_path (basic_block bb, basic_block duplicate,
+ edge e, gimple stmt, tree op)
{
gimple_stmt_iterator si, si2;
edge_iterator ei;
@@ -133,7 +182,7 @@ isolate_path (basic_block bb, basic_block duplicate, edge
e, gimple stmt)
SI2 points to the duplicate of STMT in DUPLICATE. Insert a trap
before SI2 and remove SI2 and all trailing statements. */
if (!gsi_end_p (si2))
- insert_trap_and_remove_trailing_statements (&si2);
+ insert_trap_and_remove_trailing_statements (&si2, op);
return duplicate;
}
@@ -224,7 +273,7 @@ gimple_ssa_isolate_erroneous_paths (void)
if (infer_nonnull_range (use_stmt, lhs))
{
duplicate = isolate_path (bb, duplicate,
- e, use_stmt);
+ e, use_stmt, lhs);
/* When we remove an incoming edge, we need to
reprocess the Ith element. */
@@ -247,7 +296,8 @@ gimple_ssa_isolate_erroneous_paths (void)
where a non-NULL value is required. */
if (infer_nonnull_range (stmt, null_pointer_node))
{
- insert_trap_and_remove_trailing_statements (&si);
+ insert_trap_and_remove_trailing_statements (&si,
+ null_pointer_node);
/* And finally, remove all outgoing edges from BB. */
edge e;
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-1.c
b/gcc/testsuite/gcc.dg/tree-ssa/isolate-1.c
index 6b779b4..33745ba 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/isolate-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-1.c
@@ -43,12 +43,14 @@ d_type (struct d_info *di)
return ret;
}
-/* We're testing two aspects of isolation here. First that isolation
+/* We're testing three aspects of isolation here. First that isolation
occurs, second that if we have two null dereferences in a block that
that we delete everything from the first dereferece to the end of the
- block, regardless of which comes first in the immediate use iterator. */
+ block, regardless of which comes first in the immediate use iterator
+ and finally that we set the RHS of the store to zero. */
/* { dg-final { scan-tree-dump-times "__builtin_trap" 1 "isolate-paths"} } */
-/* { dg-final { scan-tree-dump-times "->type" 1 "isolate-paths"} } */
+/* { dg-final { scan-tree-dump-times "->type = 42" 1 "isolate-paths"} } */
+/* { dg-final { scan-tree-dump-times "->type = 0" 1 "isolate-paths"} } */
/* { dg-final { scan-tree-dump-times "->zzz" 1 "isolate-paths"} } */
/* { dg-final { cleanup-tree-dump "isolate-paths" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c
b/gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c
new file mode 100644
index 0000000..a70871e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c
@@ -0,0 +1,60 @@
+
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-isolate-paths" } */
+
+
+struct demangle_component
+{
+
+ int type;
+ int zzz;
+
+};
+
+
+struct d_info
+{
+ struct demangle_component *comps;
+ int next_comp;
+ int num_comps;
+};
+
+
+static struct demangle_component *
+d_make_empty (struct d_info *di)
+{
+ struct demangle_component *p;
+
+ if (di->next_comp >= di->num_comps)
+ return ((void *)0);
+ p = &di->comps[di->next_comp];
+ return p;
+}
+
+
+
+struct demangle_component *
+d_type (struct d_info *di)
+{
+ struct demangle_component *ret;
+ ret = d_make_empty (di);
+ foo (ret->type);
+ bar (ret->zzz);
+ return ret;
+}
+
+/* We're testing two aspects of isolation here. First that isolation
+ occurs, second that if we have two null dereferences in a block that
+ that we delete everything from the first dereferece to the end of the
+ block, regardless of which comes first in the immediate use iterator.
+
+ We leave the 0->type in the IL, so expect to see ->type twice. */
+/* { dg-final { scan-tree-dump-times "__builtin_trap" 1 "isolate-paths"} } */
+/* { dg-final { scan-tree-dump-times "->type" 2 "isolate-paths"} } */
+/* { dg-final { scan-tree-dump-times "->zzz" 1 "isolate-paths"} } */
+/* { dg-final { cleanup-tree-dump "isolate-paths" } } */
+
+
+
+
+
