diff mbox series

[v2,2/2] config.mak.dev: build with -fno-common

Message ID 20201015193029.GB1129761@coredump.intra.peff.net (mailing list archive)
State Accepted
Commit c888cb8b283b14d9ee85e5324dabb7a84f6536ef
Headers show
Series [v2,1/2] usage: define a type for a reporting function | expand

Commit Message

Jeff King Oct. 15, 2020, 7:30 p.m. UTC
It's an easy mistake to define a variable in a header with "int x;" when
you really meant to only declare the variable as "extern int x;"
instead. Clang and gcc will both allow this when building with
"-fcommon"; they put these "tentative definitions" in a common block
which the linker is able to resolve.

This is the default in clang and was the default in gcc until gcc-10,
since it helps some legacy code. However, we would prefer not to rely on
this because:

  - using "extern" makes the intent more clear (so it's a style issue,
    but it's one the compiler can help us catch)

  - according to the gcc manpage, it may yield a speed and code size
    penalty

So let's build explicitly with -fno-common when the DEVELOPER knob is
set, which will let developers using clang and older versions of gcc
notice these problems.

I didn't bother making this conditional on a particular version of gcc.
As far as I know, this option has been available forever in both gcc and
clang, so old versions don't need to avoid it. And we already expect gcc
and clang options throughout config.mak.dev, so it's unlikely anybody
setting the DEVELOPER knob is using anything else. It's a noop on
gcc-10, of course, but it's not worth trying to exclude it there.

Note that there's nothing to fix in the code; we already don't have any
issues here. But if you want to test the patch, you can add a bare "int
x;" into cache.h, which will cause the link step to fail.

Signed-off-by: Jeff King <peff@peff.net>
---
New in this version. It could be queued separately from the first.

 config.mak.dev | 1 +
 1 file changed, 1 insertion(+)
diff mbox series

Patch

diff --git a/config.mak.dev b/config.mak.dev
index 89b218d11a..89fefacd94 100644
--- a/config.mak.dev
+++ b/config.mak.dev
@@ -15,6 +15,7 @@  DEVELOPER_CFLAGS += -Wpointer-arith
 DEVELOPER_CFLAGS += -Wstrict-prototypes
 DEVELOPER_CFLAGS += -Wunused
 DEVELOPER_CFLAGS += -Wvla
+DEVELOPER_CFLAGS += -fno-common
 
 ifndef COMPILER_FEATURES
 COMPILER_FEATURES := $(shell ./detect-compiler $(CC))