Only amd64 and i386 are handled at the moment, and only amd64 tested.
Details
Details
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 20173
Event Timeline
| lib/csu/amd64/reloc.c | ||
|---|---|---|
| 4 | This is what mandated by the contract I have signed with FF. | |
| lib/csu/i386/reloc.c | ||
| 60 | The code is taken literally from the Intel App Note 485 CPU Identification. Code uses XOR, so any state of the EFLAGS.ID bit is fine for it. I do not believe that we need to care about anything else. The same code is present in x86/__vdso_tc.c and in i386/gen/getcontextx.c. | |
| lib/csu/amd64/reloc.c | ||
|---|---|---|
| 4 | Eventually the Foundation contracts will be updated or amended, but the All rights reserved. text can indeed be removed from Foundation copyright templates. | |
Comment Actions
Only amd64 and i386 are handled at the moment
The intent is to set CRT_IRELOC_SUPRESS for other architectures after review, for the committable version?
Comment Actions
Exactly, I wanted each architecture to provide the explicit choice instead of doing nothing by default.
Comment Actions
Do relocs before call to tls initialization.
Block rtld from using ifunc from libc for memset/bzero.
Comment Actions
buildworld fails for me with:
===> libexec/rtld-elf (all)
/usr/bin/ld: error: duplicate symbol: bzero
>>> defined at rtld.c:5599 (/usr/home/mjg/repos/freebsd/libexec/rtld-elf/rtld.c:5599)
>>> rtld.o:(bzero)
>>> defined at ifunc.c:17 (/usr/home/mjg/repos/freebsd/lib/libc/amd64/ifunc.c:17)
>>> ifunc.nossppico:(bzero_resolver) in archive /usr/obj/usr/home/mjg/repos/freebsd/amd64.amd64/lib/libc/libc_nossp_pic.a
/usr/bin/ld: error: duplicate symbol: memset
>>> defined at rtld.c:5589 (/usr/home/mjg/repos/freebsd/libexec/rtld-elf/rtld.c:5589)
>>> rtld.o:(memset)
>>> defined at ifunc.c:8 (/usr/home/mjg/repos/freebsd/lib/libc/amd64/ifunc.c:8)
>>> ifunc.nossppico:(memset_resolver) in archive /usr/obj/usr/home/mjg/repos/freebsd/amd64.amd64/lib/libc/libc_nossp_pic.a
cc: error: linker command failed with exit code 1 (use -v to see invocation)
*** [ld-elf.so.1.full] Error code 1Note the build host runs without any of the patches.
Here is the change I was testing with, it creates all expected symbols (memmove, memcpy, bcopy, bzero, memset):
https://people.freebsd.org/~mjg/libc-ifunc.diff
Comment Actions
Right, this is due to your change defining resolvers for all functions in one .o file. You need to
- add remaining overrides to rtld.c
- split ifunc,c into per-function ifunc.
And, until ld.lld and strip bugs are not fixed, static binaries will be broken. Workaround is to not strip them on install.
Comment Actions
Ok, I confirm it works fine for dynamic and static binaries (not stripped) for the aforementioned routines on amd64. Will test i386 some time later.
| lib/csu/amd64/reloc.c | ||
|---|---|---|
| 61 | How can this work with a debugger that wishes to find the resolved address of an ifunc? I peeked at glibc and they seem to avoid passing arguments to the resolver somehow. | |
| lib/csu/amd64/reloc.c | ||
|---|---|---|
| 61 | Why would debugger require that ? What matter is the address of the function which is written into the call instruction, not the return value from the ifunc resolver. If really wanted, debugger can reconstruct the args. | |
Comment Actions
As discussed elsewhere there are a few toolchain issues with static binaries with ifuncs - one fix is in https://github.com/emaste/freebsd/commit/36131f9c4c9aa343ea2679c18b3d1d621bb837f1
| lib/csu/common/ignore_init.c | ||
|---|---|---|
| 54 | I'm slightly surprised to find this in ignore_init.c | |
| lib/csu/common/ignore_init.c | ||
|---|---|---|
| 54 | I may rename it after this change lands. | |
| lib/csu/common/ignore_init.c | ||
|---|---|---|
| 54 | Should we perhaps combine crtbrand.c and ignore_init.c into one? | |
| lib/csu/common/ignore_init.c | ||
|---|---|---|
| 54 | We can. I do not have a strong opinion. I slightly prefer a split. | |
| lib/csu/common/ignore_init.c | ||
|---|---|---|
| 54 | Just seems strange to me to combine crtbrand and my upcoming feature flags into one, and ignore_init and ifuncs into another. But either way I agree this can be a change after this patch. | |