Bug 85678 - -fno-common should be default
Summary: -fno-common should be default
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 9.0
: P3 normal
Target Milestone: 10.0
Assignee: Not yet assigned to anyone
URL:
Keywords: patch
Depends on:
Blocks: 96284
  Show dependency treegraph
 
Reported: 2018-05-07 11:25 UTC by David Brown
Modified: 2024-06-04 01:41 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-05-08 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Brown 2018-05-07 11:25:58 UTC
The use of a "common" section to match up tentative object definitions in different translation units in a program is contrary to the C standards (section 6.9p5 in C99 and C11, section 6.7 in C90), gives a high risk of error, may lead to poorer quality code, and provides no benefits except backwards compatibility with ancient code.

Unix C compilers have traditionally allowed "int x;" in file a.c to be allocated by the linker to the same storage as "int x;" in file b.c when these two C files are linked together.  gcc supports this if "-fcommon" is enabled, which is the default on most targets.  But it is contrary to the C (and C++) standards that say there shall be exactly /one/ definition of each object.  And it is very easy to have errors in code by accidentally having duplicate names in different files (if the programmer has not used "static"), perhaps even of different types.  With "-fno-common", such errors are caught at link time.

Also, data in common sections will hinder some types of optimisation, such as "-fsection-anchors".

Surely it is time to make "-fno-common" the default, at least when a modern C standard is specified indicating that the code is modern?  People who need the old behaviour can always get it with "-fcommon".
Comment 1 Eric Gallager 2018-05-07 15:20:35 UTC
I think this discussion has been had on either the main gcc mailing list or gcc-patches recently, but I forget the link...
Comment 2 Andrew Pinski 2018-05-07 15:23:10 UTC
https://gcc.gnu.org/ml/gcc/2017-09/msg00088.html
Comment 3 Richard Biener 2018-05-08 10:31:12 UTC
Confirmed (and agreed).
Comment 4 Eric Gallager 2019-10-24 22:00:40 UTC
This keeps getting brought up in bug 91766 (already added as related from the other end)
Comment 5 Jonathan Wakely 2019-10-25 08:28:09 UTC
The other bug links to a patch to change the default:

https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01549.html
Comment 6 Wilco 2019-10-25 15:47:59 UTC
(In reply to Jonathan Wakely from comment #5)
> The other bug links to a patch to change the default:
> 
> https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01549.html

Updated patch: https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01847.html
Comment 7 David Binderman 2019-11-25 16:39:28 UTC
(In reply to David Brown from comment #0)
> Surely it is time to make "-fno-common" the default, at least when a modern
> C standard is specified indicating that the code is modern?  People who need
> the old behaviour can always get it with "-fcommon".

Interestingly, use of -std=c89 or -std=gnu89 doesn't also switch on -fcommon
to get old behaviour.

So use of compiler flag indicating to compile code to old standards
means implicit use of *new* standard for common.

Looks odd to me. Possible bug ?
Comment 8 Wilco 2019-11-25 16:55:32 UTC
(In reply to David Binderman from comment #7)
> (In reply to David Brown from comment #0)
> > Surely it is time to make "-fno-common" the default, at least when a modern
> > C standard is specified indicating that the code is modern?  People who need
> > the old behaviour can always get it with "-fcommon".
> 
> Interestingly, use of -std=c89 or -std=gnu89 doesn't also switch on -fcommon
> to get old behaviour.
> 
> So use of compiler flag indicating to compile code to old standards
> means implicit use of *new* standard for common.
> 
> Looks odd to me. Possible bug ?

If required, it would be feasible to keep the old behaviour for C89 indeed, however -fno-common is not incompatible with C89 (embedded C compilers may not even support -fcommon).
Comment 9 Jonathan Wakely 2019-11-25 17:09:18 UTC
C89 6.7p4 looks equivalent to C99 6.9p5, so I don't see why -std=c89 should imply -fcommon. It's just as bad in C89 as in later standards.
Comment 10 David Binderman 2019-11-25 17:28:20 UTC
(In reply to Jonathan Wakely from comment #9)
> C89 6.7p4 looks equivalent to C99 6.9p5, so I don't see why -std=c89 should
> imply -fcommon.

To reduce costs in upgrading to post-revision 278509 compilers.

-std=c89 implies old code. Old code relies on old behaviour. Providing
old behaviour by default means that compiler users don't have to
suddenly put "-fcommon" in all their old code Makefiles to get compilation.

I am not sure about intermediate cases like c99, but certainly
newer language standards (c11, ...) can adopt the new behaviour.
Comment 11 David Brown 2019-11-25 18:12:52 UTC
Reliance on -fcommon has not been correct or compatible with any C standard (I don't think it was even right for K&R C).  If the code is written correctly (with at most one definition of all externally linked symbols) then -fcommon does not make it incorrect or conflict with the standards.  But if your code requires -fcommon to compile correctly, it does not conform to C89 or anything newer.

Personally, I'd like to see many old and dangerous C practices give errors by default.  Along with common symbols, I'd include non-prototype function declarations and definitions, and calling functions without declaring them.  These are the kind of thing you'd expect to see in pre-ANSI C - other than that, they  are probably errors in the code.  It is good that gcc still supports such code, but I think making them errors by default will reduce bugs in current code.  And surely that is worth the cost of adding a "-fold-code" flag to ancient build recipes?  (The "-fold-code" flag could probably also imply "-fno-strict-aliasing -fwrapv" to deal with other assumptions sometimes made in older code.)

There is precedence for this.  The default standard for gcc changed from "gnu89" to "gnu11".  While most "gnu89" code will compile with the same semantics in "gnu11" mode, there are a fair number of incompatibilities.  Changing the default to "-fno-common" (and ideally "-Werror=strict-prototypes -Werror=old-style-declaration -Werror=missing-parameter-type") would have a lot smaller impact than changing the default standard.
Comment 12 Wilco 2019-11-25 19:33:15 UTC
(In reply to David Brown from comment #11)

> Changing the default to "-fno-common" (and ideally
> "-Werror=strict-prototypes -Werror=old-style-declaration
> -Werror=missing-parameter-type") would have a lot smaller impact than
> changing the default standard.

Giving errors on old-style code by default sounds like a good idea. We could add -std=legacy similar to Fortran to support building old K&R code (and that would enable -fcommon by default).
Comment 13 Florian Weimer 2019-11-25 19:53:26 UTC
(In reply to Wilco from comment #12)
> Giving errors on old-style code by default sounds like a good idea. We could
> add -std=legacy similar to Fortran to support building old K&R code (and
> that would enable -fcommon by default).

It's unfortunately not that simple. A lot of these changes (admittedly I've only tried -Werror=implicit-function-declaration by default, something that trips even experienced developers) tend to produce broken, but internally consistent builds because autoconf checks give wrong results (and everything, including the test suite, depends on those results). This was true for a significant number of core GNU components until this year, including GCC itself.
Comment 14 Eric Gallager 2019-11-25 19:59:44 UTC
(In reply to David Brown from comment #11)
> Reliance on -fcommon has not been correct or compatible with any C standard
> (I don't think it was even right for K&R C).  If the code is written
> correctly (with at most one definition of all externally linked symbols)
> then -fcommon does not make it incorrect or conflict with the standards. 
> But if your code requires -fcommon to compile correctly, it does not conform
> to C89 or anything newer.
> 
> Personally, I'd like to see many old and dangerous C practices give errors
> by default.  Along with common symbols, I'd include non-prototype function
> declarations and definitions, and calling functions without declaring them. 
> These are the kind of thing you'd expect to see in pre-ANSI C - other than
> that, they  are probably errors in the code.  It is good that gcc still
> supports such code, but I think making them errors by default will reduce
> bugs in current code.  And surely that is worth the cost of adding a
> "-fold-code" flag to ancient build recipes?  (The "-fold-code" flag could
> probably also imply "-fno-strict-aliasing -fwrapv" to deal with other
> assumptions sometimes made in older code.)
> 
> There is precedence for this.  The default standard for gcc changed from
> "gnu89" to "gnu11".  While most "gnu89" code will compile with the same
> semantics in "gnu11" mode, there are a fair number of incompatibilities. 
> Changing the default to "-fno-common" (and ideally
> "-Werror=strict-prototypes -Werror=old-style-declaration
> -Werror=missing-parameter-type") would have a lot smaller impact than
> changing the default standard.

Related: bug 82922
Comment 15 David Brown 2020-07-22 13:12:16 UTC
This has been implemented in gcc 10, and -fno-common is now the default.  This "bug" can presumably now be closed.

Many thanks to the gcc developers here.
Comment 16 Sam James 2023-04-24 22:45:43 UTC
For the record, this was done on the Clang side as https://reviews.llvm.org/D75056.

Also adding -Wimplicit-function-declaration to See Also, along with -Wimplicit-int and stricter prototypes given they're all related to the same era + need care in distributions first (which Florian and I are working on).

(Maybe we want a tracker for these, not sure.)
Comment 17 Sam James 2023-04-24 22:55:24 UTC
(In reply to David Brown from comment #15)
> This has been implemented in gcc 10, and -fno-common is now the default. 
> This "bug" can presumably now be closed.
> 
> Many thanks to the gcc developers here.

Done in r10-4867-g6271dd984d7f92.