-
Notifications
You must be signed in to change notification settings - Fork 10.6k
Use __ehdr_start on ELF instead of calling dladdr().
#85012
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This PR replaces our use of `__dso_handle` and (indirectly) `dladdr()` when figuring out the base address of loaded Swift images on ELF-based platforms. Instead, we use `__ehdr_start` which refers to the start of the image's ELF header (i.e. the start of the file) and is provided by the compiler/linke for this purpose. This pointer is consumed by `swift_addNewDSOImage()` and, later, by Swift Testing when running tests, but is otherwise unused, so the less work we do on it at runtime, the better.
4d954d8 to
d164e95
Compare
|
@swift-ci test |
|
@3405691582 Can you confirm this compiles on OpenBSD 7.7 before I go merging it? Thanks! |
|
@swift-ci test |
| return __ehdr_start; | ||
| } | ||
|
|
||
| // We've hit an edge case where the linker was invoked such that the ELF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure about this? Can you point to the ELF specification where this would be the case? I believe that __ehdr_start is the edge case - it is a GNU extension.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both GNU binutils and LLVM's linker define it, and thus AFAICT Linux and FreeBSD define it. Android defines it too (see e.g. https://android.googlesource.com/platform/bionic/+/main/linker/linker_main.cpp). OpenBSD is an open question for me (no pun intended) which is why I pinged @3405691582 for assistance.
There appears to be chatter in the Google results about it either: not being defined when a linker script is used; or not being defined if the ELF header isn't in the same loadable section as the program headers. I'm having a fair bit of trouble actually getting concrete info here, which is why I've added this fallback path in the first place.
Will do, but HEAD appears to be broken for me right now. Testing this in isolation seems to work, FWIW, e.g., yields a non-null pointer. |
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test macOS |
This PR replaces our use of
__dso_handleand (indirectly)dladdr()when figuring out the base address of loaded Swift images on ELF-based platforms. Instead, we use__ehdr_startwhich refers to the start of the image's ELF header (i.e. the start of the file) and is provided by the compiler/linke for this purpose.This pointer is consumed by
swift_addNewDSOImage()and, later, by Swift Testing when running tests, but is otherwise unused, so the less work we do on it at runtime, the better.Resolves #84997.