First Last Prev Next    No search results available      Search page      Enter new bug
Bug#: 22052
Product:  
Component:  
Status: RESOLVED
Resolution: FIXED
Assigned To: Andrew Pinski <pinskia@gcc.gnu.org>
Host:
Reported against  
Priority:  
Severity:  
Target Milestone:  
 
 
Target:
Reporter: Eric Christopher <echristo@apple.com>
Add CC:
CC:
Remove selected CCs
Build:
Patch URL:
Summary:
Keywords:
Known to work:
Known to fail:

Attachment Description Type Created Size Actions
c-decl.c.diff diff1 patch 2005-06-14 01:10 1008 bytes Edit | Diff
c-decl.c.diff diff2 patch 2005-06-15 20:50 1.26 KB Edit | Diff
t.diff.txt Simplier patch patch 2005-09-16 18:50 503 bytes Edit | Diff
Create a New Attachment (proposed patch, testcase, etc.) View All

Bug 22052 depends on: 23104 Show dependency tree
Show dependency graph
Bug 22052 blocks: 16989 21975

Additional Comments:






View Bug Activity   |   Format For Printing   |   Clone This Bug


Description:   Last confirmed: 2005-06-15 18:25 Opened: 2005-06-13 19:02
This doesn't error out:

#include <stdio.h>
static inline int foo(void) {     return 1; }
static inline int foo(void) {     return 0; }
static inline int bar(void) {     return foo(); }
int main(void)
{
    if (bar())
      printf("1\n");
    else
      printf("0\n");

    return 0;
}

It appears to be a problem with inlining since if the inline keyword is removed
it works (well, fails) as expected.

------- Comment #1 From Andrew Pinski 2005-06-13 19:07 -------
Confirmed, this started after "3.5.0 20040909".

------- Comment #2 From Andrew Pinski 2005-06-13 19:11 -------
It started to fail after "4.0.0 20041124" but before "4.0.0 20050225" (the
branching).

------- Comment #3 From Andrew Pinski 2005-06-13 19:17 -------
Hmm, the most obvious patch (which is mine) is the following:
2005-01-04  Andrew Pinski  <pinskia@physics.uc.edu>

        PR c/19152
        * c-decl.c (diagnose_mismatched_decls): Accept "extern inline" declared
        after the full declaration if the are in two different TUs.

------- Comment #4 From Andrew Pinski 2005-06-13 19:35 -------
This patch should fix it (I don't have time right now, school is getting in the
way, could you test it):
Index: c-decl.c
===============================================================
====
RCS file: /cvs/gcc/gcc/gcc/c-decl.c,v
retrieving revision 1.662
diff -u -p -r1.662 c-decl.c
--- c-decl.c    6 Jun 2005 19:31:24 -0000       1.662
+++ c-decl.c    13 Jun 2005 19:35:08 -0000
@@ -1318,9 +1318,7 @@ diagnose_mismatched_decls (tree newdecl,
                  return false;
                }
              /* If both decls have not extern inline, reject the new decl.  */
-             if (!DECL_DECLARED_INLINE_P (olddecl)
-                 && !DECL_EXTERNAL (olddecl)
-                 && !DECL_DECLARED_INLINE_P (newdecl)
+             if (!DECL_EXTERNAL (olddecl)
                  && !DECL_EXTERNAL (newdecl))
                {
                  error ("%Jredefinition of %qD", newdecl, newdecl);

------- Comment #5 From Joseph S. Myers 2005-06-13 23:18 -------
There is a case the multiple static inline definitions of the same function in
a
translation unit violate the constraint of  6.9#3 (no more than one external
definition of an identifier with internal linkage in the same translation
unit),
if you take the definition of "inline definition" in 6.7.4#6 as only applying
to
functions with external linkage.

In general C99 presumes there is no more than one definition of a function in a
translation unit (in 6.7.4#6) but has no constraint requiring this to be
diagnosed.  I think it would be reasonable in C99 mode, as part of an
implementation of C99 inline semantics, to prohibit all such cases.  inline in
gnu89 mode should remain compatible with traditional GCC semantics, which in
the
case of the present bug means rejecting the problem code but in some cases may
mean accepting it (in particular where one definition is extern inline).

------- Comment #6 From Eric Christopher 2005-06-14 01:10 -------
Created an attachment (id=9083) [edit]
diff1

------- Comment #7 From Eric Christopher 2005-06-14 01:11 -------
Joseph,

So, what you're saying is that we should accept this:

extern inline int foo (void) { return 0; }
inline int foo (void) { return 1; }

unless we're in strict c99 mode.

And reject this:

static inline int foo (void) { return 0; }
static inline int foo (void) { return 1; }

in all cases?

I'm trying to get a set of tests up for everything we should and shouldn't accept :)

Current diff is attached. Mostly untested except for the above testcases.

-eric

------- Comment #8 From joseph@codesourcery.com 2005-06-14 01:19 -------
Subject: Re:  [4.0/4.1 Regression] redefinition of inline function
 succeeds

On Tue, 14 Jun 2005, echristo at redhat dot com wrote:

> So, what you're saying is that we should accept this:
> 
> extern inline int foo (void) { return 0; }
> inline int foo (void) { return 1; }
> 
> unless we're in strict c99 mode.

A C99 conditional is pointless and possibly harmful for this case without 
the rest of C99 semantics, since "inline" and "extern inline" are 
(approximately) the other way round in C99 from in gnu89.  (It's only 
approximate; for example,

inline int foo (void) { return 1; }

on its own is an inline definition only in C99 but

inline int foo (void) { return 1; }
int foo(void);

yields an external definition in C99: any declaration of foo without 
inline or with extern causes it to be exported.)

So, just accept all cases GCC has traditionally accepted here without 
regard to language version for now.

> And reject this:
> 
> static inline int foo (void) { return 0; }
> static inline int foo (void) { return 1; }
> 
> in all cases?

Yes.


------- Comment #9 From Eric Christopher 2005-06-15 18:25 -------
I'll take this. I've got a patch just completing testing.

------- Comment #10 From Eric Christopher 2005-06-15 20:49 -------
OK. This diff works, and passes bootstrap with no regressions for both
--enable-intermodule and normally on x86-linux. The --enable-intermodule bits
are why we split this out in the first place, but same_translation_unit_p in the
condition seems to handle that case.

Joseph: What do you think?

-eric

------- Comment #11 From Eric Christopher 2005-06-15 20:50 -------
Created an attachment (id=9095) [edit]
diff2

------- Comment #12 From joseph@codesourcery.com 2005-06-16 12:41 -------
Subject: Re:  [4.0/4.1 Regression] redefinition of inline function
 succeeds

On Wed, 15 Jun 2005, echristo at redhat dot com wrote:

> OK. This diff works, and passes bootstrap with no regressions for both
> --enable-intermodule and normally on x86-linux. The --enable-intermodule bits
> are why we split this out in the first place, but same_translation_unit_p in the
> condition seems to handle that case.
> 
> Joseph: What do you think?

I'm more interested in the testcases here than in the front-end changes 
which make the testcases pass.


------- Comment #13 From Eric Christopher 2005-06-17 20:21 -------
Here are the 4 testcases that I've been using to test, the first two you've
seen. (and no, the dg stuff isn't done yet, but that's merely to make the
testsuite happy.)

test1:
/* { dg-do compile } */
/* This test is expected to fail with an error for the redefinition of foo.
   This violates the constraint of 6.9#3 (no more than one external definition
   of an identifier with internal linkage in the same translation unit).  */
static inline int foo(void) { return 1; }
static inline int foo(void) { return 0; } /* { dg-error "" } */

test2:
/* { dg-do compile } */
/* This test should compile successfully.  */
extern inline int foo(void) { return 0; }
inline int foo (void) { return 1; }

test3:
/* { dg-do compile } */
/* This testcase should fail since we're redefining foo in the same
   translation unit.  */
extern inline int foo(void) { return 0; }
inline int foo (void) { return 1; }
int foo (void) { return 2; } /* { dg-error "error: redefinition of 'foo'" } */

test4:
/* { dg-do compile } */
/* This testcase should fail since we're redefining foo in the same
   translation unit.  */
int foo (void) { return 2; }
extern inline int foo (void) { return 1; }

------- Comment #14 From joseph@codesourcery.com 2005-06-17 20:48 -------
Subject: Re:  [4.0/4.1 Regression] redefinition of inline function
 succeeds

On Fri, 17 Jun 2005, echristo at redhat dot com wrote:

> Here are the 4 testcases that I've been using to test, the first two you've
> seen. (and no, the dg stuff isn't done yet, but that's merely to make the
> testsuite happy.)

These results seem OK.


------- Comment #15 From CVS Commits 2005-06-29 00:11 -------
Subject: Bug 22052

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	echristo@gcc.gnu.org	2005-06-29 00:11:37

Modified files:
	gcc            : c-decl.c ChangeLog 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/gcc.dg: inline1.c inline2.c inline3.c inline4.c 
	                      inline5.c 

Log message:
	2005-06-28  Eric Christopher  <echristo@redhat.com>
	
	PR c/22052
	PR c/21975
	* c-decl.c (diagnose_mismatched_decls): Define DECL_EXTERN_INLINE.
	Use. Fix detection of invalid extern inline redefinition.
	
	2005-06-28  Eric Christopher  <echristo@redhat.com>
	
	PR c/22052
	PR c/21975
	* gcc.dg/inline1.c: New test.
	* gcc.dg/inline2.c: Ditto.
	* gcc.dg/inline3.c: Ditto.
	* gcc.dg/inline4.c: Ditto.
	* gcc.dg/inline5.c: Ditto.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/c-decl.c.diff?cvsroot=gcc&r1=1.668&r2=1.669
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.9267&r2=2.9268
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.5702&r2=1.5703
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/inline1.c.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/inline2.c.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/inline3.c.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/inline4.c.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/inline5.c.diff?cvsroot=gcc&r1=NONE&r2=1.1


------- Comment #16 From Andrew Pinski 2005-06-29 01:59 -------
This is now rejected like it should on the mainline.

------- Comment #17 From Andrew Pinski 2005-08-19 02:36 -------
*** Bug 23474 has been marked as a duplicate of this bug. ***

------- Comment #18 From Andrew Pinski 2005-08-19 02:37 -------
*** Bug 21975 has been marked as a duplicate of this bug. ***

------- Comment #19 From Andrew Pinski 2005-09-16 18:43 -------
I have a fix for the mainline which also fixes this and 23104 and reverting
back to the old code which 
was added for 19152.

------- Comment #20 From Andrew Pinski 2005-09-16 18:50 -------
Created an attachment (id=9743) [edit]
Simplier patch

This is the simplier patch which should fix this on the 4.0 branch without
causing PR 23104 regression.

------- Comment #21 From Andrew Pinski 2005-09-30 11:46 -------
*** Bug 24140 has been marked as a duplicate of this bug. ***

------- Comment #22 From Andrew Pinski 2005-10-02 19:00 -------
Patch posted here:
http://gcc.gnu.org/ml/gcc-patches/2005-10/msg00050.html

------- Comment #23 From Andrew Pinski 2005-10-04 19:41 -------
Fixed the correct way in 4.0.3 which does not expose PR 23104.

------- Comment #24 From CVS Commits 2005-10-04 19:41 -------
Subject: Bug 22052

CVSROOT:        /cvs/gcc
Module name:    gcc
Branch:         gcc-4_0-branch
Changes by:     pinskia@gcc.gnu.org     2005-10-04 19:41:47

Modified files:
        gcc            : ChangeLog c-decl.c 
        gcc/testsuite  : ChangeLog 
Added files:
        gcc/testsuite/gcc.dg: inline1.c inline2.c inline3.c inline4.c 

Log message:
        2005-10-04  Andrew Pinski  <pinskia@physics.uc.edu>

        PR c/22052
        PR c/21975
        * c-decl.c (diagnose_mismatched_decls): Fix check for both
        decls being "extern inline".
        2005-10-04  Eric Christopher  <echristo@apple.com>

        PR c/22052
        PR c/21975
        * gcc.dg/inline1.c: New test.
        * gcc.dg/inline2.c: Ditto.
        * gcc.dg/inline3.c: Ditto.
        * gcc.dg/inline4.c: Ditto.
        * gcc.dg/inline5.c: Ditto.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=2.7592.2.445&r2=2.7592.2.446
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/c-decl.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.630.6.20&r2=1.630.6.21
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.5084.2.424&r2=1.5084.2.425
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/inline1.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=NONE&r2=1.1.18.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/inline2.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=NONE&r2=1.1.18.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/inline3.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=NONE&r2=1.1.18.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/inline4.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=NONE&r2=1.1.18.1

------- Comment #25 From Andrew Pinski 2005-11-03 15:30 -------
*** Bug 24656 has been marked as a duplicate of this bug. ***

First Last Prev Next    No search results available      Search page      Enter new bug