Auto merge of #97500 - GuillaumeGomez:rollup-ms1bvps, r=GuillaumeGomez

Rollup of 5 pull requests

Successful merges:

 - #96950 (Add regression test for #96395)
 - #97028 (Add support for embedding pretty printers via `#[debugger_visualizer]` attribute)
 - #97478 (Remove FIXME on `ExtCtxt::fn_decl()`)
 - #97479 (Make some tests check-pass)
 - #97482 (ptr::invalid is not equivalent to a int2ptr cast)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup

Author: bors

compiler/rustc_codegen_llvm/src/debuginfo/gdb.rs

@@ -5,11 +5,14 @@ use crate::llvm;
 use crate::builder::Builder;
 use crate::common::CodegenCx;
 use crate::value::Value;
+use rustc_codegen_ssa::base::collect_debugger_visualizers_transitive;
 use rustc_codegen_ssa::traits::*;
+use rustc_hir::def_id::LOCAL_CRATE;
 use rustc_middle::bug;
-use rustc_session::config::DebugInfo;
+use rustc_session::config::{CrateType, DebugInfo};
 
 use rustc_span::symbol::sym;
+use rustc_span::DebuggerVisualizerType;
 
 /// Inserts a side-effect free instruction sequence that makes sure that the
 /// .debug_gdb_scripts global is referenced, so it isn't removed by the linker.
@@ -37,9 +40,33 @@ pub fn get_or_insert_gdb_debug_scripts_section_global<'ll>(cx: &CodegenCx<'ll, '
 
     section_var.unwrap_or_else(|| {
         let section_name = b".debug_gdb_scripts\0";
-        let section_contents = b"\x01gdb_load_rust_pretty_printers.py\0";
+        let mut section_contents = Vec::new();
+
+        // Add the pretty printers for the standard library first.
+        section_contents.extend_from_slice(b"\x01gdb_load_rust_pretty_printers.py\0");
+
+        // Next, add the pretty printers that were specified via the `#[debugger_visualizer]` attribute.
+        let visualizers = collect_debugger_visualizers_transitive(
+            cx.tcx,
+            DebuggerVisualizerType::GdbPrettyPrinter,
+        );
+        let crate_name = cx.tcx.crate_name(LOCAL_CRATE);
+        for (index, visualizer) in visualizers.iter().enumerate() {
+            // The initial byte `4` instructs GDB that the following pretty printer
+            // is defined inline as opposed to in a standalone file.
+            section_contents.extend_from_slice(b"\x04");
+            let vis_name = format!("pretty-printer-{}-{}\n", crate_name.as_str(), index);
+            section_contents.extend_from_slice(vis_name.as_bytes());
+            section_contents.extend_from_slice(&visualizer.src);
+
+            // The final byte `0` tells GDB that the pretty printer has been
+            // fully defined and can continue searching for additional
+            // pretty printers.
+            section_contents.extend_from_slice(b"\0");
+        }
 
         unsafe {
+            let section_contents = section_contents.as_slice();
             let llvm_type = cx.type_array(cx.type_i8(), section_contents.len() as u64);
 
             let section_var = cx
@@ -62,7 +89,32 @@ pub fn needs_gdb_debug_scripts_section(cx: &CodegenCx<'_, '_>) -> bool {
     let omit_gdb_pretty_printer_section =
         cx.tcx.sess.contains_name(cx.tcx.hir().krate_attrs(), sym::omit_gdb_pretty_printer_section);
 
+    // To ensure the section `__rustc_debug_gdb_scripts_section__` will not create
+    // ODR violations at link time, this section will not be emitted for rlibs since
+    // each rlib could produce a different set of visualizers that would be embedded
+    // in the `.debug_gdb_scripts` section. For that reason, we make sure that the
+    // section is only emitted for leaf crates.
+    let embed_visualizers = cx.sess().crate_types().iter().any(|&crate_type| match crate_type {
+        CrateType::Executable | CrateType::Dylib | CrateType::Cdylib | CrateType::Staticlib => {
+            // These are crate types for which we will embed pretty printers since they
+            // are treated as leaf crates.
+            true
+        }
+        CrateType::ProcMacro => {
+            // We could embed pretty printers for proc macro crates too but it does not
+            // seem like a good default, since this is a rare use case and we don't
+            // want to slow down the common case.
+            false
+        }
+        CrateType::Rlib => {
+            // As per the above description, embedding pretty printers for rlibs could
+            // lead to ODR violations so we skip this crate type as well.
+            false
+        }
+    });
+
     !omit_gdb_pretty_printer_section
         && cx.sess().opts.debuginfo != DebugInfo::None
         && cx.sess().target.emit_debug_gdb_scripts
+        && embed_visualizers
 }

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -5,7 +5,7 @@ use rustc_data_structures::memmap::Mmap;
 use rustc_data_structures::temp_dir::MaybeTempDir;
 use rustc_errors::{ErrorGuaranteed, Handler};
 use rustc_fs_util::fix_windows_verbatim_for_gcc;
-use rustc_hir::def_id::{CrateNum, LOCAL_CRATE};
+use rustc_hir::def_id::CrateNum;
 use rustc_middle::middle::dependency_format::Linkage;
 use rustc_middle::middle::exported_symbols::SymbolExportKind;
 use rustc_session::config::{self, CFGuard, CrateType, DebugInfo, LdImpl, Strip};
@@ -18,6 +18,7 @@ use rustc_session::utils::NativeLibKind;
 /// need out of the shared crate context before we get rid of it.
 use rustc_session::{filesearch, Session};
 use rustc_span::symbol::Symbol;
+use rustc_span::DebuggerVisualizerFile;
 use rustc_target::spec::crt_objects::{CrtObjects, CrtObjectsFallback};
 use rustc_target::spec::{LinkOutputKind, LinkerFlavor, LldFlavor, SplitDebuginfo};
 use rustc_target::spec::{PanicStrategy, RelocModel, RelroLevel, SanitizerSet, Target};
@@ -37,6 +38,7 @@ use regex::Regex;
 use tempfile::Builder as TempFileBuilder;
 
 use std::borrow::Borrow;
+use std::collections::BTreeSet;
 use std::ffi::OsString;
 use std::fs::{File, OpenOptions};
 use std::io::{BufWriter, Write};
@@ -2099,14 +2101,16 @@ fn add_order_independent_options(
     // Pass optimization flags down to the linker.
     cmd.optimize();
 
-    let debugger_visualizer_paths = if sess.target.is_like_msvc {
-        collect_debugger_visualizers(tmpdir, sess, &codegen_results.crate_info)
-    } else {
-        Vec::new()
-    };
+    // Gather the set of NatVis files, if any, and write them out to a temp directory.
+    let natvis_visualizers = collect_natvis_visualizers(
+        tmpdir,
+        sess,
+        &codegen_results.crate_info.local_crate_name,
+        &codegen_results.crate_info.natvis_debugger_visualizers,
+    );
 
-    // Pass debuginfo and strip flags down to the linker.
-    cmd.debuginfo(strip_value(sess), &debugger_visualizer_paths);
+    // Pass debuginfo, NatVis debugger visualizers and strip flags down to the linker.
+    cmd.debuginfo(strip_value(sess), &natvis_visualizers);
 
     // We want to prevent the compiler from accidentally leaking in any system libraries,
     // so by default we tell linkers not to link to any default libraries.
@@ -2125,43 +2129,33 @@ fn add_order_independent_options(
     add_rpath_args(cmd, sess, codegen_results, out_filename);
 }
 
-// Write the debugger visualizer files for each crate to the temp directory and gather the file paths.
-fn collect_debugger_visualizers(
+// Write the NatVis debugger visualizer files for each crate to the temp directory and gather the file paths.
+fn collect_natvis_visualizers(
     tmpdir: &Path,
     sess: &Session,
-    crate_info: &CrateInfo,
+    crate_name: &Symbol,
+    natvis_debugger_visualizers: &BTreeSet<DebuggerVisualizerFile>,
 ) -> Vec<PathBuf> {
-    let mut visualizer_paths = Vec::new();
-    let debugger_visualizers = &crate_info.debugger_visualizers;
-    let mut index = 0;
+    let mut visualizer_paths = Vec::with_capacity(natvis_debugger_visualizers.len());
 
-    for (&cnum, visualizers) in debugger_visualizers {
-        let crate_name = if cnum == LOCAL_CRATE {
-            crate_info.local_crate_name.as_str()
-        } else {
-            crate_info.crate_name[&cnum].as_str()
-        };
+    for (index, visualizer) in natvis_debugger_visualizers.iter().enumerate() {
+        let visualizer_out_file = tmpdir.join(format!("{}-{}.natvis", crate_name.as_str(), index));
 
-        for visualizer in visualizers {
-            let visualizer_out_file = tmpdir.join(format!("{}-{}.natvis", crate_name, index));
-
-            match fs::write(&visualizer_out_file, &visualizer.src) {
-                Ok(()) => {
-                    visualizer_paths.push(visualizer_out_file.clone());
-                    index += 1;
-                }
-                Err(error) => {
-                    sess.warn(
-                        format!(
-                            "Unable to write debugger visualizer file `{}`: {} ",
-                            visualizer_out_file.display(),
-                            error
-                        )
-                        .as_str(),
-                    );
-                }
-            };
-        }
+        match fs::write(&visualizer_out_file, &visualizer.src) {
+            Ok(()) => {
+                visualizer_paths.push(visualizer_out_file);
+            }
+            Err(error) => {
+                sess.warn(
+                    format!(
+                        "Unable to write debugger visualizer file `{}`: {} ",
+                        visualizer_out_file.display(),
+                        error
+                    )
+                    .as_str(),
+                );
+            }
+        };
     }
     visualizer_paths
 }

compiler/rustc_codegen_ssa/src/back/linker.rs

@@ -183,7 +183,7 @@ pub trait Linker {
     fn optimize(&mut self);
     fn pgo_gen(&mut self);
     fn control_flow_guard(&mut self);
-    fn debuginfo(&mut self, strip: Strip, debugger_visualizers: &[PathBuf]);
+    fn debuginfo(&mut self, strip: Strip, natvis_debugger_visualizers: &[PathBuf]);
     fn no_crt_objects(&mut self);
     fn no_default_libraries(&mut self);
     fn export_symbols(&mut self, tmpdir: &Path, crate_type: CrateType, symbols: &[String]);
@@ -915,7 +915,7 @@ impl<'a> Linker for MsvcLinker<'a> {
         self.cmd.arg("/guard:cf");
     }
 
-    fn debuginfo(&mut self, strip: Strip, debugger_visualizers: &[PathBuf]) {
+    fn debuginfo(&mut self, strip: Strip, natvis_debugger_visualizers: &[PathBuf]) {
         match strip {
             Strip::None => {
                 // This will cause the Microsoft linker to generate a PDB file
@@ -944,7 +944,7 @@ impl<'a> Linker for MsvcLinker<'a> {
                 }
 
                 // This will cause the Microsoft linker to embed .natvis info for all crates into the PDB file
-                for path in debugger_visualizers {
+                for path in natvis_debugger_visualizers {
                     let mut arg = OsString::from("/NATVIS:");
                     arg.push(path);
                     self.cmd.arg(arg);

compiler/rustc_codegen_ssa/src/base.rs

@@ -31,11 +31,13 @@ use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf, TyAndLayout};
 use rustc_middle::ty::query::Providers;
 use rustc_middle::ty::{self, Instance, Ty, TyCtxt};
 use rustc_session::cgu_reuse_tracker::CguReuse;
-use rustc_session::config::{self, EntryFnType, OutputType};
+use rustc_session::config::{self, CrateType, EntryFnType, OutputType};
 use rustc_session::Session;
 use rustc_span::symbol::sym;
+use rustc_span::{DebuggerVisualizerFile, DebuggerVisualizerType};
 use rustc_target::abi::{Align, VariantIdx};
 
+use std::collections::BTreeSet;
 use std::convert::TryFrom;
 use std::ops::{Deref, DerefMut};
 use std::time::{Duration, Instant};
@@ -487,6 +489,29 @@ fn get_argc_argv<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     }
 }
 
+/// This function returns all of the debugger visualizers specified for the
+/// current crate as well as all upstream crates transitively that match the
+/// `visualizer_type` specified.
+pub fn collect_debugger_visualizers_transitive(
+    tcx: TyCtxt<'_>,
+    visualizer_type: DebuggerVisualizerType,
+) -> BTreeSet<DebuggerVisualizerFile> {
+    tcx.debugger_visualizers(LOCAL_CRATE)
+        .iter()
+        .chain(
+            tcx.crates(())
+                .iter()
+                .filter(|&cnum| {
+                    let used_crate_source = tcx.used_crate_source(*cnum);
+                    used_crate_source.rlib.is_some() || used_crate_source.rmeta.is_some()
+                })
+                .flat_map(|&cnum| tcx.debugger_visualizers(cnum)),
+        )
+        .filter(|visualizer| visualizer.visualizer_type == visualizer_type)
+        .cloned()
+        .collect::<BTreeSet<_>>()
+}
+
 pub fn codegen_crate<B: ExtraBackendMethods>(
     backend: B,
     tcx: TyCtxt<'_>,
@@ -838,13 +863,8 @@ impl CrateInfo {
             missing_lang_items: Default::default(),
             dependency_formats: tcx.dependency_formats(()).clone(),
             windows_subsystem,
-            debugger_visualizers: Default::default(),
+            natvis_debugger_visualizers: Default::default(),
         };
-        let debugger_visualizers = tcx.debugger_visualizers(LOCAL_CRATE).clone();
-        if !debugger_visualizers.is_empty() {
-            info.debugger_visualizers.insert(LOCAL_CRATE, debugger_visualizers);
-        }
-
         let lang_items = tcx.lang_items();
 
         let crates = tcx.crates(());
@@ -882,14 +902,29 @@ impl CrateInfo {
             let missing =
                 missing.iter().cloned().filter(|&l| lang_items::required(tcx, l)).collect();
             info.missing_lang_items.insert(cnum, missing);
+        }
 
-            // Only include debugger visualizer files from crates that will be statically linked.
-            if used_crate_source.rlib.is_some() || used_crate_source.rmeta.is_some() {
-                let debugger_visualizers = tcx.debugger_visualizers(cnum).clone();
-                if !debugger_visualizers.is_empty() {
-                    info.debugger_visualizers.insert(cnum, debugger_visualizers);
-                }
+        let embed_visualizers = tcx.sess.crate_types().iter().any(|&crate_type| match crate_type {
+            CrateType::Executable | CrateType::Dylib | CrateType::Cdylib => {
+                // These are crate types for which we invoke the linker and can embed
+                // NatVis visualizers.
+                true
+            }
+            CrateType::ProcMacro => {
+                // We could embed NatVis for proc macro crates too (to improve the debugging
+                // experience for them) but it does not seem like a good default, since
+                // this is a rare use case and we don't want to slow down the common case.
+                false
             }
+            CrateType::Staticlib | CrateType::Rlib => {
+                // We don't invoke the linker for these, so we don't need to collect the NatVis for them.
+                false
+            }
+        });
+
+        if tcx.sess.target.is_like_msvc && embed_visualizers {
+            info.natvis_debugger_visualizers =
+                collect_debugger_visualizers_transitive(tcx, DebuggerVisualizerType::Natvis);
         }
 
         info

compiler/rustc_codegen_ssa/src/lib.rs

@@ -36,6 +36,7 @@ use rustc_session::cstore::{self, CrateSource};
 use rustc_session::utils::NativeLibKind;
 use rustc_span::symbol::Symbol;
 use rustc_span::DebuggerVisualizerFile;
+use std::collections::BTreeSet;
 use std::path::{Path, PathBuf};
 
 pub mod back;
@@ -157,7 +158,7 @@ pub struct CrateInfo {
     pub missing_lang_items: FxHashMap<CrateNum, Vec<LangItem>>,
     pub dependency_formats: Lrc<Dependencies>,
     pub windows_subsystem: Option<String>,
-    pub debugger_visualizers: FxHashMap<CrateNum, Vec<DebuggerVisualizerFile>>,
+    pub natvis_debugger_visualizers: BTreeSet<DebuggerVisualizerFile>,
 }
 
 #[derive(Encodable, Decodable)]

compiler/rustc_expand/src/build.rs

@@ -514,7 +514,7 @@ impl<'a> ExtCtxt<'a> {
         }
     }
 
-    // FIXME: unused `self`
+    // `self` is unused but keep it as method for the convenience use.
     pub fn fn_decl(&self, inputs: Vec<ast::Param>, output: ast::FnRetTy) -> P<ast::FnDecl> {
         P(ast::FnDecl { inputs, output })
     }

compiler/rustc_feature/src/builtin_attrs.rs

@@ -399,7 +399,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
 
     // RFC #3191: #[debugger_visualizer] support
     gated!(
-        debugger_visualizer, Normal, template!(List: r#"natvis_file = "...""#),
+        debugger_visualizer, Normal, template!(List: r#"natvis_file = "...", gdb_script_file = "...""#),
         DuplicatesOk, experimental!(debugger_visualizer)
     ),