Skip to content
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

Fix issue #66: ignore 0th section header when sorting, don't overwrite NOBITS #149

Merged
merged 1 commit into from Mar 6, 2019

Conversation

ezquat
Copy link
Contributor

@ezquat ezquat commented Jun 21, 2018

To recount the problem symptoms: Given an executable with certain properties, including a section at offset 0, patchelf fails to set the interpreter. (With patchelf 0.8 you get an assertion failure, (off_t) rdi(hdr->e_shoff) >= startOffset'. With patchelf 0.9 you get cannot find section, which is referring to the section with the empty name, which is some kind of dummy placeholder in ELF files, at section index 0.)

Here's my understanding of the bug: Most of the code in patchelf ignores the 0th entry in the section header table, but there is a call to C++ sort() which can move the entry at index 0, in the case where some other entry also has offset 0 (which is the case for Go binaries, a .tbss section has offset 0). It is easy to skip entry 0 in the sort. Then another problem kicks in, which is that patchelf tries to overwrite some bytes (with 'X's) at that offset 0, even though the .tbss section is marked as NOBITS and thus has no content.

@colemickens
Copy link
Member

Can this be merged? I can't patch hardly any recently-built static go binaries without this.

On the other hand, I'm having complete success with this PR's branch of patchelf.

Thanks!

@gilligan
Copy link

same here, i could really use this!

@amckague
Copy link

amckague commented Jul 27, 2018

Worked for me checking out the branch and building locally in at least one case.

> file go
go: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter 
/lib64/ld-linux-x86-64.so.2, with debug_info, not stripped
> /usr/local/bin/patchelf go --set-interpreter /nix/store/j2yzfcd4wli8lj5i6yvyg6h1sbvjr55i-glibc-2.27/lib/ld-2.27.so
> file go
go: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter
/nix/store/j2yzfcd4wli8lj5i6yvyg6h1sbvjr55i-glibc-2.27/lib/ld-2.27.so, with debug_info, not stripped

@domenkozar domenkozar requested a review from edolstra July 27, 2018 15:31
@kyrofa
Copy link

kyrofa commented Aug 8, 2018

Thank you @ezquat, this works for my test cases as well! Can we please get this merged?

qubitrenegade added a commit to qubitrenegade/habitat-tidb-tikv-server that referenced this pull request Aug 22, 2018
@garrett-hopper
Copy link

garrett-hopper commented Aug 27, 2018

+1 I'm unable to build the binaries for the pulumi CLI which is written in Go.

@kyrofa
Copy link

kyrofa commented Aug 27, 2018

@edolstra, any thoughts on this?

@karlyeurl
Copy link

This patch worked for me as well.

Any chance on getting this merged eventually?

@sanmai-NL
Copy link

@puffnfresh: could you explain why this hasn't been merged yet?

@theKAKAN
Copy link

theKAKAN commented Mar 6, 2019

@puffnfresh: could you explain why this hasn't been merged yet?

He has approved the changes, it's @edolstra we are waiting for.

@edolstra edolstra merged commit 6066239 into NixOS:master Mar 6, 2019
@sjackman
Copy link

sjackman commented Mar 6, 2019

@edolstra @puffnfresh patchelf 0.9 was released three years ago. master includes a few helpful bug fixes. Are there any plans for a stable release?

@offlinehacker
Copy link

Released in 0.10 for anyone wondering

@karlyeurl
Copy link

Great! Thanks for the heads-up.

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.

None yet