Auto merge of rust-lang#135034 - Noratrieb:strip-correctly, r=<try>

Pass objcopy args for stripping on OSX

When `-Cstrip` was changed in rust-lang#131405 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`.

fixes rust-lang#135028

try-job: aarch64-apple
try-job: dist-aarch64-apple

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);
 

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.");
+}

tests/run-make/strip/rmake.rs

@@ -0,0 +1,26 @@
+//@ 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() {
+    // We use DW_AT (the start of a DWARF attribute name) to check that some debuginfo is present.
+    // Additionally, use -Cdebuginfo=2 to make the test independent of the amount of debuginfo
+    // for std.
+
+    // -Cstrip=none should preserve symbols and debuginfo.
+    rustc().arg("hello.rs").arg("-Cdebuginfo=2").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("-Cdebuginfo=2").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("-Cdebuginfo=2").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");
+}