unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: "H.J. Lu via Libc-alpha" <libc-alpha@sourceware.org>
To: "Fāng-ruì Sòng" <maskray@google.com>
Cc: GNU C Library <libc-alpha@sourceware.org>
Subject: Re: [PATCH v2 3/3] configure: Allow LD to be LLD 13.0.0 or above [BZ #26558]
Date: Thu, 5 Aug 2021 10:04:27 -0700	[thread overview]
Message-ID: <CAMe9rOqxnzDoCgbkCGjrBEPkjNQw7y=LcoZ94F4mjwUMCXY5tw@mail.gmail.com> (raw)
In-Reply-To: <CAFP8O3LCK6aBzkTmhpE54pE2zWarj3V5CHL7W8+Hpqx3fq2f9Q@mail.gmail.com>

On Thu, Aug 5, 2021 at 9:43 AM Fāng-ruì Sòng <maskray@google.com> wrote:
>
> On Thu, Aug 5, 2021 at 9:34 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Thu, Aug 5, 2021 at 9:29 AM Fangrui Song via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> > >
> > > When using LLD (LLVM linker) as the linker, configure prints a confusing
> > > message.
> > >
> > >     *** These critical programs are missing or too old: GNU ld
> > >
> > > LLD>=13.0.0 can build glibc --enable-static-pie. (8.0.0 needs one
> > > workaround for -Wl,-defsym=_begin=0. 9.0.0 works with
> > > --disable-static-pie).
> > >
> > > With BZ #28153 (glibc bug exposed by testing with LLD) fixed,
> > > `make check` only has 2 more failures with LLD than with GNU ld:
> > > BZ #28154 (LLD follows the PowerPC port of GNU ld for ifunc by
> > > placing IRELATIVE relocations in .rela.dyn).
> >
> > This is an lld bug, similar to:
> >
> > https://sourceware.org/bugzilla/show_bug.cgi?id=13302
> >
> > Please fix it at least on x86.
>
> I am afraid that I disagree. The PowerPC port of GNU ld does the right

This is just one opinion and it doesn't make it "the right thing".

> thing and LLD follows suit.
> R_*_IRELATIVE relocations should be eagerly resolved, so they belong
> to .rela.dyn instead of .rela.plt.

Ulrich and I designed/implemented IFUNC on x86 and for x86.  I consider
x86 implementation of IFUC as the gold standard.

> The objection argument
> https://sourceware.org/bugzilla/show_bug.cgi?id=28154#c3 is kinda
> weak:
>
> > This requires both linker and ld.so changes.  The PR ld/13302 linker change doesn't require ld.so change.
>
> Working around the glibc ld.so issue on GNU ld side was probably
> simpler, but that does not justify the GNU ld change.
>
> I mentioned at #c2 about the proper way on the glibc side.
>
> But our conflict here may be moot because in practice ifunc should not
> have such PLT calls, they would be unreliable on other architectures
> anyway.
>
> > > The set of dynamic symbols is the same with GNU ld and LLD,
> > > modulo unused SHN_ABS version node symbols.
> > >
> > > For comparison, gold does not support --enable-static-pie
> > > yet (--no-dynamic-linker is unsupported BZ #22221), yet
> > > has 6 failures more than LLD. gold linked libc.so has
> > > larger .dynsym differences with GNU ld and LLD
> > > (ISTM non-default version symbols are changed to default versions
> > > by a version script).
> > >
> > > ---
> > >
> > > I identified the lack of support of
> > >
> > > * version script on non-default version symbols
> > > * copy relocations on non-default version symbols
> > >
> > > in an earlier snapshot of LLD 13.0.0 and fixed them.
> > > The functionality of the LLD linked libc.so and ld.so looks pretty good.
> > > ---
> > >  configure    | 77 +++++++++++++++++++++++++++++++++++++++++++++++++---
> > >  configure.ac | 20 ++++++++++----
> > >  2 files changed, 88 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/configure b/configure
> > > index 9b966196d4..050f1a29cf 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -4664,9 +4664,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
> > > @@ -4729,8 +4730,75 @@ if test $ac_verc_fail = yes; then
> > >    LD=: critic_missing="$critic_missing GNU gold"
> > >  fi
> > >
> > > +    ;;
> > > +  "LLD"*)
> > > +  # Accept LLD 13.0.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;;
> > > +    1[3-9].*|[2-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
> > > @@ -4793,7 +4861,8 @@ if test $ac_verc_fail = yes; then
> > >    LD=: critic_missing="$critic_missing GNU ld"
> > >  fi
> > >
> > > -fi
> > > +    ;;
> > > +esac
> > >
> > >  # These programs are version sensitive.
> > >  for ac_prog in gnumake gmake make
> > > diff --git a/configure.ac b/configure.ac
> > > index 17a4c9a1ab..5632277f9c 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -995,18 +995,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 13.0.0 or higher
> > > +    AC_CHECK_PROG_VER(LD, $LD, --version,
> > > +                   [LLD.* \([0-9][0-9]*\.[0-9.]*\)],
> > > +                   [1[3-9].*|[2-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_PROG_VER(MAKE, gnumake gmake make, --version,
> > > --
> > > 2.32.0.605.g8dce9f2422-goog



--
H.J.

  reply	other threads:[~2021-08-05 17:05 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-05 16:25 [PATCH v2 0/3] Allow LLD 13.0.0 and improve compatibility with gold and clang Fangrui Song via Libc-alpha
2021-08-05 16:25 ` [PATCH v2 1/3] elf: Replace .tls_common with .tbss definition [BZ #28152] Fangrui Song via Libc-alpha
2021-08-05 16:26 ` [PATCH v2 2/3] elf: Skip tst-auditlogmod-* if the linker doesn't support --depaudit [BZ #28151] Fangrui Song via Libc-alpha
2021-08-16  4:59   ` Fāng-ruì Sòng via Libc-alpha
2021-08-24  3:56     ` Fāng-ruì Sòng via Libc-alpha
2021-08-24 12:04   ` Adhemerval Zanella via Libc-alpha
2021-08-05 16:26 ` [PATCH v2 3/3] configure: Allow LD to be LLD 13.0.0 or above [BZ #26558] Fangrui Song via Libc-alpha
2021-08-05 16:34   ` H.J. Lu via Libc-alpha
2021-08-05 16:43     ` Fāng-ruì Sòng via Libc-alpha
2021-08-05 17:04       ` H.J. Lu via Libc-alpha [this message]
2021-08-07  0:47         ` Fāng-ruì Sòng via Libc-alpha
2021-08-07 13:15           ` H.J. Lu via Libc-alpha
2021-08-08  4:17             ` Fāng-ruì Sòng via Libc-alpha
2021-08-09 17:58               ` H.J. Lu via Libc-alpha
2021-08-09 19:58                 ` Fāng-ruì Sòng via Libc-alpha
2021-08-10 14:38                   ` H.J. Lu via Libc-alpha
2021-08-10 17:42                     ` Fāng-ruì Sòng via Libc-alpha
2021-08-10 22:19   ` Fangrui Song via Libc-alpha
2021-08-23  3:18     ` Fāng-ruì Sòng via Libc-alpha
2021-08-24 17:05       ` Fāng-ruì Sòng via Libc-alpha
2021-08-30 19:52         ` Fāng-ruì Sòng via Libc-alpha
2021-08-30 20:01           ` Adhemerval Zanella via Libc-alpha
2021-08-31 21:24             ` Fāng-ruì Sòng via Libc-alpha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/libc/involved.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMe9rOqxnzDoCgbkCGjrBEPkjNQw7y=LcoZ94F4mjwUMCXY5tw@mail.gmail.com' \
    --to=libc-alpha@sourceware.org \
    --cc=hjl.tools@gmail.com \
    --cc=maskray@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).