public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Make glibc build with LLD
@ 2020-12-28 19:48 Fangrui Song
  2020-12-28 19:48 ` [PATCH 1/3] configure: Allow LD to be a linker other than GNU ld and gold Fangrui Song
                   ` (4 more replies)
  0 siblings, 5 replies; 42+ messages in thread
From: Fangrui Song @ 2020-12-28 19:48 UTC (permalink / raw)
  To: libc-alpha; +Cc: Fangrui Song

I sent the first two in April. Resending in a patch series to be
clearer.

  install: Replace scripts/output-format.sed with objdump -f

replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
by leveraging objdump -f.

With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
`make` with ld pointing to ld.lld (LLVM linker) works.

Fangrui Song (3):
  configure: Allow LD to be a linker other than GNU ld and gold
  elf: Replace a --defsym trick with an object file to be compatible
    with lld
  install: Replace scripts/output-format.sed with objdump -f

 Makerules                                     | 13 ++-----
 .../strcoll-inputs/filelist#en_US.UTF-8       |  1 -
 config.make.in                                |  1 -
 configure                                     | 32 +++++------------
 configure.ac                                  | 24 +++++--------
 elf/Makefile                                  | 11 +++---
 scripts/output-format.sed                     | 35 -------------------
 7 files changed, 23 insertions(+), 94 deletions(-)
 delete mode 100644 scripts/output-format.sed

-- 
2.29.2.729.g45daf8777d-goog


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

* [PATCH 1/3] configure: Allow LD to be a linker other than GNU ld and gold
  2020-12-28 19:48 [PATCH 0/3] Make glibc build with LLD Fangrui Song
@ 2020-12-28 19:48 ` Fangrui Song
  2020-12-28 20:46   ` H.J. Lu
  2020-12-28 19:48 ` [PATCH 2/3] elf: Replace a --defsym trick with an object file to be compatible with lld Fangrui Song
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 42+ messages in thread
From: Fangrui Song @ 2020-12-28 19:48 UTC (permalink / raw)
  To: libc-alpha; +Cc: Fangrui Song

When using lld as the linker, configure prints a confusing message.

*** These critical programs are missing or too old: GNU ld

lld>=8 can build glibc with very few patches. lld may be built with a
custom version information (e.g. git commit ID), so a version check is not
useful at all.
---
 configure    | 13 ++++++++-----
 configure.ac | 13 ++++++++-----
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/configure b/configure
index 6a35553805..6dcd2270f8 100755
--- a/configure
+++ b/configure
@@ -4601,9 +4601,10 @@ if test $ac_verc_fail = yes; then
 fi
 
 
-if test -n "`$LD --version | sed -n 's/^GNU \(gold\).*$/\1/p'`"; then
+case $($LD --version) in
+  "GNU gold"*)
   # Accept gold 1.14 or higher
-  for ac_prog in $LD
+    for ac_prog in $LD
 do
   # Extract the first word of "$ac_prog", so it can be a program name with args.
 set dummy $ac_prog; ac_word=$2
@@ -4666,8 +4667,9 @@ if test $ac_verc_fail = yes; then
   LD=: critic_missing="$critic_missing GNU gold"
 fi
 
-else
-  for ac_prog in $LD
+    ;;
+  "GNU ld"*)
+    for ac_prog in $LD
 do
   # Extract the first word of "$ac_prog", so it can be a program name with args.
 set dummy $ac_prog; ac_word=$2
@@ -4730,7 +4732,8 @@ if test $ac_verc_fail = yes; then
   LD=: critic_missing="$critic_missing GNU ld"
 fi
 
-fi
+    ;;
+esac
 
 # These programs are version sensitive.
 
diff --git a/configure.ac b/configure.ac
index 43cfac9d48..1a2054cd1a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -979,18 +979,21 @@ AC_CHECK_PROG_VER(AS, $AS, --version,
 		  [2.1[0-9][0-9]*|2.2[5-9]*|2.[3-9][0-9]*|[3-9].*|[1-9][0-9]*],
 		  AS=: critic_missing="$critic_missing as")
 
-if test -n "`$LD --version | sed -n 's/^GNU \(gold\).*$/\1/p'`"; then
+case $($LD --version) in
+  "GNU gold"*)
   # Accept gold 1.14 or higher
-  AC_CHECK_PROG_VER(LD, $LD, --version,
+    AC_CHECK_PROG_VER(LD, $LD, --version,
 		    [GNU gold.* \([0-9][0-9]*\.[0-9.]*\)],
 		    [1.1[4-9]*|1.[2-9][0-9]*|1.1[0-9][0-9]*|[2-9].*|[1-9][0-9]*],
 		    LD=: critic_missing="$critic_missing GNU gold")
-else
-  AC_CHECK_PROG_VER(LD, $LD, --version,
+    ;;
+  "GNU ld"*)
+    AC_CHECK_PROG_VER(LD, $LD, --version,
 		    [GNU ld.* \([0-9][0-9]*\.[0-9.]*\)],
 		    [2.1[0-9][0-9]*|2.2[5-9]*|2.[3-9][0-9]*|[3-9].*|[1-9][0-9]*],
 		    LD=: critic_missing="$critic_missing GNU ld")
-fi
+    ;;
+esac
 
 # These programs are version sensitive.
 AC_CHECK_TOOL_PREFIX
-- 
2.29.2.729.g45daf8777d-goog


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

* [PATCH 2/3] elf: Replace a --defsym trick with an object file to be compatible with lld
  2020-12-28 19:48 [PATCH 0/3] Make glibc build with LLD Fangrui Song
  2020-12-28 19:48 ` [PATCH 1/3] configure: Allow LD to be a linker other than GNU ld and gold Fangrui Song
@ 2020-12-28 19:48 ` Fangrui Song
  2021-01-11 20:06   ` Fāng-ruì Sòng
  2021-01-19  0:03   ` H.J. Lu
  2020-12-28 19:48 ` [PATCH 3/3] install: Replace scripts/output-format.sed with objdump -f [BZ #26559] Fangrui Song
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 42+ messages in thread
From: Fangrui Song @ 2020-12-28 19:48 UTC (permalink / raw)
  To: libc-alpha; +Cc: Fangrui Song

The existing code specifies -Wl,--defsym=malloc=0 and other malloc.os
definitions before libc_pic.a so that libc_pic.a(malloc.os) is not
fetched. This trick is used to avoid multiple definition errors which
would happen as a chain result:

  dl-allobjs.os has an undefined __libc_scratch_buffer_set_array_size
  __libc_scratch_buffer_set_array_size fetches libc_pic.a(scratch_buffer_set_array_size.os)
  libc_pic.a(scratch_buffer_set_array_size.os) has an undefined free
  free fetches libc_pic.a(malloc.os)
  libc_pic.a(malloc.os) has an undefined __libc_message
  __libc_message fetches libc_pic.a(libc_fatal.os)

  libc_fatal.os will cause a multiple definition error (__GI___libc_fatal)
  >>> defined at dl-fxstatat64.c
  >>>            /tmp/p/glibc/Release/elf/dl-allobjs.os:(__GI___libc_fatal)
  >>> defined at libc_fatal.c
  >>>            libc_fatal.os:(.text+0x240) in archive /tmp/p/glibc/Release/libc_pic.a

lld processes --defsym after all input files, so this trick does not
suppress multiple definition errors with lld. Split the step into two
and use an object file to make the intention more obvious and make lld
work.

This is conceptually more appropriate because --defsym defines a SHN_ABS
symbol while a normal definition is relative to the image base.

See https://sourceware.org/pipermail/libc-alpha/2020-March/111910.html
for discussions about the --defsym semantics.
---
 elf/Makefile | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/elf/Makefile b/elf/Makefile
index 0b4d78c874..299bf24b49 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -524,10 +524,6 @@ rtld-stubbed-symbols = \
   malloc \
   realloc \
 
-# The GCC arguments that implement $(rtld-stubbed-symbols).
-rtld-stubbed-symbols-args = \
-  $(patsubst %,-Wl$(comma)--defsym=%=0, $(rtld-stubbed-symbols))
-
 ifeq ($(have-ssp),yes)
 # rtld is not built with the stack protector, so these references will
 # go away in the rebuilds.
@@ -536,9 +532,10 @@ endif
 
 $(objpfx)librtld.map: $(objpfx)dl-allobjs.os $(common-objpfx)libc_pic.a
 	@-rm -f $@T
-	$(reloc-link) -o $@.o $(rtld-stubbed-symbols-args) \
-		'-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
-	rm -f $@.o
+	echo '$(patsubst %,.globl %;,$(rtld-stubbed-symbols)) $(patsubst %,%:,$(rtld-stubbed-symbols))' | \
+		$(CC) -o $@T.o $(ASFLAGS) -c -x assembler -
+	$(reloc-link) -o $@.o $@T.o '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
+	rm -f %@T.o $@.o
 	mv -f $@T $@
 
 # For lld, skip preceding addresses and values before matching the archive and the member.
-- 
2.29.2.729.g45daf8777d-goog


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

* [PATCH 3/3] install: Replace scripts/output-format.sed with objdump -f [BZ #26559]
  2020-12-28 19:48 [PATCH 0/3] Make glibc build with LLD Fangrui Song
  2020-12-28 19:48 ` [PATCH 1/3] configure: Allow LD to be a linker other than GNU ld and gold Fangrui Song
  2020-12-28 19:48 ` [PATCH 2/3] elf: Replace a --defsym trick with an object file to be compatible with lld Fangrui Song
@ 2020-12-28 19:48 ` Fangrui Song
  2021-01-08 19:38   ` Adhemerval Zanella
  2021-01-12 11:32   ` Andreas Schwab
  2020-12-28 21:11 ` [PATCH 0/3] Make glibc build with LLD H.J. Lu
  2021-01-08  5:04 ` Fāng-ruì Sòng
  4 siblings, 2 replies; 42+ messages in thread
From: Fangrui Song @ 2020-12-28 19:48 UTC (permalink / raw)
  To: libc-alpha; +Cc: Fangrui Song

GNU ld and gold have supported --print-output-format since 2011. glibc
requires binutils>=2.25 (2015), so if LD is GNU ld or gold, we can
assume the option is supported.

lld is by default a cross linker supporting multiple targets. It auto
detects the file format and does not need OUTPUT_FORMAT. It does not
support --print-output-format.

By parsing objdump -f, we can support all the three linkers.
---
 Makerules                                     | 13 ++-----
 .../strcoll-inputs/filelist#en_US.UTF-8       |  1 -
 config.make.in                                |  1 -
 configure                                     | 19 ----------
 configure.ac                                  | 11 ------
 scripts/output-format.sed                     | 35 -------------------
 6 files changed, 3 insertions(+), 77 deletions(-)
 delete mode 100644 scripts/output-format.sed

diff --git a/Makerules b/Makerules
index ef0fe67d9a..146d1ab650 100644
--- a/Makerules
+++ b/Makerules
@@ -1065,20 +1065,13 @@ install: $(inst_slibdir)/libc.so$(libc.so-version)
 # for the configuration we are building.  We put this statement into
 # the linker scripts we install for -lc et al so that they will not be
 # used by a link for a different format on a multi-architecture system.
-$(common-objpfx)format.lds: $(..)scripts/output-format.sed \
-			    $(common-objpfx)config.make \
+$(common-objpfx)format.lds: $(common-objpfx)config.make \
 			    $(common-objpfx)config.h $(..)Makerules
-ifneq (unknown,$(output-format))
-	echo > $@.new 'OUTPUT_FORMAT($(output-format))'
-else
 	$(LINK.o) -shared $(sysdep-LDFLAGS) $(rtld-LDFLAGS) \
 		  $(LDFLAGS.so) $(LDFLAGS-lib.so) \
-		  -x c /dev/null -o $@.so -Wl,--verbose -v 2>/dev/null \
-	| sed -n -f $< > $@.new
-	test -s $@.new
+		  -x c /dev/null -o $@.so 2>/dev/null
+	$(OBJDUMP) -f $@.so | sed -n 's/.*file format \(.*\)/OUTPUT_FORMAT(\1)/;T;p' > $@
 	rm -f $@.so
-endif
-	mv -f $@.new $@
 common-generated += format.lds
 
 ifndef subdir
diff --git a/benchtests/strcoll-inputs/filelist#en_US.UTF-8 b/benchtests/strcoll-inputs/filelist#en_US.UTF-8
index 2f4ef195bb..43eb9efb40 100644
--- a/benchtests/strcoll-inputs/filelist#en_US.UTF-8
+++ b/benchtests/strcoll-inputs/filelist#en_US.UTF-8
@@ -9450,7 +9450,6 @@ move-if-change
 check-execstack.awk
 pylint
 pylintrc
-output-format.sed
 merge-test-results.sh
 update-copyrights
 config-uname.sh
diff --git a/config.make.in b/config.make.in
index 7ae27564fd..7f47f0caa4 100644
--- a/config.make.in
+++ b/config.make.in
@@ -73,7 +73,6 @@ fno-unit-at-a-time = @fno_unit_at_a_time@
 bind-now = @bindnow@
 have-hash-style = @libc_cv_hashstyle@
 use-default-link = @use_default_link@
-output-format = @libc_cv_output_format@
 have-cxx-thread_local = @libc_cv_cxx_thread_local@
 have-loop-to-function = @libc_cv_cc_loop_to_function@
 have-textrel_ifunc = @libc_cv_textrel_ifunc@
diff --git a/configure b/configure
index 6dcd2270f8..4b0ab150cb 100755
--- a/configure
+++ b/configure
@@ -623,7 +623,6 @@ libc_cv_cc_submachine
 libc_cv_cc_nofma
 libc_cv_mtls_dialect_gnu2
 fno_unit_at_a_time
-libc_cv_output_format
 libc_cv_has_glob_dat
 libc_cv_hashstyle
 libc_cv_fpie
@@ -6077,24 +6076,6 @@ fi
 $as_echo "$libc_cv_has_glob_dat" >&6; }
 
 
-{ $as_echo "$as_me:${as_lineno-$LINENO}: checking linker output format" >&5
-$as_echo_n "checking linker output format... " >&6; }
-if ${libc_cv_output_format+:} false; then :
-  $as_echo_n "(cached) " >&6
-else
-  if libc_cv_output_format=`
-${CC-cc} -nostartfiles -nostdlib $no_ssp -Wl,--print-output-format 2>&5`
-then
-  :
-else
-  libc_cv_output_format=
-fi
-test -n "$libc_cv_output_format" || libc_cv_output_format=unknown
-fi
-{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_output_format" >&5
-$as_echo "$libc_cv_output_format" >&6; }
-
-
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for -fno-toplevel-reorder -fno-section-anchors" >&5
 $as_echo_n "checking for -fno-toplevel-reorder -fno-section-anchors... " >&6; }
 if ${libc_cv_fno_toplevel_reorder+:} false; then :
diff --git a/configure.ac b/configure.ac
index 1a2054cd1a..baf3a05ae7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1432,17 +1432,6 @@ fi
 rm -f conftest*])
 AC_SUBST(libc_cv_has_glob_dat)
 
-AC_CACHE_CHECK(linker output format, libc_cv_output_format, [dnl
-if libc_cv_output_format=`
-${CC-cc} -nostartfiles -nostdlib $no_ssp -Wl,--print-output-format 2>&AS_MESSAGE_LOG_FD`
-then
-  :
-else
-  libc_cv_output_format=
-fi
-test -n "$libc_cv_output_format" || libc_cv_output_format=unknown])
-AC_SUBST(libc_cv_output_format)
-
 AC_CACHE_CHECK(for -fno-toplevel-reorder -fno-section-anchors, libc_cv_fno_toplevel_reorder, [dnl
 cat > conftest.c <<EOF
 int foo;
diff --git a/scripts/output-format.sed b/scripts/output-format.sed
deleted file mode 100644
index 364f52059f..0000000000
--- a/scripts/output-format.sed
+++ /dev/null
@@ -1,35 +0,0 @@
-/ld.*[ 	]-E[BL]/b f
-/collect.*[ 	]-E[BL]/b f
-/OUTPUT_FORMAT[^)]*$/{N
-s/\n[	 ]*/ /
-}
-t o
-: o
-s/^.*OUTPUT_FORMAT(\([^,]*\), \1, \1).*$/OUTPUT_FORMAT(\1)/
-t q
-s/^.*OUTPUT_FORMAT(\([^,]*\), \([^,]*\), \([^,]*\)).*$/\1,\2,\3/
-t s
-s/^.*OUTPUT_FORMAT(\([^,)]*\).*$)/OUTPUT_FORMAT(\1)/
-t q
-d
-: s
-s/"//g
-G
-s/\n//
-s/^\([^,]*\),\([^,]*\),\([^,]*\),B/OUTPUT_FORMAT(\2)/p
-s/^\([^,]*\),\([^,]*\),\([^,]*\),L/OUTPUT_FORMAT(\3)/p
-s/^\([^,]*\),\([^,]*\),\([^,]*\)/OUTPUT_FORMAT(\1)/p
-/,/s|^|*** BUG in libc/scripts/output-format.sed *** |p
-q
-: q
-s/"//g
-p
-q
-: f
-s/^.*[ 	]-E\([BL]\)[ 	].*$/,\1/
-t h
-s/^.*[ 	]-E\([BL]\)$/,\1/
-t h
-d
-: h
-h
-- 
2.29.2.729.g45daf8777d-goog


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

* Re: [PATCH 1/3] configure: Allow LD to be a linker other than GNU ld and gold
  2020-12-28 19:48 ` [PATCH 1/3] configure: Allow LD to be a linker other than GNU ld and gold Fangrui Song
@ 2020-12-28 20:46   ` H.J. Lu
  2020-12-28 21:15     ` [PATCH 1/3] configure: Allow LD to be LLD 9.0.0 or above " Fangrui Song
  0 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2020-12-28 20:46 UTC (permalink / raw)
  To: Fangrui Song; +Cc: GNU C Library

On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> When using lld as the linker, configure prints a confusing message.
>
> *** These critical programs are missing or too old: GNU ld
>
> lld>=8 can build glibc with very few patches. lld may be built with a
> custom version information (e.g. git commit ID), so a version check is not
> useful at all.

But not all versions of lld can be used to build glibc.  Please find a way
to check the working lld version.

> ---
>  configure    | 13 ++++++++-----
>  configure.ac | 13 ++++++++-----
>  2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/configure b/configure
> index 6a35553805..6dcd2270f8 100755
> --- a/configure
> +++ b/configure
> @@ -4601,9 +4601,10 @@ if test $ac_verc_fail = yes; then
>  fi
>
>
> -if test -n "`$LD --version | sed -n 's/^GNU \(gold\).*$/\1/p'`"; then
> +case $($LD --version) in
> +  "GNU gold"*)
>    # Accept gold 1.14 or higher
> -  for ac_prog in $LD
> +    for ac_prog in $LD
>  do
>    # Extract the first word of "$ac_prog", so it can be a program name with args.
>  set dummy $ac_prog; ac_word=$2
> @@ -4666,8 +4667,9 @@ if test $ac_verc_fail = yes; then
>    LD=: critic_missing="$critic_missing GNU gold"
>  fi
>
> -else
> -  for ac_prog in $LD
> +    ;;
> +  "GNU ld"*)
> +    for ac_prog in $LD
>  do
>    # Extract the first word of "$ac_prog", so it can be a program name with args.
>  set dummy $ac_prog; ac_word=$2
> @@ -4730,7 +4732,8 @@ if test $ac_verc_fail = yes; then
>    LD=: critic_missing="$critic_missing GNU ld"
>  fi
>
> -fi
> +    ;;
> +esac
>
>  # These programs are version sensitive.
>
> diff --git a/configure.ac b/configure.ac
> index 43cfac9d48..1a2054cd1a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -979,18 +979,21 @@ AC_CHECK_PROG_VER(AS, $AS, --version,
>                   [2.1[0-9][0-9]*|2.2[5-9]*|2.[3-9][0-9]*|[3-9].*|[1-9][0-9]*],
>                   AS=: critic_missing="$critic_missing as")
>
> -if test -n "`$LD --version | sed -n 's/^GNU \(gold\).*$/\1/p'`"; then
> +case $($LD --version) in
> +  "GNU gold"*)
>    # Accept gold 1.14 or higher
> -  AC_CHECK_PROG_VER(LD, $LD, --version,
> +    AC_CHECK_PROG_VER(LD, $LD, --version,
>                     [GNU gold.* \([0-9][0-9]*\.[0-9.]*\)],
>                     [1.1[4-9]*|1.[2-9][0-9]*|1.1[0-9][0-9]*|[2-9].*|[1-9][0-9]*],
>                     LD=: critic_missing="$critic_missing GNU gold")
> -else
> -  AC_CHECK_PROG_VER(LD, $LD, --version,
> +    ;;
> +  "GNU ld"*)
> +    AC_CHECK_PROG_VER(LD, $LD, --version,
>                     [GNU ld.* \([0-9][0-9]*\.[0-9.]*\)],
>                     [2.1[0-9][0-9]*|2.2[5-9]*|2.[3-9][0-9]*|[3-9].*|[1-9][0-9]*],
>                     LD=: critic_missing="$critic_missing GNU ld")
> -fi
> +    ;;
> +esac
>
>  # These programs are version sensitive.
>  AC_CHECK_TOOL_PREFIX
> --
> 2.29.2.729.g45daf8777d-goog
>


-- 
H.J.

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

* Re: [PATCH 0/3] Make glibc build with LLD
  2020-12-28 19:48 [PATCH 0/3] Make glibc build with LLD Fangrui Song
                   ` (2 preceding siblings ...)
  2020-12-28 19:48 ` [PATCH 3/3] install: Replace scripts/output-format.sed with objdump -f [BZ #26559] Fangrui Song
@ 2020-12-28 21:11 ` H.J. Lu
  2020-12-28 21:45   ` Fangrui Song
  2021-01-08  5:04 ` Fāng-ruì Sòng
  4 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2020-12-28 21:11 UTC (permalink / raw)
  To: Fangrui Song; +Cc: GNU C Library

On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> I sent the first two in April. Resending in a patch series to be
> clearer.
>
>   install: Replace scripts/output-format.sed with objdump -f
>
> replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
> by leveraging objdump -f.
>
> With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
> `make` with ld pointing to ld.lld (LLVM linker) works.

I tried your branch with "LLD 11.0.0 (compatible with GNU linkers)" on
Fedora 33/x86-64,
"make check" generated:

make[4]: *** [../Makerules:767:
/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.so]
Error 1
make[4]: *** [../Makerules:767:
/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.so]
Error 1
make[4]: *** [../Makerules:767:
/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-sym-lib.so]
Error 1
make[4]: *** [../Makerules:767:
/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-zero-lib.so]
Error 1
make[4]: *** [../Makerules:767:
/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod6.so]
Error 1
make[4]: *** [../Makerules:767:
/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod5.so]
Error 1
make[4]: *** [../Rules:226:
/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit16]
Error 1
make[4]: *** [../Rules:226:
/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit14]
Error 1
make[4]: *** [../Rules:226:
/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit15]
Error 1
make[4]: *** [../Rules:226:
/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tls1]
Error 1
make[4]: *** [../Rules:226:
/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain5]
Error 1
make[4]: *** [../Rules:226:
/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain1]
Error 1
make[3]: *** [Makefile:479: elf/tests] Error 2

with error messages, like

ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.os
has an STT_TLS symbol but doesn't have an SHF_TLS section
ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.os
has an STT_TLS symbol but doesn't have an SHF_TLS section

When glibc is configured with --enable-static-pie, I got

[hjl@gnu-skx-1 build-x86_64-linux]$ ./elf/ldconfig
Segmentation fault (core dumped)
[hjl@gnu-skx-1 build-x86_64-linux]$

You need to fix lld first and give the working lld a proper version so that
configure can check it.


-- 
H.J.

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

* [PATCH 1/3] configure: Allow LD to be LLD 9.0.0 or above and gold
  2020-12-28 20:46   ` H.J. Lu
@ 2020-12-28 21:15     ` Fangrui Song
  0 siblings, 0 replies; 42+ messages in thread
From: Fangrui Song @ 2020-12-28 21:15 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library

[-- Attachment #1: Type: text/plain, Size: 836 bytes --]

On 2020-12-28, H.J. Lu wrote:
>On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song via Libc-alpha
><libc-alpha@sourceware.org> wrote:
>>
>> When using lld as the linker, configure prints a confusing message.
>>
>> *** These critical programs are missing or too old: GNU ld
>>
>> lld>=8 can build glibc with very few patches. lld may be built with a
>> custom version information (e.g. git commit ID), so a version check is not
>> useful at all.
>
>But not all versions of lld can be used to build glibc.  Please find a way
>to check the working lld version.

Replaced this with "configure: Allow LD to be LLD 9.0.0 or above" in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld

8.0.0 needs an additional workaround. Since 9.0.0 is quite common in
distributions, I just set the baseline to 9.0.0 to reduce friction.

[-- Attachment #2: 0001-configure-Allow-LD-to-be-LLD-9.0.0-or-above.patch --]
[-- Type: text/x-diff, Size: 4848 bytes --]

From 49c46455e41f1e0e396b17425eb93c5e0e776e1b Mon Sep 17 00:00:00 2001
From: Fangrui Song <maskray@google.com>
Date: Fri, 13 Mar 2020 13:29:46 -0700
Subject: [PATCH 1/3] configure: Allow LD to be LLD 9.0.0 or above

When using LLD (LLVM linker) as the linker, configure prints a confusing
message.

    *** These critical programs are missing or too old: GNU ld

LLD>=9.0.0 can build glibc with very few patches. (LLD 8.0.0 needs one
workaround for -Wl,-defsym=_begin=0). LLD>=9.0.0 is available on many
distributions, so just set the baseline version to 9.0.0.
---
 configure    | 77 +++++++++++++++++++++++++++++++++++++++++++++++++---
 configure.ac | 20 ++++++++++----
 2 files changed, 88 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index 6a35553805..f67eb287fc 100755
--- a/configure
+++ b/configure
@@ -4601,9 +4601,10 @@ if test $ac_verc_fail = yes; then
 fi
 
 
-if test -n "`$LD --version | sed -n 's/^GNU \(gold\).*$/\1/p'`"; then
+case $($LD --version) in
+  "GNU gold"*)
   # Accept gold 1.14 or higher
-  for ac_prog in $LD
+    for ac_prog in $LD
 do
   # Extract the first word of "$ac_prog", so it can be a program name with args.
 set dummy $ac_prog; ac_word=$2
@@ -4666,8 +4667,75 @@ if test $ac_verc_fail = yes; then
   LD=: critic_missing="$critic_missing GNU gold"
 fi
 
+    ;;
+  "LLD"*)
+  # Accept LLD 9.0 or higher
+    for ac_prog in $LD
+do
+  # Extract the first word of "$ac_prog", so it can be a program name with args.
+set dummy $ac_prog; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_prog_LD+:} false; then :
+  $as_echo_n "(cached) " >&6
 else
-  for ac_prog in $LD
+  if test -n "$LD"; then
+  ac_cv_prog_LD="$LD" # Let the user override the test.
+else
+as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+  IFS=$as_save_IFS
+  test -z "$as_dir" && as_dir=.
+    for ac_exec_ext in '' $ac_executable_extensions; do
+  if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+    ac_cv_prog_LD="$ac_prog"
+    $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+    break 2
+  fi
+done
+  done
+IFS=$as_save_IFS
+
+fi
+fi
+LD=$ac_cv_prog_LD
+if test -n "$LD"; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $LD" >&5
+$as_echo "$LD" >&6; }
+else
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+
+  test -n "$LD" && break
+done
+
+if test -z "$LD"; then
+  ac_verc_fail=yes
+else
+  # Found it, now check the version.
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking version of $LD" >&5
+$as_echo_n "checking version of $LD... " >&6; }
+  ac_prog_version=`$LD --version 2>&1 | sed -n 's/^.*LLD.* \([0-9][0-9]*\.[0-9.]*\).*$/\1/p'`
+  case $ac_prog_version in
+    '') ac_prog_version="v. ?.??, bad"; ac_verc_fail=yes;;
+    9.*|[1-9][0-9].*)
+       ac_prog_version="$ac_prog_version, ok"; ac_verc_fail=no;;
+    *) ac_prog_version="$ac_prog_version, bad"; ac_verc_fail=yes;;
+
+  esac
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_prog_version" >&5
+$as_echo "$ac_prog_version" >&6; }
+fi
+if test $ac_verc_fail = yes; then
+  LD=: critic_missing="$critic_missing LLD"
+fi
+
+    ;;
+  *)
+    for ac_prog in $LD
 do
   # Extract the first word of "$ac_prog", so it can be a program name with args.
 set dummy $ac_prog; ac_word=$2
@@ -4730,7 +4798,8 @@ if test $ac_verc_fail = yes; then
   LD=: critic_missing="$critic_missing GNU ld"
 fi
 
-fi
+    ;;
+esac
 
 # These programs are version sensitive.
 
diff --git a/configure.ac b/configure.ac
index 43cfac9d48..59dbf9bda8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -979,18 +979,28 @@ AC_CHECK_PROG_VER(AS, $AS, --version,
 		  [2.1[0-9][0-9]*|2.2[5-9]*|2.[3-9][0-9]*|[3-9].*|[1-9][0-9]*],
 		  AS=: critic_missing="$critic_missing as")
 
-if test -n "`$LD --version | sed -n 's/^GNU \(gold\).*$/\1/p'`"; then
+case $($LD --version) in
+  "GNU gold"*)
   # Accept gold 1.14 or higher
-  AC_CHECK_PROG_VER(LD, $LD, --version,
+    AC_CHECK_PROG_VER(LD, $LD, --version,
 		    [GNU gold.* \([0-9][0-9]*\.[0-9.]*\)],
 		    [1.1[4-9]*|1.[2-9][0-9]*|1.1[0-9][0-9]*|[2-9].*|[1-9][0-9]*],
 		    LD=: critic_missing="$critic_missing GNU gold")
-else
-  AC_CHECK_PROG_VER(LD, $LD, --version,
+    ;;
+  "LLD"*)
+  # Accept LLD 9.0 or higher
+    AC_CHECK_PROG_VER(LD, $LD, --version,
+		    [LLD.* \([0-9][0-9]*\.[0-9.]*\)],
+		    [9.*|[1-9][0-9].*],
+		    LD=: critic_missing="$critic_missing LLD")
+    ;;
+  *)
+    AC_CHECK_PROG_VER(LD, $LD, --version,
 		    [GNU ld.* \([0-9][0-9]*\.[0-9.]*\)],
 		    [2.1[0-9][0-9]*|2.2[5-9]*|2.[3-9][0-9]*|[3-9].*|[1-9][0-9]*],
 		    LD=: critic_missing="$critic_missing GNU ld")
-fi
+    ;;
+esac
 
 # These programs are version sensitive.
 AC_CHECK_TOOL_PREFIX
-- 
2.29.2.729.g45daf8777d-goog


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

* Re: [PATCH 0/3] Make glibc build with LLD
  2020-12-28 21:11 ` [PATCH 0/3] Make glibc build with LLD H.J. Lu
@ 2020-12-28 21:45   ` Fangrui Song
  2020-12-28 21:49     ` H.J. Lu
  0 siblings, 1 reply; 42+ messages in thread
From: Fangrui Song @ 2020-12-28 21:45 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library

On 2020-12-28, H.J. Lu wrote:
>On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song via Libc-alpha
><libc-alpha@sourceware.org> wrote:
>>
>> I sent the first two in April. Resending in a patch series to be
>> clearer.
>>
>>   install: Replace scripts/output-format.sed with objdump -f
>>
>> replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
>> by leveraging objdump -f.
>>
>> With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
>> `make` with ld pointing to ld.lld (LLVM linker) works.
>
>I tried your branch with "LLD 11.0.0 (compatible with GNU linkers)" on
>Fedora 33/x86-64,
>"make check" generated:
>
>make[4]: *** [../Makerules:767:
>/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.so]
>Error 1
>make[4]: *** [../Makerules:767:
>/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.so]
>Error 1
>make[4]: *** [../Makerules:767:
>/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-sym-lib.so]
>Error 1
>make[4]: *** [../Makerules:767:
>/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-zero-lib.so]
>Error 1
>make[4]: *** [../Makerules:767:
>/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod6.so]
>Error 1
>make[4]: *** [../Makerules:767:
>/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod5.so]
>Error 1
>make[4]: *** [../Rules:226:
>/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit16]
>Error 1
>make[4]: *** [../Rules:226:
>/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit14]
>Error 1
>make[4]: *** [../Rules:226:
>/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit15]
>Error 1
>make[4]: *** [../Rules:226:
>/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tls1]
>Error 1
>make[4]: *** [../Rules:226:
>/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain5]
>Error 1
>make[4]: *** [../Rules:226:
>/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain1]
>Error 1
>make[3]: *** [Makefile:479: elf/tests] Error 2
>
>with error messages, like
>
>ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.os
>has an STT_TLS symbol but doesn't have an SHF_TLS section
>ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.os
>has an STT_TLS symbol but doesn't have an SHF_TLS section

tst-tls* tests appear to be similar to .tls_common which looks very
obsoleted and not supported by Clang. I don't think ifuncmain* or
tst-audit* matters for typical usage (most users) but I can take a look.

>When glibc is configured with --enable-static-pie, I got
>
>[hjl@gnu-skx-1 build-x86_64-linux]$ ./elf/ldconfig
>Segmentation fault (core dumped)
>[hjl@gnu-skx-1 build-x86_64-linux]$
>
>You need to fix lld first and give the working lld a proper version so that
>configure can check it.

I cherry picked "Make _dl_relocate_static_pie return an int indicating whether it applied relocs."
in google/grte/v5-2.27/master, which has fixed the issue.

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

* Re: [PATCH 0/3] Make glibc build with LLD
  2020-12-28 21:45   ` Fangrui Song
@ 2020-12-28 21:49     ` H.J. Lu
  2020-12-28 22:54       ` H.J. Lu
  2021-01-05 22:57       ` Fāng-ruì Sòng
  0 siblings, 2 replies; 42+ messages in thread
From: H.J. Lu @ 2020-12-28 21:49 UTC (permalink / raw)
  To: Fangrui Song; +Cc: GNU C Library

On Mon, Dec 28, 2020 at 1:45 PM Fangrui Song <maskray@google.com> wrote:
>
> On 2020-12-28, H.J. Lu wrote:
> >On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song via Libc-alpha
> ><libc-alpha@sourceware.org> wrote:
> >>
> >> I sent the first two in April. Resending in a patch series to be
> >> clearer.
> >>
> >>   install: Replace scripts/output-format.sed with objdump -f
> >>
> >> replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
> >> by leveraging objdump -f.
> >>
> >> With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
> >> `make` with ld pointing to ld.lld (LLVM linker) works.
> >
> >I tried your branch with "LLD 11.0.0 (compatible with GNU linkers)" on
> >Fedora 33/x86-64,
> >"make check" generated:
> >
> >make[4]: *** [../Makerules:767:
> >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.so]
> >Error 1
> >make[4]: *** [../Makerules:767:
> >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.so]
> >Error 1
> >make[4]: *** [../Makerules:767:
> >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-sym-lib.so]
> >Error 1
> >make[4]: *** [../Makerules:767:
> >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-zero-lib.so]
> >Error 1
> >make[4]: *** [../Makerules:767:
> >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod6.so]
> >Error 1
> >make[4]: *** [../Makerules:767:
> >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod5.so]
> >Error 1
> >make[4]: *** [../Rules:226:
> >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit16]
> >Error 1
> >make[4]: *** [../Rules:226:
> >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit14]
> >Error 1
> >make[4]: *** [../Rules:226:
> >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit15]
> >Error 1
> >make[4]: *** [../Rules:226:
> >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tls1]
> >Error 1
> >make[4]: *** [../Rules:226:
> >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain5]
> >Error 1
> >make[4]: *** [../Rules:226:
> >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain1]
> >Error 1
> >make[3]: *** [Makefile:479: elf/tests] Error 2
> >
> >with error messages, like
> >
> >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.os
> >has an STT_TLS symbol but doesn't have an SHF_TLS section
> >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.os
> >has an STT_TLS symbol but doesn't have an SHF_TLS section
>
> tst-tls* tests appear to be similar to .tls_common which looks very
> obsoleted and not supported by Clang. I don't think ifuncmain* or
> tst-audit* matters for typical usage (most users) but I can take a look.

"make check" should be clean on Fedora 33/x86-64.

> >When glibc is configured with --enable-static-pie, I got
> >
> >[hjl@gnu-skx-1 build-x86_64-linux]$ ./elf/ldconfig
> >Segmentation fault (core dumped)
> >[hjl@gnu-skx-1 build-x86_64-linux]$
> >
> >You need to fix lld first and give the working lld a proper version so that
> >configure can check it.
>
> I cherry picked "Make _dl_relocate_static_pie return an int indicating whether it applied relocs."
> in google/grte/v5-2.27/master, which has fixed the issue.

Why isn't it needed for ld?

-- 
H.J.

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

* Re: [PATCH 0/3] Make glibc build with LLD
  2020-12-28 21:49     ` H.J. Lu
@ 2020-12-28 22:54       ` H.J. Lu
  2021-01-05 23:03         ` Fāng-ruì Sòng
  2021-01-05 22:57       ` Fāng-ruì Sòng
  1 sibling, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2020-12-28 22:54 UTC (permalink / raw)
  To: Fangrui Song; +Cc: GNU C Library

On Mon, Dec 28, 2020 at 1:49 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Dec 28, 2020 at 1:45 PM Fangrui Song <maskray@google.com> wrote:
> >
> > On 2020-12-28, H.J. Lu wrote:
> > >On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song via Libc-alpha
> > ><libc-alpha@sourceware.org> wrote:
> > >>
> > >> I sent the first two in April. Resending in a patch series to be
> > >> clearer.
> > >>
> > >>   install: Replace scripts/output-format.sed with objdump -f
> > >>
> > >> replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
> > >> by leveraging objdump -f.
> > >>
> > >> With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
> > >> `make` with ld pointing to ld.lld (LLVM linker) works.
> > >
> > >I tried your branch with "LLD 11.0.0 (compatible with GNU linkers)" on
> > >Fedora 33/x86-64,
> > >"make check" generated:
> > >
> > >make[4]: *** [../Makerules:767:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.so]
> > >Error 1
> > >make[4]: *** [../Makerules:767:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.so]
> > >Error 1
> > >make[4]: *** [../Makerules:767:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-sym-lib.so]
> > >Error 1
> > >make[4]: *** [../Makerules:767:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-zero-lib.so]
> > >Error 1
> > >make[4]: *** [../Makerules:767:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod6.so]
> > >Error 1
> > >make[4]: *** [../Makerules:767:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod5.so]
> > >Error 1
> > >make[4]: *** [../Rules:226:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit16]
> > >Error 1
> > >make[4]: *** [../Rules:226:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit14]
> > >Error 1
> > >make[4]: *** [../Rules:226:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit15]
> > >Error 1
> > >make[4]: *** [../Rules:226:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tls1]
> > >Error 1
> > >make[4]: *** [../Rules:226:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain5]
> > >Error 1
> > >make[4]: *** [../Rules:226:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain1]
> > >Error 1
> > >make[3]: *** [Makefile:479: elf/tests] Error 2
> > >
> > >with error messages, like
> > >
> > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.os
> > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.os
> > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> >
> > tst-tls* tests appear to be similar to .tls_common which looks very
> > obsoleted and not supported by Clang. I don't think ifuncmain* or
> > tst-audit* matters for typical usage (most users) but I can take a look.
>
> "make check" should be clean on Fedora 33/x86-64.

Because this lld error, "make check" didn't report test summary:

Summary of test results:
   4322 PASS
      8 UNSUPPORTED
     13 XFAIL
      6 XPASS

> > >When glibc is configured with --enable-static-pie, I got
> > >
> > >[hjl@gnu-skx-1 build-x86_64-linux]$ ./elf/ldconfig
> > >Segmentation fault (core dumped)
> > >[hjl@gnu-skx-1 build-x86_64-linux]$
> > >
> > >You need to fix lld first and give the working lld a proper version so that
> > >configure can check it.
> >
> > I cherry picked "Make _dl_relocate_static_pie return an int indicating whether it applied relocs."
> > in google/grte/v5-2.27/master, which has fixed the issue.
>
> Why isn't it needed for ld?
>

Also re-order your patches to place the enabling lld patch the last so that any
commits can build a working glibc.

Thanks.

-- 
H.J.

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

* Re: [PATCH 0/3] Make glibc build with LLD
  2020-12-28 21:49     ` H.J. Lu
  2020-12-28 22:54       ` H.J. Lu
@ 2021-01-05 22:57       ` Fāng-ruì Sòng
  1 sibling, 0 replies; 42+ messages in thread
From: Fāng-ruì Sòng @ 2021-01-05 22:57 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library

On Mon, Dec 28, 2020 at 1:50 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Dec 28, 2020 at 1:45 PM Fangrui Song <maskray@google.com> wrote:
> >
> > On 2020-12-28, H.J. Lu wrote:
> > >On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song via Libc-alpha
> > ><libc-alpha@sourceware.org> wrote:
> > >>
> > >> I sent the first two in April. Resending in a patch series to be
> > >> clearer.
> > >>
> > >>   install: Replace scripts/output-format.sed with objdump -f
> > >>
> > >> replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
> > >> by leveraging objdump -f.
> > >>
> > >> With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
> > >> `make` with ld pointing to ld.lld (LLVM linker) works.
> > >
> > >I tried your branch with "LLD 11.0.0 (compatible with GNU linkers)" on
> > >Fedora 33/x86-64,
> > >"make check" generated:
> > >
> > >make[4]: *** [../Makerules:767:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.so]
> > >Error 1
> > >make[4]: *** [../Makerules:767:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.so]
> > >Error 1
> > >make[4]: *** [../Makerules:767:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-sym-lib.so]
> > >Error 1
> > >make[4]: *** [../Makerules:767:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-zero-lib.so]
> > >Error 1
> > >make[4]: *** [../Makerules:767:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod6.so]
> > >Error 1
> > >make[4]: *** [../Makerules:767:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod5.so]
> > >Error 1
> > >make[4]: *** [../Rules:226:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit16]
> > >Error 1
> > >make[4]: *** [../Rules:226:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit14]
> > >Error 1
> > >make[4]: *** [../Rules:226:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit15]
> > >Error 1
> > >make[4]: *** [../Rules:226:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tls1]
> > >Error 1
> > >make[4]: *** [../Rules:226:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain5]
> > >Error 1
> > >make[4]: *** [../Rules:226:
> > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain1]
> > >Error 1
> > >make[3]: *** [Makefile:479: elf/tests] Error 2
> > >
> > >with error messages, like
> > >
> > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.os
> > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.os
> > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> >
> > tst-tls* tests appear to be similar to .tls_common which looks very
> > obsoleted and not supported by Clang. I don't think ifuncmain* or
> > tst-audit* matters for typical usage (most users) but I can take a look.
>
> "make check" should be clean on Fedora 33/x86-64.
>
> > >When glibc is configured with --enable-static-pie, I got
> > >
> > >[hjl@gnu-skx-1 build-x86_64-linux]$ ./elf/ldconfig
> > >Segmentation fault (core dumped)
> > >[hjl@gnu-skx-1 build-x86_64-linux]$
> > >
> > >You need to fix lld first and give the working lld a proper version so that
> > >configure can check it.
> >
> > I cherry picked "Make _dl_relocate_static_pie return an int indicating whether it applied relocs."
> > in google/grte/v5-2.27/master, which has fixed the issue.
>
> Why isn't it needed for ld?
>
> --
> H.J.

"Make _dl_relocate_static_pie return an int indicating whether it
applied relocs." makes glibc build with both GNU ld and LLD.
Without the commit, elf/ldconfig segfaults with LLD but works with GNU
ld because the code relies on the undefined weak
__rela_iplt_start/__rela_iplt_end to be null.

% ld.bfd  --verbose | grep rela_iplt_start
      PROVIDE_HIDDEN (__rela_iplt_start = .);
% ld.bfd -pie --verbose | grep rela_iplt_start
% ld.bfd -shared --verbose | grep rela_iplt_start

I don't think this justifies a different behavior in ld.

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

* Re: [PATCH 0/3] Make glibc build with LLD
  2020-12-28 22:54       ` H.J. Lu
@ 2021-01-05 23:03         ` Fāng-ruì Sòng
  2021-01-05 23:33           ` H.J. Lu
  0 siblings, 1 reply; 42+ messages in thread
From: Fāng-ruì Sòng @ 2021-01-05 23:03 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library

On Mon, Dec 28, 2020 at 2:54 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Dec 28, 2020 at 1:49 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Mon, Dec 28, 2020 at 1:45 PM Fangrui Song <maskray@google.com> wrote:
> > >
> > > On 2020-12-28, H.J. Lu wrote:
> > > >On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song via Libc-alpha
> > > ><libc-alpha@sourceware.org> wrote:
> > > >>
> > > >> I sent the first two in April. Resending in a patch series to be
> > > >> clearer.
> > > >>
> > > >>   install: Replace scripts/output-format.sed with objdump -f
> > > >>
> > > >> replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
> > > >> by leveraging objdump -f.
> > > >>
> > > >> With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
> > > >> `make` with ld pointing to ld.lld (LLVM linker) works.
> > > >
> > > >I tried your branch with "LLD 11.0.0 (compatible with GNU linkers)" on
> > > >Fedora 33/x86-64,
> > > >"make check" generated:
> > > >
> > > >make[4]: *** [../Makerules:767:
> > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.so]
> > > >Error 1
> > > >make[4]: *** [../Makerules:767:
> > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.so]
> > > >Error 1
> > > >make[4]: *** [../Makerules:767:
> > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-sym-lib.so]
> > > >Error 1
> > > >make[4]: *** [../Makerules:767:
> > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-zero-lib.so]
> > > >Error 1
> > > >make[4]: *** [../Makerules:767:
> > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod6.so]
> > > >Error 1
> > > >make[4]: *** [../Makerules:767:
> > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod5.so]
> > > >Error 1
> > > >make[4]: *** [../Rules:226:
> > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit16]
> > > >Error 1
> > > >make[4]: *** [../Rules:226:
> > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit14]
> > > >Error 1
> > > >make[4]: *** [../Rules:226:
> > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit15]
> > > >Error 1
> > > >make[4]: *** [../Rules:226:
> > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tls1]
> > > >Error 1
> > > >make[4]: *** [../Rules:226:
> > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain5]
> > > >Error 1
> > > >make[4]: *** [../Rules:226:
> > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain1]
> > > >Error 1
> > > >make[3]: *** [Makefile:479: elf/tests] Error 2
> > > >
> > > >with error messages, like
> > > >
> > > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.os
> > > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.os
> > > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > >
> > > tst-tls* tests appear to be similar to .tls_common which looks very
> > > obsoleted and not supported by Clang. I don't think ifuncmain* or
> > > tst-audit* matters for typical usage (most users) but I can take a look.
> >
> > "make check" should be clean on Fedora 33/x86-64.
>
> Because this lld error, "make check" didn't report test summary:
>
> Summary of test results:
>    4322 PASS
>       8 UNSUPPORTED
>      13 XFAIL
>       6 XPASS
>
> > > >When glibc is configured with --enable-static-pie, I got
> > > >
> > > >[hjl@gnu-skx-1 build-x86_64-linux]$ ./elf/ldconfig
> > > >Segmentation fault (core dumped)
> > > >[hjl@gnu-skx-1 build-x86_64-linux]$
> > > >
> > > >You need to fix lld first and give the working lld a proper version so that
> > > >configure can check it.
> > >
> > > I cherry picked "Make _dl_relocate_static_pie return an int indicating whether it applied relocs."
> > > in google/grte/v5-2.27/master, which has fixed the issue.
> >
> > Why isn't it needed for ld?
> >
>
> Also re-order your patches to place the enabling lld patch the last so that any
> commits can build a working glibc.
>
> Thanks.
>
> --
> H.J.

Without "configure: Allow LD to be LLD 9.0.0 or above", configure
rejects LLD at configure time and the other commits cannot be tested
at all...

The other commits are general improvement and useful on their own, and
they are independent and can be merged separately.

As I mentioned in the other reply, LLD does not want to special case
the definition of __rela_iplt_start depending on -no-pie (available in
gold and LLD, not available in GNU ld yet) ; -pie/-shared...
The TLS common issues are obsoleted features that do not make sense nowadays.
Any case, the LLD produced executables are functional.

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

* Re: [PATCH 0/3] Make glibc build with LLD
  2021-01-05 23:03         ` Fāng-ruì Sòng
@ 2021-01-05 23:33           ` H.J. Lu
  2021-01-05 23:41             ` Fāng-ruì Sòng
  0 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2021-01-05 23:33 UTC (permalink / raw)
  To: Fāng-ruì Sòng; +Cc: GNU C Library

On Tue, Jan 5, 2021 at 3:03 PM Fāng-ruì Sòng <maskray@google.com> wrote:
>
> On Mon, Dec 28, 2020 at 2:54 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Mon, Dec 28, 2020 at 1:49 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Mon, Dec 28, 2020 at 1:45 PM Fangrui Song <maskray@google.com> wrote:
> > > >
> > > > On 2020-12-28, H.J. Lu wrote:
> > > > >On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song via Libc-alpha
> > > > ><libc-alpha@sourceware.org> wrote:
> > > > >>
> > > > >> I sent the first two in April. Resending in a patch series to be
> > > > >> clearer.
> > > > >>
> > > > >>   install: Replace scripts/output-format.sed with objdump -f
> > > > >>
> > > > >> replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
> > > > >> by leveraging objdump -f.
> > > > >>
> > > > >> With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
> > > > >> `make` with ld pointing to ld.lld (LLVM linker) works.
> > > > >
> > > > >I tried your branch with "LLD 11.0.0 (compatible with GNU linkers)" on
> > > > >Fedora 33/x86-64,
> > > > >"make check" generated:
> > > > >
> > > > >make[4]: *** [../Makerules:767:
> > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.so]
> > > > >Error 1
> > > > >make[4]: *** [../Makerules:767:
> > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.so]
> > > > >Error 1
> > > > >make[4]: *** [../Makerules:767:
> > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-sym-lib.so]
> > > > >Error 1
> > > > >make[4]: *** [../Makerules:767:
> > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-zero-lib.so]
> > > > >Error 1
> > > > >make[4]: *** [../Makerules:767:
> > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod6.so]
> > > > >Error 1
> > > > >make[4]: *** [../Makerules:767:
> > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod5.so]
> > > > >Error 1
> > > > >make[4]: *** [../Rules:226:
> > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit16]
> > > > >Error 1
> > > > >make[4]: *** [../Rules:226:
> > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit14]
> > > > >Error 1
> > > > >make[4]: *** [../Rules:226:
> > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit15]
> > > > >Error 1
> > > > >make[4]: *** [../Rules:226:
> > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tls1]
> > > > >Error 1
> > > > >make[4]: *** [../Rules:226:
> > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain5]
> > > > >Error 1
> > > > >make[4]: *** [../Rules:226:
> > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain1]
> > > > >Error 1
> > > > >make[3]: *** [Makefile:479: elf/tests] Error 2
> > > > >
> > > > >with error messages, like
> > > > >
> > > > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.os
> > > > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > > > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.os
> > > > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > > >
> > > > tst-tls* tests appear to be similar to .tls_common which looks very
> > > > obsoleted and not supported by Clang. I don't think ifuncmain* or
> > > > tst-audit* matters for typical usage (most users) but I can take a look.
> > >
> > > "make check" should be clean on Fedora 33/x86-64.
> >
> > Because this lld error, "make check" didn't report test summary:
> >
> > Summary of test results:
> >    4322 PASS
> >       8 UNSUPPORTED
> >      13 XFAIL
> >       6 XPASS
> >
> > > > >When glibc is configured with --enable-static-pie, I got
> > > > >
> > > > >[hjl@gnu-skx-1 build-x86_64-linux]$ ./elf/ldconfig
> > > > >Segmentation fault (core dumped)
> > > > >[hjl@gnu-skx-1 build-x86_64-linux]$
> > > > >
> > > > >You need to fix lld first and give the working lld a proper version so that
> > > > >configure can check it.
> > > >
> > > > I cherry picked "Make _dl_relocate_static_pie return an int indicating whether it applied relocs."
> > > > in google/grte/v5-2.27/master, which has fixed the issue.
> > >
> > > Why isn't it needed for ld?
> > >
> >
> > Also re-order your patches to place the enabling lld patch the last so that any
> > commits can build a working glibc.
> >
> > Thanks.
> >
> > --
> > H.J.
>
> Without "configure: Allow LD to be LLD 9.0.0 or above", configure
> rejects LLD at configure time and the other commits cannot be tested
> at all...
>
> The other commits are general improvement and useful on their own, and
> they are independent and can be merged separately.
>
> As I mentioned in the other reply, LLD does not want to special case
> the definition of __rela_iplt_start depending on -no-pie (available in
> gold and LLD, not available in GNU ld yet) ; -pie/-shared...
> The TLS common issues are obsoleted features that do not make sense nowadays.
> Any case, the LLD produced executables are functional.

The code in question is

static void
apply_irel (void)
{
# ifdef IREL
  /* We use weak references for these so that we'll still work with a linker
     that doesn't define them.  Such a linker doesn't support IFUNC at all
     and so uses won't work, but a statically-linked program that doesn't
     use any IFUNC symbols won't have a problem.  */
  extern const IREL_T IPLT_START[] __attribute__ ((weak));
  extern const IREL_T IPLT_END[] __attribute__ ((weak));
  for (const IREL_T *ipltent = IPLT_START; ipltent < IPLT_END; ++ipltent)
    IREL (ipltent);
# endif
}

Since IPLT_START and IPLT_END are undefined, linker should set
them to zero and the loop should be skipped.  Why doesn't LLD do
that?


--
H.J.

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

* Re: [PATCH 0/3] Make glibc build with LLD
  2021-01-05 23:33           ` H.J. Lu
@ 2021-01-05 23:41             ` Fāng-ruì Sòng
  2021-01-05 23:51               ` H.J. Lu
  0 siblings, 1 reply; 42+ messages in thread
From: Fāng-ruì Sòng @ 2021-01-05 23:41 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library

On Tue, Jan 5, 2021 at 3:34 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 5, 2021 at 3:03 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> >
> > On Mon, Dec 28, 2020 at 2:54 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Mon, Dec 28, 2020 at 1:49 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > >
> > > > On Mon, Dec 28, 2020 at 1:45 PM Fangrui Song <maskray@google.com> wrote:
> > > > >
> > > > > On 2020-12-28, H.J. Lu wrote:
> > > > > >On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song via Libc-alpha
> > > > > ><libc-alpha@sourceware.org> wrote:
> > > > > >>
> > > > > >> I sent the first two in April. Resending in a patch series to be
> > > > > >> clearer.
> > > > > >>
> > > > > >>   install: Replace scripts/output-format.sed with objdump -f
> > > > > >>
> > > > > >> replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
> > > > > >> by leveraging objdump -f.
> > > > > >>
> > > > > >> With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
> > > > > >> `make` with ld pointing to ld.lld (LLVM linker) works.
> > > > > >
> > > > > >I tried your branch with "LLD 11.0.0 (compatible with GNU linkers)" on
> > > > > >Fedora 33/x86-64,
> > > > > >"make check" generated:
> > > > > >
> > > > > >make[4]: *** [../Makerules:767:
> > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.so]
> > > > > >Error 1
> > > > > >make[4]: *** [../Makerules:767:
> > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.so]
> > > > > >Error 1
> > > > > >make[4]: *** [../Makerules:767:
> > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-sym-lib.so]
> > > > > >Error 1
> > > > > >make[4]: *** [../Makerules:767:
> > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-zero-lib.so]
> > > > > >Error 1
> > > > > >make[4]: *** [../Makerules:767:
> > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod6.so]
> > > > > >Error 1
> > > > > >make[4]: *** [../Makerules:767:
> > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod5.so]
> > > > > >Error 1
> > > > > >make[4]: *** [../Rules:226:
> > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit16]
> > > > > >Error 1
> > > > > >make[4]: *** [../Rules:226:
> > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit14]
> > > > > >Error 1
> > > > > >make[4]: *** [../Rules:226:
> > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit15]
> > > > > >Error 1
> > > > > >make[4]: *** [../Rules:226:
> > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tls1]
> > > > > >Error 1
> > > > > >make[4]: *** [../Rules:226:
> > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain5]
> > > > > >Error 1
> > > > > >make[4]: *** [../Rules:226:
> > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain1]
> > > > > >Error 1
> > > > > >make[3]: *** [Makefile:479: elf/tests] Error 2
> > > > > >
> > > > > >with error messages, like
> > > > > >
> > > > > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.os
> > > > > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > > > > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.os
> > > > > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > > > >
> > > > > tst-tls* tests appear to be similar to .tls_common which looks very
> > > > > obsoleted and not supported by Clang. I don't think ifuncmain* or
> > > > > tst-audit* matters for typical usage (most users) but I can take a look.
> > > >
> > > > "make check" should be clean on Fedora 33/x86-64.
> > >
> > > Because this lld error, "make check" didn't report test summary:
> > >
> > > Summary of test results:
> > >    4322 PASS
> > >       8 UNSUPPORTED
> > >      13 XFAIL
> > >       6 XPASS
> > >
> > > > > >When glibc is configured with --enable-static-pie, I got
> > > > > >
> > > > > >[hjl@gnu-skx-1 build-x86_64-linux]$ ./elf/ldconfig
> > > > > >Segmentation fault (core dumped)
> > > > > >[hjl@gnu-skx-1 build-x86_64-linux]$
> > > > > >
> > > > > >You need to fix lld first and give the working lld a proper version so that
> > > > > >configure can check it.
> > > > >
> > > > > I cherry picked "Make _dl_relocate_static_pie return an int indicating whether it applied relocs."
> > > > > in google/grte/v5-2.27/master, which has fixed the issue.
> > > >
> > > > Why isn't it needed for ld?
> > > >
> > >
> > > Also re-order your patches to place the enabling lld patch the last so that any
> > > commits can build a working glibc.
> > >
> > > Thanks.
> > >
> > > --
> > > H.J.
> >
> > Without "configure: Allow LD to be LLD 9.0.0 or above", configure
> > rejects LLD at configure time and the other commits cannot be tested
> > at all...
> >
> > The other commits are general improvement and useful on their own, and
> > they are independent and can be merged separately.
> >
> > As I mentioned in the other reply, LLD does not want to special case
> > the definition of __rela_iplt_start depending on -no-pie (available in
> > gold and LLD, not available in GNU ld yet) ; -pie/-shared...
> > The TLS common issues are obsoleted features that do not make sense nowadays.
> > Any case, the LLD produced executables are functional.
>
> The code in question is
>
> static void
> apply_irel (void)
> {
> # ifdef IREL
>   /* We use weak references for these so that we'll still work with a linker
>      that doesn't define them.  Such a linker doesn't support IFUNC at all
>      and so uses won't work, but a statically-linked program that doesn't
>      use any IFUNC symbols won't have a problem.  */
>   extern const IREL_T IPLT_START[] __attribute__ ((weak));
>   extern const IREL_T IPLT_END[] __attribute__ ((weak));
>   for (const IREL_T *ipltent = IPLT_START; ipltent < IPLT_END; ++ipltent)
>     IREL (ipltent);
> # endif
> }
>
> Since IPLT_START and IPLT_END are undefined, linker should set
> them to zero and the loop should be skipped.  Why doesn't LLD do
> that?
>
>
> --
> H.J.

LLD defines __rela_iplt_start/__rela_iplt_end if (1) __rela_iplt_start
exists and is not defined (2) not -r (3) no .interp

LLD defines __rela_iplt_start regardless of -no-pie/-pie. This
behavior makes sense to me.
GNU ld and gold seem to only define __rela_iplt_start in -no-pie mode.

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

* Re: [PATCH 0/3] Make glibc build with LLD
  2021-01-05 23:41             ` Fāng-ruì Sòng
@ 2021-01-05 23:51               ` H.J. Lu
  2021-01-06  0:50                 ` Fāng-ruì Sòng
  0 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2021-01-05 23:51 UTC (permalink / raw)
  To: Fāng-ruì Sòng; +Cc: GNU C Library

On Tue, Jan 5, 2021 at 3:41 PM Fāng-ruì Sòng <maskray@google.com> wrote:
>
> On Tue, Jan 5, 2021 at 3:34 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Tue, Jan 5, 2021 at 3:03 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> > >
> > > On Mon, Dec 28, 2020 at 2:54 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > >
> > > > On Mon, Dec 28, 2020 at 1:49 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > >
> > > > > On Mon, Dec 28, 2020 at 1:45 PM Fangrui Song <maskray@google.com> wrote:
> > > > > >
> > > > > > On 2020-12-28, H.J. Lu wrote:
> > > > > > >On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song via Libc-alpha
> > > > > > ><libc-alpha@sourceware.org> wrote:
> > > > > > >>
> > > > > > >> I sent the first two in April. Resending in a patch series to be
> > > > > > >> clearer.
> > > > > > >>
> > > > > > >>   install: Replace scripts/output-format.sed with objdump -f
> > > > > > >>
> > > > > > >> replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
> > > > > > >> by leveraging objdump -f.
> > > > > > >>
> > > > > > >> With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
> > > > > > >> `make` with ld pointing to ld.lld (LLVM linker) works.
> > > > > > >
> > > > > > >I tried your branch with "LLD 11.0.0 (compatible with GNU linkers)" on
> > > > > > >Fedora 33/x86-64,
> > > > > > >"make check" generated:
> > > > > > >
> > > > > > >make[4]: *** [../Makerules:767:
> > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.so]
> > > > > > >Error 1
> > > > > > >make[4]: *** [../Makerules:767:
> > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.so]
> > > > > > >Error 1
> > > > > > >make[4]: *** [../Makerules:767:
> > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-sym-lib.so]
> > > > > > >Error 1
> > > > > > >make[4]: *** [../Makerules:767:
> > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-zero-lib.so]
> > > > > > >Error 1
> > > > > > >make[4]: *** [../Makerules:767:
> > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod6.so]
> > > > > > >Error 1
> > > > > > >make[4]: *** [../Makerules:767:
> > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod5.so]
> > > > > > >Error 1
> > > > > > >make[4]: *** [../Rules:226:
> > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit16]
> > > > > > >Error 1
> > > > > > >make[4]: *** [../Rules:226:
> > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit14]
> > > > > > >Error 1
> > > > > > >make[4]: *** [../Rules:226:
> > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit15]
> > > > > > >Error 1
> > > > > > >make[4]: *** [../Rules:226:
> > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tls1]
> > > > > > >Error 1
> > > > > > >make[4]: *** [../Rules:226:
> > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain5]
> > > > > > >Error 1
> > > > > > >make[4]: *** [../Rules:226:
> > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain1]
> > > > > > >Error 1
> > > > > > >make[3]: *** [Makefile:479: elf/tests] Error 2
> > > > > > >
> > > > > > >with error messages, like
> > > > > > >
> > > > > > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.os
> > > > > > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > > > > > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.os
> > > > > > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > > > > >
> > > > > > tst-tls* tests appear to be similar to .tls_common which looks very
> > > > > > obsoleted and not supported by Clang. I don't think ifuncmain* or
> > > > > > tst-audit* matters for typical usage (most users) but I can take a look.
> > > > >
> > > > > "make check" should be clean on Fedora 33/x86-64.
> > > >
> > > > Because this lld error, "make check" didn't report test summary:
> > > >
> > > > Summary of test results:
> > > >    4322 PASS
> > > >       8 UNSUPPORTED
> > > >      13 XFAIL
> > > >       6 XPASS
> > > >
> > > > > > >When glibc is configured with --enable-static-pie, I got
> > > > > > >
> > > > > > >[hjl@gnu-skx-1 build-x86_64-linux]$ ./elf/ldconfig
> > > > > > >Segmentation fault (core dumped)
> > > > > > >[hjl@gnu-skx-1 build-x86_64-linux]$
> > > > > > >
> > > > > > >You need to fix lld first and give the working lld a proper version so that
> > > > > > >configure can check it.
> > > > > >
> > > > > > I cherry picked "Make _dl_relocate_static_pie return an int indicating whether it applied relocs."
> > > > > > in google/grte/v5-2.27/master, which has fixed the issue.
> > > > >
> > > > > Why isn't it needed for ld?
> > > > >
> > > >
> > > > Also re-order your patches to place the enabling lld patch the last so that any
> > > > commits can build a working glibc.
> > > >
> > > > Thanks.
> > > >
> > > > --
> > > > H.J.
> > >
> > > Without "configure: Allow LD to be LLD 9.0.0 or above", configure
> > > rejects LLD at configure time and the other commits cannot be tested
> > > at all...
> > >
> > > The other commits are general improvement and useful on their own, and
> > > they are independent and can be merged separately.
> > >
> > > As I mentioned in the other reply, LLD does not want to special case
> > > the definition of __rela_iplt_start depending on -no-pie (available in
> > > gold and LLD, not available in GNU ld yet) ; -pie/-shared...
> > > The TLS common issues are obsoleted features that do not make sense nowadays.
> > > Any case, the LLD produced executables are functional.
> >
> > The code in question is
> >
> > static void
> > apply_irel (void)
> > {
> > # ifdef IREL
> >   /* We use weak references for these so that we'll still work with a linker
> >      that doesn't define them.  Such a linker doesn't support IFUNC at all
> >      and so uses won't work, but a statically-linked program that doesn't
> >      use any IFUNC symbols won't have a problem.  */
> >   extern const IREL_T IPLT_START[] __attribute__ ((weak));
> >   extern const IREL_T IPLT_END[] __attribute__ ((weak));
> >   for (const IREL_T *ipltent = IPLT_START; ipltent < IPLT_END; ++ipltent)
> >     IREL (ipltent);
> > # endif
> > }
> >
> > Since IPLT_START and IPLT_END are undefined, linker should set
> > them to zero and the loop should be skipped.  Why doesn't LLD do
> > that?
> >
> >
> > --
> > H.J.
>
> LLD defines __rela_iplt_start/__rela_iplt_end if (1) __rela_iplt_start
> exists and is not defined (2) not -r (3) no .interp
>
> LLD defines __rela_iplt_start regardless of -no-pie/-pie. This
> behavior makes sense to me.
> GNU ld and gold seem to only define __rela_iplt_start in -no-pie mode.

It is an lld bug:

https://bugs.llvm.org/show_bug.cgi?id=48674

-- 
H.J.

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

* Re: [PATCH 0/3] Make glibc build with LLD
  2021-01-05 23:51               ` H.J. Lu
@ 2021-01-06  0:50                 ` Fāng-ruì Sòng
  2021-01-06  1:43                   ` H.J. Lu
  0 siblings, 1 reply; 42+ messages in thread
From: Fāng-ruì Sòng @ 2021-01-06  0:50 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library

On Tue, Jan 5, 2021 at 3:52 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 5, 2021 at 3:41 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> >
> > On Tue, Jan 5, 2021 at 3:34 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Tue, Jan 5, 2021 at 3:03 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> > > >
> > > > On Mon, Dec 28, 2020 at 2:54 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > >
> > > > > On Mon, Dec 28, 2020 at 1:49 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > >
> > > > > > On Mon, Dec 28, 2020 at 1:45 PM Fangrui Song <maskray@google.com> wrote:
> > > > > > >
> > > > > > > On 2020-12-28, H.J. Lu wrote:
> > > > > > > >On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song via Libc-alpha
> > > > > > > ><libc-alpha@sourceware.org> wrote:
> > > > > > > >>
> > > > > > > >> I sent the first two in April. Resending in a patch series to be
> > > > > > > >> clearer.
> > > > > > > >>
> > > > > > > >>   install: Replace scripts/output-format.sed with objdump -f
> > > > > > > >>
> > > > > > > >> replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
> > > > > > > >> by leveraging objdump -f.
> > > > > > > >>
> > > > > > > >> With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
> > > > > > > >> `make` with ld pointing to ld.lld (LLVM linker) works.
> > > > > > > >
> > > > > > > >I tried your branch with "LLD 11.0.0 (compatible with GNU linkers)" on
> > > > > > > >Fedora 33/x86-64,
> > > > > > > >"make check" generated:
> > > > > > > >
> > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.so]
> > > > > > > >Error 1
> > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.so]
> > > > > > > >Error 1
> > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-sym-lib.so]
> > > > > > > >Error 1
> > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-zero-lib.so]
> > > > > > > >Error 1
> > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod6.so]
> > > > > > > >Error 1
> > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod5.so]
> > > > > > > >Error 1
> > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit16]
> > > > > > > >Error 1
> > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit14]
> > > > > > > >Error 1
> > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit15]
> > > > > > > >Error 1
> > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tls1]
> > > > > > > >Error 1
> > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain5]
> > > > > > > >Error 1
> > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain1]
> > > > > > > >Error 1
> > > > > > > >make[3]: *** [Makefile:479: elf/tests] Error 2
> > > > > > > >
> > > > > > > >with error messages, like
> > > > > > > >
> > > > > > > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.os
> > > > > > > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > > > > > > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.os
> > > > > > > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > > > > > >
> > > > > > > tst-tls* tests appear to be similar to .tls_common which looks very
> > > > > > > obsoleted and not supported by Clang. I don't think ifuncmain* or
> > > > > > > tst-audit* matters for typical usage (most users) but I can take a look.
> > > > > >
> > > > > > "make check" should be clean on Fedora 33/x86-64.
> > > > >
> > > > > Because this lld error, "make check" didn't report test summary:
> > > > >
> > > > > Summary of test results:
> > > > >    4322 PASS
> > > > >       8 UNSUPPORTED
> > > > >      13 XFAIL
> > > > >       6 XPASS
> > > > >
> > > > > > > >When glibc is configured with --enable-static-pie, I got
> > > > > > > >
> > > > > > > >[hjl@gnu-skx-1 build-x86_64-linux]$ ./elf/ldconfig
> > > > > > > >Segmentation fault (core dumped)
> > > > > > > >[hjl@gnu-skx-1 build-x86_64-linux]$
> > > > > > > >
> > > > > > > >You need to fix lld first and give the working lld a proper version so that
> > > > > > > >configure can check it.
> > > > > > >
> > > > > > > I cherry picked "Make _dl_relocate_static_pie return an int indicating whether it applied relocs."
> > > > > > > in google/grte/v5-2.27/master, which has fixed the issue.
> > > > > >
> > > > > > Why isn't it needed for ld?
> > > > > >
> > > > >
> > > > > Also re-order your patches to place the enabling lld patch the last so that any
> > > > > commits can build a working glibc.
> > > > >
> > > > > Thanks.
> > > > >
> > > > > --
> > > > > H.J.
> > > >
> > > > Without "configure: Allow LD to be LLD 9.0.0 or above", configure
> > > > rejects LLD at configure time and the other commits cannot be tested
> > > > at all...
> > > >
> > > > The other commits are general improvement and useful on their own, and
> > > > they are independent and can be merged separately.
> > > >
> > > > As I mentioned in the other reply, LLD does not want to special case
> > > > the definition of __rela_iplt_start depending on -no-pie (available in
> > > > gold and LLD, not available in GNU ld yet) ; -pie/-shared...
> > > > The TLS common issues are obsoleted features that do not make sense nowadays.
> > > > Any case, the LLD produced executables are functional.
> > >
> > > The code in question is
> > >
> > > static void
> > > apply_irel (void)
> > > {
> > > # ifdef IREL
> > >   /* We use weak references for these so that we'll still work with a linker
> > >      that doesn't define them.  Such a linker doesn't support IFUNC at all
> > >      and so uses won't work, but a statically-linked program that doesn't
> > >      use any IFUNC symbols won't have a problem.  */
> > >   extern const IREL_T IPLT_START[] __attribute__ ((weak));
> > >   extern const IREL_T IPLT_END[] __attribute__ ((weak));
> > >   for (const IREL_T *ipltent = IPLT_START; ipltent < IPLT_END; ++ipltent)
> > >     IREL (ipltent);
> > > # endif
> > > }
> > >
> > > Since IPLT_START and IPLT_END are undefined, linker should set
> > > them to zero and the loop should be skipped.  Why doesn't LLD do
> > > that?
> > >
> > >
> > > --
> > > H.J.
> >
> > LLD defines __rela_iplt_start/__rela_iplt_end if (1) __rela_iplt_start
> > exists and is not defined (2) not -r (3) no .interp
> >
> > LLD defines __rela_iplt_start regardless of -no-pie/-pie. This
> > behavior makes sense to me.
> > GNU ld and gold seem to only define __rela_iplt_start in -no-pie mode.
>
> It is an lld bug:
>
> https://bugs.llvm.org/show_bug.cgi?id=48674
>
> --
> H.J.

I don't consider https://bugs.llvm.org/show_bug.cgi?id=48674 an LLD
bug, so we have disagreement.

__rela_iplt_start/__rela_iplt_end is a contract between the link
editor and the dynamic loader/libc. For the perspective of a good
design, I don't think it is necessary making
"whether to define __rela_iplt_start" different for ld -no-pie (GNU ld
does not have -no-pie currently, but gold and LLD do) and ld -pie.

rtld/libc can just figure out whether it is -static-pie (since a
different crt1 rcrt1.o is used) and don't reference __rela_iplt_start
in that case.

If this difference is dropped (by taking the glibc patch "Make
_dl_relocate_static_pie return an int indicating whether it applied
relocs."),
diff -u =(ld.bfd --verbose) =(ld.bfd -pie --verbose) can have no
difference other than the start address: __executable_start.

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

* Re: [PATCH 0/3] Make glibc build with LLD
  2021-01-06  0:50                 ` Fāng-ruì Sòng
@ 2021-01-06  1:43                   ` H.J. Lu
  2021-01-06  2:00                     ` Fāng-ruì Sòng
  0 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2021-01-06  1:43 UTC (permalink / raw)
  To: Fāng-ruì Sòng; +Cc: GNU C Library

On Tue, Jan 5, 2021 at 4:51 PM Fāng-ruì Sòng <maskray@google.com> wrote:
>
> On Tue, Jan 5, 2021 at 3:52 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Tue, Jan 5, 2021 at 3:41 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> > >
> > > On Tue, Jan 5, 2021 at 3:34 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > >
> > > > On Tue, Jan 5, 2021 at 3:03 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> > > > >
> > > > > On Mon, Dec 28, 2020 at 2:54 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > >
> > > > > > On Mon, Dec 28, 2020 at 1:49 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > > >
> > > > > > > On Mon, Dec 28, 2020 at 1:45 PM Fangrui Song <maskray@google.com> wrote:
> > > > > > > >
> > > > > > > > On 2020-12-28, H.J. Lu wrote:
> > > > > > > > >On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song via Libc-alpha
> > > > > > > > ><libc-alpha@sourceware.org> wrote:
> > > > > > > > >>
> > > > > > > > >> I sent the first two in April. Resending in a patch series to be
> > > > > > > > >> clearer.
> > > > > > > > >>
> > > > > > > > >>   install: Replace scripts/output-format.sed with objdump -f
> > > > > > > > >>
> > > > > > > > >> replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
> > > > > > > > >> by leveraging objdump -f.
> > > > > > > > >>
> > > > > > > > >> With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
> > > > > > > > >> `make` with ld pointing to ld.lld (LLVM linker) works.
> > > > > > > > >
> > > > > > > > >I tried your branch with "LLD 11.0.0 (compatible with GNU linkers)" on
> > > > > > > > >Fedora 33/x86-64,
> > > > > > > > >"make check" generated:
> > > > > > > > >
> > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.so]
> > > > > > > > >Error 1
> > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.so]
> > > > > > > > >Error 1
> > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-sym-lib.so]
> > > > > > > > >Error 1
> > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-zero-lib.so]
> > > > > > > > >Error 1
> > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod6.so]
> > > > > > > > >Error 1
> > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod5.so]
> > > > > > > > >Error 1
> > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit16]
> > > > > > > > >Error 1
> > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit14]
> > > > > > > > >Error 1
> > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit15]
> > > > > > > > >Error 1
> > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tls1]
> > > > > > > > >Error 1
> > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain5]
> > > > > > > > >Error 1
> > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain1]
> > > > > > > > >Error 1
> > > > > > > > >make[3]: *** [Makefile:479: elf/tests] Error 2
> > > > > > > > >
> > > > > > > > >with error messages, like
> > > > > > > > >
> > > > > > > > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.os
> > > > > > > > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > > > > > > > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.os
> > > > > > > > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > > > > > > >
> > > > > > > > tst-tls* tests appear to be similar to .tls_common which looks very
> > > > > > > > obsoleted and not supported by Clang. I don't think ifuncmain* or
> > > > > > > > tst-audit* matters for typical usage (most users) but I can take a look.
> > > > > > >
> > > > > > > "make check" should be clean on Fedora 33/x86-64.
> > > > > >
> > > > > > Because this lld error, "make check" didn't report test summary:
> > > > > >
> > > > > > Summary of test results:
> > > > > >    4322 PASS
> > > > > >       8 UNSUPPORTED
> > > > > >      13 XFAIL
> > > > > >       6 XPASS
> > > > > >
> > > > > > > > >When glibc is configured with --enable-static-pie, I got
> > > > > > > > >
> > > > > > > > >[hjl@gnu-skx-1 build-x86_64-linux]$ ./elf/ldconfig
> > > > > > > > >Segmentation fault (core dumped)
> > > > > > > > >[hjl@gnu-skx-1 build-x86_64-linux]$
> > > > > > > > >
> > > > > > > > >You need to fix lld first and give the working lld a proper version so that
> > > > > > > > >configure can check it.
> > > > > > > >
> > > > > > > > I cherry picked "Make _dl_relocate_static_pie return an int indicating whether it applied relocs."
> > > > > > > > in google/grte/v5-2.27/master, which has fixed the issue.
> > > > > > >
> > > > > > > Why isn't it needed for ld?
> > > > > > >
> > > > > >
> > > > > > Also re-order your patches to place the enabling lld patch the last so that any
> > > > > > commits can build a working glibc.
> > > > > >
> > > > > > Thanks.
> > > > > >
> > > > > > --
> > > > > > H.J.
> > > > >
> > > > > Without "configure: Allow LD to be LLD 9.0.0 or above", configure
> > > > > rejects LLD at configure time and the other commits cannot be tested
> > > > > at all...
> > > > >
> > > > > The other commits are general improvement and useful on their own, and
> > > > > they are independent and can be merged separately.
> > > > >
> > > > > As I mentioned in the other reply, LLD does not want to special case
> > > > > the definition of __rela_iplt_start depending on -no-pie (available in
> > > > > gold and LLD, not available in GNU ld yet) ; -pie/-shared...
> > > > > The TLS common issues are obsoleted features that do not make sense nowadays.
> > > > > Any case, the LLD produced executables are functional.
> > > >
> > > > The code in question is
> > > >
> > > > static void
> > > > apply_irel (void)
> > > > {
> > > > # ifdef IREL
> > > >   /* We use weak references for these so that we'll still work with a linker
> > > >      that doesn't define them.  Such a linker doesn't support IFUNC at all
> > > >      and so uses won't work, but a statically-linked program that doesn't
> > > >      use any IFUNC symbols won't have a problem.  */
> > > >   extern const IREL_T IPLT_START[] __attribute__ ((weak));
> > > >   extern const IREL_T IPLT_END[] __attribute__ ((weak));
> > > >   for (const IREL_T *ipltent = IPLT_START; ipltent < IPLT_END; ++ipltent)
> > > >     IREL (ipltent);
> > > > # endif
> > > > }
> > > >
> > > > Since IPLT_START and IPLT_END are undefined, linker should set
> > > > them to zero and the loop should be skipped.  Why doesn't LLD do
> > > > that?
> > > >
> > > >
> > > > --
> > > > H.J.
> > >
> > > LLD defines __rela_iplt_start/__rela_iplt_end if (1) __rela_iplt_start
> > > exists and is not defined (2) not -r (3) no .interp
> > >
> > > LLD defines __rela_iplt_start regardless of -no-pie/-pie. This
> > > behavior makes sense to me.
> > > GNU ld and gold seem to only define __rela_iplt_start in -no-pie mode.
> >
> > It is an lld bug:
> >
> > https://bugs.llvm.org/show_bug.cgi?id=48674
> >
> > --
> > H.J.
>
> I don't consider https://bugs.llvm.org/show_bug.cgi?id=48674 an LLD
> bug, so we have disagreement.
>
> __rela_iplt_start/__rela_iplt_end is a contract between the link
> editor and the dynamic loader/libc. For the perspective of a good
> design, I don't think it is necessary making
> "whether to define __rela_iplt_start" different for ld -no-pie (GNU ld
> does not have -no-pie currently, but gold and LLD do) and ld -pie.
>
> rtld/libc can just figure out whether it is -static-pie (since a
> different crt1 rcrt1.o is used) and don't reference __rela_iplt_start
> in that case.
>
> If this difference is dropped (by taking the glibc patch "Make
> _dl_relocate_static_pie return an int indicating whether it applied
> relocs."),
> diff -u =(ld.bfd --verbose) =(ld.bfd -pie --verbose) can have no
> difference other than the start address: __executable_start.

I designed and implemented the whole thing in ld and glibc
many years ago.  I don't think they should be changed.  lld
should follow my design.

-- 
H.J.

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

* Re: [PATCH 0/3] Make glibc build with LLD
  2021-01-06  1:43                   ` H.J. Lu
@ 2021-01-06  2:00                     ` Fāng-ruì Sòng
  2021-01-06  2:14                       ` H.J. Lu
  0 siblings, 1 reply; 42+ messages in thread
From: Fāng-ruì Sòng @ 2021-01-06  2:00 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library

On Tue, Jan 5, 2021 at 5:44 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 5, 2021 at 4:51 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> >
> > On Tue, Jan 5, 2021 at 3:52 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Tue, Jan 5, 2021 at 3:41 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> > > >
> > > > On Tue, Jan 5, 2021 at 3:34 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > >
> > > > > On Tue, Jan 5, 2021 at 3:03 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> > > > > >
> > > > > > On Mon, Dec 28, 2020 at 2:54 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > > >
> > > > > > > On Mon, Dec 28, 2020 at 1:49 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > > > >
> > > > > > > > On Mon, Dec 28, 2020 at 1:45 PM Fangrui Song <maskray@google.com> wrote:
> > > > > > > > >
> > > > > > > > > On 2020-12-28, H.J. Lu wrote:
> > > > > > > > > >On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song via Libc-alpha
> > > > > > > > > ><libc-alpha@sourceware.org> wrote:
> > > > > > > > > >>
> > > > > > > > > >> I sent the first two in April. Resending in a patch series to be
> > > > > > > > > >> clearer.
> > > > > > > > > >>
> > > > > > > > > >>   install: Replace scripts/output-format.sed with objdump -f
> > > > > > > > > >>
> > > > > > > > > >> replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
> > > > > > > > > >> by leveraging objdump -f.
> > > > > > > > > >>
> > > > > > > > > >> With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
> > > > > > > > > >> `make` with ld pointing to ld.lld (LLVM linker) works.
> > > > > > > > > >
> > > > > > > > > >I tried your branch with "LLD 11.0.0 (compatible with GNU linkers)" on
> > > > > > > > > >Fedora 33/x86-64,
> > > > > > > > > >"make check" generated:
> > > > > > > > > >
> > > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.so]
> > > > > > > > > >Error 1
> > > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.so]
> > > > > > > > > >Error 1
> > > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-sym-lib.so]
> > > > > > > > > >Error 1
> > > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-zero-lib.so]
> > > > > > > > > >Error 1
> > > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod6.so]
> > > > > > > > > >Error 1
> > > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod5.so]
> > > > > > > > > >Error 1
> > > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit16]
> > > > > > > > > >Error 1
> > > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit14]
> > > > > > > > > >Error 1
> > > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit15]
> > > > > > > > > >Error 1
> > > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tls1]
> > > > > > > > > >Error 1
> > > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain5]
> > > > > > > > > >Error 1
> > > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain1]
> > > > > > > > > >Error 1
> > > > > > > > > >make[3]: *** [Makefile:479: elf/tests] Error 2
> > > > > > > > > >
> > > > > > > > > >with error messages, like
> > > > > > > > > >
> > > > > > > > > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.os
> > > > > > > > > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > > > > > > > > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.os
> > > > > > > > > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > > > > > > > >
> > > > > > > > > tst-tls* tests appear to be similar to .tls_common which looks very
> > > > > > > > > obsoleted and not supported by Clang. I don't think ifuncmain* or
> > > > > > > > > tst-audit* matters for typical usage (most users) but I can take a look.
> > > > > > > >
> > > > > > > > "make check" should be clean on Fedora 33/x86-64.
> > > > > > >
> > > > > > > Because this lld error, "make check" didn't report test summary:
> > > > > > >
> > > > > > > Summary of test results:
> > > > > > >    4322 PASS
> > > > > > >       8 UNSUPPORTED
> > > > > > >      13 XFAIL
> > > > > > >       6 XPASS
> > > > > > >
> > > > > > > > > >When glibc is configured with --enable-static-pie, I got
> > > > > > > > > >
> > > > > > > > > >[hjl@gnu-skx-1 build-x86_64-linux]$ ./elf/ldconfig
> > > > > > > > > >Segmentation fault (core dumped)
> > > > > > > > > >[hjl@gnu-skx-1 build-x86_64-linux]$
> > > > > > > > > >
> > > > > > > > > >You need to fix lld first and give the working lld a proper version so that
> > > > > > > > > >configure can check it.
> > > > > > > > >
> > > > > > > > > I cherry picked "Make _dl_relocate_static_pie return an int indicating whether it applied relocs."
> > > > > > > > > in google/grte/v5-2.27/master, which has fixed the issue.
> > > > > > > >
> > > > > > > > Why isn't it needed for ld?
> > > > > > > >
> > > > > > >
> > > > > > > Also re-order your patches to place the enabling lld patch the last so that any
> > > > > > > commits can build a working glibc.
> > > > > > >
> > > > > > > Thanks.
> > > > > > >
> > > > > > > --
> > > > > > > H.J.
> > > > > >
> > > > > > Without "configure: Allow LD to be LLD 9.0.0 or above", configure
> > > > > > rejects LLD at configure time and the other commits cannot be tested
> > > > > > at all...
> > > > > >
> > > > > > The other commits are general improvement and useful on their own, and
> > > > > > they are independent and can be merged separately.
> > > > > >
> > > > > > As I mentioned in the other reply, LLD does not want to special case
> > > > > > the definition of __rela_iplt_start depending on -no-pie (available in
> > > > > > gold and LLD, not available in GNU ld yet) ; -pie/-shared...
> > > > > > The TLS common issues are obsoleted features that do not make sense nowadays.
> > > > > > Any case, the LLD produced executables are functional.
> > > > >
> > > > > The code in question is
> > > > >
> > > > > static void
> > > > > apply_irel (void)
> > > > > {
> > > > > # ifdef IREL
> > > > >   /* We use weak references for these so that we'll still work with a linker
> > > > >      that doesn't define them.  Such a linker doesn't support IFUNC at all
> > > > >      and so uses won't work, but a statically-linked program that doesn't
> > > > >      use any IFUNC symbols won't have a problem.  */
> > > > >   extern const IREL_T IPLT_START[] __attribute__ ((weak));
> > > > >   extern const IREL_T IPLT_END[] __attribute__ ((weak));
> > > > >   for (const IREL_T *ipltent = IPLT_START; ipltent < IPLT_END; ++ipltent)
> > > > >     IREL (ipltent);
> > > > > # endif
> > > > > }
> > > > >
> > > > > Since IPLT_START and IPLT_END are undefined, linker should set
> > > > > them to zero and the loop should be skipped.  Why doesn't LLD do
> > > > > that?
> > > > >
> > > > >
> > > > > --
> > > > > H.J.
> > > >
> > > > LLD defines __rela_iplt_start/__rela_iplt_end if (1) __rela_iplt_start
> > > > exists and is not defined (2) not -r (3) no .interp
> > > >
> > > > LLD defines __rela_iplt_start regardless of -no-pie/-pie. This
> > > > behavior makes sense to me.
> > > > GNU ld and gold seem to only define __rela_iplt_start in -no-pie mode.
> > >
> > > It is an lld bug:
> > >
> > > https://bugs.llvm.org/show_bug.cgi?id=48674
> > >
> > > --
> > > H.J.
> >
> > I don't consider https://bugs.llvm.org/show_bug.cgi?id=48674 an LLD
> > bug, so we have disagreement.
> >
> > __rela_iplt_start/__rela_iplt_end is a contract between the link
> > editor and the dynamic loader/libc. For the perspective of a good
> > design, I don't think it is necessary making
> > "whether to define __rela_iplt_start" different for ld -no-pie (GNU ld
> > does not have -no-pie currently, but gold and LLD do) and ld -pie.
> >
> > rtld/libc can just figure out whether it is -static-pie (since a
> > different crt1 rcrt1.o is used) and don't reference __rela_iplt_start
> > in that case.
> >
> > If this difference is dropped (by taking the glibc patch "Make
> > _dl_relocate_static_pie return an int indicating whether it applied
> > relocs."),
> > diff -u =(ld.bfd --verbose) =(ld.bfd -pie --verbose) can have no
> > difference other than the start address: __executable_start.
>
> I designed and implemented the whole thing in ld and glibc
> many years ago.

I see, binutils commit 3aa14d16c669ca75f9fa4e995a2e2d13069dff3f
(2009-06-01) added __rela_iplt_start.

>  I don't think they should be changed.  lld should follow my design.

However, that was before glibc gained static pie support (2017;
http://sourceware.org/PR19574). With static pie, I think
__rela_iplt_start should be revised.
I have made many compatibility changes to LLD, but this one seems
unreasonable, so I am not inclined to folllow.
I'll personally add this to my list of incompatibilities
https://maskray.me/blog/2020-12-19-lld-and-gnu-linker-incompatibilities

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

* Re: [PATCH 0/3] Make glibc build with LLD
  2021-01-06  2:00                     ` Fāng-ruì Sòng
@ 2021-01-06  2:14                       ` H.J. Lu
  0 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2021-01-06  2:14 UTC (permalink / raw)
  To: Fāng-ruì Sòng; +Cc: GNU C Library

On Tue, Jan 5, 2021 at 6:00 PM Fāng-ruì Sòng <maskray@google.com> wrote:
>
> On Tue, Jan 5, 2021 at 5:44 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Tue, Jan 5, 2021 at 4:51 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> > >
> > > On Tue, Jan 5, 2021 at 3:52 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > >
> > > > On Tue, Jan 5, 2021 at 3:41 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> > > > >
> > > > > On Tue, Jan 5, 2021 at 3:34 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > >
> > > > > > On Tue, Jan 5, 2021 at 3:03 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> > > > > > >
> > > > > > > On Mon, Dec 28, 2020 at 2:54 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > > > >
> > > > > > > > On Mon, Dec 28, 2020 at 1:49 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > On Mon, Dec 28, 2020 at 1:45 PM Fangrui Song <maskray@google.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On 2020-12-28, H.J. Lu wrote:
> > > > > > > > > > >On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song via Libc-alpha
> > > > > > > > > > ><libc-alpha@sourceware.org> wrote:
> > > > > > > > > > >>
> > > > > > > > > > >> I sent the first two in April. Resending in a patch series to be
> > > > > > > > > > >> clearer.
> > > > > > > > > > >>
> > > > > > > > > > >>   install: Replace scripts/output-format.sed with objdump -f
> > > > > > > > > > >>
> > > > > > > > > > >> replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
> > > > > > > > > > >> by leveraging objdump -f.
> > > > > > > > > > >>
> > > > > > > > > > >> With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
> > > > > > > > > > >> `make` with ld pointing to ld.lld (LLVM linker) works.
> > > > > > > > > > >
> > > > > > > > > > >I tried your branch with "LLD 11.0.0 (compatible with GNU linkers)" on
> > > > > > > > > > >Fedora 33/x86-64,
> > > > > > > > > > >"make check" generated:
> > > > > > > > > > >
> > > > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.so]
> > > > > > > > > > >Error 1
> > > > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.so]
> > > > > > > > > > >Error 1
> > > > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-sym-lib.so]
> > > > > > > > > > >Error 1
> > > > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-absolute-zero-lib.so]
> > > > > > > > > > >Error 1
> > > > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod6.so]
> > > > > > > > > > >Error 1
> > > > > > > > > > >make[4]: *** [../Makerules:767:
> > > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod5.so]
> > > > > > > > > > >Error 1
> > > > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit16]
> > > > > > > > > > >Error 1
> > > > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit14]
> > > > > > > > > > >Error 1
> > > > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-audit15]
> > > > > > > > > > >Error 1
> > > > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tls1]
> > > > > > > > > > >Error 1
> > > > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain5]
> > > > > > > > > > >Error 1
> > > > > > > > > > >make[4]: *** [../Rules:226:
> > > > > > > > > > >/export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/ifuncmain1]
> > > > > > > > > > >Error 1
> > > > > > > > > > >make[3]: *** [Makefile:479: elf/tests] Error 2
> > > > > > > > > > >
> > > > > > > > > > >with error messages, like
> > > > > > > > > > >
> > > > > > > > > > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod2.os
> > > > > > > > > > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > > > > > > > > > >ld: error: /export/users/hjl/build/gnu/tools-build/glibc-gitlab-lld/build-x86_64-linux/elf/tst-tlsmod4.os
> > > > > > > > > > >has an STT_TLS symbol but doesn't have an SHF_TLS section
> > > > > > > > > >
> > > > > > > > > > tst-tls* tests appear to be similar to .tls_common which looks very
> > > > > > > > > > obsoleted and not supported by Clang. I don't think ifuncmain* or
> > > > > > > > > > tst-audit* matters for typical usage (most users) but I can take a look.
> > > > > > > > >
> > > > > > > > > "make check" should be clean on Fedora 33/x86-64.
> > > > > > > >
> > > > > > > > Because this lld error, "make check" didn't report test summary:
> > > > > > > >
> > > > > > > > Summary of test results:
> > > > > > > >    4322 PASS
> > > > > > > >       8 UNSUPPORTED
> > > > > > > >      13 XFAIL
> > > > > > > >       6 XPASS
> > > > > > > >
> > > > > > > > > > >When glibc is configured with --enable-static-pie, I got
> > > > > > > > > > >
> > > > > > > > > > >[hjl@gnu-skx-1 build-x86_64-linux]$ ./elf/ldconfig
> > > > > > > > > > >Segmentation fault (core dumped)
> > > > > > > > > > >[hjl@gnu-skx-1 build-x86_64-linux]$
> > > > > > > > > > >
> > > > > > > > > > >You need to fix lld first and give the working lld a proper version so that
> > > > > > > > > > >configure can check it.
> > > > > > > > > >
> > > > > > > > > > I cherry picked "Make _dl_relocate_static_pie return an int indicating whether it applied relocs."
> > > > > > > > > > in google/grte/v5-2.27/master, which has fixed the issue.
> > > > > > > > >
> > > > > > > > > Why isn't it needed for ld?
> > > > > > > > >
> > > > > > > >
> > > > > > > > Also re-order your patches to place the enabling lld patch the last so that any
> > > > > > > > commits can build a working glibc.
> > > > > > > >
> > > > > > > > Thanks.
> > > > > > > >
> > > > > > > > --
> > > > > > > > H.J.
> > > > > > >
> > > > > > > Without "configure: Allow LD to be LLD 9.0.0 or above", configure
> > > > > > > rejects LLD at configure time and the other commits cannot be tested
> > > > > > > at all...
> > > > > > >
> > > > > > > The other commits are general improvement and useful on their own, and
> > > > > > > they are independent and can be merged separately.
> > > > > > >
> > > > > > > As I mentioned in the other reply, LLD does not want to special case
> > > > > > > the definition of __rela_iplt_start depending on -no-pie (available in
> > > > > > > gold and LLD, not available in GNU ld yet) ; -pie/-shared...
> > > > > > > The TLS common issues are obsoleted features that do not make sense nowadays.
> > > > > > > Any case, the LLD produced executables are functional.
> > > > > >
> > > > > > The code in question is
> > > > > >
> > > > > > static void
> > > > > > apply_irel (void)
> > > > > > {
> > > > > > # ifdef IREL
> > > > > >   /* We use weak references for these so that we'll still work with a linker
> > > > > >      that doesn't define them.  Such a linker doesn't support IFUNC at all
> > > > > >      and so uses won't work, but a statically-linked program that doesn't
> > > > > >      use any IFUNC symbols won't have a problem.  */
> > > > > >   extern const IREL_T IPLT_START[] __attribute__ ((weak));
> > > > > >   extern const IREL_T IPLT_END[] __attribute__ ((weak));
> > > > > >   for (const IREL_T *ipltent = IPLT_START; ipltent < IPLT_END; ++ipltent)
> > > > > >     IREL (ipltent);
> > > > > > # endif
> > > > > > }
> > > > > >
> > > > > > Since IPLT_START and IPLT_END are undefined, linker should set
> > > > > > them to zero and the loop should be skipped.  Why doesn't LLD do
> > > > > > that?
> > > > > >
> > > > > >
> > > > > > --
> > > > > > H.J.
> > > > >
> > > > > LLD defines __rela_iplt_start/__rela_iplt_end if (1) __rela_iplt_start
> > > > > exists and is not defined (2) not -r (3) no .interp
> > > > >
> > > > > LLD defines __rela_iplt_start regardless of -no-pie/-pie. This
> > > > > behavior makes sense to me.
> > > > > GNU ld and gold seem to only define __rela_iplt_start in -no-pie mode.
> > > >
> > > > It is an lld bug:
> > > >
> > > > https://bugs.llvm.org/show_bug.cgi?id=48674
> > > >
> > > > --
> > > > H.J.
> > >
> > > I don't consider https://bugs.llvm.org/show_bug.cgi?id=48674 an LLD
> > > bug, so we have disagreement.
> > >
> > > __rela_iplt_start/__rela_iplt_end is a contract between the link
> > > editor and the dynamic loader/libc. For the perspective of a good
> > > design, I don't think it is necessary making
> > > "whether to define __rela_iplt_start" different for ld -no-pie (GNU ld
> > > does not have -no-pie currently, but gold and LLD do) and ld -pie.
> > >
> > > rtld/libc can just figure out whether it is -static-pie (since a
> > > different crt1 rcrt1.o is used) and don't reference __rela_iplt_start
> > > in that case.
> > >
> > > If this difference is dropped (by taking the glibc patch "Make
> > > _dl_relocate_static_pie return an int indicating whether it applied
> > > relocs."),
> > > diff -u =(ld.bfd --verbose) =(ld.bfd -pie --verbose) can have no
> > > difference other than the start address: __executable_start.
> >
> > I designed and implemented the whole thing in ld and glibc
> > many years ago.
>
> I see, binutils commit 3aa14d16c669ca75f9fa4e995a2e2d13069dff3f
> (2009-06-01) added __rela_iplt_start.
>
> >  I don't think they should be changed.  lld should follow my design.
>
> However, that was before glibc gained static pie support (2017;
> http://sourceware.org/PR19574). With static pie, I think
> __rela_iplt_start should be revised.
> I have made many compatibility changes to LLD, but this one seems
> unreasonable, so I am not inclined to folllow.
> I'll personally add this to my list of incompatibilities
> https://maskray.me/blog/2020-12-19-lld-and-gnu-linker-incompatibilities

Please move the IFUNC discussion to

https://bugs.llvm.org/show_bug.cgi?id=48674


-- 
H.J.

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

* Re: [PATCH 0/3] Make glibc build with LLD
  2020-12-28 19:48 [PATCH 0/3] Make glibc build with LLD Fangrui Song
                   ` (3 preceding siblings ...)
  2020-12-28 21:11 ` [PATCH 0/3] Make glibc build with LLD H.J. Lu
@ 2021-01-08  5:04 ` Fāng-ruì Sòng
  2021-01-08 17:09   ` H.J. Lu
  4 siblings, 1 reply; 42+ messages in thread
From: Fāng-ruì Sòng @ 2021-01-08  5:04 UTC (permalink / raw)
  To: GNU C Library

On Mon, Dec 28, 2020 at 11:48 AM Fangrui Song <maskray@google.com> wrote:
>
> I sent the first two in April. Resending in a patch series to be
> clearer.
>
>   install: Replace scripts/output-format.sed with objdump -f
>
> replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
> by leveraging objdump -f.
>
> With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
> `make` with ld pointing to ld.lld (LLVM linker) works.
>
> Fangrui Song (3):
>   configure: Allow LD to be a linker other than GNU ld and gold
>   elf: Replace a --defsym trick with an object file to be compatible
>     with lld
>   install: Replace scripts/output-format.sed with objdump -f
>
>  Makerules                                     | 13 ++-----
>  .../strcoll-inputs/filelist#en_US.UTF-8       |  1 -
>  config.make.in                                |  1 -
>  configure                                     | 32 +++++------------
>  configure.ac                                  | 24 +++++--------
>  elf/Makefile                                  | 11 +++---
>  scripts/output-format.sed                     | 35 -------------------
>  7 files changed, 23 insertions(+), 94 deletions(-)
>  delete mode 100644 scripts/output-format.sed
>
> --
> 2.29.2.729.g45daf8777d-goog
>

Hope "elf:" and "install:" can be reviewed (they are nice
simplification on their own), even if you don't want to take
"configure:" for now.

For __rela_iplt_start, I gave a fair assessment on
https://bugs.llvm.org/show_bug.cgi?id=48674#c4 . I feel bad that an
improvable binutils issue caused the block of a static-pie relocation
improvement
"Make _dl_relocate_static_pie return an int indicating whether it
applied relocs." The other few failures (e.g. not supporting ld.lld
--audit / tls common) are not really usability affecting issues.

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

* Re: [PATCH 0/3] Make glibc build with LLD
  2021-01-08  5:04 ` Fāng-ruì Sòng
@ 2021-01-08 17:09   ` H.J. Lu
  2021-01-08 18:04     ` Fāng-ruì Sòng
  0 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2021-01-08 17:09 UTC (permalink / raw)
  To: Fāng-ruì Sòng; +Cc: GNU C Library

On Thu, Jan 7, 2021 at 9:05 PM Fāng-ruì Sòng via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> On Mon, Dec 28, 2020 at 11:48 AM Fangrui Song <maskray@google.com> wrote:
> >
> > I sent the first two in April. Resending in a patch series to be
> > clearer.
> >
> >   install: Replace scripts/output-format.sed with objdump -f
> >
> > replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
> > by leveraging objdump -f.
> >
> > With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
> > `make` with ld pointing to ld.lld (LLVM linker) works.
> >
> > Fangrui Song (3):
> >   configure: Allow LD to be a linker other than GNU ld and gold
> >   elf: Replace a --defsym trick with an object file to be compatible
> >     with lld
> >   install: Replace scripts/output-format.sed with objdump -f
> >
> >  Makerules                                     | 13 ++-----
> >  .../strcoll-inputs/filelist#en_US.UTF-8       |  1 -
> >  config.make.in                                |  1 -
> >  configure                                     | 32 +++++------------
> >  configure.ac                                  | 24 +++++--------
> >  elf/Makefile                                  | 11 +++---
> >  scripts/output-format.sed                     | 35 -------------------
> >  7 files changed, 23 insertions(+), 94 deletions(-)
> >  delete mode 100644 scripts/output-format.sed
> >
> > --
> > 2.29.2.729.g45daf8777d-goog
> >
>
> Hope "elf:" and "install:" can be reviewed (they are nice
> simplification on their own), even if you don't want to take
> "configure:" for now.
>
> For __rela_iplt_start, I gave a fair assessment on
> https://bugs.llvm.org/show_bug.cgi?id=48674#c4 . I feel bad that an
> improvable binutils issue caused the block of a static-pie relocation
> improvement

No.  There is no need to work around the lld bug which can be fixed.

> "Make _dl_relocate_static_pie return an int indicating whether it
> applied relocs." The other few failures (e.g. not supporting ld.lld
> --audit / tls common) are not really usability affecting issues.

"make check" should be clean.  Otherwise, we have no way to
know if glibc is built properly on x86.

-- 
H.J.

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

* Re: [PATCH 0/3] Make glibc build with LLD
  2021-01-08 17:09   ` H.J. Lu
@ 2021-01-08 18:04     ` Fāng-ruì Sòng
  0 siblings, 0 replies; 42+ messages in thread
From: Fāng-ruì Sòng @ 2021-01-08 18:04 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library

On Fri, Jan 8, 2021 at 9:10 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Jan 7, 2021 at 9:05 PM Fāng-ruì Sòng via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> > On Mon, Dec 28, 2020 at 11:48 AM Fangrui Song <maskray@google.com> wrote:
> > >
> > > I sent the first two in April. Resending in a patch series to be
> > > clearer.
> > >
> > >   install: Replace scripts/output-format.sed with objdump -f
> > >
> > > replaced https://sourceware.org/pipermail/libc-alpha/2020-April/112733.html
> > > by leveraging objdump -f.
> > >
> > > With this patch series (available in https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld),
> > > `make` with ld pointing to ld.lld (LLVM linker) works.
> > >
> > > Fangrui Song (3):
> > >   configure: Allow LD to be a linker other than GNU ld and gold
> > >   elf: Replace a --defsym trick with an object file to be compatible
> > >     with lld
> > >   install: Replace scripts/output-format.sed with objdump -f
> > >
> > >  Makerules                                     | 13 ++-----
> > >  .../strcoll-inputs/filelist#en_US.UTF-8       |  1 -
> > >  config.make.in                                |  1 -
> > >  configure                                     | 32 +++++------------
> > >  configure.ac                                  | 24 +++++--------
> > >  elf/Makefile                                  | 11 +++---
> > >  scripts/output-format.sed                     | 35 -------------------
> > >  7 files changed, 23 insertions(+), 94 deletions(-)
> > >  delete mode 100644 scripts/output-format.sed
> > >
> > > --
> > > 2.29.2.729.g45daf8777d-goog
> > >
> >
> > Hope "elf:" and "install:" can be reviewed (they are nice
> > simplification on their own), even if you don't want to take
> > "configure:" for now.
> >
> > For __rela_iplt_start, I gave a fair assessment on
> > https://bugs.llvm.org/show_bug.cgi?id=48674#c4 . I feel bad that an
> > improvable binutils issue caused the block of a static-pie relocation
> > improvement
>
> No.  There is no need to work around the lld bug which can be fixed.

I wish the glibc change does not make people feel "oh, we change glibc
just to adapt LLD".
I have provided my reasoning
(https://sourceware.org/bugzilla/show_bug.cgi?id=27164) why this is
not an LLD bug and I believe the reasoning is pretty solid (drop a
gratuitous difference between ld and ld -pie; clarify glibc code).

This is both a (future) cleanup opportunity for GNU ld and a
clarification for glibc csu/libc-start.c

> > "Make _dl_relocate_static_pie return an int indicating whether it
> > applied relocs." The other few failures (e.g. not supporting ld.lld
> > --audit / tls common) are not really usability affecting issues.
>
> "make check" should be clean.  Otherwise, we have no way to
> know if glibc is built properly on x86.
>
> --
> H.J.

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

* Re: [PATCH 3/3] install: Replace scripts/output-format.sed with objdump -f [BZ #26559]
  2020-12-28 19:48 ` [PATCH 3/3] install: Replace scripts/output-format.sed with objdump -f [BZ #26559] Fangrui Song
@ 2021-01-08 19:38   ` Adhemerval Zanella
  2021-01-12 11:20     ` Florian Weimer
  2021-01-12 11:32   ` Andreas Schwab
  1 sibling, 1 reply; 42+ messages in thread
From: Adhemerval Zanella @ 2021-01-08 19:38 UTC (permalink / raw)
  To: Fangrui Song, libc-alpha



On 28/12/2020 16:48, Fangrui Song via Libc-alpha wrote:
> GNU ld and gold have supported --print-output-format since 2011. glibc
> requires binutils>=2.25 (2015), so if LD is GNU ld or gold, we can
> assume the option is supported.
> 
> lld is by default a cross linker supporting multiple targets. It auto
> detects the file format and does not need OUTPUT_FORMAT. It does not
> support --print-output-format.
> 
> By parsing objdump -f, we can support all the three linkers.

LGTM and this change seems orthogonal to the other lld adjustments.
I have checked that at least with a build for a handful of supported
ABIs I see no difference with ld.bfd (with multiple versions).

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  Makerules                                     | 13 ++-----
>  .../strcoll-inputs/filelist#en_US.UTF-8       |  1 -
>  config.make.in                                |  1 -
>  configure                                     | 19 ----------
>  configure.ac                                  | 11 ------
>  scripts/output-format.sed                     | 35 -------------------
>  6 files changed, 3 insertions(+), 77 deletions(-)
>  delete mode 100644 scripts/output-format.sed
> 
> diff --git a/Makerules b/Makerules
> index ef0fe67d9a..146d1ab650 100644
> --- a/Makerules
> +++ b/Makerules
> @@ -1065,20 +1065,13 @@ install: $(inst_slibdir)/libc.so$(libc.so-version)
>  # for the configuration we are building.  We put this statement into
>  # the linker scripts we install for -lc et al so that they will not be
>  # used by a link for a different format on a multi-architecture system.
> -$(common-objpfx)format.lds: $(..)scripts/output-format.sed \
> -			    $(common-objpfx)config.make \
> +$(common-objpfx)format.lds: $(common-objpfx)config.make \
>  			    $(common-objpfx)config.h $(..)Makerules
> -ifneq (unknown,$(output-format))
> -	echo > $@.new 'OUTPUT_FORMAT($(output-format))'
> -else
>  	$(LINK.o) -shared $(sysdep-LDFLAGS) $(rtld-LDFLAGS) \
>  		  $(LDFLAGS.so) $(LDFLAGS-lib.so) \
> -		  -x c /dev/null -o $@.so -Wl,--verbose -v 2>/dev/null \
> -	| sed -n -f $< > $@.new
> -	test -s $@.new
> +		  -x c /dev/null -o $@.so 2>/dev/null
> +	$(OBJDUMP) -f $@.so | sed -n 's/.*file format \(.*\)/OUTPUT_FORMAT(\1)/;T;p' > $@
>  	rm -f $@.so
> -endif
> -	mv -f $@.new $@
>  common-generated += format.lds
>  
>  ifndef subdir

Ok.

> diff --git a/benchtests/strcoll-inputs/filelist#en_US.UTF-8 b/benchtests/strcoll-inputs/filelist#en_US.UTF-8
> index 2f4ef195bb..43eb9efb40 100644
> --- a/benchtests/strcoll-inputs/filelist#en_US.UTF-8
> +++ b/benchtests/strcoll-inputs/filelist#en_US.UTF-8
> @@ -9450,7 +9450,6 @@ move-if-change
>  check-execstack.awk
>  pylint
>  pylintrc
> -output-format.sed
>  merge-test-results.sh
>  update-copyrights
>  config-uname.sh

Ok.

> diff --git a/config.make.in b/config.make.in
> index 7ae27564fd..7f47f0caa4 100644
> --- a/config.make.in
> +++ b/config.make.in
> @@ -73,7 +73,6 @@ fno-unit-at-a-time = @fno_unit_at_a_time@
>  bind-now = @bindnow@
>  have-hash-style = @libc_cv_hashstyle@
>  use-default-link = @use_default_link@
> -output-format = @libc_cv_output_format@
>  have-cxx-thread_local = @libc_cv_cxx_thread_local@
>  have-loop-to-function = @libc_cv_cc_loop_to_function@
>  have-textrel_ifunc = @libc_cv_textrel_ifunc@

Ok.

> diff --git a/configure b/configure
> index 6dcd2270f8..4b0ab150cb 100755
> --- a/configure
> +++ b/configure
> @@ -623,7 +623,6 @@ libc_cv_cc_submachine
>  libc_cv_cc_nofma
>  libc_cv_mtls_dialect_gnu2
>  fno_unit_at_a_time
> -libc_cv_output_format
>  libc_cv_has_glob_dat
>  libc_cv_hashstyle
>  libc_cv_fpie
> @@ -6077,24 +6076,6 @@ fi
>  $as_echo "$libc_cv_has_glob_dat" >&6; }
>  
>  
> -{ $as_echo "$as_me:${as_lineno-$LINENO}: checking linker output format" >&5
> -$as_echo_n "checking linker output format... " >&6; }
> -if ${libc_cv_output_format+:} false; then :
> -  $as_echo_n "(cached) " >&6
> -else
> -  if libc_cv_output_format=`
> -${CC-cc} -nostartfiles -nostdlib $no_ssp -Wl,--print-output-format 2>&5`
> -then
> -  :
> -else
> -  libc_cv_output_format=
> -fi
> -test -n "$libc_cv_output_format" || libc_cv_output_format=unknown
> -fi
> -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_output_format" >&5
> -$as_echo "$libc_cv_output_format" >&6; }
> -
> -
>  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for -fno-toplevel-reorder -fno-section-anchors" >&5
>  $as_echo_n "checking for -fno-toplevel-reorder -fno-section-anchors... " >&6; }
>  if ${libc_cv_fno_toplevel_reorder+:} false; then :
> diff --git a/configure.ac b/configure.ac
> index 1a2054cd1a..baf3a05ae7 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1432,17 +1432,6 @@ fi
>  rm -f conftest*])
>  AC_SUBST(libc_cv_has_glob_dat)
>  
> -AC_CACHE_CHECK(linker output format, libc_cv_output_format, [dnl
> -if libc_cv_output_format=`
> -${CC-cc} -nostartfiles -nostdlib $no_ssp -Wl,--print-output-format 2>&AS_MESSAGE_LOG_FD`
> -then
> -  :
> -else
> -  libc_cv_output_format=
> -fi
> -test -n "$libc_cv_output_format" || libc_cv_output_format=unknown])
> -AC_SUBST(libc_cv_output_format)
> -
>  AC_CACHE_CHECK(for -fno-toplevel-reorder -fno-section-anchors, libc_cv_fno_toplevel_reorder, [dnl
>  cat > conftest.c <<EOF
>  int foo;

Ok.

> diff --git a/scripts/output-format.sed b/scripts/output-format.sed
> deleted file mode 100644
> index 364f52059f..0000000000
> --- a/scripts/output-format.sed
> +++ /dev/null
> @@ -1,35 +0,0 @@
> -/ld.*[ 	]-E[BL]/b f
> -/collect.*[ 	]-E[BL]/b f
> -/OUTPUT_FORMAT[^)]*$/{N
> -s/\n[	 ]*/ /
> -}
> -t o
> -: o
> -s/^.*OUTPUT_FORMAT(\([^,]*\), \1, \1).*$/OUTPUT_FORMAT(\1)/
> -t q
> -s/^.*OUTPUT_FORMAT(\([^,]*\), \([^,]*\), \([^,]*\)).*$/\1,\2,\3/
> -t s
> -s/^.*OUTPUT_FORMAT(\([^,)]*\).*$)/OUTPUT_FORMAT(\1)/
> -t q
> -d
> -: s
> -s/"//g
> -G
> -s/\n//
> -s/^\([^,]*\),\([^,]*\),\([^,]*\),B/OUTPUT_FORMAT(\2)/p
> -s/^\([^,]*\),\([^,]*\),\([^,]*\),L/OUTPUT_FORMAT(\3)/p
> -s/^\([^,]*\),\([^,]*\),\([^,]*\)/OUTPUT_FORMAT(\1)/p
> -/,/s|^|*** BUG in libc/scripts/output-format.sed *** |p
> -q
> -: q
> -s/"//g
> -p
> -q
> -: f
> -s/^.*[ 	]-E\([BL]\)[ 	].*$/,\1/
> -t h
> -s/^.*[ 	]-E\([BL]\)$/,\1/
> -t h
> -d
> -: h
> -h
> 

Ok.

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

* Re: [PATCH 2/3] elf: Replace a --defsym trick with an object file to be compatible with lld
  2020-12-28 19:48 ` [PATCH 2/3] elf: Replace a --defsym trick with an object file to be compatible with lld Fangrui Song
@ 2021-01-11 20:06   ` Fāng-ruì Sòng
  2021-01-18 22:08     ` Fāng-ruì Sòng
  2021-01-19  0:03   ` H.J. Lu
  1 sibling, 1 reply; 42+ messages in thread
From: Fāng-ruì Sòng @ 2021-01-11 20:06 UTC (permalink / raw)
  To: GNU C Library

On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song <maskray@google.com> wrote:
>
> The existing code specifies -Wl,--defsym=malloc=0 and other malloc.os
> definitions before libc_pic.a so that libc_pic.a(malloc.os) is not
> fetched. This trick is used to avoid multiple definition errors which
> would happen as a chain result:
>
>   dl-allobjs.os has an undefined __libc_scratch_buffer_set_array_size
>   __libc_scratch_buffer_set_array_size fetches libc_pic.a(scratch_buffer_set_array_size.os)
>   libc_pic.a(scratch_buffer_set_array_size.os) has an undefined free
>   free fetches libc_pic.a(malloc.os)
>   libc_pic.a(malloc.os) has an undefined __libc_message
>   __libc_message fetches libc_pic.a(libc_fatal.os)
>
>   libc_fatal.os will cause a multiple definition error (__GI___libc_fatal)
>   >>> defined at dl-fxstatat64.c
>   >>>            /tmp/p/glibc/Release/elf/dl-allobjs.os:(__GI___libc_fatal)
>   >>> defined at libc_fatal.c
>   >>>            libc_fatal.os:(.text+0x240) in archive /tmp/p/glibc/Release/libc_pic.a
>
> lld processes --defsym after all input files, so this trick does not
> suppress multiple definition errors with lld. Split the step into two
> and use an object file to make the intention more obvious and make lld
> work.
>
> This is conceptually more appropriate because --defsym defines a SHN_ABS
> symbol while a normal definition is relative to the image base.
>
> See https://sourceware.org/pipermail/libc-alpha/2020-March/111910.html
> for discussions about the --defsym semantics.
> ---
>  elf/Makefile | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)

Ping on this

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

* Re: [PATCH 3/3] install: Replace scripts/output-format.sed with objdump -f [BZ #26559]
  2021-01-08 19:38   ` Adhemerval Zanella
@ 2021-01-12 11:20     ` Florian Weimer
  2021-01-12 12:00       ` Adhemerval Zanella
  0 siblings, 1 reply; 42+ messages in thread
From: Florian Weimer @ 2021-01-12 11:20 UTC (permalink / raw)
  To: Adhemerval Zanella via Libc-alpha; +Cc: Fangrui Song, Adhemerval Zanella

* Adhemerval Zanella via Libc-alpha:

> On 28/12/2020 16:48, Fangrui Song via Libc-alpha wrote:
>> GNU ld and gold have supported --print-output-format since 2011. glibc
>> requires binutils>=2.25 (2015), so if LD is GNU ld or gold, we can
>> assume the option is supported.
>> 
>> lld is by default a cross linker supporting multiple targets. It auto
>> detects the file format and does not need OUTPUT_FORMAT. It does not
>> support --print-output-format.
>> 
>> By parsing objdump -f, we can support all the three linkers.
>
> LGTM and this change seems orthogonal to the other lld adjustments.
> I have checked that at least with a build for a handful of supported
> ABIs I see no difference with ld.bfd (with multiple versions).

I see a cross-build failure during the initial static link bootstrap on
aarch64-linux-gnu during “make install”:

make[2]: Entering directory '/home/bmg/src/glibc'
aarch64-glibc-linux-gnu-gcc   -shared  -Wl,-dynamic-linker=/lib/ld-linux-aarch64.so.1 \
          -Wl,-z,combreloc -Wl,-z,relro -Wl,--hash-style=both  \
          -x c /dev/null -o /home/bmg/build/compilers/aarch64-linux-gnu/glibc/aarch64-linux-gnu/format.lds.so
/home/bmg/install/compilers/aarch64-linux-gnu/lib/gcc/aarch64-glibc-linux-gnu/10.2.1/../../../../aarch64-glibc-linux-gnu/bin/ld: cannot find crti.o: No such file or directory
/home/bmg/install/compilers/aarch64-linux-gnu/lib/gcc/aarch64-glibc-linux-gnu/10.2.1/../../../../aarch64-glibc-linux-gnu/bin/ld: cannot find crtn.o: No such file or directory
collect2: error: ld returned 1 exit status
make[2]: *** [Makerules:1070: /home/bmg/build/compilers/aarch64-linux-gnu/glibc/aarch64-linux-gnu/format.lds] Error 1

This affects other targets as well.

I guess using

  $(CC) -nostdlib -nostartfiles -shared -x assembler /dev/null

should work on most targets here.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill


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

* Re: [PATCH 3/3] install: Replace scripts/output-format.sed with objdump -f [BZ #26559]
  2020-12-28 19:48 ` [PATCH 3/3] install: Replace scripts/output-format.sed with objdump -f [BZ #26559] Fangrui Song
  2021-01-08 19:38   ` Adhemerval Zanella
@ 2021-01-12 11:32   ` Andreas Schwab
  2021-01-12 11:58     ` Florian Weimer
  1 sibling, 1 reply; 42+ messages in thread
From: Andreas Schwab @ 2021-01-12 11:32 UTC (permalink / raw)
  To: Fangrui Song via Libc-alpha; +Cc: Fangrui Song

gcc   -shared -Wl,-z,force-bti -Wl,-dynamic-linker=/lib/ld-linux-aarch64.so.1 \
          -Wl,-z,combreloc -Wl,-z,relro -Wl,--hash-style=both -Wl,-z,now -Wl,--fatal-warnings \
          -x c /dev/null -o /home/abuild/rpmbuild/BUILD/glibc-2.32.9000.527.g87d583c6e8/cc-base/format.lds.so 2>/dev/null
make[1]: *** [Makerules:1070: /home/abuild/rpmbuild/BUILD/glibc-2.32.9000.527.g87d583c6e8/cc-base/format.lds] Error 1

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

* Re: [PATCH 3/3] install: Replace scripts/output-format.sed with objdump -f [BZ #26559]
  2021-01-12 11:32   ` Andreas Schwab
@ 2021-01-12 11:58     ` Florian Weimer
  0 siblings, 0 replies; 42+ messages in thread
From: Florian Weimer @ 2021-01-12 11:58 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Fangrui Song via Libc-alpha, Fangrui Song

* Andreas Schwab:

> gcc   -shared -Wl,-z,force-bti -Wl,-dynamic-linker=/lib/ld-linux-aarch64.so.1 \
>           -Wl,-z,combreloc -Wl,-z,relro -Wl,--hash-style=both -Wl,-z,now -Wl,--fatal-warnings \
>           -x c /dev/null -o /home/abuild/rpmbuild/BUILD/glibc-2.32.9000.527.g87d583c6e8/cc-base/format.lds.so 2>/dev/null
> make[1]: *** [Makerules:1070: /home/abuild/rpmbuild/BUILD/glibc-2.32.9000.527.g87d583c6e8/cc-base/format.lds] Error 1

<https://sourceware.org/pipermail/libc-alpha/2021-January/121535.html>

I'm testing the proposed fix.  So far it's looking good.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill


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

* Re: [PATCH 3/3] install: Replace scripts/output-format.sed with objdump -f [BZ #26559]
  2021-01-12 11:20     ` Florian Weimer
@ 2021-01-12 12:00       ` Adhemerval Zanella
  2021-01-12 17:46         ` Fāng-ruì Sòng
  0 siblings, 1 reply; 42+ messages in thread
From: Adhemerval Zanella @ 2021-01-12 12:00 UTC (permalink / raw)
  To: Florian Weimer, Adhemerval Zanella via Libc-alpha; +Cc: Fangrui Song



On 12/01/2021 08:20, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> On 28/12/2020 16:48, Fangrui Song via Libc-alpha wrote:
>>> GNU ld and gold have supported --print-output-format since 2011. glibc
>>> requires binutils>=2.25 (2015), so if LD is GNU ld or gold, we can
>>> assume the option is supported.
>>>
>>> lld is by default a cross linker supporting multiple targets. It auto
>>> detects the file format and does not need OUTPUT_FORMAT. It does not
>>> support --print-output-format.
>>>
>>> By parsing objdump -f, we can support all the three linkers.
>>
>> LGTM and this change seems orthogonal to the other lld adjustments.
>> I have checked that at least with a build for a handful of supported
>> ABIs I see no difference with ld.bfd (with multiple versions).
> 
> I see a cross-build failure during the initial static link bootstrap on
> aarch64-linux-gnu during “make install”:
> 
> make[2]: Entering directory '/home/bmg/src/glibc'
> aarch64-glibc-linux-gnu-gcc   -shared  -Wl,-dynamic-linker=/lib/ld-linux-aarch64.so.1 \
>           -Wl,-z,combreloc -Wl,-z,relro -Wl,--hash-style=both  \
>           -x c /dev/null -o /home/bmg/build/compilers/aarch64-linux-gnu/glibc/aarch64-linux-gnu/format.lds.so
> /home/bmg/install/compilers/aarch64-linux-gnu/lib/gcc/aarch64-glibc-linux-gnu/10.2.1/../../../../aarch64-glibc-linux-gnu/bin/ld: cannot find crti.o: No such file or directory
> /home/bmg/install/compilers/aarch64-linux-gnu/lib/gcc/aarch64-glibc-linux-gnu/10.2.1/../../../../aarch64-glibc-linux-gnu/bin/ld: cannot find crtn.o: No such file or directory
> collect2: error: ld returned 1 exit status
> make[2]: *** [Makerules:1070: /home/bmg/build/compilers/aarch64-linux-gnu/glibc/aarch64-linux-gnu/format.lds] Error 1
> 
> This affects other targets as well.
> 
> I guess using
> 
>   $(CC) -nostdlib -nostartfiles -shared -x assembler /dev/null
> 
> should work on most targets here.

I haven't see it when a stage2 gcc, make install works as intended.  However I
did see it now with a stage1 gcc with build-many-glibcs.py.

The -nostdlib -nostartfiles seems to fix it indeed, I am checking with a
bootstrap using build-many-glibcs.py.

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

* Re: [PATCH 3/3] install: Replace scripts/output-format.sed with objdump -f [BZ #26559]
  2021-01-12 12:00       ` Adhemerval Zanella
@ 2021-01-12 17:46         ` Fāng-ruì Sòng
  0 siblings, 0 replies; 42+ messages in thread
From: Fāng-ruì Sòng @ 2021-01-12 17:46 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Adhemerval Zanella via Libc-alpha, Adhemerval Zanella

On Tue, Jan 12, 2021 at 4:00 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 12/01/2021 08:20, Florian Weimer wrote:
> > * Adhemerval Zanella via Libc-alpha:
> >
> >> On 28/12/2020 16:48, Fangrui Song via Libc-alpha wrote:
> >>> GNU ld and gold have supported --print-output-format since 2011. glibc
> >>> requires binutils>=2.25 (2015), so if LD is GNU ld or gold, we can
> >>> assume the option is supported.
> >>>
> >>> lld is by default a cross linker supporting multiple targets. It auto
> >>> detects the file format and does not need OUTPUT_FORMAT. It does not
> >>> support --print-output-format.
> >>>
> >>> By parsing objdump -f, we can support all the three linkers.
> >>
> >> LGTM and this change seems orthogonal to the other lld adjustments.
> >> I have checked that at least with a build for a handful of supported
> >> ABIs I see no difference with ld.bfd (with multiple versions).
> >
> > I see a cross-build failure during the initial static link bootstrap on
> > aarch64-linux-gnu during “make install”:
> >
> > make[2]: Entering directory '/home/bmg/src/glibc'
> > aarch64-glibc-linux-gnu-gcc   -shared  -Wl,-dynamic-linker=/lib/ld-linux-aarch64.so.1 \
> >           -Wl,-z,combreloc -Wl,-z,relro -Wl,--hash-style=both  \
> >           -x c /dev/null -o /home/bmg/build/compilers/aarch64-linux-gnu/glibc/aarch64-linux-gnu/format.lds.so
> > /home/bmg/install/compilers/aarch64-linux-gnu/lib/gcc/aarch64-glibc-linux-gnu/10.2.1/../../../../aarch64-glibc-linux-gnu/bin/ld: cannot find crti.o: No such file or directory
> > /home/bmg/install/compilers/aarch64-linux-gnu/lib/gcc/aarch64-glibc-linux-gnu/10.2.1/../../../../aarch64-glibc-linux-gnu/bin/ld: cannot find crtn.o: No such file or directory
> > collect2: error: ld returned 1 exit status
> > make[2]: *** [Makerules:1070: /home/bmg/build/compilers/aarch64-linux-gnu/glibc/aarch64-linux-gnu/format.lds] Error 1
> >
> > This affects other targets as well.
> >
> > I guess using
> >
> >   $(CC) -nostdlib -nostartfiles -shared -x assembler /dev/null
> >
> > should work on most targets here.
>
> I haven't see it when a stage2 gcc, make install works as intended.  However I
> did see it now with a stage1 gcc with build-many-glibcs.py.
>
> The -nostdlib -nostartfiles seems to fix it indeed, I am checking with a
> bootstrap using build-many-glibcs.py.

Thanks for the fix 0400f928335a5e04c788e1c831d8825d42612c49!

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

* Re: [PATCH 2/3] elf: Replace a --defsym trick with an object file to be compatible with lld
  2021-01-11 20:06   ` Fāng-ruì Sòng
@ 2021-01-18 22:08     ` Fāng-ruì Sòng
  0 siblings, 0 replies; 42+ messages in thread
From: Fāng-ruì Sòng @ 2021-01-18 22:08 UTC (permalink / raw)
  To: GNU C Library

On Mon, Jan 11, 2021 at 12:06 PM Fāng-ruì Sòng <maskray@google.com> wrote:
>
> On Mon, Dec 28, 2020 at 11:49 AM Fangrui Song <maskray@google.com> wrote:
> >
> > The existing code specifies -Wl,--defsym=malloc=0 and other malloc.os
> > definitions before libc_pic.a so that libc_pic.a(malloc.os) is not
> > fetched. This trick is used to avoid multiple definition errors which
> > would happen as a chain result:
> >
> >   dl-allobjs.os has an undefined __libc_scratch_buffer_set_array_size
> >   __libc_scratch_buffer_set_array_size fetches libc_pic.a(scratch_buffer_set_array_size.os)
> >   libc_pic.a(scratch_buffer_set_array_size.os) has an undefined free
> >   free fetches libc_pic.a(malloc.os)
> >   libc_pic.a(malloc.os) has an undefined __libc_message
> >   __libc_message fetches libc_pic.a(libc_fatal.os)
> >
> >   libc_fatal.os will cause a multiple definition error (__GI___libc_fatal)
> >   >>> defined at dl-fxstatat64.c
> >   >>>            /tmp/p/glibc/Release/elf/dl-allobjs.os:(__GI___libc_fatal)
> >   >>> defined at libc_fatal.c
> >   >>>            libc_fatal.os:(.text+0x240) in archive /tmp/p/glibc/Release/libc_pic.a
> >
> > lld processes --defsym after all input files, so this trick does not
> > suppress multiple definition errors with lld. Split the step into two
> > and use an object file to make the intention more obvious and make lld
> > work.
> >
> > This is conceptually more appropriate because --defsym defines a SHN_ABS
> > symbol while a normal definition is relative to the image base.
> >
> > See https://sourceware.org/pipermail/libc-alpha/2020-March/111910.html
> > for discussions about the --defsym semantics.
> > ---
> >  elf/Makefile | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
>
> Ping on this

PING^2

https://sourceware.org/pipermail/libc-alpha/2020-December/121144.html
You can also find the commit in
https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld

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

* Re: [PATCH 2/3] elf: Replace a --defsym trick with an object file to be compatible with lld
  2020-12-28 19:48 ` [PATCH 2/3] elf: Replace a --defsym trick with an object file to be compatible with lld Fangrui Song
  2021-01-11 20:06   ` Fāng-ruì Sòng
@ 2021-01-19  0:03   ` H.J. Lu
  2021-01-19  8:50     ` Fāng-ruì Sòng
  1 sibling, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2021-01-19  0:03 UTC (permalink / raw)
  To: Fangrui Song; +Cc: GNU C Library

On Mon, Dec 28, 2020 at 11:50 AM Fangrui Song via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> The existing code specifies -Wl,--defsym=malloc=0 and other malloc.os
> definitions before libc_pic.a so that libc_pic.a(malloc.os) is not
> fetched. This trick is used to avoid multiple definition errors which
> would happen as a chain result:
>
>   dl-allobjs.os has an undefined __libc_scratch_buffer_set_array_size
>   __libc_scratch_buffer_set_array_size fetches libc_pic.a(scratch_buffer_set_array_size.os)
>   libc_pic.a(scratch_buffer_set_array_size.os) has an undefined free
>   free fetches libc_pic.a(malloc.os)
>   libc_pic.a(malloc.os) has an undefined __libc_message
>   __libc_message fetches libc_pic.a(libc_fatal.os)
>
>   libc_fatal.os will cause a multiple definition error (__GI___libc_fatal)
>   >>> defined at dl-fxstatat64.c
>   >>>            /tmp/p/glibc/Release/elf/dl-allobjs.os:(__GI___libc_fatal)
>   >>> defined at libc_fatal.c
>   >>>            libc_fatal.os:(.text+0x240) in archive /tmp/p/glibc/Release/libc_pic.a
>
> lld processes --defsym after all input files, so this trick does not
> suppress multiple definition errors with lld. Split the step into two
> and use an object file to make the intention more obvious and make lld
> work.
>
> This is conceptually more appropriate because --defsym defines a SHN_ABS
> symbol while a normal definition is relative to the image base.
>

This is irrelevant since  --defsym was used to create a temporary file which was
removed immediately after it was created:

/usr/bin/gcc   -nostdlib -nostartfiles -r -o
/export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map.o
-Wl,--defsym=calloc=0 -Wl,--defsym=free=0 -Wl,--defsym=malloc=0
-Wl,--defsym=realloc=0 -Wl,--defsym=__stack_chk_fail=0
-Wl,--defsym=__stack_chk_fail_local=0 \
        '-Wl,-('
/export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/dl-allobjs.os
/export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/libc_pic.a
-lgcc '-Wl,-)' -Wl,-Map,/export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.mapT
rm -f /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map.o
mv -f /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.mapT
/export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map

> ---
>  elf/Makefile | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/elf/Makefile b/elf/Makefile
> index 0b4d78c874..299bf24b49 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -524,10 +524,6 @@ rtld-stubbed-symbols = \
>    malloc \
>    realloc \
>
> -# The GCC arguments that implement $(rtld-stubbed-symbols).
> -rtld-stubbed-symbols-args = \
> -  $(patsubst %,-Wl$(comma)--defsym=%=0, $(rtld-stubbed-symbols))
> -
>  ifeq ($(have-ssp),yes)
>  # rtld is not built with the stack protector, so these references will
>  # go away in the rebuilds.
> @@ -536,9 +532,10 @@ endif
>
>  $(objpfx)librtld.map: $(objpfx)dl-allobjs.os $(common-objpfx)libc_pic.a
>         @-rm -f $@T
> -       $(reloc-link) -o $@.o $(rtld-stubbed-symbols-args) \
> -               '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
> -       rm -f $@.o
> +       echo '$(patsubst %,.globl %;,$(rtld-stubbed-symbols)) $(patsubst %,%:,$(rtld-stubbed-symbols))' | \
> +               $(CC) -o $@T.o $(ASFLAGS) -c -x assembler -

Please generate

.globl symbol;
symbol = 0;

to make it closer to -Wl,--defsym=symbol=0.

> +       $(reloc-link) -o $@.o $@T.o '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
> +       rm -f %@T.o $@.o
>         mv -f $@T $@
>
>  # For lld, skip preceding addresses and values before matching the archive and the member.
> --
> 2.29.2.729.g45daf8777d-goog
>


-- 
H.J.

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

* Re: [PATCH 2/3] elf: Replace a --defsym trick with an object file to be compatible with lld
  2021-01-19  0:03   ` H.J. Lu
@ 2021-01-19  8:50     ` Fāng-ruì Sòng
  2021-01-19 12:30       ` H.J. Lu
  0 siblings, 1 reply; 42+ messages in thread
From: Fāng-ruì Sòng @ 2021-01-19  8:50 UTC (permalink / raw)
  To: H.J. Lu, Florian Weimer; +Cc: GNU C Library

On Mon, Jan 18, 2021 at 4:04 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Dec 28, 2020 at 11:50 AM Fangrui Song via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> > The existing code specifies -Wl,--defsym=malloc=0 and other malloc.os
> > definitions before libc_pic.a so that libc_pic.a(malloc.os) is not
> > fetched. This trick is used to avoid multiple definition errors which
> > would happen as a chain result:
> >
> >   dl-allobjs.os has an undefined __libc_scratch_buffer_set_array_size
> >   __libc_scratch_buffer_set_array_size fetches libc_pic.a(scratch_buffer_set_array_size.os)
> >   libc_pic.a(scratch_buffer_set_array_size.os) has an undefined free
> >   free fetches libc_pic.a(malloc.os)
> >   libc_pic.a(malloc.os) has an undefined __libc_message
> >   __libc_message fetches libc_pic.a(libc_fatal.os)
> >
> >   libc_fatal.os will cause a multiple definition error (__GI___libc_fatal)
> >   >>> defined at dl-fxstatat64.c
> >   >>>            /tmp/p/glibc/Release/elf/dl-allobjs.os:(__GI___libc_fatal)
> >   >>> defined at libc_fatal.c
> >   >>>            libc_fatal.os:(.text+0x240) in archive /tmp/p/glibc/Release/libc_pic.a
> >
> > lld processes --defsym after all input files, so this trick does not
> > suppress multiple definition errors with lld. Split the step into two
> > and use an object file to make the intention more obvious and make lld
> > work.
> >
> > This is conceptually more appropriate because --defsym defines a SHN_ABS
> > symbol while a normal definition is relative to the image base.
> >
>
> This is irrelevant since  --defsym was used to create a temporary file which was
> removed immediately after it was created:
>
> /usr/bin/gcc   -nostdlib -nostartfiles -r -o
> /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map.o
> -Wl,--defsym=calloc=0 -Wl,--defsym=free=0 -Wl,--defsym=malloc=0
> -Wl,--defsym=realloc=0 -Wl,--defsym=__stack_chk_fail=0
> -Wl,--defsym=__stack_chk_fail_local=0 \
>         '-Wl,-('
> /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/dl-allobjs.os
> /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/libc_pic.a
> -lgcc '-Wl,-)' -Wl,-Map,/export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.mapT
> rm -f /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map.o
> mv -f /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.mapT
> /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map
>
> > ---
> >  elf/Makefile | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/elf/Makefile b/elf/Makefile
> > index 0b4d78c874..299bf24b49 100644
> > --- a/elf/Makefile
> > +++ b/elf/Makefile
> > @@ -524,10 +524,6 @@ rtld-stubbed-symbols = \
> >    malloc \
> >    realloc \
> >
> > -# The GCC arguments that implement $(rtld-stubbed-symbols).
> > -rtld-stubbed-symbols-args = \
> > -  $(patsubst %,-Wl$(comma)--defsym=%=0, $(rtld-stubbed-symbols))
> > -
> >  ifeq ($(have-ssp),yes)
> >  # rtld is not built with the stack protector, so these references will
> >  # go away in the rebuilds.
> > @@ -536,9 +532,10 @@ endif
> >
> >  $(objpfx)librtld.map: $(objpfx)dl-allobjs.os $(common-objpfx)libc_pic.a
> >         @-rm -f $@T
> > -       $(reloc-link) -o $@.o $(rtld-stubbed-symbols-args) \
> > -               '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
> > -       rm -f $@.o
> > +       echo '$(patsubst %,.globl %;,$(rtld-stubbed-symbols)) $(patsubst %,%:,$(rtld-stubbed-symbols))' | \
> > +               $(CC) -o $@T.o $(ASFLAGS) -c -x assembler -
>
> Please generate
>
> .globl symbol;
> symbol = 0;
>
> to make it closer to -Wl,--defsym=symbol=0.

If symbol: and symbol = 0; work, isn't symbol: slightly better because
it looks more normal?
Or is the intention here using SHN_ABS to make it clear these symbols
are special?

> > +       $(reloc-link) -o $@.o $@T.o '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
> > +       rm -f %@T.o $@.o
> >         mv -f $@T $@
> >
> >  # For lld, skip preceding addresses and values before matching the archive and the member.
> > --
> > 2.29.2.729.g45daf8777d-goog
> >
>
>
> --
> H.J.

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

* Re: [PATCH 2/3] elf: Replace a --defsym trick with an object file to be compatible with lld
  2021-01-19  8:50     ` Fāng-ruì Sòng
@ 2021-01-19 12:30       ` H.J. Lu
  2021-01-20 18:51         ` Fāng-ruì Sòng
  0 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2021-01-19 12:30 UTC (permalink / raw)
  To: Fāng-ruì Sòng; +Cc: Florian Weimer, GNU C Library

On Tue, Jan 19, 2021 at 12:50 AM Fāng-ruì Sòng <maskray@google.com> wrote:
>
> On Mon, Jan 18, 2021 at 4:04 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Mon, Dec 28, 2020 at 11:50 AM Fangrui Song via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> > >
> > > The existing code specifies -Wl,--defsym=malloc=0 and other malloc.os
> > > definitions before libc_pic.a so that libc_pic.a(malloc.os) is not
> > > fetched. This trick is used to avoid multiple definition errors which
> > > would happen as a chain result:
> > >
> > >   dl-allobjs.os has an undefined __libc_scratch_buffer_set_array_size
> > >   __libc_scratch_buffer_set_array_size fetches libc_pic.a(scratch_buffer_set_array_size.os)
> > >   libc_pic.a(scratch_buffer_set_array_size.os) has an undefined free
> > >   free fetches libc_pic.a(malloc.os)
> > >   libc_pic.a(malloc.os) has an undefined __libc_message
> > >   __libc_message fetches libc_pic.a(libc_fatal.os)
> > >
> > >   libc_fatal.os will cause a multiple definition error (__GI___libc_fatal)
> > >   >>> defined at dl-fxstatat64.c
> > >   >>>            /tmp/p/glibc/Release/elf/dl-allobjs.os:(__GI___libc_fatal)
> > >   >>> defined at libc_fatal.c
> > >   >>>            libc_fatal.os:(.text+0x240) in archive /tmp/p/glibc/Release/libc_pic.a
> > >
> > > lld processes --defsym after all input files, so this trick does not
> > > suppress multiple definition errors with lld. Split the step into two
> > > and use an object file to make the intention more obvious and make lld
> > > work.
> > >
> > > This is conceptually more appropriate because --defsym defines a SHN_ABS
> > > symbol while a normal definition is relative to the image base.
> > >
> >
> > This is irrelevant since  --defsym was used to create a temporary file which was
> > removed immediately after it was created:
> >
> > /usr/bin/gcc   -nostdlib -nostartfiles -r -o
> > /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map.o
> > -Wl,--defsym=calloc=0 -Wl,--defsym=free=0 -Wl,--defsym=malloc=0
> > -Wl,--defsym=realloc=0 -Wl,--defsym=__stack_chk_fail=0
> > -Wl,--defsym=__stack_chk_fail_local=0 \
> >         '-Wl,-('
> > /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/dl-allobjs.os
> > /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/libc_pic.a
> > -lgcc '-Wl,-)' -Wl,-Map,/export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.mapT
> > rm -f /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map.o
> > mv -f /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.mapT
> > /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map
> >
> > > ---
> > >  elf/Makefile | 11 ++++-------
> > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/elf/Makefile b/elf/Makefile
> > > index 0b4d78c874..299bf24b49 100644
> > > --- a/elf/Makefile
> > > +++ b/elf/Makefile
> > > @@ -524,10 +524,6 @@ rtld-stubbed-symbols = \
> > >    malloc \
> > >    realloc \
> > >
> > > -# The GCC arguments that implement $(rtld-stubbed-symbols).
> > > -rtld-stubbed-symbols-args = \
> > > -  $(patsubst %,-Wl$(comma)--defsym=%=0, $(rtld-stubbed-symbols))
> > > -
> > >  ifeq ($(have-ssp),yes)
> > >  # rtld is not built with the stack protector, so these references will
> > >  # go away in the rebuilds.
> > > @@ -536,9 +532,10 @@ endif
> > >
> > >  $(objpfx)librtld.map: $(objpfx)dl-allobjs.os $(common-objpfx)libc_pic.a
> > >         @-rm -f $@T
> > > -       $(reloc-link) -o $@.o $(rtld-stubbed-symbols-args) \
> > > -               '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
> > > -       rm -f $@.o
> > > +       echo '$(patsubst %,.globl %;,$(rtld-stubbed-symbols)) $(patsubst %,%:,$(rtld-stubbed-symbols))' | \
> > > +               $(CC) -o $@T.o $(ASFLAGS) -c -x assembler -
> >
> > Please generate
> >
> > .globl symbol;
> > symbol = 0;
> >
> > to make it closer to -Wl,--defsym=symbol=0.
>
> If symbol: and symbol = 0; work, isn't symbol: slightly better because
> it looks more normal?
> Or is the intention here using SHN_ABS to make it clear these symbols
> are special?

Generate machine independent assembly codes is tricky.   We should
make before and after the change as close as possible to avoid ANY
surprises.   BTW, you need to check HAVE_ASM_SET_DIRECTIVE
to decide using ".set" vs "=".

-- 
H.J.

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

* Re: [PATCH 2/3] elf: Replace a --defsym trick with an object file to be compatible with lld
  2021-01-19 12:30       ` H.J. Lu
@ 2021-01-20 18:51         ` Fāng-ruì Sòng
  2021-01-28  1:03           ` Fāng-ruì Sòng
  0 siblings, 1 reply; 42+ messages in thread
From: Fāng-ruì Sòng @ 2021-01-20 18:51 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Florian Weimer, GNU C Library

On Tue, Jan 19, 2021 at 4:31 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 19, 2021 at 12:50 AM Fāng-ruì Sòng <maskray@google.com> wrote:
> >
> > On Mon, Jan 18, 2021 at 4:04 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Mon, Dec 28, 2020 at 11:50 AM Fangrui Song via Libc-alpha
> > > <libc-alpha@sourceware.org> wrote:
> > > >
> > > > The existing code specifies -Wl,--defsym=malloc=0 and other malloc.os
> > > > definitions before libc_pic.a so that libc_pic.a(malloc.os) is not
> > > > fetched. This trick is used to avoid multiple definition errors which
> > > > would happen as a chain result:
> > > >
> > > >   dl-allobjs.os has an undefined __libc_scratch_buffer_set_array_size
> > > >   __libc_scratch_buffer_set_array_size fetches libc_pic.a(scratch_buffer_set_array_size.os)
> > > >   libc_pic.a(scratch_buffer_set_array_size.os) has an undefined free
> > > >   free fetches libc_pic.a(malloc.os)
> > > >   libc_pic.a(malloc.os) has an undefined __libc_message
> > > >   __libc_message fetches libc_pic.a(libc_fatal.os)
> > > >
> > > >   libc_fatal.os will cause a multiple definition error (__GI___libc_fatal)
> > > >   >>> defined at dl-fxstatat64.c
> > > >   >>>            /tmp/p/glibc/Release/elf/dl-allobjs.os:(__GI___libc_fatal)
> > > >   >>> defined at libc_fatal.c
> > > >   >>>            libc_fatal.os:(.text+0x240) in archive /tmp/p/glibc/Release/libc_pic.a
> > > >
> > > > lld processes --defsym after all input files, so this trick does not
> > > > suppress multiple definition errors with lld. Split the step into two
> > > > and use an object file to make the intention more obvious and make lld
> > > > work.
> > > >
> > > > This is conceptually more appropriate because --defsym defines a SHN_ABS
> > > > symbol while a normal definition is relative to the image base.
> > > >
> > >
> > > This is irrelevant since  --defsym was used to create a temporary file which was
> > > removed immediately after it was created:
> > >
> > > /usr/bin/gcc   -nostdlib -nostartfiles -r -o
> > > /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map.o
> > > -Wl,--defsym=calloc=0 -Wl,--defsym=free=0 -Wl,--defsym=malloc=0
> > > -Wl,--defsym=realloc=0 -Wl,--defsym=__stack_chk_fail=0
> > > -Wl,--defsym=__stack_chk_fail_local=0 \
> > >         '-Wl,-('
> > > /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/dl-allobjs.os
> > > /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/libc_pic.a
> > > -lgcc '-Wl,-)' -Wl,-Map,/export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.mapT
> > > rm -f /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map.o
> > > mv -f /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.mapT
> > > /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map
> > >
> > > > ---
> > > >  elf/Makefile | 11 ++++-------
> > > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/elf/Makefile b/elf/Makefile
> > > > index 0b4d78c874..299bf24b49 100644
> > > > --- a/elf/Makefile
> > > > +++ b/elf/Makefile
> > > > @@ -524,10 +524,6 @@ rtld-stubbed-symbols = \
> > > >    malloc \
> > > >    realloc \
> > > >
> > > > -# The GCC arguments that implement $(rtld-stubbed-symbols).
> > > > -rtld-stubbed-symbols-args = \
> > > > -  $(patsubst %,-Wl$(comma)--defsym=%=0, $(rtld-stubbed-symbols))
> > > > -
> > > >  ifeq ($(have-ssp),yes)
> > > >  # rtld is not built with the stack protector, so these references will
> > > >  # go away in the rebuilds.
> > > > @@ -536,9 +532,10 @@ endif
> > > >
> > > >  $(objpfx)librtld.map: $(objpfx)dl-allobjs.os $(common-objpfx)libc_pic.a
> > > >         @-rm -f $@T
> > > > -       $(reloc-link) -o $@.o $(rtld-stubbed-symbols-args) \
> > > > -               '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
> > > > -       rm -f $@.o
> > > > +       echo '$(patsubst %,.globl %;,$(rtld-stubbed-symbols)) $(patsubst %,%:,$(rtld-stubbed-symbols))' | \
> > > > +               $(CC) -o $@T.o $(ASFLAGS) -c -x assembler -
> > >
> > > Please generate
> > >
> > > .globl symbol;
> > > symbol = 0;
> > >
> > > to make it closer to -Wl,--defsym=symbol=0.
> >
> > If symbol: and symbol = 0; work, isn't symbol: slightly better because
> > it looks more normal?
> > Or is the intention here using SHN_ABS to make it clear these symbols
> > are special?
>
> Generate machine independent assembly codes is tricky.   We should
> make before and after the change as close as possible to avoid ANY
> surprises.   BTW, you need to check HAVE_ASM_SET_DIRECTIVE
> to decide using ".set" vs "=".
>
> --
> H.J.

Thanks for the tip. In this case I think the patch as is (sticking
with `foo:`, don't use `foo = 0` or set) is better.
foo: works everywhere.

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

* Re: [PATCH 2/3] elf: Replace a --defsym trick with an object file to be compatible with lld
  2021-01-20 18:51         ` Fāng-ruì Sòng
@ 2021-01-28  1:03           ` Fāng-ruì Sòng
  2021-01-29 14:38             ` Adhemerval Zanella
  0 siblings, 1 reply; 42+ messages in thread
From: Fāng-ruì Sòng @ 2021-01-28  1:03 UTC (permalink / raw)
  To: Florian Weimer; +Cc: H.J. Lu, GNU C Library

On Wed, Jan 20, 2021 at 10:51 AM Fāng-ruì Sòng <maskray@google.com> wrote:
>
> On Tue, Jan 19, 2021 at 4:31 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Tue, Jan 19, 2021 at 12:50 AM Fāng-ruì Sòng <maskray@google.com> wrote:
> > >
> > > On Mon, Jan 18, 2021 at 4:04 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > >
> > > > On Mon, Dec 28, 2020 at 11:50 AM Fangrui Song via Libc-alpha
> > > > <libc-alpha@sourceware.org> wrote:
> > > > >
> > > > > The existing code specifies -Wl,--defsym=malloc=0 and other malloc.os
> > > > > definitions before libc_pic.a so that libc_pic.a(malloc.os) is not
> > > > > fetched. This trick is used to avoid multiple definition errors which
> > > > > would happen as a chain result:
> > > > >
> > > > >   dl-allobjs.os has an undefined __libc_scratch_buffer_set_array_size
> > > > >   __libc_scratch_buffer_set_array_size fetches libc_pic.a(scratch_buffer_set_array_size.os)
> > > > >   libc_pic.a(scratch_buffer_set_array_size.os) has an undefined free
> > > > >   free fetches libc_pic.a(malloc.os)
> > > > >   libc_pic.a(malloc.os) has an undefined __libc_message
> > > > >   __libc_message fetches libc_pic.a(libc_fatal.os)
> > > > >
> > > > >   libc_fatal.os will cause a multiple definition error (__GI___libc_fatal)
> > > > >   >>> defined at dl-fxstatat64.c
> > > > >   >>>            /tmp/p/glibc/Release/elf/dl-allobjs.os:(__GI___libc_fatal)
> > > > >   >>> defined at libc_fatal.c
> > > > >   >>>            libc_fatal.os:(.text+0x240) in archive /tmp/p/glibc/Release/libc_pic.a
> > > > >
> > > > > lld processes --defsym after all input files, so this trick does not
> > > > > suppress multiple definition errors with lld. Split the step into two
> > > > > and use an object file to make the intention more obvious and make lld
> > > > > work.
> > > > >
> > > > > This is conceptually more appropriate because --defsym defines a SHN_ABS
> > > > > symbol while a normal definition is relative to the image base.
> > > > >
> > > >
> > > > This is irrelevant since  --defsym was used to create a temporary file which was
> > > > removed immediately after it was created:
> > > >
> > > > /usr/bin/gcc   -nostdlib -nostartfiles -r -o
> > > > /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map.o
> > > > -Wl,--defsym=calloc=0 -Wl,--defsym=free=0 -Wl,--defsym=malloc=0
> > > > -Wl,--defsym=realloc=0 -Wl,--defsym=__stack_chk_fail=0
> > > > -Wl,--defsym=__stack_chk_fail_local=0 \
> > > >         '-Wl,-('
> > > > /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/dl-allobjs.os
> > > > /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/libc_pic.a
> > > > -lgcc '-Wl,-)' -Wl,-Map,/export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.mapT
> > > > rm -f /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map.o
> > > > mv -f /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.mapT
> > > > /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map
> > > >
> > > > > ---
> > > > >  elf/Makefile | 11 ++++-------
> > > > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/elf/Makefile b/elf/Makefile
> > > > > index 0b4d78c874..299bf24b49 100644
> > > > > --- a/elf/Makefile
> > > > > +++ b/elf/Makefile
> > > > > @@ -524,10 +524,6 @@ rtld-stubbed-symbols = \
> > > > >    malloc \
> > > > >    realloc \
> > > > >
> > > > > -# The GCC arguments that implement $(rtld-stubbed-symbols).
> > > > > -rtld-stubbed-symbols-args = \
> > > > > -  $(patsubst %,-Wl$(comma)--defsym=%=0, $(rtld-stubbed-symbols))
> > > > > -
> > > > >  ifeq ($(have-ssp),yes)
> > > > >  # rtld is not built with the stack protector, so these references will
> > > > >  # go away in the rebuilds.
> > > > > @@ -536,9 +532,10 @@ endif
> > > > >
> > > > >  $(objpfx)librtld.map: $(objpfx)dl-allobjs.os $(common-objpfx)libc_pic.a
> > > > >         @-rm -f $@T
> > > > > -       $(reloc-link) -o $@.o $(rtld-stubbed-symbols-args) \
> > > > > -               '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
> > > > > -       rm -f $@.o
> > > > > +       echo '$(patsubst %,.globl %;,$(rtld-stubbed-symbols)) $(patsubst %,%:,$(rtld-stubbed-symbols))' | \
> > > > > +               $(CC) -o $@T.o $(ASFLAGS) -c -x assembler -
> > > >
> > > > Please generate
> > > >
> > > > .globl symbol;
> > > > symbol = 0;
> > > >
> > > > to make it closer to -Wl,--defsym=symbol=0.
> > >
> > > If symbol: and symbol = 0; work, isn't symbol: slightly better because
> > > it looks more normal?
> > > Or is the intention here using SHN_ABS to make it clear these symbols
> > > are special?
> >
> > Generate machine independent assembly codes is tricky.   We should
> > make before and after the change as close as possible to avoid ANY
> > surprises.   BTW, you need to check HAVE_ASM_SET_DIRECTIVE
> > to decide using ".set" vs "=".
> >
> > --
> > H.J.
>
> Thanks for the tip. In this case I think the patch as is (sticking
> with `foo:`, don't use `foo = 0` or set) is better.
> foo: works everywhere.

Ping. I think the patch as-is is better than the alternative approach
using either `=` or `.set`.

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

* Re: [PATCH 2/3] elf: Replace a --defsym trick with an object file to be compatible with lld
  2021-01-28  1:03           ` Fāng-ruì Sòng
@ 2021-01-29 14:38             ` Adhemerval Zanella
  2021-01-29 15:29               ` H.J. Lu
  0 siblings, 1 reply; 42+ messages in thread
From: Adhemerval Zanella @ 2021-01-29 14:38 UTC (permalink / raw)
  To: Fāng-ruì Sòng, Florian Weimer; +Cc: GNU C Library



On 27/01/2021 22:03, Fāng-ruì Sòng via Libc-alpha wrote:
> On Wed, Jan 20, 2021 at 10:51 AM Fāng-ruì Sòng <maskray@google.com> wrote:
>>
>> On Tue, Jan 19, 2021 at 4:31 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>
>>> On Tue, Jan 19, 2021 at 12:50 AM Fāng-ruì Sòng <maskray@google.com> wrote:
>>>>
>>>> On Mon, Jan 18, 2021 at 4:04 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>
>>>>> On Mon, Dec 28, 2020 at 11:50 AM Fangrui Song via Libc-alpha
>>>>> <libc-alpha@sourceware.org> wrote:
>>>>>>
>>>>>> The existing code specifies -Wl,--defsym=malloc=0 and other malloc.os
>>>>>> definitions before libc_pic.a so that libc_pic.a(malloc.os) is not
>>>>>> fetched. This trick is used to avoid multiple definition errors which
>>>>>> would happen as a chain result:
>>>>>>
>>>>>>   dl-allobjs.os has an undefined __libc_scratch_buffer_set_array_size
>>>>>>   __libc_scratch_buffer_set_array_size fetches libc_pic.a(scratch_buffer_set_array_size.os)
>>>>>>   libc_pic.a(scratch_buffer_set_array_size.os) has an undefined free
>>>>>>   free fetches libc_pic.a(malloc.os)
>>>>>>   libc_pic.a(malloc.os) has an undefined __libc_message
>>>>>>   __libc_message fetches libc_pic.a(libc_fatal.os)
>>>>>>
>>>>>>   libc_fatal.os will cause a multiple definition error (__GI___libc_fatal)
>>>>>>   >>> defined at dl-fxstatat64.c
>>>>>>   >>>            /tmp/p/glibc/Release/elf/dl-allobjs.os:(__GI___libc_fatal)
>>>>>>   >>> defined at libc_fatal.c
>>>>>>   >>>            libc_fatal.os:(.text+0x240) in archive /tmp/p/glibc/Release/libc_pic.a
>>>>>>
>>>>>> lld processes --defsym after all input files, so this trick does not
>>>>>> suppress multiple definition errors with lld. Split the step into two
>>>>>> and use an object file to make the intention more obvious and make lld
>>>>>> work.
>>>>>>
>>>>>> This is conceptually more appropriate because --defsym defines a SHN_ABS
>>>>>> symbol while a normal definition is relative to the image base.
>>>>>>
>>>>>
>>>>> This is irrelevant since  --defsym was used to create a temporary file which was
>>>>> removed immediately after it was created:
>>>>>
>>>>> /usr/bin/gcc   -nostdlib -nostartfiles -r -o
>>>>> /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map.o
>>>>> -Wl,--defsym=calloc=0 -Wl,--defsym=free=0 -Wl,--defsym=malloc=0
>>>>> -Wl,--defsym=realloc=0 -Wl,--defsym=__stack_chk_fail=0
>>>>> -Wl,--defsym=__stack_chk_fail_local=0 \
>>>>>         '-Wl,-('
>>>>> /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/dl-allobjs.os
>>>>> /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/libc_pic.a
>>>>> -lgcc '-Wl,-)' -Wl,-Map,/export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.mapT
>>>>> rm -f /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map.o
>>>>> mv -f /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.mapT
>>>>> /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map
>>>>>
>>>>>> ---
>>>>>>  elf/Makefile | 11 ++++-------
>>>>>>  1 file changed, 4 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/elf/Makefile b/elf/Makefile
>>>>>> index 0b4d78c874..299bf24b49 100644
>>>>>> --- a/elf/Makefile
>>>>>> +++ b/elf/Makefile
>>>>>> @@ -524,10 +524,6 @@ rtld-stubbed-symbols = \
>>>>>>    malloc \
>>>>>>    realloc \
>>>>>>
>>>>>> -# The GCC arguments that implement $(rtld-stubbed-symbols).
>>>>>> -rtld-stubbed-symbols-args = \
>>>>>> -  $(patsubst %,-Wl$(comma)--defsym=%=0, $(rtld-stubbed-symbols))
>>>>>> -
>>>>>>  ifeq ($(have-ssp),yes)
>>>>>>  # rtld is not built with the stack protector, so these references will
>>>>>>  # go away in the rebuilds.
>>>>>> @@ -536,9 +532,10 @@ endif
>>>>>>
>>>>>>  $(objpfx)librtld.map: $(objpfx)dl-allobjs.os $(common-objpfx)libc_pic.a
>>>>>>         @-rm -f $@T
>>>>>> -       $(reloc-link) -o $@.o $(rtld-stubbed-symbols-args) \
>>>>>> -               '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
>>>>>> -       rm -f $@.o
>>>>>> +       echo '$(patsubst %,.globl %;,$(rtld-stubbed-symbols)) $(patsubst %,%:,$(rtld-stubbed-symbols))' | \
>>>>>> +               $(CC) -o $@T.o $(ASFLAGS) -c -x assembler -
>>>>>
>>>>> Please generate
>>>>>
>>>>> .globl symbol;
>>>>> symbol = 0;
>>>>>
>>>>> to make it closer to -Wl,--defsym=symbol=0.
>>>>
>>>> If symbol: and symbol = 0; work, isn't symbol: slightly better because
>>>> it looks more normal?
>>>> Or is the intention here using SHN_ABS to make it clear these symbols
>>>> are special?
>>>
>>> Generate machine independent assembly codes is tricky.   We should
>>> make before and after the change as close as possible to avoid ANY
>>> surprises.   BTW, you need to check HAVE_ASM_SET_DIRECTIVE
>>> to decide using ".set" vs "=".
>>>
>>> --
>>> H.J.
>>
>> Thanks for the tip. In this case I think the patch as is (sticking
>> with `foo:`, don't use `foo = 0` or set) is better.
>> foo: works everywhere.
> 
> Ping. I think the patch as-is is better than the alternative approach
> using either `=` or `.set`.

The patch breaks both arc-linux-gnuhf and hppa-linux-gnu with recent
ld.bfd 2.26, as below.

We will need to sort these out.

arc-linux-gnuhf:
make[2]: Entering directory '/home/azanella/glibc/glibc-git/elf'
echo '.globl calloc; .globl free; .globl malloc; .globl realloc; .globl __stack_chk_fail; .globl __stack_chk_fail_local; calloc: free: malloc: realloc: __stack_chk_fail: __stack_chk_fail_local:' | \
        /home/azanella/toolchain/install/compilers/arc-linux-gnuhf/bin/arc-glibc-linux-gnuhf-gcc -o /home/azanella/glibc/build/arc-linux-gnuhf/elf/librtld.mapT.o -g -Werror=undef -Wa,--noexecstack  -c -x assembler -
/home/azanella/toolchain/install/compilers/arc-linux-gnuhf/bin/arc-glibc-linux-gnuhf-gcc   -nostdlib -nostartfiles -r -o /home/azanella/glibc/build/arc-linux-gnuhf/elf/librtld.map.o /home/azanella/glibc/build/arc-linux-gnuhf/elf/librtld.mapT.o '-Wl,-(' /home/azanella/glibc/build/arc-linux-gnuhf/elf/dl-allobjs.os /home/azanella/glibc/build/arc-linux-gnuhf/libc_pic.a -lgcc '-Wl,-)' -Wl,-Map,/home/azanella/glibc/build/arc-linux-gnuhf/elf/librtld.mapT
/home/azanella/toolchain/install/compilers/arc-linux-gnuhf/lib/gcc/arc-glibc-linux-gnuhf/10.2.1/../../../../arc-glibc-linux-gnuhf/bin/ld: /home/azanella/glibc/build/arc-linux-gnuhf/libc_pic.a(sbrk.os): in function `__GI___sbrk':
/home/azanella/glibc/glibc-git/misc/sbrk.c:37: multiple definition of `__sbrk'; /home/azanella/glibc/build/arc-linux-gnuhf/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/../misc/sbrk.c:37: first defined here
/home/azanella/toolchain/install/compilers/arc-linux-gnuhf/lib/gcc/arc-glibc-linux-gnuhf/10.2.1/../../../../arc-glibc-linux-gnuhf/bin/ld: /home/azanella/glibc/build/arc-linux-gnuhf/libc_pic.a(libc_fatal.os): in function `__GI___libc_fatal':
/home/azanella/glibc/glibc-git/libio/../sysdeps/posix/libc_fatal.c:161: multiple definition of `__GI___libc_fatal'; /home/azanella/glibc/build/arc-linux-gnuhf/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/dl-minimal.c:267: first defined here
/home/azanella/toolchain/install/compilers/arc-linux-gnuhf/lib/gcc/arc-glibc-linux-gnuhf/10.2.1/../../../../arc-glibc-linux-gnuhf/bin/ld: /home/azanella/glibc/build/arc-linux-gnuhf/libc_pic.a(libc_fatal.os): in function `__GI___libc_fatal':
/home/azanella/glibc/glibc-git/libio/../sysdeps/posix/libc_fatal.c:161: multiple definition of `__libc_fatal'; /home/azanella/glibc/build/arc-linux-gnuhf/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/dl-minimal.c:267: first defined here
/home/azanella/toolchain/install/compilers/arc-linux-gnuhf/lib/gcc/arc-glibc-linux-gnuhf/10.2.1/../../../../arc-glibc-linux-gnuhf/bin/ld: /home/azanella/glibc/build/arc-linux-gnuhf/libc_pic.a(_itoa.os): in function `_itoa':
/home/azanella/glibc/glibc-git/stdio-common/_itoa.c:196: multiple definition of `_itoa'; /home/azanella/glibc/build/arc-linux-gnuhf/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/dl-minimal.c:323: first defined here
/home/azanella/toolchain/install/compilers/arc-linux-gnuhf/lib/gcc/arc-glibc-linux-gnuhf/10.2.1/../../../../arc-glibc-linux-gnuhf/bin/ld: /home/azanella/glibc/build/arc-linux-gnuhf/libc_pic.a(getcwd.os): in function `__GI___getcwd':
/home/azanella/glibc/glibc-git/io/../sysdeps/unix/sysv/linux/getcwd.c:50: multiple definition of `__getcwd'; /home/azanella/glibc/build/arc-linux-gnuhf/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/../sysdeps/unix/sysv/linux/getcwd.c:50: first defined here
/home/azanella/toolchain/install/compilers/arc-linux-gnuhf/lib/gcc/arc-glibc-linux-gnuhf/10.2.1/../../../../arc-glibc-linux-gnuhf/bin/ld: /home/azanella/glibc/build/arc-linux-gnuhf/libc_pic.a(dl-error.os): in function `__GI__dl_signal_exception':
/home/azanella/glibc/glibc-git/elf/dl-error-skeleton.c:91: multiple definition of `_dl_signal_exception'; /home/azanella/glibc/build/arc-linux-gnuhf/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/dl-error-skeleton.c:91: first defined here
/home/azanella/toolchain/install/compilers/arc-linux-gnuhf/lib/gcc/arc-glibc-linux-gnuhf/10.2.1/../../../../arc-glibc-linux-gnuhf/bin/ld: /home/azanella/glibc/build/arc-linux-gnuhf/libc_pic.a(dl-error.os): in function `__GI__dl_signal_error':
/home/azanella/glibc/glibc-git/elf/dl-error-skeleton.c:109: multiple definition of `_dl_signal_error'; /home/azanella/glibc/build/arc-linux-gnuhf/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/dl-error-skeleton.c:109: first defined here
/home/azanella/toolchain/install/compilers/arc-linux-gnuhf/lib/gcc/arc-glibc-linux-gnuhf/10.2.1/../../../../arc-glibc-linux-gnuhf/bin/ld: /home/azanella/glibc/build/arc-linux-gnuhf/libc_pic.a(dl-error.os): in function `__GI__dl_catch_exception':
/home/azanella/glibc/glibc-git/elf/dl-error-skeleton.c:175: multiple definition of `_dl_catch_exception'; /home/azanella/glibc/build/arc-linux-gnuhf/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/dl-error-skeleton.c:175: first defined here
/home/azanella/toolchain/install/compilers/arc-linux-gnuhf/lib/gcc/arc-glibc-linux-gnuhf/10.2.1/../../../../arc-glibc-linux-gnuhf/bin/ld: /home/azanella/glibc/build/arc-linux-gnuhf/libc_pic.a(dl-error.os): in function `__GI__dl_catch_error':
/home/azanella/glibc/glibc-git/elf/dl-error-skeleton.c:225: multiple definition of `_dl_catch_error'; /home/azanella/glibc/build/arc-linux-gnuhf/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/dl-error-skeleton.c:225: first defined here
collect2: error: ld returned 1 exit status
make[2]: *** [Makefile:538: /home/azanella/glibc/build/arc-linux-gnuhf/elf/librtld.map] Error 1

hppa-linux-gnu:
echo '.globl calloc; .globl free; .globl malloc; .globl realloc; calloc: free: malloc: realloc:' | \
        /home/azanella/toolchain/install/compilers/hppa-linux-gnu/bin/hppa-glibc-linux-gnu-gcc -o /home/azanella/glibc/build/hppa-linux-gnu/elf/librtld.mapT.o -g -Werror=undef   -c -x assembler -
/home/azanella/toolchain/install/compilers/hppa-linux-gnu/bin/hppa-glibc-linux-gnu-gcc   -nostdlib -nostartfiles -r -o /home/azanella/glibc/build/hppa-linux-gnu/elf/librtld.map.o /home/azanella/glibc/build/hppa-linux-gnu/elf/librtld.mapT.o '-Wl,-(' /home/azanella/glibc/build/hppa-linux-gnu/elf/dl-allobjs.os /home/azanella/glibc/build/hppa-linux-gnu/libc_pic.a -lgcc '-Wl,-)' -Wl,-Map,/home/azanella/glibc/build/hppa-linux-gnu/elf/librtld.mapT
/home/azanella/toolchain/install/compilers/hppa-linux-gnu/lib/gcc/hppa-glibc-linux-gnu/10.2.1/../../../../hppa-glibc-linux-gnu/bin/ld: /home/azanella/glibc/build/hppa-linux-gnu/libc_pic.a(sbrk.os): in function `__GI___sbrk':
/home/azanella/glibc/glibc-git/misc/sbrk.c:37: multiple definition of `__sbrk'; /home/azanella/glibc/build/hppa-linux-gnu/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/../misc/sbrk.c:37: first defined here
/home/azanella/toolchain/install/compilers/hppa-linux-gnu/lib/gcc/hppa-glibc-linux-gnu/10.2.1/../../../../hppa-glibc-linux-gnu/bin/ld: /home/azanella/glibc/build/hppa-linux-gnu/libc_pic.a(libc_fatal.os): in function `__GI___libc_fatal':
/home/azanella/glibc/glibc-git/libio/../sysdeps/posix/libc_fatal.c:161: multiple definition of `__GI___libc_fatal'; /home/azanella/glibc/build/hppa-linux-gnu/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/dl-minimal.c:268: first defined here
/home/azanella/toolchain/install/compilers/hppa-linux-gnu/lib/gcc/hppa-glibc-linux-gnu/10.2.1/../../../../hppa-glibc-linux-gnu/bin/ld: /home/azanella/glibc/build/hppa-linux-gnu/libc_pic.a(libc_fatal.os): in function `__GI___libc_fatal':
/home/azanella/glibc/glibc-git/libio/../sysdeps/posix/libc_fatal.c:161: multiple definition of `__libc_fatal'; /home/azanella/glibc/build/hppa-linux-gnu/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/dl-minimal.c:268: first defined here
/home/azanella/toolchain/install/compilers/hppa-linux-gnu/lib/gcc/hppa-glibc-linux-gnu/10.2.1/../../../../hppa-glibc-linux-gnu/bin/ld: /home/azanella/glibc/build/hppa-linux-gnu/libc_pic.a(_itoa.os): in function `_itoa':
/home/azanella/glibc/glibc-git/stdio-common/_itoa.c:196: multiple definition of `_itoa'; /home/azanella/glibc/build/hppa-linux-gnu/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/dl-minimal.c:323: first defined here
/home/azanella/toolchain/install/compilers/hppa-linux-gnu/lib/gcc/hppa-glibc-linux-gnu/10.2.1/../../../../hppa-glibc-linux-gnu/bin/ld: /home/azanella/glibc/build/hppa-linux-gnu/libc_pic.a(getcwd.os): in function `__GI___getcwd':
/home/azanella/glibc/glibc-git/io/../sysdeps/unix/sysv/linux/getcwd.c:50: multiple definition of `__getcwd'; /home/azanella/glibc/build/hppa-linux-gnu/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/../sysdeps/unix/sysv/linux/getcwd.c:50: first defined here
/home/azanella/toolchain/install/compilers/hppa-linux-gnu/lib/gcc/hppa-glibc-linux-gnu/10.2.1/../../../../hppa-glibc-linux-gnu/bin/ld: /home/azanella/glibc/build/hppa-linux-gnu/libc_pic.a(dl-error.os): in function `__GI__dl_signal_exception':
/home/azanella/glibc/glibc-git/elf/dl-error-skeleton.c:91: multiple definition of `_dl_signal_exception'; /home/azanella/glibc/build/hppa-linux-gnu/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/dl-error-skeleton.c:91: first defined here
/home/azanella/toolchain/install/compilers/hppa-linux-gnu/lib/gcc/hppa-glibc-linux-gnu/10.2.1/../../../../hppa-glibc-linux-gnu/bin/ld: /home/azanella/glibc/build/hppa-linux-gnu/libc_pic.a(dl-error.os): in function `__GI__dl_signal_error':
/home/azanella/glibc/glibc-git/elf/dl-error-skeleton.c:109: multiple definition of `_dl_signal_error'; /home/azanella/glibc/build/hppa-linux-gnu/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/dl-error-skeleton.c:109: first defined here
/home/azanella/toolchain/install/compilers/hppa-linux-gnu/lib/gcc/hppa-glibc-linux-gnu/10.2.1/../../../../hppa-glibc-linux-gnu/bin/ld: /home/azanella/glibc/build/hppa-linux-gnu/libc_pic.a(dl-error.os): in function `__GI__dl_catch_exception':
/home/azanella/glibc/glibc-git/elf/dl-error-skeleton.c:175: multiple definition of `_dl_catch_exception'; /home/azanella/glibc/build/hppa-linux-gnu/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/dl-error-skeleton.c:175: first defined here
/home/azanella/toolchain/install/compilers/hppa-linux-gnu/lib/gcc/hppa-glibc-linux-gnu/10.2.1/../../../../hppa-glibc-linux-gnu/bin/ld: /home/azanella/glibc/build/hppa-linux-gnu/libc_pic.a(dl-error.os): in function `__GI__dl_catch_error':
/home/azanella/glibc/glibc-git/elf/dl-error-skeleton.c:225: multiple definition of `_dl_catch_error'; /home/azanella/glibc/build/hppa-linux-gnu/elf/dl-allobjs.os:/home/azanella/glibc/glibc-git/elf/dl-error-skeleton.c:225: first defined here
collect2: error: ld returned 1 exit status

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

* Re: [PATCH 2/3] elf: Replace a --defsym trick with an object file to be compatible with lld
  2021-01-29 14:38             ` Adhemerval Zanella
@ 2021-01-29 15:29               ` H.J. Lu
  2021-01-29 18:04                 ` Adhemerval Zanella
  0 siblings, 1 reply; 42+ messages in thread
From: H.J. Lu @ 2021-01-29 15:29 UTC (permalink / raw)
  To: Adhemerval Zanella
  Cc: Fāng-ruì Sòng, Florian Weimer, GNU C Library

On Fri, Jan 29, 2021 at 7:26 AM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
>
> On 27/01/2021 22:03, Fāng-ruì Sòng via Libc-alpha wrote:
> > On Wed, Jan 20, 2021 at 10:51 AM Fāng-ruì Sòng <maskray@google.com> wrote:
> >>
> >> On Tue, Jan 19, 2021 at 4:31 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>>
> >>> On Tue, Jan 19, 2021 at 12:50 AM Fāng-ruì Sòng <maskray@google.com> wrote:
> >>>>
> >>>> On Mon, Jan 18, 2021 at 4:04 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>>>>
> >>>>> On Mon, Dec 28, 2020 at 11:50 AM Fangrui Song via Libc-alpha
> >>>>> <libc-alpha@sourceware.org> wrote:
> >>>>>>
> >>>>>> The existing code specifies -Wl,--defsym=malloc=0 and other malloc.os
> >>>>>> definitions before libc_pic.a so that libc_pic.a(malloc.os) is not
> >>>>>> fetched. This trick is used to avoid multiple definition errors which
> >>>>>> would happen as a chain result:
> >>>>>>
> >>>>>>   dl-allobjs.os has an undefined __libc_scratch_buffer_set_array_size
> >>>>>>   __libc_scratch_buffer_set_array_size fetches libc_pic.a(scratch_buffer_set_array_size.os)
> >>>>>>   libc_pic.a(scratch_buffer_set_array_size.os) has an undefined free
> >>>>>>   free fetches libc_pic.a(malloc.os)
> >>>>>>   libc_pic.a(malloc.os) has an undefined __libc_message
> >>>>>>   __libc_message fetches libc_pic.a(libc_fatal.os)
> >>>>>>
> >>>>>>   libc_fatal.os will cause a multiple definition error (__GI___libc_fatal)
> >>>>>>   >>> defined at dl-fxstatat64.c
> >>>>>>   >>>            /tmp/p/glibc/Release/elf/dl-allobjs.os:(__GI___libc_fatal)
> >>>>>>   >>> defined at libc_fatal.c
> >>>>>>   >>>            libc_fatal.os:(.text+0x240) in archive /tmp/p/glibc/Release/libc_pic.a
> >>>>>>
> >>>>>> lld processes --defsym after all input files, so this trick does not
> >>>>>> suppress multiple definition errors with lld. Split the step into two
> >>>>>> and use an object file to make the intention more obvious and make lld
> >>>>>> work.
> >>>>>>
> >>>>>> This is conceptually more appropriate because --defsym defines a SHN_ABS
> >>>>>> symbol while a normal definition is relative to the image base.
> >>>>>>
> >>>>>
> >>>>> This is irrelevant since  --defsym was used to create a temporary file which was
> >>>>> removed immediately after it was created:
> >>>>>
> >>>>> /usr/bin/gcc   -nostdlib -nostartfiles -r -o
> >>>>> /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map.o
> >>>>> -Wl,--defsym=calloc=0 -Wl,--defsym=free=0 -Wl,--defsym=malloc=0
> >>>>> -Wl,--defsym=realloc=0 -Wl,--defsym=__stack_chk_fail=0
> >>>>> -Wl,--defsym=__stack_chk_fail_local=0 \
> >>>>>         '-Wl,-('
> >>>>> /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/dl-allobjs.os
> >>>>> /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/libc_pic.a
> >>>>> -lgcc '-Wl,-)' -Wl,-Map,/export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.mapT
> >>>>> rm -f /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map.o
> >>>>> mv -f /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.mapT
> >>>>> /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map
> >>>>>
> >>>>>> ---
> >>>>>>  elf/Makefile | 11 ++++-------
> >>>>>>  1 file changed, 4 insertions(+), 7 deletions(-)
> >>>>>>
> >>>>>> diff --git a/elf/Makefile b/elf/Makefile
> >>>>>> index 0b4d78c874..299bf24b49 100644
> >>>>>> --- a/elf/Makefile
> >>>>>> +++ b/elf/Makefile
> >>>>>> @@ -524,10 +524,6 @@ rtld-stubbed-symbols = \
> >>>>>>    malloc \
> >>>>>>    realloc \
> >>>>>>
> >>>>>> -# The GCC arguments that implement $(rtld-stubbed-symbols).
> >>>>>> -rtld-stubbed-symbols-args = \
> >>>>>> -  $(patsubst %,-Wl$(comma)--defsym=%=0, $(rtld-stubbed-symbols))
> >>>>>> -
> >>>>>>  ifeq ($(have-ssp),yes)
> >>>>>>  # rtld is not built with the stack protector, so these references will
> >>>>>>  # go away in the rebuilds.
> >>>>>> @@ -536,9 +532,10 @@ endif
> >>>>>>
> >>>>>>  $(objpfx)librtld.map: $(objpfx)dl-allobjs.os $(common-objpfx)libc_pic.a
> >>>>>>         @-rm -f $@T
> >>>>>> -       $(reloc-link) -o $@.o $(rtld-stubbed-symbols-args) \
> >>>>>> -               '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
> >>>>>> -       rm -f $@.o
> >>>>>> +       echo '$(patsubst %,.globl %;,$(rtld-stubbed-symbols)) $(patsubst %,%:,$(rtld-stubbed-symbols))' | \
> >>>>>> +               $(CC) -o $@T.o $(ASFLAGS) -c -x assembler -
> >>>>>
> >>>>> Please generate
> >>>>>
> >>>>> .globl symbol;
> >>>>> symbol = 0;
> >>>>>
> >>>>> to make it closer to -Wl,--defsym=symbol=0.
> >>>>
> >>>> If symbol: and symbol = 0; work, isn't symbol: slightly better because
> >>>> it looks more normal?
> >>>> Or is the intention here using SHN_ABS to make it clear these symbols
> >>>> are special?
> >>>
> >>> Generate machine independent assembly codes is tricky.   We should
> >>> make before and after the change as close as possible to avoid ANY
> >>> surprises.   BTW, you need to check HAVE_ASM_SET_DIRECTIVE
> >>> to decide using ".set" vs "=".
> >>>
> >>> --
> >>> H.J.
> >>
> >> Thanks for the tip. In this case I think the patch as is (sticking
> >> with `foo:`, don't use `foo = 0` or set) is better.
> >> foo: works everywhere.
> >
> > Ping. I think the patch as-is is better than the alternative approach
> > using either `=` or `.set`.
>
> The patch breaks both arc-linux-gnuhf and hppa-linux-gnu with recent
> ld.bfd 2.26, as below.
>
> We will need to sort these out.
>

Does my suggestion work?

-- 
H.J.

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

* Re: [PATCH 2/3] elf: Replace a --defsym trick with an object file to be compatible with lld
  2021-01-29 15:29               ` H.J. Lu
@ 2021-01-29 18:04                 ` Adhemerval Zanella
  2021-01-29 19:28                   ` [PATCH v2] elf: Replace a --defsym trick with an object file to be compatible with LLD Fangrui Song
  0 siblings, 1 reply; 42+ messages in thread
From: Adhemerval Zanella @ 2021-01-29 18:04 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Fāng-ruì Sòng, Florian Weimer, GNU C Library



On 29/01/2021 12:29, H.J. Lu wrote:
> On Fri, Jan 29, 2021 at 7:26 AM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>>
>>
>> On 27/01/2021 22:03, Fāng-ruì Sòng via Libc-alpha wrote:
>>> On Wed, Jan 20, 2021 at 10:51 AM Fāng-ruì Sòng <maskray@google.com> wrote:
>>>>
>>>> On Tue, Jan 19, 2021 at 4:31 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>
>>>>> On Tue, Jan 19, 2021 at 12:50 AM Fāng-ruì Sòng <maskray@google.com> wrote:
>>>>>>
>>>>>> On Mon, Jan 18, 2021 at 4:04 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>
>>>>>>> On Mon, Dec 28, 2020 at 11:50 AM Fangrui Song via Libc-alpha
>>>>>>> <libc-alpha@sourceware.org> wrote:
>>>>>>>>
>>>>>>>> The existing code specifies -Wl,--defsym=malloc=0 and other malloc.os
>>>>>>>> definitions before libc_pic.a so that libc_pic.a(malloc.os) is not
>>>>>>>> fetched. This trick is used to avoid multiple definition errors which
>>>>>>>> would happen as a chain result:
>>>>>>>>
>>>>>>>>   dl-allobjs.os has an undefined __libc_scratch_buffer_set_array_size
>>>>>>>>   __libc_scratch_buffer_set_array_size fetches libc_pic.a(scratch_buffer_set_array_size.os)
>>>>>>>>   libc_pic.a(scratch_buffer_set_array_size.os) has an undefined free
>>>>>>>>   free fetches libc_pic.a(malloc.os)
>>>>>>>>   libc_pic.a(malloc.os) has an undefined __libc_message
>>>>>>>>   __libc_message fetches libc_pic.a(libc_fatal.os)
>>>>>>>>
>>>>>>>>   libc_fatal.os will cause a multiple definition error (__GI___libc_fatal)
>>>>>>>>   >>> defined at dl-fxstatat64.c
>>>>>>>>   >>>            /tmp/p/glibc/Release/elf/dl-allobjs.os:(__GI___libc_fatal)
>>>>>>>>   >>> defined at libc_fatal.c
>>>>>>>>   >>>            libc_fatal.os:(.text+0x240) in archive /tmp/p/glibc/Release/libc_pic.a
>>>>>>>>
>>>>>>>> lld processes --defsym after all input files, so this trick does not
>>>>>>>> suppress multiple definition errors with lld. Split the step into two
>>>>>>>> and use an object file to make the intention more obvious and make lld
>>>>>>>> work.
>>>>>>>>
>>>>>>>> This is conceptually more appropriate because --defsym defines a SHN_ABS
>>>>>>>> symbol while a normal definition is relative to the image base.
>>>>>>>>
>>>>>>>
>>>>>>> This is irrelevant since  --defsym was used to create a temporary file which was
>>>>>>> removed immediately after it was created:
>>>>>>>
>>>>>>> /usr/bin/gcc   -nostdlib -nostartfiles -r -o
>>>>>>> /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map.o
>>>>>>> -Wl,--defsym=calloc=0 -Wl,--defsym=free=0 -Wl,--defsym=malloc=0
>>>>>>> -Wl,--defsym=realloc=0 -Wl,--defsym=__stack_chk_fail=0
>>>>>>> -Wl,--defsym=__stack_chk_fail_local=0 \
>>>>>>>         '-Wl,-('
>>>>>>> /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/dl-allobjs.os
>>>>>>> /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/libc_pic.a
>>>>>>> -lgcc '-Wl,-)' -Wl,-Map,/export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.mapT
>>>>>>> rm -f /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map.o
>>>>>>> mv -f /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.mapT
>>>>>>> /export/users/hjl/build/gnu/tools-build/glibc-gitlab/build-x86_64-linux/elf/librtld.map
>>>>>>>
>>>>>>>> ---
>>>>>>>>  elf/Makefile | 11 ++++-------
>>>>>>>>  1 file changed, 4 insertions(+), 7 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/elf/Makefile b/elf/Makefile
>>>>>>>> index 0b4d78c874..299bf24b49 100644
>>>>>>>> --- a/elf/Makefile
>>>>>>>> +++ b/elf/Makefile
>>>>>>>> @@ -524,10 +524,6 @@ rtld-stubbed-symbols = \
>>>>>>>>    malloc \
>>>>>>>>    realloc \
>>>>>>>>
>>>>>>>> -# The GCC arguments that implement $(rtld-stubbed-symbols).
>>>>>>>> -rtld-stubbed-symbols-args = \
>>>>>>>> -  $(patsubst %,-Wl$(comma)--defsym=%=0, $(rtld-stubbed-symbols))
>>>>>>>> -
>>>>>>>>  ifeq ($(have-ssp),yes)
>>>>>>>>  # rtld is not built with the stack protector, so these references will
>>>>>>>>  # go away in the rebuilds.
>>>>>>>> @@ -536,9 +532,10 @@ endif
>>>>>>>>
>>>>>>>>  $(objpfx)librtld.map: $(objpfx)dl-allobjs.os $(common-objpfx)libc_pic.a
>>>>>>>>         @-rm -f $@T
>>>>>>>> -       $(reloc-link) -o $@.o $(rtld-stubbed-symbols-args) \
>>>>>>>> -               '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
>>>>>>>> -       rm -f $@.o
>>>>>>>> +       echo '$(patsubst %,.globl %;,$(rtld-stubbed-symbols)) $(patsubst %,%:,$(rtld-stubbed-symbols))' | \
>>>>>>>> +               $(CC) -o $@T.o $(ASFLAGS) -c -x assembler -
>>>>>>>
>>>>>>> Please generate
>>>>>>>
>>>>>>> .globl symbol;
>>>>>>> symbol = 0;
>>>>>>>
>>>>>>> to make it closer to -Wl,--defsym=symbol=0.
>>>>>>
>>>>>> If symbol: and symbol = 0; work, isn't symbol: slightly better because
>>>>>> it looks more normal?
>>>>>> Or is the intention here using SHN_ABS to make it clear these symbols
>>>>>> are special?
>>>>>
>>>>> Generate machine independent assembly codes is tricky.   We should
>>>>> make before and after the change as close as possible to avoid ANY
>>>>> surprises.   BTW, you need to check HAVE_ASM_SET_DIRECTIVE
>>>>> to decide using ".set" vs "=".
>>>>>
>>>>> --
>>>>> H.J.
>>>>
>>>> Thanks for the tip. In this case I think the patch as is (sticking
>>>> with `foo:`, don't use `foo = 0` or set) is better.
>>>> foo: works everywhere.
>>>
>>> Ping. I think the patch as-is is better than the alternative approach
>>> using either `=` or `.set`.
>>
>> The patch breaks both arc-linux-gnuhf and hppa-linux-gnu with recent
>> ld.bfd 2.26, as below.
>>
>> We will need to sort these out.

The issue was in fact that hppa and arc uses no default asm line separators
(represented by ASM_LINE_SEP within code): hppa uses '!' while arc '`'. 
By replacing echo with printf and issuing a new line explicit I see that
all targets build:

---

diff --git a/elf/Makefile b/elf/Makefile
index 5d666b1b0c..e5c04f682c 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -526,10 +526,6 @@ rtld-stubbed-symbols = \
   malloc \
   realloc \

-# The GCC arguments that implement $(rtld-stubbed-symbols).
-rtld-stubbed-symbols-args = \
-  $(patsubst %,-Wl$(comma)--defsym=%=0, $(rtld-stubbed-symbols))
-
 ifeq ($(have-ssp),yes)
 # rtld is not built with the stack protector, so these references will
 # go away in the rebuilds.
@@ -538,9 +534,10 @@ endif

 $(objpfx)librtld.map: $(objpfx)dl-allobjs.os $(common-objpfx)libc_pic.a
        @-rm -f $@T
-       $(reloc-link) -o $@.o $(rtld-stubbed-symbols-args) \
-               '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
-       rm -f $@.o
+       printf '$(patsubst %,.globl %;\n,$(rtld-stubbed-symbols)) $(patsubst %,% = 0;\n,$(rtld-stubbed-symbols))' | \
+               $(CC) -o $@T.o $(ASFLAGS) -c -x assembler -
+       $(reloc-link) -o $@.o $@T.o '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
+       rm -f %@T.o $@.o
        mv -f $@T $@

 # For lld, skip preceding addresses and values before matching the archive and the member.

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

* [PATCH v2] elf: Replace a --defsym trick with an object file to be compatible with LLD
  2021-01-29 18:04                 ` Adhemerval Zanella
@ 2021-01-29 19:28                   ` Fangrui Song
  2021-02-01 13:19                     ` Adhemerval Zanella
  2021-02-01 13:43                     ` Florian Weimer
  0 siblings, 2 replies; 42+ messages in thread
From: Fangrui Song @ 2021-01-29 19:28 UTC (permalink / raw)
  To: libc-alpha, Adhemerval Zanella; +Cc: H.J. Lu, Fangrui Song

The existing code specifies -Wl,--defsym=malloc=0 and other malloc.os
definitions before libc_pic.a so that libc_pic.a(malloc.os) is not
fetched. This trick is used to avoid multiple definition errors which
would happen as a chain result:

  dl-allobjs.os has an undefined __libc_scratch_buffer_set_array_size
  __libc_scratch_buffer_set_array_size fetches libc_pic.a(scratch_buffer_set_array_size.os)
  libc_pic.a(scratch_buffer_set_array_size.os) has an undefined free
  free fetches libc_pic.a(malloc.os)
  libc_pic.a(malloc.os) has an undefined __libc_message
  __libc_message fetches libc_pic.a(libc_fatal.os)

  libc_fatal.os will cause a multiple definition error (__GI___libc_fatal)
  >>> defined at dl-fxstatat64.c
  >>>            /tmp/p/glibc/Release/elf/dl-allobjs.os:(__GI___libc_fatal)
  >>> defined at libc_fatal.c
  >>>            libc_fatal.os:(.text+0x240) in archive /tmp/p/glibc/Release/libc_pic.a

LLD processes --defsym after all input files, so this trick does not
suppress multiple definition errors with LLD. Split the step into two
and use an object file to make the intention more obvious and make LLD
work.

This is conceptually more appropriate because --defsym defines a SHN_ABS
symbol while a normal definition is relative to the image base.

See https://sourceware.org/pipermail/libc-alpha/2020-March/111910.html
for discussions about the --defsym semantics.

This commit is available on https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld
---
 elf/Makefile | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/elf/Makefile b/elf/Makefile
index 5d666b1b0c..03e4034cc7 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -526,10 +526,6 @@ rtld-stubbed-symbols = \
   malloc \
   realloc \
 
-# The GCC arguments that implement $(rtld-stubbed-symbols).
-rtld-stubbed-symbols-args = \
-  $(patsubst %,-Wl$(comma)--defsym=%=0, $(rtld-stubbed-symbols))
-
 ifeq ($(have-ssp),yes)
 # rtld is not built with the stack protector, so these references will
 # go away in the rebuilds.
@@ -538,9 +534,10 @@ endif
 
 $(objpfx)librtld.map: $(objpfx)dl-allobjs.os $(common-objpfx)libc_pic.a
 	@-rm -f $@T
-	$(reloc-link) -o $@.o $(rtld-stubbed-symbols-args) \
-		'-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
-	rm -f $@.o
+	printf '$(patsubst %,.globl %\n,$(rtld-stubbed-symbols)) $(patsubst %,%:\n,$(rtld-stubbed-symbols))' | \
+		$(CC) -o $@T.o $(ASFLAGS) -c -x assembler -
+	$(reloc-link) -o $@.o $@T.o '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
+	rm -f %@T.o $@.o
 	mv -f $@T $@
 
 # For lld, skip preceding addresses and values before matching the archive and the member.
-- 
2.30.0.365.g02bc693789-goog


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

* Re: [PATCH v2] elf: Replace a --defsym trick with an object file to be compatible with LLD
  2021-01-29 19:28                   ` [PATCH v2] elf: Replace a --defsym trick with an object file to be compatible with LLD Fangrui Song
@ 2021-02-01 13:19                     ` Adhemerval Zanella
  2021-02-01 13:45                       ` H.J. Lu
  2021-02-01 13:43                     ` Florian Weimer
  1 sibling, 1 reply; 42+ messages in thread
From: Adhemerval Zanella @ 2021-02-01 13:19 UTC (permalink / raw)
  To: Fangrui Song, libc-alpha



On 29/01/2021 16:28, Fangrui Song wrote:
> The existing code specifies -Wl,--defsym=malloc=0 and other malloc.os
> definitions before libc_pic.a so that libc_pic.a(malloc.os) is not
> fetched. This trick is used to avoid multiple definition errors which
> would happen as a chain result:
> 
>   dl-allobjs.os has an undefined __libc_scratch_buffer_set_array_size
>   __libc_scratch_buffer_set_array_size fetches libc_pic.a(scratch_buffer_set_array_size.os)
>   libc_pic.a(scratch_buffer_set_array_size.os) has an undefined free
>   free fetches libc_pic.a(malloc.os)
>   libc_pic.a(malloc.os) has an undefined __libc_message
>   __libc_message fetches libc_pic.a(libc_fatal.os)
> 
>   libc_fatal.os will cause a multiple definition error (__GI___libc_fatal)
>   >>> defined at dl-fxstatat64.c
>   >>>            /tmp/p/glibc/Release/elf/dl-allobjs.os:(__GI___libc_fatal)
>   >>> defined at libc_fatal.c
>   >>>            libc_fatal.os:(.text+0x240) in archive /tmp/p/glibc/Release/libc_pic.a
> 
> LLD processes --defsym after all input files, so this trick does not
> suppress multiple definition errors with LLD. Split the step into two
> and use an object file to make the intention more obvious and make LLD
> work.
> 
> This is conceptually more appropriate because --defsym defines a SHN_ABS
> symbol while a normal definition is relative to the image base.
> 
> See https://sourceware.org/pipermail/libc-alpha/2020-March/111910.html
> for discussions about the --defsym semantics.
> 
> This commit is available on https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld
> ---
>  elf/Makefile | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index 5d666b1b0c..03e4034cc7 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -526,10 +526,6 @@ rtld-stubbed-symbols = \
>    malloc \
>    realloc \
>  
> -# The GCC arguments that implement $(rtld-stubbed-symbols).
> -rtld-stubbed-symbols-args = \
> -  $(patsubst %,-Wl$(comma)--defsym=%=0, $(rtld-stubbed-symbols))
> -
>  ifeq ($(have-ssp),yes)
>  # rtld is not built with the stack protector, so these references will
>  # go away in the rebuilds.
> @@ -538,9 +534,10 @@ endif
>  
>  $(objpfx)librtld.map: $(objpfx)dl-allobjs.os $(common-objpfx)libc_pic.a
>  	@-rm -f $@T
> -	$(reloc-link) -o $@.o $(rtld-stubbed-symbols-args) \
> -		'-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
> -	rm -f $@.o
> +	printf '$(patsubst %,.globl %\n,$(rtld-stubbed-symbols)) $(patsubst %,%:\n,$(rtld-stubbed-symbols))' | \
> +		$(CC) -o $@T.o $(ASFLAGS) -c -x assembler -
> +	$(reloc-link) -o $@.o $@T.o '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
> +	rm -f %@T.o $@.o
>  	mv -f $@T $@
>  
>  # For lld, skip preceding addresses and values before matching the archive and the member.

Looks ok, I haven't see any build issue.  The '=' seems unrequired, although '=' 
seems to work on all supported architectures (HAVE_ASM_SET_DIRECTIVE does not 
seem to be necessary to handle here).

It is ok for 2.34 from my side.  H.J, do you see any issues of not setting the
symbol to '0'?

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

* Re: [PATCH v2] elf: Replace a --defsym trick with an object file to be compatible with LLD
  2021-01-29 19:28                   ` [PATCH v2] elf: Replace a --defsym trick with an object file to be compatible with LLD Fangrui Song
  2021-02-01 13:19                     ` Adhemerval Zanella
@ 2021-02-01 13:43                     ` Florian Weimer
  1 sibling, 0 replies; 42+ messages in thread
From: Florian Weimer @ 2021-02-01 13:43 UTC (permalink / raw)
  To: Fangrui Song via Libc-alpha; +Cc: Adhemerval Zanella, Fangrui Song

* Fangrui Song via Libc-alpha:

>  $(objpfx)librtld.map: $(objpfx)dl-allobjs.os $(common-objpfx)libc_pic.a
>  	@-rm -f $@T
> -	$(reloc-link) -o $@.o $(rtld-stubbed-symbols-args) \
> -		'-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
> -	rm -f $@.o
> +	printf '$(patsubst %,.globl %\n,$(rtld-stubbed-symbols)) $(patsubst %,%:\n,$(rtld-stubbed-symbols))' | \
> +		$(CC) -o $@T.o $(ASFLAGS) -c -x assembler -
> +	$(reloc-link) -o $@.o $@T.o '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
> +	rm -f %@T.o $@.o
>  	mv -f $@T $@

It may be a little bit cleaner to replace the shell/make mix with just
shell, like (untested):

	for symbol in $(rtld-stubbed-symbols) ; do \
	   echo ".globl $$symbol" ; \
	   echo "$$symbol:" ; \
	done | $(CC) -o $@T.o $(ASFLAGS) -c -x assembler -

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill


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

* Re: [PATCH v2] elf: Replace a --defsym trick with an object file to be compatible with LLD
  2021-02-01 13:19                     ` Adhemerval Zanella
@ 2021-02-01 13:45                       ` H.J. Lu
  0 siblings, 0 replies; 42+ messages in thread
From: H.J. Lu @ 2021-02-01 13:45 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: Fangrui Song, GNU C Library

On Mon, Feb 1, 2021 at 5:19 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 29/01/2021 16:28, Fangrui Song wrote:
> > The existing code specifies -Wl,--defsym=malloc=0 and other malloc.os
> > definitions before libc_pic.a so that libc_pic.a(malloc.os) is not
> > fetched. This trick is used to avoid multiple definition errors which
> > would happen as a chain result:
> >
> >   dl-allobjs.os has an undefined __libc_scratch_buffer_set_array_size
> >   __libc_scratch_buffer_set_array_size fetches libc_pic.a(scratch_buffer_set_array_size.os)
> >   libc_pic.a(scratch_buffer_set_array_size.os) has an undefined free
> >   free fetches libc_pic.a(malloc.os)
> >   libc_pic.a(malloc.os) has an undefined __libc_message
> >   __libc_message fetches libc_pic.a(libc_fatal.os)
> >
> >   libc_fatal.os will cause a multiple definition error (__GI___libc_fatal)
> >   >>> defined at dl-fxstatat64.c
> >   >>>            /tmp/p/glibc/Release/elf/dl-allobjs.os:(__GI___libc_fatal)
> >   >>> defined at libc_fatal.c
> >   >>>            libc_fatal.os:(.text+0x240) in archive /tmp/p/glibc/Release/libc_pic.a
> >
> > LLD processes --defsym after all input files, so this trick does not
> > suppress multiple definition errors with LLD. Split the step into two
> > and use an object file to make the intention more obvious and make LLD
> > work.
> >
> > This is conceptually more appropriate because --defsym defines a SHN_ABS
> > symbol while a normal definition is relative to the image base.
> >
> > See https://sourceware.org/pipermail/libc-alpha/2020-March/111910.html
> > for discussions about the --defsym semantics.
> >
> > This commit is available on https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/lld
> > ---
> >  elf/Makefile | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/elf/Makefile b/elf/Makefile
> > index 5d666b1b0c..03e4034cc7 100644
> > --- a/elf/Makefile
> > +++ b/elf/Makefile
> > @@ -526,10 +526,6 @@ rtld-stubbed-symbols = \
> >    malloc \
> >    realloc \
> >
> > -# The GCC arguments that implement $(rtld-stubbed-symbols).
> > -rtld-stubbed-symbols-args = \
> > -  $(patsubst %,-Wl$(comma)--defsym=%=0, $(rtld-stubbed-symbols))
> > -
> >  ifeq ($(have-ssp),yes)
> >  # rtld is not built with the stack protector, so these references will
> >  # go away in the rebuilds.
> > @@ -538,9 +534,10 @@ endif
> >
> >  $(objpfx)librtld.map: $(objpfx)dl-allobjs.os $(common-objpfx)libc_pic.a
> >       @-rm -f $@T
> > -     $(reloc-link) -o $@.o $(rtld-stubbed-symbols-args) \
> > -             '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
> > -     rm -f $@.o
> > +     printf '$(patsubst %,.globl %\n,$(rtld-stubbed-symbols)) $(patsubst %,%:\n,$(rtld-stubbed-symbols))' | \
> > +             $(CC) -o $@T.o $(ASFLAGS) -c -x assembler -
> > +     $(reloc-link) -o $@.o $@T.o '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
> > +     rm -f %@T.o $@.o
> >       mv -f $@T $@
> >
> >  # For lld, skip preceding addresses and values before matching the archive and the member.
>
> Looks ok, I haven't see any build issue.  The '=' seems unrequired, although '='
> seems to work on all supported architectures (HAVE_ASM_SET_DIRECTIVE does not
> seem to be necessary to handle here).
>
> It is ok for 2.34 from my side.  H.J, do you see any issues of not setting the
> symbol to '0'?

LGTM.

Thanks.

-- 
H.J.

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

end of thread, other threads:[~2021-02-01 13:46 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-28 19:48 [PATCH 0/3] Make glibc build with LLD Fangrui Song
2020-12-28 19:48 ` [PATCH 1/3] configure: Allow LD to be a linker other than GNU ld and gold Fangrui Song
2020-12-28 20:46   ` H.J. Lu
2020-12-28 21:15     ` [PATCH 1/3] configure: Allow LD to be LLD 9.0.0 or above " Fangrui Song
2020-12-28 19:48 ` [PATCH 2/3] elf: Replace a --defsym trick with an object file to be compatible with lld Fangrui Song
2021-01-11 20:06   ` Fāng-ruì Sòng
2021-01-18 22:08     ` Fāng-ruì Sòng
2021-01-19  0:03   ` H.J. Lu
2021-01-19  8:50     ` Fāng-ruì Sòng
2021-01-19 12:30       ` H.J. Lu
2021-01-20 18:51         ` Fāng-ruì Sòng
2021-01-28  1:03           ` Fāng-ruì Sòng
2021-01-29 14:38             ` Adhemerval Zanella
2021-01-29 15:29               ` H.J. Lu
2021-01-29 18:04                 ` Adhemerval Zanella
2021-01-29 19:28                   ` [PATCH v2] elf: Replace a --defsym trick with an object file to be compatible with LLD Fangrui Song
2021-02-01 13:19                     ` Adhemerval Zanella
2021-02-01 13:45                       ` H.J. Lu
2021-02-01 13:43                     ` Florian Weimer
2020-12-28 19:48 ` [PATCH 3/3] install: Replace scripts/output-format.sed with objdump -f [BZ #26559] Fangrui Song
2021-01-08 19:38   ` Adhemerval Zanella
2021-01-12 11:20     ` Florian Weimer
2021-01-12 12:00       ` Adhemerval Zanella
2021-01-12 17:46         ` Fāng-ruì Sòng
2021-01-12 11:32   ` Andreas Schwab
2021-01-12 11:58     ` Florian Weimer
2020-12-28 21:11 ` [PATCH 0/3] Make glibc build with LLD H.J. Lu
2020-12-28 21:45   ` Fangrui Song
2020-12-28 21:49     ` H.J. Lu
2020-12-28 22:54       ` H.J. Lu
2021-01-05 23:03         ` Fāng-ruì Sòng
2021-01-05 23:33           ` H.J. Lu
2021-01-05 23:41             ` Fāng-ruì Sòng
2021-01-05 23:51               ` H.J. Lu
2021-01-06  0:50                 ` Fāng-ruì Sòng
2021-01-06  1:43                   ` H.J. Lu
2021-01-06  2:00                     ` Fāng-ruì Sòng
2021-01-06  2:14                       ` H.J. Lu
2021-01-05 22:57       ` Fāng-ruì Sòng
2021-01-08  5:04 ` Fāng-ruì Sòng
2021-01-08 17:09   ` H.J. Lu
2021-01-08 18:04     ` Fāng-ruì Sòng

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 read-only IMAP folder(s) and NNTP newsgroup(s).