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

Hard-coded strip path breaks Linux -> Darwin builds #131206

Closed
rvolosatovs opened this issue Oct 3, 2024 · 12 comments · Fixed by #131405 or #131480
Closed

Hard-coded strip path breaks Linux -> Darwin builds #131206

rvolosatovs opened this issue Oct 3, 2024 · 12 comments · Fixed by #131405 or #131480
Assignees
Labels
C-bug Category: This is a bug. O-linux Operating system: Linux O-macos Operating system: macOS P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@rvolosatovs
Copy link

rvolosatovs commented Oct 3, 2024

I cross-compile code for Darwin from Linux, which now fails due to the strip path being hard-coded in latest nightly, which I believe is caused by this PR #130781

I expected to see this happen: successful compilation

Instead, this happened:

error: unable to run `/usr/bin/strip`: No such file or directory (os error 2)

https://github.com/rvolosatovs/nixify/actions/runs/11163623704/job/31031050404?pr=265

I'm not sure why /usr/bin/strip was chosen as the strip to use, as an end user - that is surprising, in fact I would have expected the strip from the environment to be used by default. If the intention is to set a default to a hardcoded path, then it looks like a configuration option must be provided.

Alternatively, perhaps cargo could use /usr/bin/strip if found and otherwise use strip from environment?

Version it worked on

It most recently worked on: 1.80.1

Version with regression

rustc --version --verbose:

cargo 1.83.0-nightly (80d82ca22 2024-09-27)
@rvolosatovs rvolosatovs added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Oct 3, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed regression-untriaged Untriaged performance or correctness regression. labels Oct 3, 2024
@taiki-e
Copy link
Member

taiki-e commented Oct 3, 2024

Duplicate of #114411

(The results are not exactly the same, but things are broken for the same cause.)

@rvolosatovs
Copy link
Author

Duplicate of #114411

@taiki-e no, that's a different issue, since strip does work fine on Linux to strip Darwin binaries. The issue here is the hardcoded path introduced a week ago

rvolosatovs added a commit to rvolosatovs/nixify that referenced this issue Oct 3, 2024
pin to version before rust-lang-ci/rust@0399709
rust-lang/rust#130781
rust-lang/rust#131206

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
rvolosatovs added a commit to rvolosatovs/nixify that referenced this issue Oct 3, 2024
pin to version before rust-lang-ci/rust@0399709
rust-lang/rust#130781
rust-lang/rust#131206

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
@taiki-e
Copy link
Member

taiki-e commented Oct 3, 2024

Oh, sorry, I missed the information that that used to work.

(In any case, I think the correct solution to these problems is the same one: #123151)

@rvolosatovs
Copy link
Author

Oh, sorry, I missed the information that that used to work.

(In any case, I think the correct solution to these problems is the same one: #123151)

indeed, LLVM strip works great for my use case, on current nightly, however, there is no way to make sure it's used other than explicitly putting the binary in /usr/bin/strip.

Will #123151 land before 1.83? I don't think #130781 should land in a release and seems that it currently targets 1.83

rvolosatovs added a commit to rvolosatovs/nixify that referenced this issue Oct 3, 2024
pin to version before rust-lang-ci/rust@0399709
rust-lang/rust#130781
rust-lang/rust#131206

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
@jieyouxu
Copy link
Member

jieyouxu commented Oct 3, 2024

#130781 cc @monkeydbobo and @davidtwco
Not sure how to solve the strip situation...

@jieyouxu jieyouxu added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 3, 2024
@ChrisDenton
Copy link
Member

Not sure how to solve the strip situation...

Also check that the host is darwin too?

@Noratrieb
Copy link
Member

how hard is it to implement "our own" (based on crates) Mach-O stripping instead of using the command? Using the strip command leads to so many problems.. (on all platforms :D)

@bjorn3
Copy link
Member

bjorn3 commented Oct 4, 2024

since strip does work fine on Linux to strip Darwin binaries.

If you are using binutils strip to strip arm64 macOS binaries, it does not work fine. It produces broken binaries with invalid code signature. On arm64 macOS mandates the use of code signatures, so "unsigned" binaries use an ad-hoc code signature, but binutils seems to forget to update the ad-hoc code signature when modifying binaries. This is exactly the problem #130781 fixed by forcing XCode's strip to be used instead.

@rvolosatovs
Copy link
Author

since strip does work fine on Linux to strip Darwin binaries.

If you are using binutils strip to strip arm64 macOS binaries, it does not work fine. It produces broken binaries with invalid code signature. On arm64 macOS mandates the use of code signatures, so "unsigned" binaries use an ad-hoc code signature, but binutils seems to forget to update the ad-hoc code signature when modifying binaries. This is exactly the problem #130781 fixed by forcing XCode's strip to be used instead.

I have been compiling Mac binaries on Linux for over a year now and I originally have experienced code signatures being broken by GNU strip on aarch64 darwin binaries myself ( IIRC it only happens for binaries that require particular MacOS frameworks).

I just tried to use 2 different strips on a x86_64-linux box directly to strip a binary compiled with cargo 1.81.0 (2dbb1af80 2024-08-20) on arm Mac and here's what I got, however:

  • GNU strip: file format not recognized - the binary still works
  • LLVM strip: success and stripped binary (I gave it a debug build with over 100k lines in objdump -s)

For reference:
GNU strip:

GNU strip (GNU Binutils) 2.41
Copyright (C) 2023 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) any later version.
This program has absolutely no warranty.

LLVM strip:

llvm-strip, compatible with GNU strip
LLVM (http://llvm.org/):
  LLVM version 18.1.8
  Optimized build.

So clearly LLVM strip works just fine for stripping Mac binaries.

@bjorn3
Copy link
Member

bjorn3 commented Oct 4, 2024

Ah, right. LLVM's strip should work fine.

@saethlin saethlin added O-linux Operating system: Linux O-macos Operating system: macOS and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Oct 4, 2024
@apiraino
Copy link
Contributor

apiraino commented Oct 7, 2024

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Oct 7, 2024
@davidtwco davidtwco self-assigned this Oct 8, 2024
@madsmtm
Copy link
Contributor

madsmtm commented Oct 10, 2024

Opened #131480 to fix this directly, in case #131405 ends up taking some time.

@bors bors closed this as completed in 6831362 Oct 10, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 10, 2024
Rollup merge of rust-lang#131480 - madsmtm:macos-fix-strip-binary, r=nnethercote

Fix hardcoded strip path when cross-compiling from Linux to Darwin

Fixes rust-lang#131206.

I fear that rust-lang#131405 might end up taking some time, so opening this PR to resolve the regression.

`@rustbot` label O-apple
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 5, 2024
…=jieyouxu,albertlarsan68

bootstrap/codegen_ssa: ship llvm-strip and use it for -Cstrip

Fixes rust-lang#131206.

- Includes `llvm-strip` (a symlink to `llvm-objcopy`) in the compiler dist artifact so that it can be used for `-Cstrip` instead of the system tooling.
- Uses `llvm-strip` instead of `/usr/bin/strip` for macOS. macOS needs a specific linker and the system one is preferred, hence rust-lang#130781 but that doesn't work when cross-compiling, so use the `llvm-strip` utility instead.

cc rust-lang#123151
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Nov 6, 2024
Rollup merge of rust-lang#131405 - davidtwco:hardcoded-strip-macos, r=jieyouxu,albertlarsan68

bootstrap/codegen_ssa: ship llvm-strip and use it for -Cstrip

Fixes rust-lang#131206.

- Includes `llvm-strip` (a symlink to `llvm-objcopy`) in the compiler dist artifact so that it can be used for `-Cstrip` instead of the system tooling.
- Uses `llvm-strip` instead of `/usr/bin/strip` for macOS. macOS needs a specific linker and the system one is preferred, hence rust-lang#130781 but that doesn't work when cross-compiling, so use the `llvm-strip` utility instead.

cc rust-lang#123151
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-linux Operating system: Linux O-macos Operating system: macOS P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet