Skip to content

Fix assertion failure when setting the interpreter of a debug-only executable #595

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

corngood
Copy link
Contributor

The included test currently fails only on binutils 2.44, which is available on nixpkgs-unstable. I've only tested on x86_64-linux.

Perhaps lastReplaced should be renamed because it no longer ends up pointing to the actual last replaced section.

This avoids an assertion failure when 'lastReplaced' is followed by one
or mo NOBITS sections, since they won't have a valid startOffset.

Fixes: NixOS#373
@corngood corngood marked this pull request as draft April 21, 2025 00:06
@corngood
Copy link
Contributor Author

Should we also bail out of setInterpreter if .interp is NOBITS? If so, should it be an error?

Unfortunately auto-patchelf calls --set-interpreter on any ELF where .interp exists, even if it's NOBITS, so if we make it an error, changes will still be required there.

@corngood
Copy link
Contributor Author

The test I added showed another problem when run with clang. It looks like clang outputs ET_DYN, which means it doesn't even hit the other fix in rewriteSectionsExecutable.

The problem with clang (with both old and new binutils) is that it's accessing .dynamic even if it's NOBITS. I've pushed a fix for that in ad3d3d4.

@corngood corngood marked this pull request as ready for review April 21, 2025 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant