ipa: Fix segfault when remapping debug_binds with expressions (PR 103132)
authorMartin Jambor <mjambor@suse.cz>
Tue, 9 Nov 2021 10:32:20 +0000 (11:32 +0100)
committerMartin Jambor <mjambor@suse.cz>
Tue, 9 Nov 2021 10:37:17 +0000 (11:37 +0100)
My initial implementation of the method
ipa_param_body_adjustments::remap_with_debug_expressions was based on
the assumption that if it was asked to remap an expression (as opposed
to a simple SSA_NAME), the expression would not contain an SSA_NAME
operand which is to be debug-reset.  While that is true for when
called from ipa_param_body_adjustments::prepare_debug_expressions, it
turns out it is not true when invoked from remap_gimple_stmt in
tree-inline.c.  This patch adds a simple logic to handle such cases
and simply map the entire value to NULL_TREE in those cases.

gcc/ChangeLog:

2021-11-08  Martin Jambor  <mjambor@suse.cz>

PR ipa/103132
* ipa-param-manipulation.c (replace_with_mapped_expr): Early
return with error_mark_mode when part of expression is mapped to
NULL.
(ipa_param_body_adjustments::remap_with_debug_expressions): Set
mapped value to NULL if walk_tree returns error_mark_mode.

gcc/testsuite/ChangeLog:

2021-11-08  Martin Jambor  <mjambor@suse.cz>

PR ipa/103132
* gcc.dg/ipa/pr103132.c: New test.

gcc/ipa-param-manipulation.c
gcc/testsuite/gcc.dg/ipa/pr103132.c [new file with mode: 0644]

index 44f3bed..4610fc4 100644 (file)
@@ -1071,8 +1071,9 @@ ipa_param_body_adjustments::mark_dead_statements (tree dead_param,
 }
 
 /* Callback to walk_tree.  If REMAP is an SSA_NAME that is present in hash_map
-   passed in DATA, replace it with unshared version of what it was mapped
-   to.  */
+   passed in DATA, replace it with unshared version of what it was mapped to.
+   If an SSA argument would be remapped to NULL, the whole operation needs to
+   abort which is signaled by returning error_mark_node.  */
 
 static tree
 replace_with_mapped_expr (tree *remap, int *walk_subtrees, void *data)
@@ -1089,7 +1090,11 @@ replace_with_mapped_expr (tree *remap, int *walk_subtrees, void *data)
 
   hash_map<tree, tree> *equivs = (hash_map<tree, tree> *) data;
   if (tree *p = equivs->get (*remap))
-    *remap = unshare_expr (*p);
+    {
+      if (!*p)
+       return error_mark_node;
+      *remap = unshare_expr (*p);
+    }
   return 0;
 }
 
@@ -1100,16 +1105,22 @@ void
 ipa_param_body_adjustments::remap_with_debug_expressions (tree *t)
 {
   /* If *t is an SSA_NAME which should have its debug statements reset, it is
-     mapped to NULL in the hash_map.  We need to handle that case separately or
-     otherwise the walker would segfault.  No expression that is more
-     complicated than that can have its operands mapped to NULL.  */
+     mapped to NULL in the hash_map.
+
+     It is perhaps simpler to handle the SSA_NAME cases directly and only
+     invoke walk_tree on more complex expressions.  When
+     remap_with_debug_expressions is called from tree-inline.c, a to-be-reset
+     SSA_NAME can be an operand to such expressions and the entire debug
+     variable we are remapping should be reset.  This is signaled by walk_tree
+     returning error_mark_node and done by setting *t to NULL.  */
   if (TREE_CODE (*t) == SSA_NAME)
     {
       if (tree *p = m_dead_ssa_debug_equiv.get (*t))
        *t = *p;
     }
-  else
-    walk_tree (t, replace_with_mapped_expr, &m_dead_ssa_debug_equiv, NULL);
+  else if (walk_tree (t, replace_with_mapped_expr,
+                     &m_dead_ssa_debug_equiv, NULL) == error_mark_node)
+    *t = NULL_TREE;
 }
 
 /* For an SSA_NAME DEAD_SSA which is about to be DCEd because it is based on a
diff --git a/gcc/testsuite/gcc.dg/ipa/pr103132.c b/gcc/testsuite/gcc.dg/ipa/pr103132.c
new file mode 100644 (file)
index 0000000..bef5649
--- /dev/null
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -g" } */
+
+int globus_i_GLOBUS_GRIDFTP_SERVER_debug_handle_1;
+int globus_l_gfs_ipc_unpack_data__sz;
+void globus_i_GLOBUS_GRIDFTP_SERVER_debug_printf(const char *, ...);
+static void globus_l_gfs_ipc_unpack_cred(int len) {
+  if (globus_i_GLOBUS_GRIDFTP_SERVER_debug_handle_1)
+    globus_i_GLOBUS_GRIDFTP_SERVER_debug_printf("", __func__);
+}
+static void globus_l_gfs_ipc_unpack_data(int len) {
+  for (; globus_l_gfs_ipc_unpack_data__sz;)
+    len--;
+  len -= 4;
+  len -= 4;
+  globus_l_gfs_ipc_unpack_cred(len);
+}
+void globus_l_gfs_ipc_reply_read_body_cb(int len)
+{ globus_l_gfs_ipc_unpack_data(len); }