[gcc r12-5045] ipa: Fix segfault when remapping debug_binds with expressions (PR 103132)

[gcc r12-5045] ipa: Fix segfault when remapping debug_binds with expressions (PR 103132)

[Martin Jambor]

https://gcc.gnu.org/g:df8690f28379795a44aa4b6c737df08828168b6a

commit r12-5045-gdf8690f28379795a44aa4b6c737df08828168b6a
Author: Martin Jambor <mjambor@suse.cz>
Date:   Tue Nov 9 11:32:20 2021 +0100

    ipa: Fix segfault when remapping debug_binds with expressions (PR 103132)
    
    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.

Diff:
---
 gcc/ipa-param-manipulation.c        | 27 +++++++++++++++++++--------
 gcc/testsuite/gcc.dg/ipa/pr103132.c | 19 +++++++++++++++++++
 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
index 44f3bed2640..4610fc4ac03 100644
--- a/gcc/ipa-param-manipulation.c
+++ b/gcc/ipa-param-manipulation.c
@@ -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
index 00000000000..bef56494c03
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr103132.c
@@ -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); }