From 26309e5edafc7412f16def9b9a1df7ef4930d27c Mon Sep 17 00:00:00 2001
From: Noratrieb <48135649+Noratrieb@users.noreply.github.com>
Date: Thu, 2 Jan 2025 16:50:21 +0100
Subject: [PATCH] Pass objcopy args for stripping on OSX

When `-Cstrip` was changed to use the bundled rust-objcopy instead of
/usr/bin/strip on OSX, strip-like arguments were preserved.

But strip and objcopy are, while being the same binary, different, they
have different defaults depending on which binary they are.
Notably, strip strips everything by default, and objcopy doesn't strip
anything by default.

Additionally, `-S` actually means `--strip-all`, so debuginfo stripped
everything and symbols didn't strip anything.

We now correctly pass `--strip-debug` and `--strip-all`.
---
 compiler/rustc_codegen_ssa/src/back/link.rs | 25 ++++++---------------
 tests/run-make/strip/hello.rs               |  8 +++++++
 tests/run-make/strip/rmake.rs               | 23 +++++++++++++++++++
 3 files changed, 38 insertions(+), 18 deletions(-)
 create mode 100644 tests/run-make/strip/hello.rs
 create mode 100644 tests/run-make/strip/rmake.rs

diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs
index 4bc064528f3f0..e2081ad75633c 100644
--- a/compiler/rustc_codegen_ssa/src/back/link.rs
+++ b/compiler/rustc_codegen_ssa/src/back/link.rs
@@ -1104,14 +1104,14 @@ fn link_natively(
         let stripcmd = "rust-objcopy";
         match (strip, crate_type) {
             (Strip::Debuginfo, _) => {
-                strip_symbols_with_external_utility(sess, stripcmd, out_filename, &["-S"])
+                strip_with_external_utility(sess, stripcmd, out_filename, &["--strip-debug"])
             }
             // Per the manpage, `-x` is the maximum safe strip level for dynamic libraries. (#93988)
             (Strip::Symbols, CrateType::Dylib | CrateType::Cdylib | CrateType::ProcMacro) => {
-                strip_symbols_with_external_utility(sess, stripcmd, out_filename, &["-x"])
+                strip_with_external_utility(sess, stripcmd, out_filename, &["-x"])
             }
             (Strip::Symbols, _) => {
-                strip_symbols_with_external_utility(sess, stripcmd, out_filename, &[])
+                strip_with_external_utility(sess, stripcmd, out_filename, &["--strip-all"])
             }
             (Strip::None, _) => {}
         }
@@ -1127,9 +1127,7 @@ fn link_natively(
         let stripcmd = if !sess.host.is_like_solaris { "rust-objcopy" } else { "/usr/bin/strip" };
         match strip {
             // Always preserve the symbol table (-x).
-            Strip::Debuginfo => {
-                strip_symbols_with_external_utility(sess, stripcmd, out_filename, &["-x"])
-            }
+            Strip::Debuginfo => strip_with_external_utility(sess, stripcmd, out_filename, &["-x"]),
             // Strip::Symbols is handled via the --strip-all linker option.
             Strip::Symbols => {}
             Strip::None => {}
@@ -1145,15 +1143,11 @@ fn link_natively(
         match strip {
             Strip::Debuginfo => {
                 // FIXME: AIX's strip utility only offers option to strip line number information.
-                strip_symbols_with_external_utility(sess, stripcmd, out_filename, &[
-                    "-X32_64", "-l",
-                ])
+                strip_with_external_utility(sess, stripcmd, out_filename, &["-X32_64", "-l"])
             }
             Strip::Symbols => {
                 // Must be noted this option might remove symbol __aix_rust_metadata and thus removes .info section which contains metadata.
-                strip_symbols_with_external_utility(sess, stripcmd, out_filename, &[
-                    "-X32_64", "-r",
-                ])
+                strip_with_external_utility(sess, stripcmd, out_filename, &["-X32_64", "-r"])
             }
             Strip::None => {}
         }
@@ -1166,12 +1160,7 @@ fn link_natively(
     }
 }
 
-fn strip_symbols_with_external_utility(
-    sess: &Session,
-    util: &str,
-    out_filename: &Path,
-    options: &[&str],
-) {
+fn strip_with_external_utility(sess: &Session, util: &str, out_filename: &Path, options: &[&str]) {
     let mut cmd = Command::new(util);
     cmd.args(options);
 
diff --git a/tests/run-make/strip/hello.rs b/tests/run-make/strip/hello.rs
new file mode 100644
index 0000000000000..2dc0376650bfe
--- /dev/null
+++ b/tests/run-make/strip/hello.rs
@@ -0,0 +1,8 @@
+fn main() {
+    hey_i_get_compiled();
+}
+
+#[inline(never)]
+fn hey_i_get_compiled() {
+    println!("Hi! Do or do not strip me, your choice.");
+}
diff --git a/tests/run-make/strip/rmake.rs b/tests/run-make/strip/rmake.rs
new file mode 100644
index 0000000000000..dfed528ac6c4c
--- /dev/null
+++ b/tests/run-make/strip/rmake.rs
@@ -0,0 +1,23 @@
+//@ ignore-windows Windows does not actually strip
+
+// Test that -Cstrip correctly strips/preserves debuginfo and symbols.
+
+use run_make_support::{bin_name, llvm_dwarfdump, llvm_nm, rustc};
+
+fn main() {
+    // -Cstrip=none should preserve symbols and debuginfo.
+    // We use DW_AT (the start of a DWARF attribute name) to ensure that some debuginfo is present.
+    rustc().arg("hello.rs").arg("-Cstrip=none").run();
+    llvm_nm().input(bin_name("hello")).run().assert_stdout_contains("hey_i_get_compiled");
+    llvm_dwarfdump().input(bin_name("hello")).run().assert_stdout_contains("DW_AT");
+
+    // -Cstrip=debuginfo should preserve symbols and strip debuginfo.
+    rustc().arg("hello.rs").arg("-Cstrip=debuginfo").run();
+    llvm_nm().input(bin_name("hello")).run().assert_stdout_contains("hey_i_get_compiled");
+    llvm_dwarfdump().input(bin_name("hello")).run().assert_stdout_not_contains("DW_AT");
+
+    // -Cstrip=symbols should strip symbols and strip debuginfo.
+    rustc().arg("hello.rs").arg("-Cstrip=symbols").run();
+    llvm_nm().input(bin_name("hello")).run().assert_stderr_contains("no symbols");
+    llvm_dwarfdump().input(bin_name("hello")).run().assert_stdout_not_contains("DW_AT");
+}