diff mbox series

LoongArch: Restore pointer_equality_needed for GOT data relocations

Message ID 20260730093443.18174-1-yangyj.ee@gmail.com
State New
Headers
Series LoongArch: Restore pointer_equality_needed for GOT data relocations |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Yanjun Yang July 30, 2026, 9:34 a.m. UTC 
  From: Pluto Yang <yangyj.ee@gmail.com>

Commit 7f9d6d6ead5 ("LoongArch: Fix Build pr29655") removed the
setting of pointer_equality_needed for GOT relocations
(R_LARCH_GOT_PC_HI20, R_LARCH_GOT_PCADD_HI20, R_LARCH_GOT_HI20,
R_LARCH_SOP_PUSH_GPREL).  This was correct for function symbols to
fix PR 29655 (function pointer equality in PIE executables), but it
had an unintended side effect: the CRT startup code uses
R_LARCH_GOT_PC_HI20 to access data symbols like __environ,
__rela_iplt_start, __rela_iplt_end, and _dl_ns.  Without
pointer_equality_needed, the GOT layout for these data symbols is
altered, corrupting the static binary and causing runtime segfaults.

Restore pointer_equality_needed for GOT relocations, but only for
non-function symbols (h->type != STT_FUNC).  This preserves the
PR 29655 fix for function symbols while fixing static executables
that use data symbols accessed via GOT relocations.

This bug was discovered when building GCC for loongarch64: the
libstdc++ configure script tests TLS support by running a statically
linked program.  The corrupted GOT caused the test to crash, leading
to gcc_cv_have_tls=no, which disabled TLS support in libstdc++ and
broke ABI compatibility with system libraries.

Signed-off-by: Pluto Yang <yangyj.ee@gmail.com>
---
 bfd/elfnn-loongarch.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Xi Ruoyao Aug. 1, 2026, 11:26 p.m. UTC | #1 
On Thu, 2026-07-30 at 17:34 +0800, Yanjun Yang wrote:
> From: Pluto Yang <yangyj.ee@gmail.com>
> 
> Commit 7f9d6d6ead5 ("LoongArch: Fix Build pr29655") removed the
> setting of pointer_equality_needed for GOT relocations
> (R_LARCH_GOT_PC_HI20, R_LARCH_GOT_PCADD_HI20, R_LARCH_GOT_HI20,
> R_LARCH_SOP_PUSH_GPREL).  This was correct for function symbols to
> fix PR 29655 (function pointer equality in PIE executables), but it
> had an unintended side effect: the CRT startup code uses
> R_LARCH_GOT_PC_HI20 to access data symbols like __environ,
> __rela_iplt_start, __rela_iplt_end, and _dl_ns.  Without
> pointer_equality_needed, the GOT layout for these data symbols is
> altered, corrupting the static binary and causing runtime segfaults.

This does not make sense to me.  Global data symbols are accessed via
R_LARCH_GOT_PC_HI20 everywhere, not only the CRT startup code.  Thus if
accessing data symbols with R_LARCH_GOT_PC_HI20 is broken we'd have a
much more wide-spreading breakage.

> Restore pointer_equality_needed for GOT relocations, but only for
> non-function symbols (h->type != STT_FUNC).  This preserves the
> PR 29655 fix for function symbols while fixing static executables
> that use data symbols accessed via GOT relocations.

This again does not make sense to me.  Pointer equality matters because
if you end up comparing the address of the PLT stub and the address of
the real function you'd break pointer equality.  You cannot use a PLT
stub for data symbols so pointer equality has nothing to do with data
symbols.

> This bug was discovered when building GCC for loongarch64: the
> libstdc++ configure script tests TLS support by running a statically
> linked program.  The corrupted GOT caused the test to crash, leading
> to gcc_cv_have_tls=no, which disabled TLS support in libstdc++ and
> broke ABI compatibility with system libraries.

I cannot reproduce the issue by building GCC: GCC builds fine with
gcc_cv_have_tls=yes in config.log files.  But maybe this test case:

int f1() { return 42; }
int f2() { return 47; }

void *fx() { return f1; }

[[gnu::ifunc("fx")]] int f();

int main() { int (*p)() = f; asm("":"+r"(p)); return p();}

This thing indeed crashes with ld-2.47 and -static: debugger shows the
value of p is now __strrchr_lasx in glibc (!!!) which will definitely
not work at the place of f1().

It "works" with ld-2.46 but the value of p is the address of the PLT
entry, not the address of f1.  On x86 it behaves the same way but on
aarch64 we get the address of f1.  I don't know which way is better yet.

And I'm also unsure if the malfunction demonstrated with the test case
above is the same "GOT corruption" in your case.

> Signed-off-by: Pluto Yang <yangyj.ee@gmail.com>
> ---
>  bfd/elfnn-loongarch.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index 4bdc3f26285..1c643c587f2 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -1249,6 +1249,11 @@ loongarch_elf_check_relocs (bfd *abfd, struct
> bfd_link_info *info,
>  	case R_LARCH_GOT_PCADD_HI20:
>  	case R_LARCH_GOT_HI20:
>  	case R_LARCH_SOP_PUSH_GPREL:
> +	  /* For la.global: set pointer_equality_needed for data
> symbols

This line especially makes no sense...

> +	     to ensure correct GOT layout.  Function symbols are
> excluded
> +	     to avoid breaking function pointer equality (PR 29655). 
> */
> +	  if (h && h->type != STT_FUNC)

So, maybe just h->type == STT_GNU_IFUNC?

> +	    h->pointer_equality_needed = 1;
>  	  if (!loongarch_elf_record_tls_and_got_reference (abfd,
> info, h,
>  							   r_symndx,
>  							  
> GOT_NORMAL,
Yanjun Yang Aug. 3, 2026, 7:35 a.m. UTC | #2 
On Sun, Aug 02, 2026 at 07:26:13AM +0800, Xi Ruoyao wrote:
> On Thu, 2026-07-30 at 17:34 +0800, Yanjun Yang wrote:
> > From: Pluto Yang <yangyj.ee@gmail.com>
> > 
> > Commit 7f9d6d6ead5 ("LoongArch: Fix Build pr29655") removed the
> > setting of pointer_equality_needed for GOT relocations
> > (R_LARCH_GOT_PC_HI20, R_LARCH_GOT_PCADD_HI20, R_LARCH_GOT_HI20,
> > R_LARCH_SOP_PUSH_GPREL).  This was correct for function symbols to
> > fix PR 29655 (function pointer equality in PIE executables), but it
> > had an unintended side effect: the CRT startup code uses
> > R_LARCH_GOT_PC_HI20 to access data symbols like __environ,
> > __rela_iplt_start, __rela_iplt_end, and _dl_ns.  Without
> > pointer_equality_needed, the GOT layout for these data symbols is
> > altered, corrupting the static binary and causing runtime segfaults.
> 
> This does not make sense to me.  Global data symbols are accessed via
> R_LARCH_GOT_PC_HI20 everywhere, not only the CRT startup code.  Thus if
> accessing data symbols with R_LARCH_GOT_PC_HI20 is broken we'd have a
> much more wide-spreading breakage.
> 
> > Restore pointer_equality_needed for GOT relocations, but only for
> > non-function symbols (h->type != STT_FUNC).  This preserves the
> > PR 29655 fix for function symbols while fixing static executables
> > that use data symbols accessed via GOT relocations.
> 
> This again does not make sense to me.  Pointer equality matters because
> if you end up comparing the address of the PLT stub and the address of
> the real function you'd break pointer equality.  You cannot use a PLT
> stub for data symbols so pointer equality has nothing to do with data
> symbols.
> 

You are right, and thank you for the analysis.

I bisected the regression and found commit 7f9d6d6ead5 ("LoongArch:
Fix Build pr29655") was the first bad commit, which removed the
pointer_equality_needed = 1 for GOT relocations.  Based on that,
I incorrectly assumed the removal itself was the problem.

> > This bug was discovered when building GCC for loongarch64: the
> > libstdc++ configure script tests TLS support by running a statically
> > linked program.  The corrupted GOT caused the test to crash, leading
> > to gcc_cv_have_tls=no, which disabled TLS support in libstdc++ and
> > broke ABI compatibility with system libraries.
> 
> I cannot reproduce the issue by building GCC: GCC builds fine with
> gcc_cv_have_tls=yes in config.log files.  But maybe this test case:
> 

This is likely a difference in the build environment.  I hit this when
packaging GCC 16.1.1 for Arch Linux on loongarch64.  The chroot has
binutils 2.47-1, where the libstdc++ configure TLS test runs a
statically linked program that crashed due to the .got.plt bug,
resulting in gcc_cv_have_tls=no.

> int f1() { return 42; }
> int f2() { return 47; }
> 
> void *fx() { return f1; }
> 
> [[gnu::ifunc("fx")]] int f();
> 
> int main() { int (*p)() = f; asm("":"+r"(p)); return p();}
> 
> This thing indeed crashes with ld-2.47 and -static: debugger shows the
> value of p is now __strrchr_lasx in glibc (!!!) which will definitely
> not work at the place of f1().
> 
> It "works" with ld-2.46 but the value of p is the address of the PLT
> entry, not the address of f1.  On x86 it behaves the same way but on
> aarch64 we get the address of f1.  I don't know which way is better yet.
> 
> And I'm also unsure if the malfunction demonstrated with the test case
> above is the same "GOT corruption" in your case.

Yes, it is the same bug, and your ifunc_got_off patch (from: [PATCH]
LoongArch: fix .got.plt dislocation in static PDE) is the proper fix.
I have applied it on top of my tree and verified GCC libstdc++ configure
TLS test now passes.

Best regards,
Yanjun Yang

> 
> > Signed-off-by: Pluto Yang <yangyj.ee@gmail.com>
> > ---
> >  bfd/elfnn-loongarch.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> > index 4bdc3f26285..1c643c587f2 100644
> > --- a/bfd/elfnn-loongarch.c
> > +++ b/bfd/elfnn-loongarch.c
> > @@ -1249,6 +1249,11 @@ loongarch_elf_check_relocs (bfd *abfd, struct
> > bfd_link_info *info,
> >  	case R_LARCH_GOT_PCADD_HI20:
> >  	case R_LARCH_GOT_HI20:
> >  	case R_LARCH_SOP_PUSH_GPREL:
> > +	  /* For la.global: set pointer_equality_needed for data
> > symbols
> 
> This line especially makes no sense...
> 
> > +	     to ensure correct GOT layout.  Function symbols are
> > excluded
> > +	     to avoid breaking function pointer equality (PR 29655). 
> > */
> > +	  if (h && h->type != STT_FUNC)
> 
> So, maybe just h->type == STT_GNU_IFUNC?
> 
> > +	    h->pointer_equality_needed = 1;
> >  	  if (!loongarch_elf_record_tls_and_got_reference (abfd,
> > info, h,
> >  							   r_symndx,
> >  							  
> > GOT_NORMAL,
> 
> -- 
> Xi Ruoyao <xry111@xry111.site>
diff mbox series

Patch

diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index 4bdc3f26285..1c643c587f2 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -1249,6 +1249,11 @@  loongarch_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
 	case R_LARCH_GOT_PCADD_HI20:
 	case R_LARCH_GOT_HI20:
 	case R_LARCH_SOP_PUSH_GPREL:
+	  /* For la.global: set pointer_equality_needed for data symbols
+	     to ensure correct GOT layout.  Function symbols are excluded
+	     to avoid breaking function pointer equality (PR 29655).  */
+	  if (h && h->type != STT_FUNC)
+	    h->pointer_equality_needed = 1;
 	  if (!loongarch_elf_record_tls_and_got_reference (abfd, info, h,
 							   r_symndx,
 							   GOT_NORMAL,