[PowerPC] powerpc64 rtld IFUNC handling code
ClosedPublic
Actions

Authored by bdragon on Dec 12 2019, 11:37 PM.

Details

Reviewers

jhibbits
kib

Commits

rS356055: [PowerPC] powerpc64 rtld IFUNC handling code

Summary

I am finally putting this up for formal review.

This is the rtld powerpc64 ifunc handling code, colloquially known as "RTLD WIP3."

I have only done minor changes to it since creation, but it does have some things I may still need to address. I will point them out in comments.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bdragon created this revision.Dec 12 2019, 11:37 PM

Herald added subscribers: alfredo, luporl, imp. · View Herald TranscriptDec 12 2019, 11:37 PM

bdragon added inline comments.Dec 12 2019, 11:50 PM

libexec/rtld-elf/powerpc64/reloc.c
164 ↗	(On Diff #65571)	A lot of the diff here is reformatting to more closely fit the structure that the other platforms use.
200 ↗	(On Diff #65571)	As of rS343484 it is impossible to reach the rtld_resolve_ifunc call, because nothing is calling us with SYMLOOK_IFUNC set. As far as I can tell, there is no processing for non-plt gnu ifuncs anymore due to that. (Whether you actually WANT non-plt gnu ifunc to ever occur in userspace is an open question. Due to the constraints on it, it's generally only used for libc microoptimizations and kernel code.) This is also applicible to the other platforms that have ifunc support.
412 ↗	(On Diff #65571)	I would like some confirmation on my interpretation of the ABI here.
567 ↗	(On Diff #65571)	It may be cleaner to leave the __unused in instead. Opinions welcome.
594 ↗	(On Diff #65571)	TODO: Check lld9 behavior, it's possible LLD9 no longer does this, and I can toss this.
678 ↗	(On Diff #65571)	These are actually defined already by machine/cpu.h, but we were not actually providing them until now.
libexec/rtld-elf/powerpc64/rtld_machdep.h
60 ↗	(On Diff #65571)	This may or may not need some tweaking depending on feedback on D22787.

About the tests part, I've been using this patch for a couple months now (actually, "RTLD WIP3"), along with ELFv2 and libc optimization patches (D15118, D15368, D15369), and I have observed no issues so far.

luporl added a child revision: D15368: strcpy optimization for PowerPC64.Dec 13 2019, 2:03 PM

luporl added a child revision: D15118: memcpy optimization for powerpc64.

luporl added a child revision: D15369: strncpy optimization for PowerPC64.Dec 13 2019, 2:09 PM

This should work both for v1 and v2 ?

I do not see why would you sit on this patch so long. Even if there are ABI mistakes, they are cheap on Tier-2.

libexec/rtld-elf/powerpc64/reloc.c
200 ↗	(On Diff #65571)	If you can provide a binary with such relocs, I will fix it. On the other hand, I do not want to remove SYMLOOK_IFUNC stage support. I do not see a problem with adding it to new arches.

This revision is now accepted and ready to land.Dec 13 2019, 3:42 PM

bdragon added inline comments.Dec 13 2019, 5:53 PM

libexec/rtld-elf/powerpc64/reloc.c
200 ↗	(On Diff #65571)	It looks like clang/lld9 will refuse to generate them outside of freestanding code nowadays. At least, I can't seem to generate them anymore. All of my ifunc tests other than my old "ifunc2" from last year ( http://drop.rtk0.net/ifunc2 ) seem to be working fine. And I lost the source to "ifunc2" so I can't reproduce anymore. It was probably just miscompiled in the first place. It's really only useful for microoptimizing intra-library calls and ifuncs embedded in the main program anyway, and is probably only really worth it in kernel.

The reason I was sitting on this for so long is that I kept getting distracted with other things and never found the time to revisit this, and I was never quite happy with it.

bdragon added inline comments.Dec 13 2019, 10:54 PM

libexec/rtld-elf/powerpc64/reloc.c
594 ↗	(On Diff #65571)	Looks like we're stuck with this search until LLD10 unless we backport some stuff. I suppose I could set a flag independent of irelative to avoid this in most cases though. References: https://bugs.llvm.org/show_bug.cgi?id=42759 https://reviews.llvm.org/D65651 https://reviews.llvm.org/D65755 https://reviews.llvm.org/D65995