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

Avoid inflating file sizes needlessly and allow binaries to be stripped #117

Merged
merged 1 commit into from
Apr 24, 2017

Conversation

rpurdie
Copy link
Contributor

@rpurdie rpurdie commented Mar 7, 2017

The current approach to changing sections in ET_DYN executables is to move
the INTERP section to the end of the file. +This means changing PT_PHDR to
add an extra PT_LOAD section so that the new section is mmaped into memory
by the elf loader in the kernel. In order to extend PHDR, this means moving
it to the end of the file.

Its documented in BUGS there is a kernel 'bug' which means that if you have holes
in memory between the base load address and the PT_LOAD segment that contains PHDR,
it will pass an incorrect PHDR address to ld.so and fail to load the binary, segfaulting.

To avoid this, the code currently inserts space into the binary to ensure that when
loaded into memory there are no holes between the PT_LOAD sections. This inflates the
binaries by many MBs in some cases. Whilst we could make them sparse, there is a second
issue which is that strip can fail to process these binaries:

$ strip fixincl
Not enough room for program headers, try linking with -N
[.note.ABI-tag]: Bad value

This turns out to be due to libbfd not liking the relocated PHDR section either
(#10).

Instead this patch implements a different approach, leaving PHDR where it is but extending
it in place to allow addition of a new PT_LOAD section. This overwrites sections in the
binary but those get moved to the end of the file in the new PT_LOAD section.

This is based on patches linked from the above github issue, however whilst the idea
was good, the implementation wasn't correct and they've been rewritten here.

Signed-off-by: Richard Purdie richard.purdie@linuxfoundation.org

The current approach to changing sections in ET_DYN executables is to move
the INTERP section to the end of the file. +This means changing PT_PHDR to
add an extra PT_LOAD section so that the new section is mmaped into memory
by the elf loader in the kernel. In order to extend PHDR, this means moving
it to the end of the file.

Its documented in BUGS there is a kernel 'bug' which means that if you have holes
in memory between the base load address and the PT_LOAD segment that contains PHDR,
it will pass an incorrect PHDR address to ld.so and fail to load the binary, segfaulting.

To avoid this, the code currently inserts space into the binary to ensure that when
loaded into memory there are no holes between the PT_LOAD sections. This inflates the
binaries by many MBs in some cases. Whilst we could make them sparse, there is a second
issue which is that strip can fail to process these binaries:

$ strip fixincl
Not enough room for program headers, try linking with -N
[.note.ABI-tag]: Bad value

This turns out to be due to libbfd not liking the relocated PHDR section either
(NixOS#10).

Instead this patch implements a different approach, leaving PHDR where it is but extending
it in place to allow addition of a new PT_LOAD section. This overwrites sections in the
binary but those get moved to the end of the file in the new PT_LOAD section.

This is based on patches linked from the above github issue, however whilst the idea
was good, the implementation wasn't correct and they've been rewritten here.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
@probonopd
Copy link

Fixing stripping would indeed be very valuable. Thanks!

@domenkozar
Copy link
Member

@edolstra

@EvenOldridge
Copy link

I'd love to see this committed as well. Right now the numpy build process uses this and stripping binaries is essential for aws lambda deployment.

@edolstra
Copy link
Member

Thanks, merged!

JonathonReinhart added a commit to JonathonReinhart/staticx that referenced this pull request Jun 18, 2017
Older versions of patchelf (including the latest release, 0.9) can render
binaries unable to be stripped. Provided strip only replaces the input
binary if it is successful, we can safely hide and ignore this error.

See NixOS/patchelf#117
JonathonReinhart added a commit to JonathonReinhart/staticx that referenced this pull request Jul 15, 2017
Older versions of patchelf (including the latest release, 0.9) can render
binaries unable to be stripped. Provided strip only replaces the input
binary if it is successful, we can safely hide and ignore this error.

See NixOS/patchelf#117
JonathonReinhart added a commit to JonathonReinhart/staticx that referenced this pull request Jul 16, 2017
Note that we strip the user program before modifying it with patchelf.
This avoids the issue where older versions of patchelf (including the
latest release, 0.9) can render binaries unable to be stripped.

See NixOS/patchelf#117
@zeerd
Copy link

zeerd commented Aug 23, 2017

was this issue closed?

I met this today in the 0.9 version.

arch:x86_64

added: sorry, i found this fixed after 0.9 release. jusr ignore this.

@rpurdie
Copy link
Contributor Author

rpurdie commented Aug 23, 2017

There was a bug in my change which #127 addresses and may help.

jsirex added a commit to jsirex/core-plans that referenced this pull request Jul 12, 2019
Sometimes patchelf failed to work because of bug:
* NixOS/patchelf#117
* https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=903396

Signed-off-by: Yauhen Artsiukhou <jsirex@gmail.com>
raspbian-autopush pushed a commit to raspbian-packages/pyside2 that referenced this pull request Jan 13, 2020
Update patchelf to current git version

The current version fixes a bug which broke the ability to strip
binaries with debug symbols:
NixOS/patchelf#117

With the former version of patchelf, I would get "not enough room for
program headers, try linking with -N" when trying to strip the binaries.

This patch brings patchelf in sync with the commit
27ffe8ae871e7a186018d66020ef3f6162c12c69 of patchelf's git
repository.

Also update patchelf's build process to match the logic in upstream's
automake files (thanks to Frédéric Bonnard for the patch to properly use
getconf PAGESIZE).

Bug: https://bugreports.qt.io/browse/PYSIDE-740
Last-Update: 2018-07-09


Gbp-Pq: Name update-patchelf.patch
raspbian-autopush pushed a commit to raspbian-packages/pyside2 that referenced this pull request Jan 13, 2020
Update patchelf to current git version

The current version fixes a bug which broke the ability to strip
binaries with debug symbols:
NixOS/patchelf#117

With the former version of patchelf, I would get "not enough room for
program headers, try linking with -N" when trying to strip the binaries.

This patch brings patchelf in sync with the commit
27ffe8ae871e7a186018d66020ef3f6162c12c69 of patchelf's git
repository.

Also update patchelf's build process to match the logic in upstream's
automake files (thanks to Frédéric Bonnard for the patch to properly use
getconf PAGESIZE).

Bug: https://bugreports.qt.io/browse/PYSIDE-740
Last-Update: 2018-07-09


Gbp-Pq: Name update-patchelf.patch
raspbian-autopush pushed a commit to raspbian-packages/pyside2 that referenced this pull request Jan 13, 2020
Update patchelf to current git version

The current version fixes a bug which broke the ability to strip
binaries with debug symbols:
NixOS/patchelf#117

With the former version of patchelf, I would get "not enough room for
program headers, try linking with -N" when trying to strip the binaries.

This patch brings patchelf in sync with the commit
27ffe8ae871e7a186018d66020ef3f6162c12c69 of patchelf's git
repository.

Also update patchelf's build process to match the logic in upstream's
automake files (thanks to Frédéric Bonnard for the patch to properly use
getconf PAGESIZE).

Bug: https://bugreports.qt.io/browse/PYSIDE-740
Last-Update: 2018-07-09


Gbp-Pq: Name update-patchelf.patch
raspbian-autopush pushed a commit to raspbian-packages/pyside2 that referenced this pull request Jan 13, 2020
Update patchelf to current git version

The current version fixes a bug which broke the ability to strip
binaries with debug symbols:
NixOS/patchelf#117

With the former version of patchelf, I would get "not enough room for
program headers, try linking with -N" when trying to strip the binaries.

This patch brings patchelf in sync with the commit
27ffe8ae871e7a186018d66020ef3f6162c12c69 of patchelf's git
repository.

Also update patchelf's build process to match the logic in upstream's
automake files (thanks to Frédéric Bonnard for the patch to properly use
getconf PAGESIZE).

Bug: https://bugreports.qt.io/browse/PYSIDE-740
Last-Update: 2018-07-09


Gbp-Pq: Name update-patchelf.patch
raspbian-autopush pushed a commit to raspbian-packages/pyside2 that referenced this pull request Jan 13, 2020
Update patchelf to current git version

The current version fixes a bug which broke the ability to strip
binaries with debug symbols:
NixOS/patchelf#117

With the former version of patchelf, I would get "not enough room for
program headers, try linking with -N" when trying to strip the binaries.

This patch brings patchelf in sync with the commit
27ffe8ae871e7a186018d66020ef3f6162c12c69 of patchelf's git
repository.

Also update patchelf's build process to match the logic in upstream's
automake files (thanks to Frédéric Bonnard for the patch to properly use
getconf PAGESIZE).

Bug: https://bugreports.qt.io/browse/PYSIDE-740
Last-Update: 2018-07-09

Gbp-Pq: Name update-patchelf.patch
raspbian-autopush pushed a commit to raspbian-packages/pyside2 that referenced this pull request May 23, 2020
Update patchelf to current git version

The current version fixes a bug which broke the ability to strip
binaries with debug symbols:
NixOS/patchelf#117

With the former version of patchelf, I would get "not enough room for
program headers, try linking with -N" when trying to strip the binaries.

This patch brings patchelf in sync with the commit
27ffe8ae871e7a186018d66020ef3f6162c12c69 of patchelf's git
repository.

Also update patchelf's build process to match the logic in upstream's
automake files (thanks to Frédéric Bonnard for the patch to properly use
getconf PAGESIZE).

Bug: https://bugreports.qt.io/browse/PYSIDE-740
Last-Update: 2018-07-09

Gbp-Pq: Name update-patchelf.patch
raspbian-autopush pushed a commit to raspbian-packages/pyside2 that referenced this pull request Jul 3, 2020
Update patchelf to current git version

The current version fixes a bug which broke the ability to strip
binaries with debug symbols:
NixOS/patchelf#117

With the former version of patchelf, I would get "not enough room for
program headers, try linking with -N" when trying to strip the binaries.

This patch brings patchelf in sync with the commit
27ffe8ae871e7a186018d66020ef3f6162c12c69 of patchelf's git
repository.

Also update patchelf's build process to match the logic in upstream's
automake files (thanks to Frédéric Bonnard for the patch to properly use
getconf PAGESIZE).

Bug: https://bugreports.qt.io/browse/PYSIDE-740
Last-Update: 2018-07-09

Gbp-Pq: Name update-patchelf.patch
raspbian-autopush pushed a commit to raspbian-packages/pyside2 that referenced this pull request Jul 16, 2020
Update patchelf to current git version

The current version fixes a bug which broke the ability to strip
binaries with debug symbols:
NixOS/patchelf#117

With the former version of patchelf, I would get "not enough room for
program headers, try linking with -N" when trying to strip the binaries.

This patch brings patchelf in sync with the commit
27ffe8ae871e7a186018d66020ef3f6162c12c69 of patchelf's git
repository.

Also update patchelf's build process to match the logic in upstream's
automake files (thanks to Frédéric Bonnard for the patch to properly use
getconf PAGESIZE).

Bug: https://bugreports.qt.io/browse/PYSIDE-740
Last-Update: 2018-07-09

Gbp-Pq: Name update-patchelf.patch
raspbian-autopush pushed a commit to raspbian-packages/pyside2 that referenced this pull request Oct 26, 2020
Update patchelf to current git version

The current version fixes a bug which broke the ability to strip
binaries with debug symbols:
NixOS/patchelf#117

With the former version of patchelf, I would get "not enough room for
program headers, try linking with -N" when trying to strip the binaries.

This patch brings patchelf in sync with the commit
27ffe8ae871e7a186018d66020ef3f6162c12c69 of patchelf's git
repository.

Also update patchelf's build process to match the logic in upstream's
automake files (thanks to Frédéric Bonnard for the patch to properly use
getconf PAGESIZE).

Bug: https://bugreports.qt.io/browse/PYSIDE-740
Last-Update: 2018-07-09

Gbp-Pq: Name update-patchelf.patch
raspbian-autopush pushed a commit to raspbian-packages/pyside2 that referenced this pull request Dec 18, 2020
Update patchelf to current git version

The current version fixes a bug which broke the ability to strip
binaries with debug symbols:
NixOS/patchelf#117

With the former version of patchelf, I would get "not enough room for
program headers, try linking with -N" when trying to strip the binaries.

This patch brings patchelf in sync with the commit
27ffe8ae871e7a186018d66020ef3f6162c12c69 of patchelf's git
repository.

Also update patchelf's build process to match the logic in upstream's
automake files (thanks to Frédéric Bonnard for the patch to properly use
getconf PAGESIZE).

Bug: https://bugreports.qt.io/browse/PYSIDE-740
Last-Update: 2018-07-09

Gbp-Pq: Name update-patchelf.patch
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

6 participants