[ELF][PPC] Don't relax ifunc toc-indirect accesses to toc-relative

[MaskRay]

Summary

Fixes PR42759.

// If ifunc is taken address in -fPIC code, it may have a toc entry
.section .toc,"aw",@progbits
  .quad ifunc

// ifunc may be defined as STT_GNU_IFUNC in another object file
.type ifunc, %gnu_indirect_function

If ifunc is non-preemptable (e.g. when linking an executable), the toc
entry will be relocated by R_PPC64_IRELATIVE.

R_*_IRELATIVE represents the symbolic value of a
non-preemptable ifunc (not associated with a canonical PLT) in a writable location. It has an unknown value at
link time, so we cannot apply toc-indirect to toc-relative relaxation.

[luporl]

Nice, I've confirmed that it fixes PR42759 on FreeBSD.

[sfertile]

ELF/Arch/PPC64.cpp

175 ↗

(On Diff #213383)

minor nit: I think it would help readability if the new comment and check were added after the existing preemptible check.

176 ↗

(On Diff #213383)

It might be better to spell out why the address is not representable as a toc-relative value. IIUC: The symbol being referenced is a resolver function that the dynamic loader will run to determine what function should be used. Relaxing to a toc-relative reference will manifest the address of the resolver function as opposed to the address of the resolved function (which is not know until load time).

test/ELF/ppc64-toc-relax-ifunc.s

6–7 ↗

(On Diff #213383)

FileCheck run-step is missing.

[MaskRay]

Add a missed FileCheck

Improve a comment

[MaskRay]

ELF/Arch/PPC64.cpp

175 ↗

(On Diff #213383)

I think it is probably fine to have them on one line.. The first sentence makes it clear that preemptable and non-Defined symbols are excluded.

Only non-preemptable defined symbols can be relaxed.

The ifunc comment discusses the possible IRELATIVE case.

176 ↗

(On Diff #213383)

We have more detailed comment at Relocations.cpp:1231. I reworded the comment here but I think we probably don't have to delve too much into the details.

test/ELF/ppc64-toc-relax-ifunc.s

6–7 ↗

(On Diff #213383)

Thanks! Fixed.

[sfertile]

LGTM.

ELF/Arch/PPC64.cpp

175 ↗

(On Diff #213383)

Sure. My thoughts were that the Ifunc case is a special case of a defined symbol not being relaxable, splitting the code/comment out separately highlights that. I'm Ok with it the way it is as well.

176 ↗

(On Diff #213383)

Thanks, I agree: your reword covers it well enough for here.

lld/trunk/ELF/Arch/PPC64.cpp

bool elf::tryRelaxPPC64TocIndirection(RelType type, const Relocation &rel,
   Defined *d;
   int64_t addend;
   auto *tocISB = cast<InputSectionBase>(defSym->section);
   std::tie(d, addend) =
       config->isLE ? getRelaTocSymAndAddend<ELF64LE>(tocISB, rel.addend)
                    : getRelaTocSymAndAddend<ELF64BE>(tocISB, rel.addend);
 
   // Only non-preemptable defined symbols can be relaxed.
-  if (!d || d->isPreemptible)
+  //
+  // The toc entry of a non-preemptable ifunc is relocated by R_PPC64_IRELATIVE,
+  // which will run at load time to determine the relocated value. It is not
+  // known until load time, so the access cannot be relaxed.
+  if (!d || d->isPreemptible || d->isGnuIFunc())
     return false;
 
   // Two instructions can materialize a 32-bit signed offset from the toc base.
   uint64_t tocRelative = d->getVA(addend) - getPPC64TocBase();
   if (!isInt<32>(tocRelative))
     return false;
 
   // Add PPC64TocOffset that will be subtracted by relocateOne().

lld/trunk/test/ELF/ppc64-toc-relax-ifunc.s

+# REQUIRES: ppc
+
+# RUN: llvm-mc -filetype=obj -triple=powerpc64le %s -o %t.o
+# RUN: echo '.globl ifunc; .type ifunc, %gnu_indirect_function; ifunc:' | \
+# RUN:   llvm-mc -filetype=obj -triple=powerpc64le - -o %t1.o
+# RUN: ld.lld %t.o %t1.o -o %t
+# RUN: llvm-objdump -d %t | FileCheck %s
+
+## ifunc is a non-preemptable STT_GNU_IFUNC. Its toc entry will be
+## relocated by R_PPC64_IRELATIVE, not representable by a toc-relative value.
+## Check the toc-indirect access is not relaxed.
+
+# CHECK:      nop
+# CHECK-NEXT: ld 3, -32768(2)
+
+addis 3, 2, .toc@toc@ha
+ld 3, .toc@toc@l(3)
+
+.section .toc,"aw",@progbits
+  .quad ifunc