Page MenuHomeFreeBSD

[PowerPC] powerpc64 rtld IFUNC handling code
ClosedPublic

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

Details

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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.

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
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.

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

libexec/rtld-elf/powerpc64/reloc.c
200 ↗(On Diff #65571)

Actually, I was accidentally doing all my tests today with base/binutils installed.

So I do still hit them.

This revision was automatically updated to reflect the committed changes.