| Summary: | [4.0 Regression] redefinition of inline function succeeds | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Eric Christopher <echristo> |
| Component: | c | Assignee: | Andrew Pinski <pinskia> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | akim, doko, gcc-bugs, j, jsm28 |
| Priority: | P2 | Keywords: | accepts-invalid, patch |
| Version: | 4.0.0 | ||
| Target Milestone: | 4.0.3 | ||
| URL: | http://gcc.gnu.org/ml/gcc-patches/2005-10/msg00050.html | ||
| Host: | Target: | ||
| Build: | Known to work: | 3.4.4 4.1.0 | |
| Known to fail: | 4.0.0 | Last reconfirmed: | 2005-06-15 18:25:47 |
| Bug Depends on: | 23104 | ||
| Bug Blocks: | 16989, 21975 | ||
| Attachments: |
diff1
diff2 Simplier patch |
||
Confirmed, this started after "3.5.0 20040909". It started to fail after "4.0.0 20041124" but before "4.0.0 20050225" (the branching). 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. 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);
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). Created attachment 9083 [details]
diff1
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
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. I'll take this. I've got a patch just completing testing. 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 Created attachment 9095 [details]
diff2
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.
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; }
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.
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 This is now rejected like it should on the mainline. *** Bug 23474 has been marked as a duplicate of this bug. *** *** Bug 21975 has been marked as a duplicate of this bug. *** 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. Created attachment 9743 [details] Simplier patch This is the simplier patch which should fix this on the 4.0 branch without causing PR 23104 regression. *** Bug 24140 has been marked as a duplicate of this bug. *** Patch posted here: http://gcc.gnu.org/ml/gcc-patches/2005-10/msg00050.html Fixed the correct way in 4.0.3 which does not expose PR 23104. 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 *** Bug 24656 has been marked as a duplicate of this bug. *** |
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.