On Wed, Sep 03, 2014 at 10:45:34AM +0200, 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?
>
After Jeff's approval, I have committed the patch to the trunk. The
patch for 4.9 is however slightly larger, because before cgraph API
C++ification analyze function was a static function (now it is a
method of cgraph_node). Because my fix needs to call it from another
compilation unit, I need to make it externally visible. The 4.9 patch
is below, it also passes bootstrap and test on an x86_64-linux, i386
testing is underway. Given that the original patch has been approved,
I intend to commit this one to the branch tomorrow if the test
finishes fine.
Thanks,
Martin
2014-09-10 Martin Jambor <mjam...@suse.cz>
PR ipa/61654
* cgraph.h (cgraph_analyze_function): Declare.
* cgraphunit.c: (analyze_function): Remove forward declaration,
rename to cgraph_analyze_function, made external.
* 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.
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 6c3be6d..0d13ebe 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -797,6 +797,7 @@ void cgraph_unnest_node (struct cgraph_node *);
enum availability cgraph_function_body_availability (struct cgraph_node *);
void cgraph_add_new_function (tree, bool);
+void cgraph_analyze_function (struct cgraph_node *);
const char* cgraph_inline_failed_string (cgraph_inline_failed_t);
cgraph_inline_failed_type_t cgraph_inline_failed_type (cgraph_inline_failed_t);
diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index 972ca07..9ad76dd 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));
@@ -358,6 +374,11 @@ duplicate_thunk_for_node (cgraph_node *thunk, cgraph_node
*node)
cgraph_call_edge_duplication_hooks (thunk->callees, e);
if (!expand_thunk (new_thunk, false))
new_thunk->analyzed = true;
+ else
+ {
+ new_thunk->thunk.thunk_p = false;
+ cgraph_analyze_function (new_thunk);
+ }
cgraph_call_node_duplication_hooks (thunk, new_thunk);
return new_thunk;
}
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 8abdc5d..f486055 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -219,7 +219,6 @@ cgraph_node_set cgraph_new_nodes;
static void expand_all_functions (void);
static void mark_functions_to_output (void);
static void expand_function (struct cgraph_node *);
-static void analyze_function (struct cgraph_node *);
static void handle_alias_pairs (void);
FILE *cgraph_dump_file;
@@ -331,7 +330,7 @@ cgraph_process_new_functions (void)
gimple_register_cfg_hooks ();
if (!node->analyzed)
- analyze_function (node);
+ cgraph_analyze_function (node);
push_cfun (DECL_STRUCT_FUNCTION (fndecl));
if (cgraph_state == CGRAPH_STATE_IPA_SSA
&& !gimple_in_ssa_p (DECL_STRUCT_FUNCTION (fndecl)))
@@ -541,7 +540,7 @@ cgraph_add_new_function (tree fndecl, bool lowered)
if (lowered)
node->lowered = true;
node->definition = true;
- analyze_function (node);
+ cgraph_analyze_function (node);
push_cfun (DECL_STRUCT_FUNCTION (fndecl));
gimple_register_cfg_hooks ();
bitmap_obstack_initialize (NULL);
@@ -598,8 +597,8 @@ output_asm_statements (void)
}
/* Analyze the function scheduled to be output. */
-static void
-analyze_function (struct cgraph_node *node)
+void
+cgraph_analyze_function (struct cgraph_node *node)
{
tree decl = node->decl;
location_t saved_loc = input_location;
@@ -1014,7 +1013,7 @@ analyze_functions (void)
}
if (!cnode->analyzed)
- analyze_function (cnode);
+ cgraph_analyze_function (cnode);
for (edge = cnode->callees; edge; edge = edge->next_callee)
if (edge->callee->definition)
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;
+}
