mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] [PATCH] Explicitly pass the -fno-common flag
@ 2022-01-08  7:45 Rui Ueyama
  2022-01-08 23:25 ` Szabolcs Nagy
  0 siblings, 1 reply; 5+ messages in thread
From: Rui Ueyama @ 2022-01-08  7:45 UTC (permalink / raw)
  To: musl

Common symbol is a special type of symbol that allows a linker to merge
multiple common symbols into one and turn it into a defined symbol.
This feature was used in C to allow tentative definitions. That is,
you can write `int foo;` instead of `extern int foo;` in a header file.

Common symbols are discouraged these days because they can easily
hide unintentional duplicate symbol errors. For that reason, GCC
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85678)
and Clang (https://github.com/llvm/llvm-project/commit/0a9fc9233e172601e26381810d093e02ef410f65)
now default to `-fno-common`.

That means, musl libc's global variables are compiled to common symbols
or regular defined symbols depending on the compiler. That's not an issue
per se, but it's unnecessary churn.

This patch always passes the `-fno-common` flag to the compiler.
---
 configure | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/configure b/configure
index e1aefed7..73ec966d 100755
--- a/configure
+++ b/configure
@@ -492,6 +492,14 @@ tryflag CFLAGS_AUTO -fno-asynchronous-unwind-tables
 tryflag CFLAGS_AUTO -ffunction-sections
 tryflag CFLAGS_AUTO -fdata-sections

+#
+# The use of common symbol is discouraged recently because it can
+# easily hide unintentional duplicate symbol errors. Recent versions
+# of GCC and Clang default to -fno-common. To get a consistent output
+# with older versions of compilers, explicitly pass that flag.
+#
+tryflag CFLAGS_AUTO -fno-common
+
 #
 # On x86, make sure we don't have incompatible instruction set
 # extensions enabled by default. This is bad for making static binaries.
--
2.33.0.610.gcefe983a32

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] [PATCH] Explicitly pass the -fno-common flag
  2022-01-08  7:45 [musl] [PATCH] Explicitly pass the -fno-common flag Rui Ueyama
@ 2022-01-08 23:25 ` Szabolcs Nagy
  2022-01-09  1:49   ` Rich Felker
  0 siblings, 1 reply; 5+ messages in thread
From: Szabolcs Nagy @ 2022-01-08 23:25 UTC (permalink / raw)
  To: Rui Ueyama; +Cc: musl

* Rui Ueyama <rui314@gmail.com> [2022-01-08 16:45:51 +0900]:
> Common symbol is a special type of symbol that allows a linker to merge
> multiple common symbols into one and turn it into a defined symbol.
> This feature was used in C to allow tentative definitions. That is,
> you can write `int foo;` instead of `extern int foo;` in a header file.
> 
> Common symbols are discouraged these days because they can easily
> hide unintentional duplicate symbol errors. For that reason, GCC
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85678)
> and Clang (https://github.com/llvm/llvm-project/commit/0a9fc9233e172601e26381810d093e02ef410f65)
> now default to `-fno-common`.
> 
> That means, musl libc's global variables are compiled to common symbols
> or regular defined symbols depending on the compiler. That's not an issue
> per se, but it's unnecessary churn.
> 
> This patch always passes the `-fno-common` flag to the compiler.

building x86_64 musl with -fcommon i see these symbols in COM section:

obj/src/malloc/replaced.lo:
 __aligned_alloc_replaced
 __malloc_replaced
obj/src/malloc/mallocng/malloc.lo:
 __malloc_lock
obj/src/misc/getopt.lo:
 __optpos
 optarg
 optopt
obj/src/env/__stack_chk_fail.lo:
 __stack_chk_guard
obj/src/env/__init_tls.lo:
 __thread_list_lock
obj/src/aio/aio.lo:
 __aio_fut
obj/src/locale/locale_map.lo:
 __locale_lock
obj/src/internal/libc.lo:
 __hwcap
 __libc
obj/src/internal/defsysinfo.lo:
 __sysinfo
obj/src/signal/sigaction.lo:
 __eintr_valid_flag
obj/src/exit/abort_lock.lo:
 __abort_lock
obj/src/network/h_errno.lo:
 h_errno
obj/src/time/getdate.lo:
 getdate_err

i'm not sure how this can cause trouble (maybe static linking?), but
if we care then another solution is to change

 int x;

to

 int x = 0;

instead of forcing -fno-common.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] [PATCH] Explicitly pass the -fno-common flag
  2022-01-08 23:25 ` Szabolcs Nagy
@ 2022-01-09  1:49   ` Rich Felker
  2022-01-10 16:50     ` Markus Wichmann
  0 siblings, 1 reply; 5+ messages in thread
From: Rich Felker @ 2022-01-09  1:49 UTC (permalink / raw)
  To: Rui Ueyama, musl

On Sun, Jan 09, 2022 at 12:25:36AM +0100, Szabolcs Nagy wrote:
> * Rui Ueyama <rui314@gmail.com> [2022-01-08 16:45:51 +0900]:
> > Common symbol is a special type of symbol that allows a linker to merge
> > multiple common symbols into one and turn it into a defined symbol.
> > This feature was used in C to allow tentative definitions. That is,
> > you can write `int foo;` instead of `extern int foo;` in a header file.
> > 
> > Common symbols are discouraged these days because they can easily
> > hide unintentional duplicate symbol errors. For that reason, GCC
> > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85678)
> > and Clang (https://github.com/llvm/llvm-project/commit/0a9fc9233e172601e26381810d093e02ef410f65)
> > now default to `-fno-common`.
> > 
> > That means, musl libc's global variables are compiled to common symbols
> > or regular defined symbols depending on the compiler. That's not an issue
> > per se, but it's unnecessary churn.
> > 
> > This patch always passes the `-fno-common` flag to the compiler.
> 
> building x86_64 musl with -fcommon i see these symbols in COM section:
> 
> obj/src/malloc/replaced.lo:
>  __aligned_alloc_replaced
>  __malloc_replaced
> obj/src/malloc/mallocng/malloc.lo:
>  __malloc_lock
> obj/src/misc/getopt.lo:
>  __optpos
>  optarg
>  optopt
> obj/src/env/__stack_chk_fail.lo:
>  __stack_chk_guard
> obj/src/env/__init_tls.lo:
>  __thread_list_lock
> obj/src/aio/aio.lo:
>  __aio_fut
> obj/src/locale/locale_map.lo:
>  __locale_lock
> obj/src/internal/libc.lo:
>  __hwcap
>  __libc
> obj/src/internal/defsysinfo.lo:
>  __sysinfo
> obj/src/signal/sigaction.lo:
>  __eintr_valid_flag
> obj/src/exit/abort_lock.lo:
>  __abort_lock
> obj/src/network/h_errno.lo:
>  h_errno
> obj/src/time/getdate.lo:
>  getdate_err
> 
> i'm not sure how this can cause trouble (maybe static linking?), but
> if we care then another solution is to change
> 
>  int x;
> 
> to
> 
>  int x = 0;
> 
> instead of forcing -fno-common.

I don't really see a need for us to care whether these are commons or
bss. Having them be bss would give slightly better ability to error on
build-time UB in static linked programs, but doesn't do anything at
all for dynamic linking, and for static linking the problem is only
diagnosable when the data object is in a file that's pulled in for an
undefined symbol other than that of the data object (e.g. a function
or other data object in the same TU).

The one time bss is significantly preferable to common is for
zero-initialized const objects (rare but they might appear in some
places) since commons don't actually get made const. I don't think we
have any of those that are commons now though.

Rich

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] [PATCH] Explicitly pass the -fno-common flag
  2022-01-09  1:49   ` Rich Felker
@ 2022-01-10 16:50     ` Markus Wichmann
  2022-01-10 18:45       ` Szabolcs Nagy
  0 siblings, 1 reply; 5+ messages in thread
From: Markus Wichmann @ 2022-01-10 16:50 UTC (permalink / raw)
  To: musl; +Cc: Rui Ueyama

On Sat, Jan 08, 2022 at 08:49:17PM -0500, Rich Felker wrote:
> The one time bss is significantly preferable to common is for
> zero-initialized const objects (rare but they might appear in some
> places) since commons don't actually get made const. I don't think we
> have any of those that are commons now though.
>

Wait, that's a possibility? I thought all constant data would be added
to .rodata. I thought that was the whole point of .rodata.

Ciao,
Markus

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] [PATCH] Explicitly pass the -fno-common flag
  2022-01-10 16:50     ` Markus Wichmann
@ 2022-01-10 18:45       ` Szabolcs Nagy
  0 siblings, 0 replies; 5+ messages in thread
From: Szabolcs Nagy @ 2022-01-10 18:45 UTC (permalink / raw)
  To: Markus Wichmann; +Cc: musl, Rui Ueyama

* Markus Wichmann <nullplan@gmx.net> [2022-01-10 17:50:15 +0100]:
> On Sat, Jan 08, 2022 at 08:49:17PM -0500, Rich Felker wrote:
> > The one time bss is significantly preferable to common is for
> > zero-initialized const objects (rare but they might appear in some
> > places) since commons don't actually get made const. I don't think we
> > have any of those that are commons now though.
> >
> 
> Wait, that's a possibility? I thought all constant data would be added
> to .rodata. I thought that was the whole point of .rodata.

const int x;

with -fcommon the x is a common symbol.
(the fact that it was const is lost in the .o)

common symbol is treated as bss at link time,
if there is no actual definition, and thus writable.

with -fno-common x is rodata.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-01-10 18:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-08  7:45 [musl] [PATCH] Explicitly pass the -fno-common flag Rui Ueyama
2022-01-08 23:25 ` Szabolcs Nagy
2022-01-09  1:49   ` Rich Felker
2022-01-10 16:50     ` Markus Wichmann
2022-01-10 18:45       ` Szabolcs Nagy

Code repositories for project(s) associated with this inbox:

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).