[CHKP, PATCH] Fix LTO cgraph merge for instrumented functions

[Ilya Enkovich <enkovich dot gnu at gmail dot com>]

Hi,

Currently cgraph merge has several issues with instrumented code:
 - original function node may be removed => no assembler name conflict is detected between function and variable
 - only orig_decl name is privatized for instrumented function => node still shares assembler name which causes infinite privatization loop
 - information about changed name is stored in file_data of instrumented node => original section name may be not found for original function
 - chkp reference is not fixed when nodes are merged

This patch should fix theese problems by keeping instrumentation thunks reachable, privatizing both nodes and fixing chkp references.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for trunk?


Thanks,
Ilya
--
gcc/

2015-03-12  Ilya Enkovich  <ilya.enkovich@intel.com>

	* ipa-chkp.h (chkp_maybe_fix_chkp_ref): New.
	* ipa-chkp.c (chkp_maybe_fix_chkp_ref): New.
	* ipa.c (symbol_table::remove_unreachable_nodes): Don't
	remove instumentation thunks calling reachable functions.
	* lto-cgraph.c: Include ipa-chkp.h.
	(input_symtab): Fix chkp references for boundary nodes.
	* lto/lto-partition.c (privatize_symbol_name_1): New.
	(privatize_symbol_name): Privatize both decl and orig_decl
	names for instrumented functions.
	* lto/lto-symtab.c: Include ipa-chkp.h.
	(lto_cgraph_replace_node): Fix chkp references for merged
	function nodes.

gcc/testsuite/

2015-03-12  Ilya Enkovich  <ilya.enkovich@intel.com>

	* gcc.dg/lto/chkp-privatize-1_0.c: New.
	* gcc.dg/lto/chkp-privatize-1_1.c: New.
	* gcc.dg/lto/chkp-privatize-2_0.c: New.
	* gcc.dg/lto/chkp-privatize-2_1.c: New.


diff --git a/gcc/ipa-chkp.c b/gcc/ipa-chkp.c
index 0b857ff..223f4ed 100644
--- a/gcc/ipa-chkp.c
+++ b/gcc/ipa-chkp.c
@@ -414,6 +414,36 @@ chkp_instrumentable_p (tree fndecl)
 	  && (!fn || !copy_forbidden (fn, fndecl)));
 }
 
+/* Check NODE has a correct IPA_REF_CHKP reference.
+   Create a new reference if required.  */
+
+void
+chkp_maybe_fix_chkp_ref (cgraph_node *node)
+{
+  /* Firstly check node needs IPA_REF_CHKP.  */
+  if (node->instrumentation_clone
+      || !node->instrumented_version)
+    return;
+
+  /* Check we already have a proper IPA_REF_CHKP.
+     Remove incorrect refs.  */
+  int i;
+  ipa_ref *ref = NULL;
+  for (i = 0; node->iterate_reference (i, ref); i++)
+    if (ref->use == IPA_REF_CHKP)
+      {
+	/* Found proper reference.  */
+	if (ref->referred == node->instrumented_version)
+	  return;
+
+	/* Need to recreate reference.  */
+	ref->remove_reference ();
+	break;
+      }
+
+  node->create_reference (node->instrumented_version, IPA_REF_CHKP, NULL);
+}
+
 /* Return clone created for instrumentation of NODE or NULL.  */
 
 cgraph_node *
diff --git a/gcc/ipa-chkp.h b/gcc/ipa-chkp.h
index 6708fe9..5fa7d88 100644
--- a/gcc/ipa-chkp.h
+++ b/gcc/ipa-chkp.h
@@ -24,5 +24,6 @@ extern tree chkp_copy_function_type_adding_bounds (tree orig_type);
 extern tree chkp_maybe_clone_builtin_fndecl (tree fndecl);
 extern cgraph_node *chkp_maybe_create_clone (tree fndecl);
 extern bool chkp_instrumentable_p (tree fndecl);
+extern void chkp_maybe_fix_chkp_ref (cgraph_node *node);
 
 #endif /* GCC_IPA_CHKP_H */
diff --git a/gcc/ipa.c b/gcc/ipa.c
index b3752de..ae6269f 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -492,7 +492,18 @@ symbol_table::remove_unreachable_nodes (FILE *file)
 	    }
 	  else if (cnode->thunk.thunk_p)
 	    enqueue_node (cnode->callees->callee, &first, &reachable);
-	      
+
+	  /* For instrumentation clones we always need original
+	     function node for proper LTO privatization.  */
+	  if (cnode->instrumentation_clone
+	      && reachable.contains (cnode)
+	      && cnode->definition)
+	    {
+	      gcc_assert (cnode->instrumented_version);
+	      enqueue_node (cnode->instrumented_version, &first, &reachable);
+	      reachable.add (cnode->instrumented_version);
+	    }
+
 	  /* If any reachable function has simd clones, mark them as
 	     reachable as well.  */
 	  if (cnode->simd_clones)
diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
index c875fed..b9196eb 100644
--- a/gcc/lto-cgraph.c
+++ b/gcc/lto-cgraph.c
@@ -80,6 +80,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "pass_manager.h"
 #include "ipa-utils.h"
 #include "omp-low.h"
+#include "ipa-chkp.h"
 
 /* True when asm nodes has been output.  */
 bool asm_nodes_output = false;
@@ -1888,6 +1889,10 @@ input_symtab (void)
 	 context of the nested function.  */
       if (node->lto_file_data)
 	node->aux = NULL;
+
+      /* May need to fix chkp reference because we don't stream
+	 them for boundary symbols.  */
+      chkp_maybe_fix_chkp_ref (node);
     }
 }
 
diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
index 235b735..7d117e9 100644
--- a/gcc/lto/lto-partition.c
+++ b/gcc/lto/lto-partition.c
@@ -877,27 +877,13 @@ validize_symbol_for_target (symtab_node *node)
     }
 }
 
-/* Mangle NODE symbol name into a local name.  
-   This is necessary to do
-   1) if two or more static vars of same assembler name
-      are merged into single ltrans unit.
-   2) if previously static var was promoted hidden to avoid possible conflict
-      with symbols defined out of the LTO world.  */
+/* Helper for privatize_symbol_name.  Mangle NODE symbol name
+   represented by DECL.  */
 
 static bool
-privatize_symbol_name (symtab_node *node)
+privatize_symbol_name_1 (symtab_node *node, tree decl)
 {
-  tree decl = node->decl;
-  const char *name;
-  cgraph_node *cnode = dyn_cast <cgraph_node *> (node);
-
-  /* If we want to privatize instrumentation clone
-     then we need to change original function name
-     which is used via transparent alias chain.  */
-  if (cnode && cnode->instrumentation_clone)
-    decl = cnode->orig_decl;
-
-  name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
+  const char *name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
 
   if (must_not_rename (node, name))
     return false;
@@ -906,31 +892,57 @@ privatize_symbol_name (symtab_node *node)
   symtab->change_decl_assembler_name (decl,
 				      clone_function_name_1 (name,
 							     "lto_priv"));
+
   if (node->lto_file_data)
     lto_record_renamed_decl (node->lto_file_data, name,
 			     IDENTIFIER_POINTER
 			     (DECL_ASSEMBLER_NAME (decl)));
+
+  if (symtab->dump_file)
+    fprintf (symtab->dump_file,
+	     "Privatizing symbol name: %s -> %s\n",
+	     name, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)));
+
+  return true;
+}
+
+/* Mangle NODE symbol name into a local name.
+   This is necessary to do
+   1) if two or more static vars of same assembler name
+      are merged into single ltrans unit.
+   2) if previously static var was promoted hidden to avoid possible conflict
+      with symbols defined out of the LTO world.  */
+
+static bool
+privatize_symbol_name (symtab_node *node)
+{
+  if (!privatize_symbol_name_1 (node, node->decl))
+    return false;
+
   /* We could change name which is a target of transparent alias
      chain of instrumented function name.  Fix alias chain if so  .*/
-  if (cnode)
+  if (cgraph_node *cnode = dyn_cast <cgraph_node *> (node))
     {
       tree iname = NULL_TREE;
       if (cnode->instrumentation_clone)
-	iname = DECL_ASSEMBLER_NAME (cnode->decl);
+	{
+	  /* If we want to privatize instrumentation clone
+	     then we also need to privatize original function.  */
+	  if (cnode->instrumented_version)
+	    privatize_symbol_name (cnode->instrumented_version);
+	  else
+	    privatize_symbol_name_1 (cnode, cnode->orig_decl);
+	  iname = DECL_ASSEMBLER_NAME (cnode->decl);
+	  TREE_CHAIN (iname) = DECL_ASSEMBLER_NAME (cnode->orig_decl);
+	}
       else if (cnode->instrumented_version
-	       && cnode->instrumented_version->orig_decl == decl)
-	iname = DECL_ASSEMBLER_NAME (cnode->instrumented_version->decl);
-
-      if (iname)
+	       && cnode->instrumented_version->orig_decl == cnode->decl)
 	{
-	  gcc_assert (IDENTIFIER_TRANSPARENT_ALIAS (iname));
-	  TREE_CHAIN (iname) = DECL_ASSEMBLER_NAME (decl);
+	  iname = DECL_ASSEMBLER_NAME (cnode->instrumented_version->decl);
+	  TREE_CHAIN (iname) = DECL_ASSEMBLER_NAME (cnode->decl);
 	}
     }
-  if (symtab->dump_file)
-    fprintf (symtab->dump_file,
-	    "Privatizing symbol name: %s -> %s\n",
-	    name, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)));
+
   return true;
 }
 
diff --git a/gcc/lto/lto-symtab.c b/gcc/lto/lto-symtab.c
index c00fd87..e0b9762 100644
--- a/gcc/lto/lto-symtab.c
+++ b/gcc/lto/lto-symtab.c
@@ -56,6 +56,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "ipa-prop.h"
 #include "ipa-inline.h"
 #include "builtins.h"
+#include "ipa-chkp.h"
 
 /* Replace the cgraph node NODE with PREVAILING_NODE in the cgraph, merging
    all edges and removing the old node.  */
@@ -116,7 +117,11 @@ lto_cgraph_replace_node (struct cgraph_node *node,
 		  == prevailing_node->instrumentation_clone);
       node->instrumented_version->instrumented_version = prevailing_node;
       if (!prevailing_node->instrumented_version)
-	prevailing_node->instrumented_version = node->instrumented_version;
+	{
+	  prevailing_node->instrumented_version = node->instrumented_version;
+	  chkp_maybe_fix_chkp_ref (prevailing_node);
+	}
+      chkp_maybe_fix_chkp_ref (node->instrumented_version);
       /* Need to reset node->instrumented_version to NULL,
 	 otherwise node removal code would reset
 	 node->instrumented_version->instrumented_version.  */
diff --git a/gcc/testsuite/gcc.dg/lto/chkp-privatize-1_0.c b/gcc/testsuite/gcc.dg/lto/chkp-privatize-1_0.c
new file mode 100644
index 0000000..2054aa15
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/chkp-privatize-1_0.c
@@ -0,0 +1,17 @@
+/* { dg-lto-do link } */
+/* { dg-require-effective-target mpx } */
+/* { dg-lto-options { { -Ofast -flto -fcheck-pointer-bounds -mmpx } } } */
+
+extern int __attribute__((noinline)) f1 (int i);
+
+static int __attribute__((noinline))
+f2 (int i)
+{
+  return i + 6;
+}
+
+int
+main (int argc, char **argv)
+{
+  return f1 (argc) + f2 (argc);
+}
diff --git a/gcc/testsuite/gcc.dg/lto/chkp-privatize-1_1.c b/gcc/testsuite/gcc.dg/lto/chkp-privatize-1_1.c
new file mode 100644
index 0000000..4fa8656
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/chkp-privatize-1_1.c
@@ -0,0 +1,11 @@
+static int __attribute__((noinline))
+f2 (int i)
+{
+  return 2 * i;
+}
+
+int __attribute__((noinline))
+f1 (int i)
+{
+  return f2 (i) + 10;
+}
diff --git a/gcc/testsuite/gcc.dg/lto/chkp-privatize-2_0.c b/gcc/testsuite/gcc.dg/lto/chkp-privatize-2_0.c
new file mode 100644
index 0000000..be7f601
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/chkp-privatize-2_0.c
@@ -0,0 +1,18 @@
+/* { dg-lto-do link } */
+/* { dg-require-effective-target mpx } */
+/* { dg-lto-options { { -Ofast -flto -fcheck-pointer-bounds -mmpx } } } */
+
+static int
+__attribute__ ((noinline))
+func1 (int i)
+{
+  return i + 2;
+}
+
+extern int func2 (int i);
+
+int
+main (int argc, char **argv)
+{
+  return func1 (argc) + func2 (argc) + 1;
+}
diff --git a/gcc/testsuite/gcc.dg/lto/chkp-privatize-2_1.c b/gcc/testsuite/gcc.dg/lto/chkp-privatize-2_1.c
new file mode 100644
index 0000000..db39e7f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/chkp-privatize-2_1.c
@@ -0,0 +1,8 @@
+int func1 = 10;
+
+int
+func2 (int i)
+{
+  func1++;
+  return i + func1;
+}