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

llvm-strip breaks symlink #59848

Open
pawelsopensource opened this issue Jan 5, 2023 · 9 comments
Open

llvm-strip breaks symlink #59848

pawelsopensource opened this issue Jan 5, 2023 · 9 comments

Comments

@pawelsopensource
Copy link

pawelsopensource commented Jan 5, 2023

Please consider following shell testcase:

touch empty.c
clang empty.c -shared -o empty.so.1
ln -sf empty.so.1 empty.so
strip --strip-unneeded empty.so
[ -L empty.so ] || echo "KO1"
llvm-strip --strip-unneeded empty.so
[ -L empty.so ] || echo "KO2"

llvm-strip (15.0.6) breaks empty.so symlink and reports KO2. binutils strip works fine.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 5, 2023

@llvm/issue-subscribers-tools-llvm-objcopy-strip

@chestnykh
Copy link

Candidate patch: https://reviews.llvm.org/D141145

@MaskRay
Copy link
Member

MaskRay commented Jan 6, 2023

I think we have made a decision (for crbug.com/1108880 c2a8477 @manojgupta) that we always create a new file instead of reusing the existing file. This is more in line with most other llvm-* clang* utilities which may create an output file (and gcc, as, etc, but not cp).
It can be argued that our behavior has stronger security characteristics since it doesn't follow a symlink (see https://sourceware.org/bugzilla/show_bug.cgi?id=26945 ).
Perhaps we should just accept this incompatibility with GNU objcopy. This normally isn't an issue for users.

The binutils behavior of following symlinks and doing smart_rename is quite complex.

@chestnykh
Copy link

@MaskRay So this issue is not a bug and then i close the review?

@MaskRay
Copy link
Member

MaskRay commented Jan 6, 2023

@MaskRay So this issue is not a bug and then i close the review?

I think so but you may wait a bit for more feedback.

@pawelsopensource
Copy link
Author

imho, breaking devel symlinks (foo.so -> foo.so.x -> foo.so.x.y) by simple llvm-srip foo.so* should be reported as an error. in the current state compiler/linker/runtime-loader may pick different files after "foo upgrade"

@chestnykh
Copy link

I think this is not very critical if llvm-strip will break symlink and create regular file instead, but such behaviour may cause negative side-effects for example in someone scripts which expect a symlink but instead we provide regular file

@brad0
Copy link
Contributor

brad0 commented Jan 21, 2023

This normally isn't an issue for users.

Build infrastructure are users.

We tried switching over to llvm-strip to deal with an issue we are having with GNU strip and ran into this issue which broken building our ports / packages.

The old strip replaces the contents of the file.
llvm-strip moves a new file into place, breaking hard links.

This shows up when fake creates hard links to a binary and the
DEBUG_PACKAGES infrastructure runs strip during packaging.
The problem is caught as PLIST changes involving @link if you
build with PLIST_DB.
CVSROOT:	/cvs
Module name:	src
Changes by:	kettenis@cvs.openbsd.org	2023/01/18 17:18:19

Modified files:
	gnu/usr.bin/binutils-2.17: Makefile.bsd-wrapper 
	gnu/usr.bin/clang/llvm-objcopy: Makefile 
Added files:
	gnu/usr.bin/clang/llvm-objcopy: strip.1 

Log message:
The binutils strip damages GNU_RELRO on binaries linked by ld.lld on at
least amd64.  Fix this by switching to the llvm strip on architectures
that use ld.lld.
CVSROOT:	/cvs
Module name:	src
Changes by:	kettenis@cvs.openbsd.org	2023/01/19 15:54:45

Modified files:
	gnu/usr.bin/binutils-2.17: Makefile.bsd-wrapper 
	gnu/usr.bin/clang/llvm-objcopy: Makefile 

Log message:
Revert previous commit (but leave the man page around); llvm-strip behaves
differently on files that are hardlinked and this is tripping up ports.

bob-beck pushed a commit to openbsd/ports that referenced this issue Jan 22, 2023
- Use llvm-objcopy instead of (binutils) objcopy when creating detached
debug symbols and adding the gnu-debuglink section.

- For llvm-objcopy and strip, output to a temporary file and copy it
back to the original file. The LLVM tools create a new file, so hard links
are not normally maintained, but this method keeps them.

llvm/llvm-project#59848 (comment)

ok naddy@.

This should allow us to move back to llvm-strip in base on archs where
we use ld.lld, which (alongside the change to llvm-objcopy) avoids problems
that have been seen during mimmutable(2) development with GNU_RELRO with
the version of binutils that we have.
@brad0
Copy link
Contributor

brad0 commented Feb 5, 2023

CVSROOT:	/cvs
Module name:	src
Changes by:	kettenis@cvs.openbsd.org	2023/01/27 15:01:02

Modified files:
	gnu/usr.bin/clang/llvm-objcopy: Makefile 
	gnu/usr.bin/binutils-2.17: Makefile.bsd-wrapper 

Log message:
Recommit the switch to use llvm-strip on architectures that use ld.lld.

The part that needed a workaround in ports..

openbsd/ports@9a85588

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants