public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Xi Ruoyao <xry111@xry111.site>
To: libc-alpha@sourceware.org
Cc: caiyinyu <caiyinyu@loongson.cn>,
	Joseph Myers <joseph@codesourcery.com>,
	Wang Xuerui <i@xen0n.name>
Subject: Ping: [PATCH] LoongArch: Fix the condition to use PC-relative addressing in start.S
Date: Fri, 07 Oct 2022 17:54:18 +0800	[thread overview]
Message-ID: <7ceff00687e8f768388e877d8d0c732c323238d5.camel@xry111.site> (raw)
In-Reply-To: <20221002142309.900714-1-xry111@xry111.site>

Gentle ping.

I see the conclusion from review meeting is:

"Should just commit these as machine maintainers."

Has caiyinyu been formally promoted as a machine maintainer now?

On Sun, 2022-10-02 at 22:23 +0800, Xi Ruoyao wrote:
> A start.o compiled from start.S with -DPIC and no -DSHARED is used by
> both crt1.o and rcrt1.o.  So the LoongArch static PIE patch
> unintentionally introduced PC-relative addressing for main and
> __libc_start_main into crt1.o.
> 
> While the latest Binutils (trunk, which will be released as 2.40)
> supports the PC-relative relocs against an external function by
> creating
> a PLT entry, the 2.39 release branch doesn't (and won't) support this.
> An error is raised:
> 
>     "PLT stub does not represent and symbol not defined."
> 
> So, we need the following changes:
> 
> 1. Check if ld supports the PC-relative relocs against an external
>    function.  If it's not supported, we deem static PIE unsupported.
> 2. Change start.S.  If static PIE is supported, use PC-relative
>    addressing for main and __libc_start_main and rely on the linker to
>    create PLT entries.  Otherwise, restore the old behavior (using GOT
>    to address these functions).
> 
> An alternative would be adding a new "static-pie-start.S", and some
> custom logic into Makefile to build rcrt1.o with it.  And, restore
> start.S to the state before static PIE change so crt1.o won't contain
> PC-relative relocs against external symbols.  But I can't see any
> benefit of this alternative, so I'd just keep it simple.
> 
> Tested by building glibc with the following configurations:
> 
> 1. Binutils trunk + GCC trunk.  Static PIE enabled.  All tests
>    passed.
> 2. Binutils 2.39 branch + GCC trunk.  Static PIE disabled.  Tests
>    related to ifunc failed (it's a known issue).  All other tests
>    passed.
> 3. Binutils 2.39 branch + GCC 12 branch, cross compilation with
>    build-many-glibcs.py from x86_64-linux-gnu.  Static PIE disabled.
>    Build succeeded.
> ---
>  sysdeps/loongarch/configure    | 29 +++++++++++++++++++++++++----
>  sysdeps/loongarch/configure.ac | 19 +++++++++++++++----
>  sysdeps/loongarch/start.S      | 11 +++++++----
>  3 files changed, 47 insertions(+), 12 deletions(-)
> 
> diff --git a/sysdeps/loongarch/configure b/sysdeps/loongarch/configure
> index 6edd6d08a5..3046915ce2 100644
> --- a/sysdeps/loongarch/configure
> +++ b/sysdeps/loongarch/configure
> @@ -10,7 +10,7 @@ if ${libc_cv_static_pie_on_loongarch+:} false; then
> :
>    $as_echo_n "(cached) " >&6
>  else
>  
> -  cat > conftest.S << EOF
> +  cat > conftest1.S << EOF
>  .global _start
>  .type _start, @function
>  _start:
> @@ -26,14 +26,35 @@ x:
>    /* This should produce an R_LARCH_RELATIVE in the static PIE.  */
>    .dword _start
>  EOF
> +  cat > conftest2.S << EOF
> +.global f
> +.type f, @function
> +f:
> +  /* The linker should be able to handle this and produce a PLT
> entry.  */
> +  la.pcrel \$t0, \$t0, external_func
> +  jirl \$zero, \$t0, 0
> +EOF
> +
>    libc_cv_static_pie_on_loongarch=no
> -  if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -static-pie -
> nostdlib -fPIE -o conftest conftest.S'
> +  if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -static-pie -
> nostdlib -fPIE -o conftest1 conftest1.S'
> +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; }
> >&5
> +  (eval $ac_try) 2>&5
> +  ac_status=$?
> +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> +  test $ac_status = 0; }; } \
> +     && { ac_try='LC_ALL=C $READELF -Wr conftest1 | grep -q
> R_LARCH_RELATIVE'
> +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; }
> >&5
> +  (eval $ac_try) 2>&5
> +  ac_status=$?
> +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> +  test $ac_status = 0; }; } \
> +     && { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -shared -fPIC -
> o conftest2.so conftest2.S'
>    { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; }
> >&5
>    (eval $ac_try) 2>&5
>    ac_status=$?
>    $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
>    test $ac_status = 0; }; } \
> -     && { ac_try='LC_ALL=C $READELF -Wr conftest | grep -q
> R_LARCH_RELATIVE'
> +     && { ac_try='LC_ALL=C $READELF -Wr conftest2.so | grep -q
> 'R_LARCH_JUMP_SLOT.*external_func''
>    { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; }
> >&5
>    (eval $ac_try) 2>&5
>    ac_status=$?
> @@ -42,7 +63,7 @@ EOF
>    then
>      libc_cv_static_pie_on_loongarch=yes
>    fi
> -  rm -rf conftest.*
> +  rm -rf conftest*
>  fi
>  { $as_echo "$as_me:${as_lineno-$LINENO}: result:
> $libc_cv_static_pie_on_loongarch" >&5
>  $as_echo "$libc_cv_static_pie_on_loongarch" >&6; }
> diff --git a/sysdeps/loongarch/configure.ac
> b/sysdeps/loongarch/configure.ac
> index a8a373bea3..06dd408ad9 100644
> --- a/sysdeps/loongarch/configure.ac
> +++ b/sysdeps/loongarch/configure.ac
> @@ -13,7 +13,7 @@ dnl satisify the requirement, but a distro may
> backport static PIE support into
>  dnl earlier GCC or Binutils releases as well.
>  AC_CACHE_CHECK([if the toolchain is sufficient to build static PIE on
> LoongArch],
>  libc_cv_static_pie_on_loongarch, [
> -  cat > conftest.S << EOF
> +  cat > conftest1.S << EOF
>  .global _start
>  .type _start, @function
>  _start:
> @@ -29,13 +29,24 @@ x:
>    /* This should produce an R_LARCH_RELATIVE in the static PIE.  */
>    .dword _start
>  EOF
> +  cat > conftest2.S << EOF
> +.global f
> +.type f, @function
> +f:
> +  /* The linker should be able to handle this and produce a PLT
> entry.  */
> +  la.pcrel \$t0, \$t0, external_func
> +  jirl \$zero, \$t0, 0
> +EOF
> +
>    libc_cv_static_pie_on_loongarch=no
> -  if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -static-pie
> -nostdlib -fPIE -o conftest conftest.S]) \
> -     && AC_TRY_COMMAND([LC_ALL=C $READELF -Wr conftest | grep -q
> R_LARCH_RELATIVE])
> +  if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -static-pie
> -nostdlib -fPIE -o conftest1 conftest1.S]) \
> +     && AC_TRY_COMMAND([LC_ALL=C $READELF -Wr conftest1 | grep -q
> R_LARCH_RELATIVE]) \
> +     && AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -shared -
> fPIC -o conftest2.so conftest2.S]) \
> +     && AC_TRY_COMMAND([LC_ALL=C $READELF -Wr conftest2.so | grep -q
> 'R_LARCH_JUMP_SLOT.*external_func'])
>    then
>      libc_cv_static_pie_on_loongarch=yes
>    fi
> -  rm -rf conftest.*])
> +  rm -rf conftest* ])
>  
>  if test "$libc_cv_static_pie_on_loongarch" = yes; then
>    AC_DEFINE(SUPPORT_STATIC_PIE)
> diff --git a/sysdeps/loongarch/start.S b/sysdeps/loongarch/start.S
> index 05cabd9b96..09e5a3c59c 100644
> --- a/sysdeps/loongarch/start.S
> +++ b/sysdeps/loongarch/start.S
> @@ -60,13 +60,16 @@ ENTRY (ENTRY_POINT)
>         cfi_undefined (1)
>         or              a5, a0, zero /* rtld_fini */
>  
> -#if defined(PIC) && !defined(SHARED)
> +#if ENABLE_STATIC_PIE
>  /* For static PIE, the GOT cannot be used in _start because the GOT
> entries are
> -   offsets instead of real addresses before __libc_start_main.  */
> +   offsets instead of real addresses before __libc_start_main.
> +   __libc_start_main and/or main may be not local, so we rely on the
> linker to
> +   produce PLT entries for them.  GNU ld >= 2.40 supports this.  */
>  # define LA la.pcrel
>  #else
> -/* We must get symbol main through GOT table, since main may not be
> local.
> -   For instance: googletest defines main in dynamic library.  */
> +/* Old GNU ld (< 2.40) cannot handle PC relative address against a
> non-local
> +   function correctly.  We deem these old linkers failing to support
> static PIE
> +   and load the addresses from GOT.  */
>  # define LA la.got
>  #endif
>  

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

      reply	other threads:[~2022-10-07  9:54 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-02 14:23 Xi Ruoyao
2022-10-07  9:54 ` Xi Ruoyao [this message]

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

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

  git send-email \
    --in-reply-to=7ceff00687e8f768388e877d8d0c732c323238d5.camel@xry111.site \
    --to=xry111@xry111.site \
    --cc=caiyinyu@loongson.cn \
    --cc=i@xen0n.name \
    --cc=joseph@codesourcery.com \
    --cc=libc-alpha@sourceware.org \
    /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).