-
-
Notifications
You must be signed in to change notification settings - Fork 490
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
Teach --replace-needed to update .gnu.version_r table #85
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If the ELF binary that we're patching is linked to a DSO that uses symbol versioning, then the DSO's SONAME appears in two different places: once as a DT_NEEDED entry, and once in the .gnu.version_r version requirements section. Previously, patchelf --replace-needed would update DT_NEEDED entry, but fail to update the .gnu.version_r table. This resulted in completely broken binaries -- trying to load them would trigger an assertion failure inside the dynamic loader, as it tries to check the version of a library that was never loaded: Inconsistency detected by ld.so: dl-version.c: 224: _dl_check_map_versions: Assertion `needed != ((void *)0)' failed! This commit teaches --replace-needed to update the .gnu.version_r table. Fixes: NixOSgh-84
No semantic changes, but I noticed some small errors in the DT_NEEDED handling loop while I was adding the .gnu.version_r handling, so might as well fix them while I'm here.
Thanks, merged! |
njsmith
added a commit
to njsmith/patchelf
that referenced
this pull request
Apr 2, 2016
When writing the code to teach --replace-needed to modify the .gnu.version_r section (NixOSgh-85), I misunderstood how the ->vn_next pointers in the Elf_Verneed structs are supposed to be interpreted: I thought they gave an offset from the beginning of the section, but in fact they give an offset relative to the current struct. The resulting bug was very odd: generally, patchelf would complete without signalling an error, but it would only successfully replace filenames that occurred as either the first or second entries in the .gnu.version_r section, while the third or later entries would be left untouched. This commit fixes the interpretation of the ->vn_next pointers, so that now --replace-needed should work correctly even on ELF files with more than two version needed structs. Thanks to @matthew-brett for finding the bug / providing a test case, and to @rmcgibbo for helping me diagnose it.
1 task
bgamari
added a commit
to bgamari/ghc-artefact-nix
that referenced
this pull request
Feb 27, 2019
Debian 9 still uses ncurses 5, which complicates installation. Specifically, ncurses 5 provided by nixpkgs appears not to provide shared object version information, whereas Debian does. While `patchelfUnstable` can [1] update library version requirements, it cannot remove them entirely. Rather than find some terrible workaround, it's much easier to simply use a bindist that expects ncurses 6 with symbol versioning. Fedora 27 provides exactly this. [1] NixOS/patchelf#85
bgamari
added a commit
to bgamari/ghc-artefact-nix
that referenced
this pull request
Feb 27, 2019
Debian 9 still uses ncurses 5, which complicates installation. Specifically, ncurses 5 provided by nixpkgs appears not to provide shared object version information, whereas Debian does. While `patchelfUnstable` can [1] update library version requirements, it cannot remove them entirely. Rather than find some terrible workaround, it's much easier to simply use a bindist that expects ncurses 6 with symbol versioning. Fedora 27 provides exactly this. [1] NixOS/patchelf#85
Sorry for necro bumping but '--remove-needed' have exactly the same issue. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the ELF binary that we're patching is linked to a DSO that uses
symbol versioning, then the DSO's SONAME appears in two different
places: once as a DT_NEEDED entry, and once in the .gnu.version_r
version requirements section. Previously,
patchelf --replace-needed
would update DT_NEEDED entry, but fail to update the .gnu.version_r
table. This resulted in completely broken binaries -- trying to load
them would trigger an assertion failure inside the dynamic loader, as it
tries to check the version of a library that was never loaded:
This PR teaches
--replace-needed
to update the .gnu.version_rtable.
It also contains a few unrelated cleanups to
replaceNeeded
in a separate commit.There currently isn't any test included, because I couldn't find an existing test for
--replace-needed
so I guess that might be okay, plus I don't really understand the automake test framework and I need to go to sleep :-). If you want a test then I can probably figure it out, but would appreciate any hints...Fixes: gh-84