Auto merge of #60970 - MaulingMonkey:pr-compiletest-cdb-support, r=alexcrichton

Add basic CDB support to debuginfo compiletest s, to help catch `*.natvis` regressions, like those fixed in #60687.

First draft, feedback welcome.

Several Microsoft debuggers (VS, VS Code, WinDbg, CDB, ...) consume the `*.natvis` files we embed into rust `*.pdb` files.  While this only tests CDB, that test coverage should help for all of them.

# Changes

## src\bootstrap
  - test.rs:  Run CDB debuginfo tests on MSVC targets

## src\test\debuginfo
  - issue-13213.rs:  CDB has trouble with this, skip for now (newly discovered regression?)
  - pretty-std.rs:  Was ignored, re-enable for CDB only to start with, add CDB tests.
  - should-fail.rs:  Add CDB tests.

## src\tools\compiletest:
  - Added "-cdb" option
  - Added Mode::DebugInfoCdb ("debuginfo-cdb")
  - Added run_debuginfo_cdb_test[_no_opt]
  - Renamed Mode::DebugInfoBoth -> DebugInfoGdbLldb ("debuginfo-gdb+lldb") since it's no longer clear what "Both" means.
  - Find CDB at the default Win10 SDK install path "C:\Program Files (x86)\Windows Kits\10\Debugger\\*\cdb.exe"
  - Ignore CDB tests if CDB not found.

# Issues

  - `compute_stamp_hash`: not sure if there's any point in hashing `%ProgramFiles(x86)%`
  - `OsString` lacks any `*.natvis` entries (would be nice to add in a followup changelist)
  - DSTs (array/string slices) which work in VS & VS Code fail in CDB.
  - I've avoided `Mode::DebugInfoAll` as 3 debuggers leads to pow(2,3)=8 possible combinations.

# Reference

CDB is not part of the base Visual Studio install, but can be added via the Windows 10 SDK:
  https://developer.microsoft.com/en-us/windows/downloads/windows-10-sdk
Installing just "Debugging Tools for Windows" is sufficient.

CDB appears to already be installed on appveyor CI, where this changelist can find it, based on it's use here:
  https://github.com/rust-lang/rust/blob/0ffc57311030a1930edfa721fe57d0000a063af4/appveyor.yml#L227

CDB commands and command line reference:
  https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/debugger-reference

Author: bors

src/bootstrap/test.rs

@@ -976,14 +976,10 @@ impl Step for Compiletest {
         }
 
         if suite == "debuginfo" {
-            // Skip debuginfo tests on MSVC
-            if builder.config.build.contains("msvc") {
-                return;
-            }
-
+            let msvc = builder.config.build.contains("msvc");
             if mode == "debuginfo" {
                 return builder.ensure(Compiletest {
-                    mode: "debuginfo-both",
+                    mode: if msvc { "debuginfo-cdb" } else { "debuginfo-gdb+lldb" },
                     ..self
                 });
             }

src/test/debuginfo/issue-13213.rs

@@ -1,4 +1,5 @@
 // min-lldb-version: 310
+// ignore-cdb: Fails with exit code 0xc0000135 ("the application failed to initialize properly")
 
 // aux-build:issue-13213-aux.rs
 

src/test/debuginfo/pretty-std.rs

@@ -1,6 +1,5 @@
-// ignore-windows failing on win32 bot
 // ignore-freebsd: gdb package too new
-// ignore-test // Test temporarily ignored due to debuginfo tests being disabled, see PR 47155
+// only-cdb // "Temporarily" ignored on GDB/LLDB due to debuginfo tests being disabled, see PR 47155
 // ignore-android: FIXME(#10381)
 // compile-flags:-g
 // min-gdb-version 7.7
@@ -63,6 +62,56 @@
 // lldb-check:[...]$5 = None
 
 
+// === CDB TESTS ==================================================================================
+
+// cdb-command: g
+
+// cdb-command: dx slice,d
+// cdb-check:slice,d [...]
+// NOTE: While slices have a .natvis entry that works in VS & VS Code, it fails in CDB 10.0.18362.1
+
+// cdb-command: dx vec,d
+// cdb-check:vec,d [...] : { size=4 } [Type: [...]::Vec<u64>]
+// cdb-check:    [size]           : 4 [Type: [...]]
+// cdb-check:    [capacity]       : [...] [Type: [...]]
+// cdb-check:    [0]              : 4 [Type: unsigned __int64]
+// cdb-check:    [1]              : 5 [Type: unsigned __int64]
+// cdb-check:    [2]              : 6 [Type: unsigned __int64]
+// cdb-check:    [3]              : 7 [Type: unsigned __int64]
+
+// cdb-command: dx str_slice
+// cdb-check:str_slice [...]
+// NOTE: While string slices have a .natvis entry that works in VS & VS Code, it fails in CDB
+
+// cdb-command: dx string
+// cdb-check:string           : "IAMA string!" [Type: [...]::String]
+// cdb-check:    [<Raw View>]     [Type: [...]::String]
+// cdb-check:    [size]           : 0xc [Type: [...]]
+// cdb-check:    [capacity]       : 0xc [Type: [...]]
+// cdb-check:    [0]              : 73 'I' [Type: char]
+// cdb-check:    [1]              : 65 'A' [Type: char]
+// cdb-check:    [2]              : 77 'M' [Type: char]
+// cdb-check:    [3]              : 65 'A' [Type: char]
+// cdb-check:    [4]              : 32 ' ' [Type: char]
+// cdb-check:    [5]              : 115 's' [Type: char]
+// cdb-check:    [6]              : 116 't' [Type: char]
+// cdb-check:    [7]              : 114 'r' [Type: char]
+// cdb-check:    [8]              : 105 'i' [Type: char]
+// cdb-check:    [9]              : 110 'n' [Type: char]
+// cdb-check:    [10]             : 103 'g' [Type: char]
+// cdb-check:    [11]             : 33 '!' [Type: char]
+
+// cdb-command: dx os_string
+// cdb-check:os_string        [Type: [...]::OsString]
+// NOTE: OsString doesn't have a .natvis entry yet.
+
+// cdb-command: dx some
+// cdb-check:some             : { Some 8 } [Type: [...]::Option<i16>]
+// cdb-command: dx none
+// cdb-check:none             : { None } [Type: [...]::Option<i64>]
+// cdb-command: dx some_string
+// cdb-check:some_string      : { Some "IAMA optional string!" } [[...]::Option<[...]::String>]
+
 #![allow(unused_variables)]
 use std::ffi::OsString;
 

src/test/debuginfo/should-fail.rs

@@ -18,6 +18,13 @@
 // lldb-command:print x
 // lldb-check:[...]$0 = 5
 
+// === CDB TESTS ==================================================================================
+
+// cdb-command:g
+
+// cdb-command:dx x
+// cdb-check:string [...] : 5 [Type: [...]]
+
 fn main() {
     let x = 1;
 

src/tools/compiletest/src/common.rs

@@ -1,5 +1,6 @@
 pub use self::Mode::*;
 
+use std::ffi::OsString;
 use std::fmt;
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
@@ -15,7 +16,8 @@ pub enum Mode {
     RunPass,
     RunPassValgrind,
     Pretty,
-    DebugInfoBoth,
+    DebugInfoCdb,
+    DebugInfoGdbLldb,
     DebugInfoGdb,
     DebugInfoLldb,
     Codegen,
@@ -33,9 +35,10 @@ impl Mode {
     pub fn disambiguator(self) -> &'static str {
         // Run-pass and pretty run-pass tests could run concurrently, and if they do,
         // they need to keep their output segregated. Same is true for debuginfo tests that
-        // can be run both on gdb and lldb.
+        // can be run on cdb, gdb, and lldb.
         match self {
             Pretty => ".pretty",
+            DebugInfoCdb => ".cdb",
             DebugInfoGdb => ".gdb",
             DebugInfoLldb => ".lldb",
             _ => "",
@@ -52,7 +55,8 @@ impl FromStr for Mode {
             "run-pass" => Ok(RunPass),
             "run-pass-valgrind" => Ok(RunPassValgrind),
             "pretty" => Ok(Pretty),
-            "debuginfo-both" => Ok(DebugInfoBoth),
+            "debuginfo-cdb" => Ok(DebugInfoCdb),
+            "debuginfo-gdb+lldb" => Ok(DebugInfoGdbLldb),
             "debuginfo-lldb" => Ok(DebugInfoLldb),
             "debuginfo-gdb" => Ok(DebugInfoGdb),
             "codegen" => Ok(Codegen),
@@ -77,7 +81,8 @@ impl fmt::Display for Mode {
             RunPass => "run-pass",
             RunPassValgrind => "run-pass-valgrind",
             Pretty => "pretty",
-            DebugInfoBoth => "debuginfo-both",
+            DebugInfoCdb => "debuginfo-cdb",
+            DebugInfoGdbLldb => "debuginfo-gdb+lldb",
             DebugInfoGdb => "debuginfo-gdb",
             DebugInfoLldb => "debuginfo-lldb",
             Codegen => "codegen",
@@ -198,6 +203,9 @@ pub struct Config {
     /// Host triple for the compiler being invoked
     pub host: String,
 
+    /// Path to / name of the Microsoft Console Debugger (CDB) executable
+    pub cdb: Option<OsString>,
+
     /// Path to / name of the GDB executable
     pub gdb: Option<String>,
 

src/tools/compiletest/src/header.rs

@@ -57,9 +57,9 @@ enum ParsedNameDirective {
     NoMatch,
     /// Match.
     Match,
-    /// Mode was DebugInfoBoth and this matched gdb.
+    /// Mode was DebugInfoGdbLldb and this matched gdb.
     MatchGdb,
-    /// Mode was DebugInfoBoth and this matched lldb.
+    /// Mode was DebugInfoGdbLldb and this matched lldb.
     MatchLldb,
 }
 
@@ -81,13 +81,17 @@ impl EarlyProps {
             revisions: vec![],
         };
 
-        if config.mode == common::DebugInfoBoth {
+        if config.mode == common::DebugInfoGdbLldb {
             if config.lldb_python_dir.is_none() {
                 props.ignore = props.ignore.no_lldb();
             }
             if config.gdb_version.is_none() {
                 props.ignore = props.ignore.no_gdb();
             }
+        } else if config.mode == common::DebugInfoCdb {
+            if config.cdb.is_none() {
+                props.ignore = Ignore::Ignore;
+            }
         }
 
         let rustc_has_profiler_support = env::var_os("RUSTC_PROFILER_SUPPORT").is_some();
@@ -133,12 +137,12 @@ impl EarlyProps {
                 }
             }
 
-            if (config.mode == common::DebugInfoGdb || config.mode == common::DebugInfoBoth) &&
+            if (config.mode == common::DebugInfoGdb || config.mode == common::DebugInfoGdbLldb) &&
                 props.ignore.can_run_gdb() && ignore_gdb(config, ln) {
                 props.ignore = props.ignore.no_gdb();
             }
 
-            if (config.mode == common::DebugInfoLldb || config.mode == common::DebugInfoBoth) &&
+            if (config.mode == common::DebugInfoLldb || config.mode == common::DebugInfoGdbLldb) &&
                 props.ignore.can_run_lldb() && ignore_lldb(config, ln) {
                 props.ignore = props.ignore.no_lldb();
             }
@@ -804,7 +808,7 @@ impl Config {
                 ParsedNameDirective::Match
             } else {
                 match self.mode {
-                    common::DebugInfoBoth => {
+                    common::DebugInfoGdbLldb => {
                         if name == "gdb" {
                             ParsedNameDirective::MatchGdb
                         } else if name == "lldb" {
@@ -813,6 +817,11 @@ impl Config {
                             ParsedNameDirective::NoMatch
                         }
                     },
+                    common::DebugInfoCdb => if name == "cdb" {
+                        ParsedNameDirective::Match
+                    } else {
+                        ParsedNameDirective::NoMatch
+                    },
                     common::DebugInfoGdb => if name == "gdb" {
                         ParsedNameDirective::Match
                     } else {

src/tools/compiletest/src/main.rs

@@ -8,7 +8,7 @@ extern crate test;
 use crate::common::CompareMode;
 use crate::common::{expected_output_path, output_base_dir, output_relative_path, UI_EXTENSIONS};
 use crate::common::{Config, TestPaths};
-use crate::common::{DebugInfoBoth, DebugInfoGdb, DebugInfoLldb, Mode, Pretty};
+use crate::common::{DebugInfoCdb, DebugInfoGdbLldb, DebugInfoGdb, DebugInfoLldb, Mode, Pretty};
 use getopts::Options;
 use std::env;
 use std::ffi::OsString;
@@ -164,6 +164,12 @@ pub fn parse_config(args: Vec<String>) -> Config {
         .optopt("", "logfile", "file to log test execution to", "FILE")
         .optopt("", "target", "the target to build for", "TARGET")
         .optopt("", "host", "the host to build for", "HOST")
+        .optopt(
+            "",
+            "cdb",
+            "path to CDB to use for CDB debuginfo tests",
+            "PATH",
+        )
         .optopt(
             "",
             "gdb",
@@ -273,6 +279,7 @@ pub fn parse_config(args: Vec<String>) -> Config {
 
     let target = opt_str2(matches.opt_str("target"));
     let android_cross_path = opt_path(matches, "android-cross-path");
+    let cdb = analyze_cdb(matches.opt_str("cdb"), &target);
     let (gdb, gdb_version, gdb_native_rust) = analyze_gdb(matches.opt_str("gdb"), &target,
                                                           &android_cross_path);
     let (lldb_version, lldb_native_rust) = extract_lldb_version(matches.opt_str("lldb-version"));
@@ -319,6 +326,7 @@ pub fn parse_config(args: Vec<String>) -> Config {
         target_rustcflags: matches.opt_str("target-rustcflags"),
         target: target,
         host: opt_str2(matches.opt_str("host")),
+        cdb,
         gdb,
         gdb_version,
         gdb_native_rust,
@@ -421,7 +429,7 @@ pub fn opt_str2(maybestr: Option<String>) -> String {
 
 pub fn run_tests(config: &Config) {
     if config.target.contains("android") {
-        if config.mode == DebugInfoGdb || config.mode == DebugInfoBoth {
+        if config.mode == DebugInfoGdb || config.mode == DebugInfoGdbLldb {
             println!(
                 "{} debug-info test uses tcp 5039 port.\
                  please reserve it",
@@ -440,8 +448,8 @@ pub fn run_tests(config: &Config) {
 
     match config.mode {
         // Note that we don't need to emit the gdb warning when
-        // DebugInfoBoth, so it is ok to list that here.
-        DebugInfoBoth | DebugInfoLldb => {
+        // DebugInfoGdbLldb, so it is ok to list that here.
+        DebugInfoGdbLldb | DebugInfoLldb => {
             if let Some(lldb_version) = config.lldb_version.as_ref() {
                 if is_blacklisted_lldb_version(&lldb_version[..]) {
                     println!(
@@ -470,7 +478,8 @@ pub fn run_tests(config: &Config) {
                 return;
             }
         }
-        _ => { /* proceed */ }
+
+        DebugInfoCdb | _ => { /* proceed */ }
     }
 
     // FIXME(#33435) Avoid spurious failures in codegen-units/partitioning tests.
@@ -667,7 +676,7 @@ pub fn make_test(config: &Config, testpaths: &TestPaths) -> Vec<test::TestDescAn
                     &early_props,
                     revision.map(|s| s.as_str()),
                 )
-                || ((config.mode == DebugInfoBoth ||
+                || ((config.mode == DebugInfoGdbLldb || config.mode == DebugInfoCdb ||
                      config.mode == DebugInfoGdb || config.mode == DebugInfoLldb)
                     && config.target.contains("emscripten"))
                 || (config.mode == DebugInfoGdb && !early_props.ignore.can_run_gdb())
@@ -815,7 +824,7 @@ fn make_test_closure(
     revision: Option<&String>,
 ) -> test::TestFn {
     let mut config = config.clone();
-    if config.mode == DebugInfoBoth {
+    if config.mode == DebugInfoGdbLldb {
         // If both gdb and lldb were ignored, then the test as a whole
         // would be ignored.
         if !ignore.can_run_gdb() {
@@ -841,6 +850,47 @@ fn is_android_gdb_target(target: &String) -> bool {
     }
 }
 
+/// Returns `true` if the given target is a MSVC target for the purpouses of CDB testing.
+fn is_pc_windows_msvc_target(target: &String) -> bool {
+    target.ends_with("-pc-windows-msvc")
+}
+
+fn find_cdb(target: &String) -> Option<OsString> {
+    if !(cfg!(windows) && is_pc_windows_msvc_target(target)) {
+        return None;
+    }
+
+    let pf86 = env::var_os("ProgramFiles(x86)").or(env::var_os("ProgramFiles"))?;
+    let cdb_arch = if cfg!(target_arch="x86") {
+        "x86"
+    } else if cfg!(target_arch="x86_64") {
+        "x64"
+    } else if cfg!(target_arch="aarch64") {
+        "arm64"
+    } else if cfg!(target_arch="arm") {
+        "arm"
+    } else {
+        return None; // No compatible CDB.exe in the Windows 10 SDK
+    };
+
+    let mut path = PathBuf::new();
+    path.push(pf86);
+    path.push(r"Windows Kits\10\Debuggers"); // We could check 8.1 etc. too?
+    path.push(cdb_arch);
+    path.push(r"cdb.exe");
+
+    if !path.exists() {
+        return None;
+    }
+
+    Some(path.into_os_string())
+}
+
+/// Returns Path to CDB
+fn analyze_cdb(cdb: Option<String>, target: &String) -> Option<OsString> {
+    cdb.map(|s| OsString::from(s)).or(find_cdb(target))
+}
+
 /// Returns (Path to GDB, GDB Version, GDB has Rust Support)
 fn analyze_gdb(gdb: Option<String>, target: &String, android_cross_path: &PathBuf)
                -> (Option<String>, Option<u32>, bool) {