> On Sun, 29 Jun 2014, Chen Gang wrote:
>
> > NEWDECL dereferences shared data of OLDDECL, or when free NEWDECL, also
> > have effect with OLDDECL. At present, only fix the related reference for
> > current issue. If find another related issue, can follow this fix.
>
> I'm afraid I can't make any sense of this.
>
> We need a carefully written analysis of the issue you are trying to fix
> that explains what is going wrong and why, in terms of the underlying
> design of the code, the proposed change is logically correct to fix the
> issue.
>
> That is, explain how NEWDECL gets used after merge_decls returns, why your
> proposed changes to NEWDECL are correct for the subsequent use of NEWDECL,
> why the previous contents of NEWDECL would be incorrect for the subsequent
> uses of NEWDECL, and why it is correct for the subsequent uses to make
> whatever use of NEWDECL is causing problems. Where does the segfault
> occur anyway?
Actually this problem was introduced by me (and I made patch for it once already
but apparently it did not reach the ML). THe problem is that we now create
symbols
to hold information about decl that is no longer represented in trees -
sections,
comdats etc.
Now in duplicate_decl we have two symbol nodes for the duplicated declarations
and remove one of them at the end. cgraph_remove_node calls
cgraph_release_function_body that assumes that we never have two functions with
same DECL_STRUCT_FUNCTION.
So Cheng is right we need to clear DECL_STRUCT_FUNCTION or we end up ggc_freeing
it that later ICEs in push_cfun. We do not need to clear the other fields.
The patch bellow just copy identical hunk of code from cp/decl.c
>
> > root@gchen:/upstream/linux# cat elevator.i
> > extern int __attribute__ ((__section__(".init.text"))) elv_register(void)
> > {
> > return 0;
> > }
> > extern typeof(elv_register) elv_register;
>
> Patch submissions should add a test to the testsuite, when it's something
> small like this.
Added.
thanks for working on this! I am bootstrapping/regtesting the attacked patch
on x86_64-linux
OK?
Honza
Chen Gang <gang.chen.5...@gmail.com>
Jan Hubicka <hubi...@ucw.cz>
* c-decl.c (duplicate_decls): CLear DECL_STRUCT_FUNCTION before
releasing
symbol.
* gcc.c-torture/compile/section.c: New testcase
Index: testsuite/gcc.c-torture/compile/section.c
===================================================================
--- testsuite/gcc.c-torture/compile/section.c (revision 0)
+++ testsuite/gcc.c-torture/compile/section.c (revision 0)
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+
+/* { dg-require-effective-target named_sections } */
+extern int __attribute__ ((__section__(".init.text"))) elv_register(void)
+{
+ return 0;
+}
+extern typeof(elv_register) elv_register;
Index: c/c-decl.c
===================================================================
--- c/c-decl.c (revision 212098)
+++ c/c-decl.c (working copy)
@@ -2575,7 +2575,14 @@ duplicate_decls (tree newdecl, tree oldd
merge_decls (newdecl, olddecl, newtype, oldtype);
- /* The NEWDECL will no longer be needed. */
+ /* The NEWDECL will no longer be needed.
+
+ Before releasing the node, be sore to remove function from symbol
+ table that might have been inserted there to record comdat group.
+ Be sure to however do not free DECL_STRUCT_FUNCTION becuase this
+ structure is shared in between newdecl and oldecl. */
+ if (TREE_CODE (newdecl) == FUNCTION_DECL)
+ DECL_STRUCT_FUNCTION (newdecl) = NULL;
if (TREE_CODE (newdecl) == FUNCTION_DECL
|| TREE_CODE (newdecl) == VAR_DECL)
{
