Auto merge of #115120 - icedrocket:ignore-strip-on-msvc, r=michaelwoerister

Ignore `-C strip` on MSVC

tl;dr - Define `-Cstrip` to only ever affect the binary; no other build artifacts.

This is necessary to improve cross-platform behavior consistency: if someone wanted debug information to be contained only in separate files on all platforms, they would set `-Cstrip=symbols` and `-Csplit-debuginfo=packed`, but this would result in no PDB files on MSVC.

Resolves #114215

compiler/rustc_codegen_ssa/src/back/linker.rs

@@ -902,52 +902,45 @@ impl<'a> Linker for MsvcLinker<'a> {
         }
     }
 
-    fn debuginfo(&mut self, strip: Strip, natvis_debugger_visualizers: &[PathBuf]) {
+    fn debuginfo(&mut self, _strip: Strip, natvis_debugger_visualizers: &[PathBuf]) {
-        match strip {
+        // This will cause the Microsoft linker to generate a PDB file
-            Strip::None => {
+        // from the CodeView line tables in the object files.
-                // This will cause the Microsoft linker to generate a PDB file
+        self.cmd.arg("/DEBUG");
-                // from the CodeView line tables in the object files.
-                self.cmd.arg("/DEBUG");
 
-                // Default to emitting only the file name of the PDB file into
+        // Default to emitting only the file name of the PDB file into
-                // the binary instead of the full path. Emitting the full path
+        // the binary instead of the full path. Emitting the full path
-                // may leak private information (such as user names).
+        // may leak private information (such as user names).
-                // See https://github.com/rust-lang/rust/issues/87825.
+        // See https://github.com/rust-lang/rust/issues/87825.
-                //
+        //
-                // This default behavior can be overridden by explicitly passing
+        // This default behavior can be overridden by explicitly passing
-                // `-Clink-arg=/PDBALTPATH:...` to rustc.
+        // `-Clink-arg=/PDBALTPATH:...` to rustc.
-                self.cmd.arg("/PDBALTPATH:%_PDB%");
+        self.cmd.arg("/PDBALTPATH:%_PDB%");
 
-                // This will cause the Microsoft linker to embed .natvis info into the PDB file
+        // This will cause the Microsoft linker to embed .natvis info into the PDB file
-                let natvis_dir_path = self.sess.sysroot.join("lib\\rustlib\\etc");
+        let natvis_dir_path = self.sess.sysroot.join("lib\\rustlib\\etc");
-                if let Ok(natvis_dir) = fs::read_dir(&natvis_dir_path) {
+        if let Ok(natvis_dir) = fs::read_dir(&natvis_dir_path) {
-                    for entry in natvis_dir {
+            for entry in natvis_dir {
-                        match entry {
+                match entry {
-                            Ok(entry) => {
+                    Ok(entry) => {
-                                let path = entry.path();
+                        let path = entry.path();
-                                if path.extension() == Some("natvis".as_ref()) {
+                        if path.extension() == Some("natvis".as_ref()) {
-                                    let mut arg = OsString::from("/NATVIS:");
+                            let mut arg = OsString::from("/NATVIS:");
-                                    arg.push(path);
+                            arg.push(path);
-                                    self.cmd.arg(arg);
+                            self.cmd.arg(arg);
-                                }
-                            }
-                            Err(error) => {
-                                self.sess.dcx().emit_warn(errors::NoNatvisDirectory { error });
-                            }
                         }
                     }
+                    Err(error) => {
+                        self.sess.dcx().emit_warn(errors::NoNatvisDirectory { error });
+                    }
                 }
+            }
+        }
 
-                // This will cause the Microsoft linker to embed .natvis info for all crates into the PDB file
+        // This will cause the Microsoft linker to embed .natvis info for all crates into the PDB file
-                for path in natvis_debugger_visualizers {
+        for path in natvis_debugger_visualizers {
-                    let mut arg = OsString::from("/NATVIS:");
+            let mut arg = OsString::from("/NATVIS:");
-                    arg.push(path);
+            arg.push(path);
-                    self.cmd.arg(arg);
+            self.cmd.arg(arg);
-                }
-            }
-            Strip::Debuginfo | Strip::Symbols => {
-                self.cmd.arg("/DEBUG:NONE");
-            }
         }
     }
 

src/doc/rustc/src/codegen-options/index.md

@@ -568,22 +568,23 @@ data from binaries during linking.
 
 Supported values for this option are:
 
-- `none` - debuginfo and symbols (if they exist) are copied to the produced
+- `none` - debuginfo and symbols are not modified.
-  binary or separate files depending on the target (e.g. `.pdb` files in case
-  of MSVC).
 - `debuginfo` - debuginfo sections and debuginfo symbols from the symbol table
-  section are stripped at link time and are not copied to the produced binary
+  section are stripped at link time and are not copied to the produced binary.
-  or separate files. This should leave backtraces mostly-intact but may make
+  This should leave backtraces mostly-intact but may make using a debugger like
-  using a debugger like gdb or lldb ineffectual.
+  gdb or lldb ineffectual. Prior to 1.79, this unintentionally disabled the
-- `symbols` - same as `debuginfo`, but the rest of the symbol table section is stripped as well,
+  generation of `*.pdb` files on MSVC, resulting in the absence of symbols.
-  depending on platform support. On platforms which depend on this symbol table for backtraces,
+- `symbols` - same as `debuginfo`, but the rest of the symbol table section is
-  profiling, and similar, this can affect them so negatively as to make the trace incomprehensible.
+  stripped as well, depending on platform support. On platforms which depend on
-  Programs which may be combined with others, such as CLI pipelines and developer tooling,
+  this symbol table for backtraces, profiling, and similar, this can affect
-  or even anything which wants crash-reporting, should usually avoid `-Cstrip=symbols`.
+  them so negatively as to make the trace incomprehensible. Programs which may
+  be combined with others, such as CLI pipelines and developer tooling, or even
+  anything which wants crash-reporting, should usually avoid `-Cstrip=symbols`.
 
-Note that, at any level, removing debuginfo only necessarily impacts "friendly" introspection.
+Note that, at any level, removing debuginfo only necessarily impacts "friendly"
-`-Cstrip` cannot be relied on as a meaningful security or obfuscation measure, as disassemblers
+introspection. `-Cstrip` cannot be relied on as a meaningful security or
-and decompilers can extract considerable information even in the absence of symbols.
+obfuscation measure, as disassemblers and decompilers can extract considerable
+information even in the absence of symbols.
 
 ## symbol-mangling-version
 

tests/ui/debuginfo/msvc-strip-debuginfo.rs

@@ -0,0 +1,26 @@
+//@ compile-flags: -C strip=debuginfo
+//@ only-msvc
+//@ run-pass
+
+use std::path::Path;
+
+pub fn is_related_pdb<P: AsRef<Path>>(path: &P, exe: &P) -> bool {
+    let (exe, path) = (exe.as_ref(), path.as_ref());
+
+    path.extension()
+        .map(|x| x.to_ascii_lowercase())
+        .is_some_and(|x| x == "pdb")
+        && path.file_stem() == exe.file_stem()
+}
+
+pub fn main() {
+    let curr_exe = std::env::current_exe().unwrap();
+    let curr_dir = curr_exe.parent().unwrap();
+
+    let entries = std::fs::read_dir(curr_dir).unwrap();
+
+    assert!(entries
+        .map_while(|x| x.ok())
+        .find(|x| is_related_pdb(&x.path(), &curr_exe))
+        .is_some());
+}

tests/ui/debuginfo/msvc-strip-symbols.rs

@@ -0,0 +1,26 @@
+//@ compile-flags: -C strip=symbols
+//@ only-msvc
+//@ run-pass
+
+use std::path::Path;
+
+pub fn is_related_pdb<P: AsRef<Path>>(path: &P, exe: &P) -> bool {
+    let (exe, path) = (exe.as_ref(), path.as_ref());
+
+    path.extension()
+        .map(|x| x.to_ascii_lowercase())
+        .is_some_and(|x| x == "pdb")
+        && path.file_stem() == exe.file_stem()
+}
+
+pub fn main() {
+    let curr_exe = std::env::current_exe().unwrap();
+    let curr_dir = curr_exe.parent().unwrap();
+
+    let entries = std::fs::read_dir(curr_dir).unwrap();
+
+    assert!(entries
+        .map_while(|x| x.ok())
+        .find(|x| is_related_pdb(&x.path(), &curr_exe))
+        .is_some());
+}