[PATCH] D65995: [ELF] Don't special case symbolic relocations with 0 addend to ifunc in writable locations

Fri Aug 9 08:40:52 PDT 2019

peter.smith added a comment.

If I've understood correctly, this will make the case "address of an ifunc is taken from RW but there are no other relocations that would create a canonical PLT entry" worse as we would always create the canonical PLT entry even if it isn't strictly needed. The trade off is simpler code and a possible size saving from only needing one irelative relocation?

I think that taking the address of Ifunc's is rare in most user code, so even if we make this case worse it isn't likely to make a lot of difference, however it would be good to hear from PCC as he may have a special use case using lots more ifuncs than usual. I think FreeBSD also make  heavy use of ifuncs so they may have some concerns.

Personally I'd like to avoid canonical PLT entries as much as possible as if we can show that the PLT addresses don't leak, the BTI PLT entries can be optimised to remove the BTI as it is only needed if the PLT entry is called indirectly. I recognise that this is only of theoretical interest at the moment though so I'm happy to go with the consensus.

================
Comment at: ELF/Relocations.cpp:1095

 struct IRelativeReloc {
   RelType type;
----------------
I think that this is only used by code that you have deleted. Could be removed as well?

================
Comment at: test/ELF/aarch64-gnu-ifunc-nonpreemptable2.s:1
+# REQUIRES: aarch64
+# RUN: llvm-mc -filetype=obj -triple=aarch64-none-linux-gnu %s -o %t.o
----------------
Assuming I've not made a mistake somewhere, this test is passing with lld prior to this change. Is this intentional?

Possibly the reference from RO is causing the canonical PLT entry even with the previous LLD?

Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65995/new/

https://reviews.llvm.org/D65995