[PATCH] ld/elf: Don't take IFUNC address in the shared object

[Fangrui Song]

On 2021-01-16, H.J. Lu via Binutils wrote:
>On x86, glibc 2.33 starts to issue a fatal error message when calling
>IFUNC function defined in the unrelocated executable from a shared
>library.
>
>1. Update x86 ELF linker to always convert IFUNC function defined in
>position-dependent executable (PDE) to the normal function.  GOT in PDE
>will be updated by R_*_IRELATIVE at run-time.
>2. Update PR ld/23169 tests not to compare function address of external
>IFUNC function in the shared object to avoid calling the IFUNC function
>defined in the unrelocated executable.

I think the description is less clear.
If the declaration (defined as ifunc in another translation unit) is always
referenced via GOT-generating or PLT-generating relocation types,
there is no need converting it to STT_FUNC.

I guess the original instructions in
https://sourceware.org/bugzilla/show_bug.cgi?id=23169#c0 missed -fno-pic
because many x86-64 Linux distributions default to -fpie now.

-fpie uses R_X86_64_REX_GOTPCRELX to take a function pointer. It likely
worked before PR23169.
-fno-pic uses R_X86_64_32S which needed a fix in PR23169.

It is unclear whether something regressed the PR23169 fix or the PR23169 fix was incomplete.



>bfd/
>
>	PR ld/23169
>	* elfxx-x86.c (_bfd_x86_elf_link_fixup_ifunc_symbol): Don't
>	check pointer_equality_needed.
>
>ld/
>
>	PR ld/23169
>	* testsuite/ld-ifunc/ifunc.exp: Replace pr23169c.rd with
>	pr23169a.rd for pr23169c and pr23169f.  Remove pr23169e tests.
>	* testsuite/ld-ifunc/pr23169a.c: Don't take function address.
>---
> bfd/elfxx-x86.c                  |  3 +--
> ld/testsuite/ld-ifunc/ifunc.exp  | 22 ++--------------------
> ld/testsuite/ld-ifunc/pr23169a.c |  2 +-
> 3 files changed, 4 insertions(+), 23 deletions(-)
>
>diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
>index 8cda8d2d97c..93ad38c5eb0 100644
>--- a/bfd/elfxx-x86.c
>+++ b/bfd/elfxx-x86.c
>@@ -1711,8 +1711,7 @@ _bfd_x86_elf_link_fixup_ifunc_symbol (struct bfd_link_info *info,
>       && h->def_regular
>       && h->dynindx != -1
>       && h->plt.offset != (bfd_vma) -1
>-      && h->type == STT_GNU_IFUNC
>-      && h->pointer_equality_needed)
>+      && h->type == STT_GNU_IFUNC)
>     {
>       asection *plt_s;
>       bfd_vma plt_offset;
>diff --git a/ld/testsuite/ld-ifunc/ifunc.exp b/ld/testsuite/ld-ifunc/ifunc.exp
>index 31666d0f29c..9d3ace65216 100644
>--- a/ld/testsuite/ld-ifunc/ifunc.exp
>+++ b/ld/testsuite/ld-ifunc/ifunc.exp
>@@ -640,7 +640,7 @@ run_cc_link_tests [list \
> 	"$NOPIE_LDFLAGS -Wl,--no-as-needed tmpdir/libpr23169a.so" \
> 	"-fPIE -O2 -g" \
> 	{ pr23169b.c pr23169c.c } \
>-	{{readelf {--dyn-syms} pr23169c.rd} \
>+	{{readelf {--dyn-syms} pr23169a.rd} \
> 	 {readelf {-r -W} pr23169b.rd}} \
> 	"pr23169c" \
>     ] \
>@@ -653,21 +653,12 @@ run_cc_link_tests [list \
> 	 {readelf {-r -W} pr23169b.rd}} \
> 	"pr23169d" \
>     ] \
>-    [list \
>-	"Build pr23169e" \
>-	"-pie -Wl,--no-as-needed,-z,now tmpdir/libpr23169b.so" \
>-	"-fPIE -O2 -g" \
>-	{ pr23169b.c pr23169c.c } \
>-	{{readelf {--dyn-syms} pr23169c.rd} \
>-	 {readelf {-r -W} pr23169b.rd}} \
>-	"pr23169e" \
>-    ] \
>     [list \
> 	"Build pr23169f" \
> 	"$NOPIE_LDFLAGS -Wl,--no-as-needed,-z,now tmpdir/libpr23169b.so" \
> 	"-fPIE -O2 -g" \
> 	{ pr23169b.c pr23169c.c } \
>-	{{readelf {--dyn-syms} pr23169c.rd} \
>+	{{readelf {--dyn-syms} pr23169a.rd} \
> 	 {readelf {-r -W} pr23169b.rd}} \
> 	"pr23169f" \
>     ] \
>@@ -774,15 +765,6 @@ run_ld_link_exec_tests [list \
> 	"pass.out" \
> 	"$NOPIE_CFLAGS -O2 -g" \
>     ] \
>-    [list \
>-	"Run pr23169e" \
>-	"-pie -Wl,--no-as-needed,-z,now tmpdir/libpr23169b.so" \
>-	"" \
>-	{ pr23169b.c pr23169c.c } \
>-	"pr23169e" \
>-	"pass.out" \
>-	"-fPIE -O2 -g" \
>-    ] \
>     [list \
> 	"Run pr23169f" \
> 	"$NOPIE_LDFLAGS -Wl,--no-as-needed,-z,now tmpdir/libpr23169b.so" \
>diff --git a/ld/testsuite/ld-ifunc/pr23169a.c b/ld/testsuite/ld-ifunc/pr23169a.c
>index 02bf2208904..8beed283bc8 100644
>--- a/ld/testsuite/ld-ifunc/pr23169a.c
>+++ b/ld/testsuite/ld-ifunc/pr23169a.c
>@@ -4,6 +4,6 @@ extern int func (void);
> void
> foo (void)
> {
>-  if (func_p != &func || func_p () != 0xbadbeef)
>+  if (func () != 0xbadbeef || func_p () != 0xbadbeef)
>     __builtin_abort ();
> }

There is lost test coverage. func_p != &func  is untested now.