[PATCH, CHKP] Fix cdtor merge for instrumented functions

[PATCH, CHKP] Fix cdtor merge for instrumented functions

[Ilya Enkovich]

Hi,

This patch doesn't allow instrumentation thunks calls while merging constructors and destructors.  Not isntrumented code is not affeceted.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for trunk?

Thanks,
Ilya
--
gcc/

2015-04-02  Ilya Enkovich  <ilya.enkovich@intel.com>

	* ipa.c (ipa_cdtor_merge): Skip instrumentation thunks.

gcc/testsuite/

2015-04-02  Ilya Enkovich  <ilya.enkovich@intel.com>

	* gcc.dg/lto/chkp-ctor-merge_0.c: New.


diff --git a/gcc/ipa.c b/gcc/ipa.c
index b3752de..84ab542 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -1159,8 +1159,10 @@ ipa_cdtor_merge (void)
 {
   struct cgraph_node *node;
   FOR_EACH_DEFINED_FUNCTION (node)
-    if (DECL_STATIC_CONSTRUCTOR (node->decl)
-	|| DECL_STATIC_DESTRUCTOR (node->decl))
+    if ((DECL_STATIC_CONSTRUCTOR (node->decl)
+	 || DECL_STATIC_DESTRUCTOR (node->decl))
+	&& !(node->thunk.thunk_p
+	     && node->thunk.add_pointer_bounds_args))
        record_cdtor_fn (node);
   build_cdtor_fns ();
   static_ctors.release ();
diff --git a/gcc/testsuite/gcc.dg/lto/chkp-ctor-merge_0.c b/gcc/testsuite/gcc.dg/lto/chkp-ctor-merge_0.c
new file mode 100644
index 0000000..ac4095b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/chkp-ctor-merge_0.c
@@ -0,0 +1,23 @@
+/* { dg-lto-do run } */
+/* { dg-require-effective-target mpx } */
+/* { dg-lto-options { { -O2 -flto -fcheck-pointer-bounds -mmpx -nodefaultlibs -lc } } } */
+
+int glob = 1;
+
+void __attribute__((constructor))
+ctor1 ()
+{
+  glob += 1;
+}
+
+
+void __attribute__((constructor))
+ctor2 ()
+{
+  glob -= 2;
+}
+
+int main (int argc, const char **argv)
+{
+  return glob;
+}

Re: [PATCH, CHKP] Fix cdtor merge for instrumented functions

[Jan Hubicka]

> Hi,
> 
> This patch doesn't allow instrumentation thunks calls while merging constructors and destructors.  Not isntrumented code is not affeceted.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for trunk?
> 
> Thanks,
> Ilya
> --
> gcc/
> 
> 2015-04-02  Ilya Enkovich  <ilya.enkovich@intel.com>
> 
> 	* ipa.c (ipa_cdtor_merge): Skip instrumentation thunks.

So the problem here is that you do have two names for the function, one that
is not instrumented and other that is instrumented?  I am bit surprised we get
instrumentation on ctors that should not take or return pointer parameter,
but I see one can trigger that at least by manually adding constructor attribute.

I think what you need is to drop DECL_STATIC_CONSTRUCTOR/DESTRUCTURO flags when
producing the transparent alias.

Honza

Re: [PATCH, CHKP] Fix cdtor merge for instrumented functions

[Ilya Enkovich]

On 02 Apr 22:21, Jan Hubicka wrote:
> > Hi,
> > 
> > This patch doesn't allow instrumentation thunks calls while merging constructors and destructors.  Not isntrumented code is not affeceted.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for trunk?
> > 
> > Thanks,
> > Ilya
> > --
> > gcc/
> > 
> > 2015-04-02  Ilya Enkovich  <ilya.enkovich@intel.com>
> > 
> > 	* ipa.c (ipa_cdtor_merge): Skip instrumentation thunks.
> 
> So the problem here is that you do have two names for the function, one that
> is not instrumented and other that is instrumented?  I am bit surprised we get
> instrumentation on ctors that should not take or return pointer parameter,
> but I see one can trigger that at least by manually adding constructor attribute.
> 
> I think what you need is to drop DECL_STATIC_CONSTRUCTOR/DESTRUCTURO flags when
> producing the transparent alias.
> 
> Honza

Dropping flag is a good option.  Here is a corresponding patch.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  Does it look OK?

Thanks,
Ilya
--
gcc/

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

	* ipa-chkp.c (chkp_maybe_create_clone): Reset cdtor
	flags for instrumentation thunk.
	(chkp_produce_thunks): Likewise.

gcc/testsuite/

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

	* gcc.dg/lto/chkp-ctor-merge_0.c: New.


diff --git a/gcc/ipa-chkp.c b/gcc/ipa-chkp.c
index a9933e2..0c16f71 100644
--- a/gcc/ipa-chkp.c
+++ b/gcc/ipa-chkp.c
@@ -550,6 +550,9 @@ chkp_maybe_create_clone (tree fndecl)
 	      clone->thunk.thunk_p = true;
 	      clone->thunk.add_pointer_bounds_args = true;
 	      clone->create_edge (node, NULL, 0, CGRAPH_FREQ_BASE);
+	      /* Thunk shouldn't be a cdtor.  */
+	      DECL_STATIC_CONSTRUCTOR (clone->decl) = 0;
+	      DECL_STATIC_DESTRUCTOR (clone->decl) = 0;
 	    }
 	  else
 	    {
@@ -713,6 +716,9 @@ chkp_produce_thunks (bool early)
 			     0, CGRAPH_FREQ_BASE);
 	  node->create_reference (node->instrumented_version,
 			       IPA_REF_CHKP, NULL);
+	  /* Thunk shouldn't be a cdtor.  */
+	  DECL_STATIC_CONSTRUCTOR (node->decl) = 0;
+	  DECL_STATIC_DESTRUCTOR (node->decl) = 0;
 	}
     }
 
diff --git a/gcc/testsuite/gcc.dg/lto/chkp-ctor-merge_0.c b/gcc/testsuite/gcc.dg/lto/chkp-ctor-merge_0.c
new file mode 100644
index 0000000..ac4095b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/chkp-ctor-merge_0.c
@@ -0,0 +1,23 @@
+/* { dg-lto-do run } */
+/* { dg-require-effective-target mpx } */
+/* { dg-lto-options { { -O2 -flto -fcheck-pointer-bounds -mmpx -nodefaultlibs -lc } } } */
+
+int glob = 1;
+
+void __attribute__((constructor))
+ctor1 ()
+{
+  glob += 1;
+}
+
+
+void __attribute__((constructor))
+ctor2 ()
+{
+  glob -= 2;
+}
+
+int main (int argc, const char **argv)
+{
+  return glob;
+}

Re: [PATCH, CHKP] Fix cdtor merge for instrumented functions

[Jan Hubicka]

> On 02 Apr 22:21, Jan Hubicka wrote:
> > > Hi,
> > > 
> > > This patch doesn't allow instrumentation thunks calls while merging constructors and destructors.  Not isntrumented code is not affeceted.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for trunk?
> > > 
> > > Thanks,
> > > Ilya
> > > --
> > > gcc/
> > > 
> > > 2015-04-02  Ilya Enkovich  <ilya.enkovich@intel.com>
> > > 
> > > 	* ipa.c (ipa_cdtor_merge): Skip instrumentation thunks.
> > 
> > So the problem here is that you do have two names for the function, one that
> > is not instrumented and other that is instrumented?  I am bit surprised we get
> > instrumentation on ctors that should not take or return pointer parameter,
> > but I see one can trigger that at least by manually adding constructor attribute.
> > 
> > I think what you need is to drop DECL_STATIC_CONSTRUCTOR/DESTRUCTURO flags when
> > producing the transparent alias.
> > 
> > Honza
> 
> Dropping flag is a good option.  Here is a corresponding patch.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  Does it look OK?
> 
> Thanks,
> Ilya
> --
> gcc/
> 
> 2015-04-03  Ilya Enkovich  <ilya.enkovich@intel.com>
> 
> 	* ipa-chkp.c (chkp_maybe_create_clone): Reset cdtor
> 	flags for instrumentation thunk.
> 	(chkp_produce_thunks): Likewise.

OK,
Thanks!
Honza