Bug 33763 - [4.6/4.7/4.8 Regression] Bogus inlining failed in call to `xxx': redefined extern inline functions are not considered for inlining
[4.6/4.7/4.8 Regression] Bogus inlining failed in call to `xxx': redefined ex...
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: c
4.1.2
: P2 normal
: 4.6.4
Assigned To: Not yet assigned to anyone
: wrong-code
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-10-13 19:14 UTC by Jakub Jelinek
Modified: 2013-09-20 22:25 UTC (History)
11 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 4.6.0
Last reconfirmed: 2011-03-04 10:26:41


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelinek 2007-10-13 19:14:43 UTC
/* { dg-do compile } */
/* { dg-options "-O2" } */

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);
}

issues bogus sorry diagnostics, while simpler testcases are handled just fine:
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);
}

When a gnu_inline (or extern inline gnu89) function has a real is redefined, we don't want any inlining for that function even if it is always_inline, at least GCC always behaved that way and even on the simpler testcase behaves that way.
Even the original testcase works just fine with GCC 3.2.x, so this is a regression.  On the trunk it is enough to e.g. remove one of the 3 baz calls in inl and it compiles just fine (none of the baz calls is inlined as expected), but
e.g. on 4.1 branch the compilation still fails.

arts hits this problem when building against recent glibcs.
Comment 1 Richard Biener 2007-10-14 10:26:41 UTC
Confirmed.
Comment 2 Jan Hubicka 2007-10-14 12:10:09 UTC
This is result of ugly hack handling redefined extern inline functions.  The most sane semantics (not what 3.2.x did) would be that function if inlined should get the extern inline body, but the body from redefined non-extern inline function should be output off-line and used when called.

Cgraph code can deal with this via cgraph->global->inline_decl.  The problem is that frontend currently behave in a way parsing extern inline body and finalizing it in a cgraph, but before unit is finished and cgraph has chance to use it, the redefinition overwrite the body.

If frontend was updated to produce 2 separate declarations, one for the extern inline body, one for the offline body and set the inline_decl of the second declaration to point to first, things should just work.

There is one extra problem to this - extern inline functions are declared as public and handled so by quite large part of compiler. This means that when doing --combine (or future LTO), we end up having multiple copies of global extern inline function and we end up not inlining them at all too.

I think frontend should be told to make those functions static (or we need to update rest of fronend code to special case this)

Honza
Comment 3 Jan Hubicka 2007-10-14 12:50:01 UTC
This patch should disable the sorry message in this case, but I would argue that outputting sorry here is right - we fail to inline alway_inline function because of implementation limitation. So I would claim 3.2.0 being buggy rather than mainline.

Honza

Index: tree-inline.c
===================================================================
*** tree-inline.c	(revision 129072)
--- tree-inline.c	(working copy)
*************** expand_call_inline (basic_block bb, tree
*** 2560,2565 ****
--- 2560,2571 ----
    if (!cgraph_inline_p (cg_edge, &reason))
      {
        if (lookup_attribute ("always_inline", DECL_ATTRIBUTES (fn))
+ 	  /* For extern inline functions that get redefined we always
+ 	     silently ignored alway_inline flag. Better behaviour would
+ 	     be to be able to keep both bodies and use extern inline body
+ 	     for inlining, but we can't do that because frontends overwrite
+ 	     the body.  */
+ 	  && !cg_edge->callee->local.redefined_extern_inline
  	  /* Avoid warnings during early inline pass. */
  	  && (!flag_unit_at_a_time || cgraph_global_info_ready))
  	{
Comment 4 Jakub Jelinek 2007-10-14 21:10:51 UTC
At least for gcc-4_1-branch and gcc-4_2-branch your patch is IMHO the best thing we can do.  I agree that with cgraph it shouldn't be very hard to actually
register two bodies, one for extern inline fn and one for the real definition,
but am not sure if such changes are appropriate this late in 4.3 development, so
perhaps it should be applied also for 4.3 and early in 4.4 we should come up with the real fixes.
Comment 5 Richard Biener 2008-01-08 16:49:55 UTC
I would suggest to go with the patch in comment #3.
Comment 6 Jakub Jelinek 2008-01-08 17:25:06 UTC
So do I and even am using that patch for quite some time in our gcc packages.
But Mark objected to that: http://gcc.gnu.org/ml/gcc-patches/2007-11/msg00589.html
The current state of things is bad, because with glibc 2.7+ several valid programs don't compile.  And this late in stage3 is probably dangerous to play with two different bodies in cgraph.
Comment 7 rguenther@suse.de 2008-01-08 17:27:43 UTC
Subject: Re:  [4.1/4.2/4.3 Regression] Bogus
 inlining failed in call to `xxx': redefined extern inline functions are not
 considered for inlining

On Tue, 8 Jan 2008, jakub at gcc dot gnu dot org wrote:

> ------- Comment #6 from jakub at gcc dot gnu dot org  2008-01-08 17:25 -------
> So do I and even am using that patch for quite some time in our gcc packages.

So do I.

> But Mark objected to that:
> http://gcc.gnu.org/ml/gcc-patches/2007-11/msg00589.html
> The current state of things is bad, because with glibc 2.7+ several valid
> programs don't compile.  And this late in stage3 is probably dangerous to play
> with two different bodies in cgraph.

Right, this is something for stage1.

Richard.
Comment 8 Jan Hubicka 2008-01-12 13:55:34 UTC
I can definitely commit the patch to silence the (IMO valid) diagnostics.  However, why programs are using always_inline and extern inline combination at all?  Just extern inline should be enough.
  /* For GNU C extern inline functions disregard inline limits.  */
  if (DECL_EXTERNAL (fndecl)
      && DECL_DECLARED_INLINE_P (fndecl))
    DECL_DISREGARD_INLINE_LIMITS (fndecl) = 1;
  
So perhaps just modifying glibc headers to not do both would do the trick?

I agree that solving this problem correctly via two functions (not only bodies, but simply two declarations) is probably bit tricky for stage3. We however have related PR34609 and PR31529 demonstrating same problem in different context.


Honza
Comment 9 rguenther@suse.de 2008-01-12 15:13:50 UTC
Subject: Re:  [4.1/4.2/4.3 Regression] Bogus
 inlining failed in call to `xxx': redefined extern inline functions are not
 considered for inlining

On Sat, 12 Jan 2008, hubicka at gcc dot gnu dot org wrote:

> ------- Comment #8 from hubicka at gcc dot gnu dot org  2008-01-12 13:55 -------
> I can definitely commit the patch to silence the (IMO valid) diagnostics. 
> However, why programs are using always_inline and extern inline combination at
> all?  Just extern inline should be enough.
>   /* For GNU C extern inline functions disregard inline limits.  */
>   if (DECL_EXTERNAL (fndecl)
>       && DECL_DECLARED_INLINE_P (fndecl))
>     DECL_DISREGARD_INLINE_LIMITS (fndecl) = 1;
> 
> So perhaps just modifying glibc headers to not do both would do the trick?
> 
> I agree that solving this problem correctly via two functions (not only bodies,
> but simply two declarations) is probably bit tricky for stage3. We however have
> related PR34609 and PR31529 demonstrating same problem in different context.

Can you give it a try?  If it looks reasonably safe I would certainly
prefer such a solution.

Richard.
Comment 10 Joseph S. Myers 2008-02-01 16:54:54 UTC
4.2.3 is being released now, changing milestones of open bugs to 4.2.4.
Comment 11 Jan Hubicka 2008-02-13 18:15:44 UTC
I must say that I don't see it that safe: we can't clone the functions because at the time fronend decide to rewrite the body by new one we are not having functions gimplified yet.  (well we can clone, but we want to get rid of non-gimple inlining code too). Keeping the unclonned body around seems difficult in combination with one declaration rule.  But perhaps it is just my limited knowledge of C and C++ frontends here..
Comment 12 Joseph S. Myers 2008-05-19 20:23:29 UTC
4.2.4 is being released, changing milestones to 4.2.5.
Comment 13 Joseph S. Myers 2008-07-04 22:51:47 UTC
Closing 4.1 branch.
Comment 14 Jakub Jelinek 2008-11-13 13:55:07 UTC
The reason for using always_inline attribute in glibc headers is that whether these (tiny) wrappers are inlined or not is a security matter for the program
(if they are inlined, -D_FORTIFY_SOURCE checking is performed, if they are not,
then no checking is done), and that checking shouldn't be affected by how many
functions were already inlined into some function etc.

As this isn't going to be fixed in 4.4 either, could we get the sorry quieted up?
Comment 15 Joseph S. Myers 2009-03-31 20:12:59 UTC
Closing 4.2 branch.
Comment 16 Richard Biener 2009-08-04 12:28:25 UTC
GCC 4.3.4 is being released, adjusting target milestone.
Comment 17 Andrew Pinski 2010-01-16 06:52:17 UTC
Note on the trunk it fails at -O1 but not -O2.
Comment 18 Richard Biener 2010-05-22 18:11:43 UTC
GCC 4.3.5 is being released, adjusting target milestone.
Comment 19 Richard Biener 2011-03-04 12:36:56 UTC
Still fails to inline baz (not a regression) and diagnoses this (this is
the regression).

> When a gnu_inline (or extern inline gnu89) function has a real is redefined, we
> don't want any inlining for that function even if it is always_inline, at least
> GCC always behaved that way and even on the simpler testcase behaves that way.

I don't think this is the desired behavior (nor is it documented).  If it is
then we should simply drop the always-inline attribute when merging the
function decls (basically throw away the inline definition).
Comment 20 Jan Hubicka 2011-03-05 12:45:29 UTC
> I don't think this is the desired behavior (nor is it documented).  If it is
> then we should simply drop the always-inline attribute when merging the
> function decls (basically throw away the inline definition).

Well, the desired behaviour IMO is to get one body for inline copies, other
body for offline copy (this is also only consistent interpretation with LTO.
Current way of inlining them early and then droppipng is bit weird).  This is
quite easy to do at cgraph level once frontend is able to produce both function
bodies at two different declarations (one static inline, other external) tells
so to the backend.
No one volunteed to do this at frontend side yet :(

Honza
Comment 21 Pat Haugen 2011-03-11 22:16:12 UTC
An example from libhugetlbfs. Note the original call wasn't recursive, following is reduced code I got from delta.

$ cat gethugepagesizes.c 
extern __inline __attribute__ ((__always_inline__))
     int open (__const char *__path, int __oflag, ...)
{
}

void cleanup (void);
open (const char *file, int flags, ...)
{
  char fname[4096 + 1];
  int fd;
  cleanup ();
  fd = open (fname, 01 | 0100);
}

void cleanup (void)
{
  cleanup_fake_data ();
}
$ ~/install/gcc/trunk/bin/gcc -O3 -c gethugepagesizes.c
gethugepagesizes.c: In function 'open':
gethugepagesizes.c:7:1: sorry, unimplemented: inlining failed in call to 'open': redefined extern inline functions are not considered for inlining
gethugepagesizes.c:12:6: sorry, unimplemented: called from here
Comment 22 Richard Biener 2011-06-27 12:14:32 UTC
4.3 branch is being closed, moving to 4.4.7 target.
Comment 23 Richard Biener 2012-01-12 13:42:39 UTC
Shorter testcase

extern __inline __attribute__ ((__always_inline__,__gnu_inline__))
void open ()
{
}
void open ()
{
  open ();
}

fails on trunk like

> ./cc1 -quiet t.c -O
t.c: In function 'open':
t.c:5:6: error: inlining failed in call to always_inline 'open': function not inlinable
t.c:7:8: error: called from here

and creates wrong code at -O0 (and probably would, at -On):

open:
.LFB1:
        .cfi_startproc
        pushq   %rbp
        .cfi_def_cfa_offset 16
        .cfi_offset 6, -16
        movq    %rsp, %rbp
        .cfi_def_cfa_register 6
        movl    $0, %eax
        call    open
        popq    %rbp
        .cfi_def_cfa 7, 8
        ret

as the always-inline body was not inlined.

Note that the initial callgraph is already wrong:

Initial callgraph:

open/0 @0x7ffff5a2b6c0 (asm: open) analyzed needed reachable body finalized redefined_extern_inline
  called by: open/0 (1.00 per call)
  calls: open/0 (1.00 per call)
  References:
  Refering this function:
variable pool:

as said, the C frontend needs to create two decls for open for this to
properly work.

OTOH, it is time to deprecate this extension and warn about it (after
all we miscompile this since quite some time, GCC 3.3 and 4.1 already produce
the recursive open - how was this intended to work? ...)  I don't have
3.2 (and 2.95 does not have always_inline).

The "proper" way to write the testcase is

extern __inline __attribute__ ((__always_inline__,__gnu_inline__))
void open ()
{
}
void open_1 () asm("open");
void open_1 ()
{
  open ();
}

So, Joseph - would you be fine with changing behavior for this testcase
from ICEing at -On, miscompiling at -O0 to rejecting it?  Thus,

Index: c-decl.c
===================================================================
--- c-decl.c    (revision 183121)
+++ c-decl.c    (working copy)
@@ -1855,21 +1855,10 @@ diagnose_mismatched_decls (tree newdecl,
        {
          if (DECL_INITIAL (olddecl))
            {
-             /* If both decls are in the same TU and the new declaration
-                isn't overriding an extern inline reject the new decl.
+             /* If both decls are in the same TU reject the new decl.
                 In c99, no overriding is allowed in the same translation
                 unit.  */
-             if ((!DECL_EXTERN_INLINE (olddecl)
-                  || DECL_EXTERN_INLINE (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))))
-                 )
-                 && same_translation_unit_p (newdecl, olddecl))
+             if (same_translation_unit_p (newdecl, olddecl))
                {
                  error ("redefinition of %q+D", newdecl);
                  locate_old_decl (olddecl);
Comment 24 Jan Hubicka 2012-01-12 14:22:52 UTC
> OTOH, it is time to deprecate this extension and warn about it (after
> all we miscompile this since quite some time, GCC 3.3 and 4.1 already produce
> the recursive open - how was this intended to work? ...)  I don't have
> 3.2 (and 2.95 does not have always_inline).

pre cgraph compilers handled it in a way that inline body was kept after parsing
extern inline version and inlined into every new parsed function until
the offline version was reached. Then the function was marked uninlinable,
offline body was produced and all subsequentely parsed calls was not inlined
(including calls in the offline body).

I think extern inlines are sadly rather common to be deprecated...

Honza
Comment 25 rguenther@suse.de 2012-01-12 14:27:12 UTC
On Thu, 12 Jan 2012, hubicka at ucw dot cz wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33763
> 
> --- Comment #24 from Jan Hubicka <hubicka at ucw dot cz> 2012-01-12 14:22:52 UTC ---
> > OTOH, it is time to deprecate this extension and warn about it (after
> > all we miscompile this since quite some time, GCC 3.3 and 4.1 already produce
> > the recursive open - how was this intended to work? ...)  I don't have
> > 3.2 (and 2.95 does not have always_inline).
> 
> pre cgraph compilers handled it in a way that inline body was kept after
> parsing
> extern inline version and inlined into every new parsed function until
> the offline version was reached. Then the function was marked uninlinable,
> offline body was produced and all subsequentely parsed calls was not inlined
> (including calls in the offline body).
> 
> I think extern inlines are sadly rather common to be deprecated...

Well, not deprecating extern inlines but re-definitions in the
same TU which miscompiles once the extern inline is used in that
TU (ok, so you can say in 99% of all cases that won't happen).

Thus, another workaround would be to avoid transitioning the
attribute to the redefinition in merge_decls (under the exact
same condition as we allow the redefinition).

Richard.
Comment 26 Richard Biener 2012-01-12 14:30:31 UTC
The patch fails to bootstrap in libquadmath btw:

/space/rguenther/src/svn/trunk/libquadmath/math/cimagq.c:24:1: error: redefinition of 'cimagq'
In file included from /space/rguenther/src/svn/trunk/libquadmath/quadmath-imp.h:26:0,
                 from /space/rguenther/src/svn/trunk/libquadmath/math/cimagq.c:21:
/space/rguenther/src/svn/trunk/libquadmath/quadmath.h:175:1: note: previous definition of 'cimagq' was here
make[3]: *** [math/cimagq.lo] Error 1

which seems to make heavy use of gnu-inline always-inline.
Comment 27 Richard Biener 2012-01-12 14:31:28 UTC
(In reply to comment #25)
> On Thu, 12 Jan 2012, hubicka at ucw dot cz wrote:
> 
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33763
> > 
> > --- Comment #24 from Jan Hubicka <hubicka at ucw dot cz> 2012-01-12 14:22:52 UTC ---
> > > OTOH, it is time to deprecate this extension and warn about it (after
> > > all we miscompile this since quite some time, GCC 3.3 and 4.1 already produce
> > > the recursive open - how was this intended to work? ...)  I don't have
> > > 3.2 (and 2.95 does not have always_inline).
> > 
> > pre cgraph compilers handled it in a way that inline body was kept after
> > parsing
> > extern inline version and inlined into every new parsed function until
> > the offline version was reached. Then the function was marked uninlinable,
> > offline body was produced and all subsequentely parsed calls was not inlined
> > (including calls in the offline body).
> > 
> > I think extern inlines are sadly rather common to be deprecated...
> 
> Well, not deprecating extern inlines but re-definitions in the
> same TU which miscompiles once the extern inline is used in that
> TU (ok, so you can say in 99% of all cases that won't happen).
> 
> Thus, another workaround would be to avoid transitioning the
> attribute to the redefinition in merge_decls (under the exact
> same condition as we allow the redefinition).

Only a workaround for the ICE, of course, not the miscompile (can
anyone check if we ever did not miscompile this case?)
Comment 28 joseph@codesourcery.com 2012-01-12 14:35:58 UTC
On Thu, 12 Jan 2012, rguenther at suse dot de wrote:

> > I think extern inlines are sadly rather common to be deprecated...
> 
> Well, not deprecating extern inlines but re-definitions in the
> same TU which miscompiles once the extern inline is used in that
> TU (ok, so you can say in 99% of all cases that won't happen).

I haven't tested it, but I'd expect extern inline + redefinition to be 
occur in various cases building glibc, or any other library with extern 
inlines in its headers.  Though those cases may be less likely then to use 
the redefined function in the same TU.
Comment 29 Richard Biener 2012-01-12 14:49:42 UTC
Btw, GCC 3.2.3 produces for

extern __inline __attribute__ ((__always_inline__))
void open ()
{
}
void open ()
{
  open ();
}

open:
        pushl   %ebp
        movl    %esp, %ebp
        call    open
        popl    %ebp
        ret

so it's "broken" as well.
Comment 30 Richard Biener 2012-01-12 14:54:04 UTC
Of course the question is what we should really do here wrt name-lookup.
Comment 31 Richard Biener 2012-01-13 09:26:45 UTC
Ok, so the following patch makes

extern __inline __attribute__ ((__always_inline__))
void open ()
{
}
void bar ()
{
  open ();
}
void open ()
{
  open ();
}

"work" as in the extern inline variant is used for the callin bar()
and the call in open () is recursive and all following calls would
refer to it.  That means, we end up with two cgraph nodes (but do
not magically use one for inlining and one for the offline copy - that
would need to be fixed in the C frontend by _not_ changing name-lookup
to lookup the 2nd version, but I'm not sure that is desired).

At least you can say the behavior makes sense and it avoids the ICEs, too.

I'm going to test this.

Index: gcc/c-decl.c
===================================================================
--- gcc/c-decl.c        (revision 183121)
+++ gcc/c-decl.c        (working copy)
@@ -2513,6 +2513,24 @@ duplicate_decls (tree newdecl, tree oldd
       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;
 }
Comment 32 Jakub Jelinek 2012-03-13 12:47:59 UTC
4.4 branch is being closed, moving to 4.5.4 target.
Comment 33 Richard Biener 2012-07-02 11:50:11 UTC
The 4.5 branch is being closed, adjusting target milestone.
Comment 34 Daniel Schepler 2012-07-09 15:02:42 UTC
(In reply to comment #31)
Just so you know, the proposed patch would break glibc builds.  Here's a reduced test case that reproduces an issue compiling glibc's s_isnan.c on amd64...

extern int foo() __attribute__((__const__, __nothrow__));
extern int foo() __asm__("__GI_foo") __attribute__ ((visibility("hidden")));
extern __inline int __attribute__((__always_inline__)) foo() {
  return 0;
}
int foo() {
  return 0;
}
extern __typeof__(foo) __EI_foo __asm__("foo") __attribute__((alias("__GI_foo")));

On Debian, gcc-4.6 compiles this code OK, while gcc-4.7 (which has this patch applied) errors out with:
/tmp/pr33763_broken.c:9:24: error: '__EI_foo' aliased to external symbol '__GI_foo'
Comment 35 Richard Biener 2012-07-09 15:39:35 UTC
(In reply to comment #34)
> (In reply to comment #31)
> Just so you know, the proposed patch would break glibc builds.  Here's a
> reduced test case that reproduces an issue compiling glibc's s_isnan.c on
> amd64...
> 
> extern int foo() __attribute__((__const__, __nothrow__));
> extern int foo() __asm__("__GI_foo") __attribute__ ((visibility("hidden")));
> extern __inline int __attribute__((__always_inline__)) foo() {
>   return 0;
> }
> int foo() {
>   return 0;
> }
> extern __typeof__(foo) __EI_foo __asm__("foo")
> __attribute__((alias("__GI_foo")));
> 
> On Debian, gcc-4.6 compiles this code OK, while gcc-4.7 (which has this patch
> applied) errors out with:
> /tmp/pr33763_broken.c:9:24: error: '__EI_foo' aliased to external symbol
> '__GI_foo'

Yes, I know - the patch had fallout in the testsuite and I dropped the ball
on this bugreport.
Comment 36 Jakub Jelinek 2012-10-05 11:43:43 UTC
Author: jakub
Date: Fri Oct  5 11:43:38 2012
New Revision: 192119

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=192119
Log:
	PR tree-optimization/33763
	* tree-inline.c (expand_call_inline): Silently ignore always_inline
	attribute for redefined extern inline functions.

	* c-c++-common/pr33763.c: New test.

Added:
    trunk/gcc/testsuite/c-c++-common/pr33763.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-inline.c
Comment 37 Jakub Jelinek 2012-10-05 11:58:49 UTC
Author: jakub
Date: Fri Oct  5 11:58:46 2012
New Revision: 192121

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=192121
Log:
	PR tree-optimization/33763
	* tree-inline.c (expand_call_inline): Silently ignore always_inline
	attribute for redefined extern inline functions.

	* c-c++-common/pr33763.c: New test.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/c-c++-common/pr33763.c
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_7-branch/gcc/tree-inline.c
Comment 38 Jakub Jelinek 2012-10-05 12:02:03 UTC
Author: jakub
Date: Fri Oct  5 12:01:59 2012
New Revision: 192122

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=192122
Log:
	PR tree-optimization/33763
	* tree-inline.c (expand_call_inline): Silently ignore always_inline
	attribute for redefined extern inline functions.

	* c-c++-common/pr33763.c: New test.

Added:
    branches/gcc-4_6-branch/gcc/testsuite/c-c++-common/pr33763.c
Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_6-branch/gcc/tree-inline.c
Comment 39 Steven Bosscher 2012-11-11 21:27:12 UTC
Is there still a bug here, after the commits of comment #36 to #38?
Comment 40 Jakub Jelinek 2012-11-11 21:28:46 UTC
Fixed.
Comment 41 Juha Kallioinen 2013-09-20 22:14:55 UTC
Maybe this failure with a newly built gcc 4.8 is related to this bug.

Built gcc today from
commit a846b696b9f3e9b9caab7e43ef8450cbded2715c
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gcc-4_8-branch@202762 138bc75d-0d04-0410-961f-82ee72b

~/gcc48/bin$ ./gcc -v
Using built-in specs.
COLLECT_GCC=./gcc
COLLECT_LTO_WRAPPER=/home/kaltsi/gcc48/libexec/gcc/x86_64-unknown-linux-gnu/4.8.2/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../gcc/configure --prefix=/home/kaltsi/gcc48 --disable-bootstrap --with-tune=generic --disable-multilib --enable-checking=release --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-lto --enable-linker-build-id --enable-languages=c,c++ --enable-threads=posix --enable-shared --disable-libgcj
Thread model: posix
gcc version 4.8.2 20130920 (prerelease) (GCC) 

Compiling the following simple program with these options on an Ubuntu 12.04 causes a failure:

$ gcc -Wp,-D_FORTIFY_SOURCE=2 -O2 -D_GNU_SOURCE=1




-- foo.c --
#include <sys/socket.h>
 
extern ssize_t recvfrom(int s, void *buf, size_t len, int flags, struct sockaddr *from, socklen_t *fromlen);
 
int main()
{
  int s, flags; void *buf; size_t len; struct sockaddr *from; socklen_t* fromlen;
  return(recvfrom(s, buf, len, flags, from, fromlen));
}

-- foo.c --

~/gcc48/bin$ ./gcc -Wp,-D_FORTIFY_SOURCE=2 -O2 -D_GNU_SOURCE=1 foo.c
In file included from /usr/include/x86_64-linux-gnu/sys/socket.h:251:0,
                 from foo.c:5:
foo.c: In function ‘main’:
/usr/include/x86_64-linux-gnu/bits/socket2.h:65:1: error: inlining failed in call to always_inline ‘recvfrom’: mismatched arguments
 recvfrom (int __fd, void *__restrict __buf, size_t __n, int __flags,
 ^
foo.c:12:18: error: called from here
   return(recvfrom(s, buf, len, flags, from, fromlen));
                  ^
Comment 42 Jakub Jelinek 2013-09-20 22:20:56 UTC
(In reply to Juha Kallioinen from comment #41)
> Maybe this failure with a newly built gcc 4.8 is related to this bug.

No, that is just a buggy testcase.  Redefining a prototype, especially if you have no idea how the system function prototype looks like, is a bug.
In the testcaseif you redefine the prototype to something different and obviously it then fails to inline it.  Don't do that.
Comment 43 Juha Kallioinen 2013-09-20 22:25:54 UTC
(In reply to Jakub Jelinek from comment #42)
> (In reply to Juha Kallioinen from comment #41)
> > Maybe this failure with a newly built gcc 4.8 is related to this bug.
> 
> No, that is just a buggy testcase.  Redefining a prototype, especially if
> you have no idea how the system function prototype looks like, is a bug.
> In the testcaseif you redefine the prototype to something different and
> obviously it then fails to inline it.  Don't do that.

This was just a reduction of a piece of code that started to fail with gcc 4.8, but compiles with earlier versions. I'll try to understand what is different in the prototype redefinition.