diff mbox

[1/2,PR,61654] Handle newly truly expanded artificial_thunks

Message ID 20140903084534.GA18073@virgil.suse
State New
Headers show

Commit Message

Martin Jambor Sept. 3, 2014, 8:45 a.m. UTC 
Hi,

I did not think it was possible, but it can happen that when
duplicate_thunk_for_node creates a duplicate of a thunk which
previously expand_thunk left alone to be expanded into assembly by the
back end, the newly created thunk does get expanded by expand_thunk.
When this happens, we end up with an un-analyzed node which triggers
an assert later on.

This patch deals with the situation by analyzing the newly expanded
thunk.  This revealed that DECL_ARGUMENTS were insufficiently copied
for the new decl and it was sharing them with the old one.  So this
patch fixes this as well.

Bootstrapped and tested on x86_64-linux and i686-linux (where the bug
triggered), OK for trunk and the 4.9 branch?

Thanks,

Martin


2014-09-01  Martin Jambor  <mjambor@suse.cz>

	PR ipa/61654
	* cgraphclones.c (duplicate_thunk_for_node): Copy arguments of the
	new decl properly.  Analyze the new thunk if it is expanded.

gcc/testsuite/
	* g++.dg/ipa/pr61654.C: New test.
---
 gcc/cgraphclones.c                 | 21 ++++++++++++++++++++
 gcc/testsuite/g++.dg/ipa/pr61654.C | 40 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/ipa/pr61654.C

Comments

Jeff Law Sept. 3, 2014, 6:29 p.m. UTC | #1 
On 09/03/14 02:45, Martin Jambor wrote:
> Hi,
>
> I did not think it was possible, but it can happen that when
> duplicate_thunk_for_node creates a duplicate of a thunk which
> previously expand_thunk left alone to be expanded into assembly by the
> back end, the newly created thunk does get expanded by expand_thunk.
> When this happens, we end up with an un-analyzed node which triggers
> an assert later on.
>
> This patch deals with the situation by analyzing the newly expanded
> thunk.  This revealed that DECL_ARGUMENTS were insufficiently copied
> for the new decl and it was sharing them with the old one.  So this
> patch fixes this as well.
>
> Bootstrapped and tested on x86_64-linux and i686-linux (where the bug
> triggered), OK for trunk and the 4.9 branch?
>
> Thanks,
>
> Martin
>
>
> 2014-09-01  Martin Jambor  <mjambor@suse.cz>
>
> 	PR ipa/61654
> 	* cgraphclones.c (duplicate_thunk_for_node): Copy arguments of the
> 	new decl properly.  Analyze the new thunk if it is expanded.
>
> gcc/testsuite/
> 	* g++.dg/ipa/pr61654.C: New test.
OK.
Jeff
diff mbox

Patch

diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index eb04418..2a17de5 100644
--- a/gcc/cgraphclones.c
+++ b/gcc/cgraphclones.c
@@ -334,6 +334,22 @@  duplicate_thunk_for_node (cgraph_node *thunk, cgraph_node *node)
 						node->clone.args_to_skip,
 						false);
     }
+
+  tree *link = &DECL_ARGUMENTS (new_decl);
+  int i = 0;
+  for (tree pd = DECL_ARGUMENTS (thunk->decl); pd; pd = DECL_CHAIN (pd), i++)
+    {
+      if (!node->clone.args_to_skip
+	  || !bitmap_bit_p (node->clone.args_to_skip, i))
+	{
+	  tree nd = copy_node (pd);
+	  DECL_CONTEXT (nd) = new_decl;
+	  *link = nd;
+	  link = &DECL_CHAIN (nd);
+	}
+    }
+  *link = NULL_TREE;
+
   gcc_checking_assert (!DECL_STRUCT_FUNCTION (new_decl));
   gcc_checking_assert (!DECL_INITIAL (new_decl));
   gcc_checking_assert (!DECL_RESULT (new_decl));
@@ -357,6 +373,11 @@  duplicate_thunk_for_node (cgraph_node *thunk, cgraph_node *node)
   symtab->call_edge_duplication_hooks (thunk->callees, e);
   if (!new_thunk->expand_thunk (false, false))
     new_thunk->analyzed = true;
+  else
+    {
+      new_thunk->thunk.thunk_p = false;
+      new_thunk->analyze ();
+    }
 
   symtab->call_cgraph_duplication_hooks (thunk, new_thunk);
   return new_thunk;
diff --git a/gcc/testsuite/g++.dg/ipa/pr61654.C b/gcc/testsuite/g++.dg/ipa/pr61654.C
new file mode 100644
index 0000000..d07e458
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr61654.C
@@ -0,0 +1,40 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+/* The bug only presented itself on a 32 bit i386 but in theory it might also
+   pop up elsewhere and we do not want to put -m32 options to testcase
+   options.  */
+
+struct A
+{
+  virtual int a (int, int = 0) = 0;
+  void b ();
+  void c ();
+  int d;
+};
+
+struct B : virtual A
+{
+  int a (int, int);
+  int e;
+};
+
+int f;
+
+void
+A::b ()
+{
+  a (0);
+}
+
+void
+A::c ()
+{
+  a (f);
+}
+
+int
+B::a (int, int)
+{
+  return e;
+}