git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jeff King <peff@peff.net>
Date
Oct 15, 2020, 19:30 UTC
Message-ID
<20201015193029.GB1129761@coredump.intra.peff.net>
In-Reply-To
<20201015192845.GD1108210@coredump.intra.peff.net>

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 --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))
-- 
2.29.0.rc1.562.g7bd9bc8902
Previous: Jeff King
Message 15 of 15 in “clean up extern decl of functions”
  1. clean up extern decl of functionsJunio C Hamano, Oct 8, 2020
  2. Denton LiuOct 9, 2020
  3. Junio C HamanoOct 9, 2020
  4. Junio C HamanoOct 9, 2020
  5. [RFC] CodingGuidelines: mark external declarations with "extern"Junio C Hamano, Oct 9, 2020
  6. Jeff KingOct 9, 2020
  7. Junio C HamanoOct 9, 2020
  8. Denton LiuOct 9, 2020
  9. Junio C HamanoOct 9, 2020
  10. Đoàn Trần Công DanhOct 10, 2020
  11. Jeff KingOct 15, 2020
  12. Junio C HamanoOct 15, 2020
  13. Jeff KingOct 15, 2020
  14. 1/2 usage: define a type for a reporting functionJeff King, Oct 15, 2020
  15. 2/2 config.mak.dev: build with -fno-commonJeff King, Oct 15, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.