On Fri, 13 Jan 2012, Jakub Jelinek wrote:
> On Fri, Jan 13, 2012 at 11:05:36AM +0100, Richard Guenther wrote:
> > This fixes the ICEs that occur with redeclared extern inline functions
> > in some circumstances. It avoids the cgraph confusion by _not_ merging
> > the two decls in this case but simply drops the old (extern inline)
> > one on the floor. This causes the cgraph to be properly presented
> > with two different decls and thus two different cgraph nodes will
> > be created. I didn't try to change name-lookup to always find
> > the extern inline copy to preserve the ever existing recursive
> > case
> >
> > extern __inline __attribute__ ((__always_inline__))
> > void open ()
> > {
> > }
> > void open ()
> > {
> > open ();
> > }
> >
> > which even in 3.2 where the ICEs appearantly did not exist compiled
> > to a self-recursive open () (trivially explained by how 3.2 worked,
> > function-at-a-time).
>
> That won't e.g. copy over any attributes from the extern inline to
> the out of line, or asm redirection, or type merging, etc.
But that's of course intended. Attributes or redirection on the
extern inline variant are completely meaningless.
> If you want to keep olddecl as is, then IMHO we should add a new bool
> argument to merge_decls and if the flag is set, make sure we only ever
> modify newdecl and not olddecl.
I don't think that is necesary nor warranted. What legitimate use
would break you think?
Btw, the following patch now passed bootstrap and regtest on
x86_64-unknown-linux-gnu.
Ok for trunk?
Thanks,
Richard.
2012-01-13 Richard Guenther <[email protected]>
PR c/33763
* c-decl.c (duplicate_decls): Do not merge re-declared extern
inline function decls with their re-declaration.
(pushdecl): Do not put extern inlines into external_scope.
* gcc.dg/torture/pr33763-1.c: New testcase.
* gcc.dg/torture/pr33763-2.c: Likewise.
* gcc.dg/torture/pr33763-3.c: Likewise.
Index: gcc/c-decl.c
===================================================================
*** gcc/c-decl.c (revision 183121)
--- gcc/c-decl.c (working copy)
*************** duplicate_decls (tree newdecl, tree oldd
*** 2513,2518 ****
--- 2513,2536 ----
return false;
}
+ /* If we have a redeclared extern inline function simply drop olddecl
+ on the floor instead of merging it with newdecl. */
+ if (TREE_CODE (newdecl) == FUNCTION_DECL
+ && DECL_INITIAL (newdecl)
+ && DECL_INITIAL (olddecl)
+ && !(!(DECL_DECLARED_INLINE_P (olddecl)
+ && DECL_EXTERNAL (olddecl))
+ || (DECL_DECLARED_INLINE_P (newdecl)
+ && DECL_EXTERNAL (newdecl))
+ || (!flag_gnu89_inline
+ && (!DECL_DECLARED_INLINE_P (olddecl)
+ || !lookup_attribute ("gnu_inline",
+ DECL_ATTRIBUTES (olddecl)))
+ && (!DECL_DECLARED_INLINE_P (newdecl)
+ || !lookup_attribute ("gnu_inline",
+ DECL_ATTRIBUTES (newdecl))))))
+ return false;
+
merge_decls (newdecl, olddecl, newtype, oldtype);
return true;
}
*************** pushdecl (tree x)
*** 2776,2782 ****
nested = true;
x = visdecl;
}
! else
{
bind (name, x, external_scope, /*invisible=*/true,
/*nested=*/false, locus);
--- 2794,2802 ----
nested = true;
x = visdecl;
}
! else if (TREE_CODE (x) != FUNCTION_DECL
! || !(DECL_DECLARED_INLINE_P (x)
! && DECL_EXTERNAL (x)))
{
bind (name, x, external_scope, /*invisible=*/true,
/*nested=*/false, locus);
Index: gcc/testsuite/gcc.dg/torture/pr33763-1.c
===================================================================
*** gcc/testsuite/gcc.dg/torture/pr33763-1.c (revision 0)
--- gcc/testsuite/gcc.dg/torture/pr33763-1.c (revision 0)
***************
*** 0 ****
--- 1,43 ----
+ /* { dg-do run } */
+
+ extern void abort (void);
+
+ extern __inline __attribute__ ((__always_inline__))
+ int foo ()
+ {
+ return 1;
+ }
+ int test1 ()
+ {
+ /* Name-lookup should find foo that returns 1. */
+ return foo ();
+ }
+ int foo ()
+ {
+ return 0;
+ }
+
+ extern __inline __attribute__ ((__always_inline__))
+ int bar ()
+ {
+ return 1;
+ }
+ int bar ()
+ {
+ return 0;
+ }
+ int test2 ()
+ {
+ /* Name-lookup should find bar that returns 0. */
+ return bar ();
+ }
+
+ int
+ main()
+ {
+ if (test1 () != 1)
+ abort ();
+ if (test2 () != 0)
+ abort ();
+ return 0;
+ }
Index: gcc/testsuite/gcc.dg/torture/pr33763-2.c
===================================================================
*** gcc/testsuite/gcc.dg/torture/pr33763-2.c (revision 0)
--- gcc/testsuite/gcc.dg/torture/pr33763-2.c (revision 0)
***************
*** 0 ****
--- 1,30 ----
+ /* { dg-do compile } */
+
+ extern int foo (const char *, int);
+ extern int baz (const char *, int);
+
+ extern inline __attribute__ ((always_inline, gnu_inline)) int
+ baz (const char *x, int y)
+ {
+ return 2;
+ }
+
+ int
+ baz (const char *x, int y)
+ {
+ return 1;
+ }
+
+ int xa, xb;
+
+ static int
+ inl (const char *x, int y)
+ {
+ return baz (x, y);
+ }
+
+ int
+ foo (const char *x, int y)
+ {
+ return inl (x, y);
+ }
Index: gcc/testsuite/gcc.dg/torture/pr33763-3.c
===================================================================
*** gcc/testsuite/gcc.dg/torture/pr33763-3.c (revision 0)
--- gcc/testsuite/gcc.dg/torture/pr33763-3.c (revision 0)
***************
*** 0 ****
--- 1,58 ----
+ /* { dg-do compile } */
+
+ typedef struct
+ {
+ void *a;
+ void *b;
+ } T;
+ extern void *foo (const char *, const char *);
+ extern void *bar (void *, const char *, T);
+ extern int baz (const char *, int);
+
+ extern inline __attribute__ ((always_inline, gnu_inline)) int
+ baz (const char *x, int y)
+ {
+ return 2;
+ }
+
+ int
+ baz (const char *x, int y)
+ {
+ return 1;
+ }
+
+ int xa, xb;
+
+ static void *
+ inl (const char *x, const char *y)
+ {
+ T t = { &xa, &xb };
+ int *f = (int *) __builtin_malloc (sizeof (int));
+ const char *z;
+ int o = 0;
+ void *r = 0;
+
+ for (z = y; *z; z++)
+ {
+ if (*z == 'r')
+ o |= 1;
+ if (*z == 'w')
+ o |= 2;
+ }
+ if (o == 1)
+ *f = baz (x, 0);
+ if (o == 2)
+ *f = baz (x, 1);
+ if (o == 3)
+ *f = baz (x, 2);
+
+ if (o && *f > 0)
+ r = bar (f, "w", t);
+ return r;
+ }
+
+ void *
+ foo (const char *x, const char *y)
+ {
+ return inl (x, y);
+ }
