Skip to content

Commit

Permalink
Add basic CDB support to debuginfo compiletest s, to help catch `*.na…
Browse files Browse the repository at this point in the history
…tvis` regressions, like those fixed in rust-lang#60687.

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
  • Loading branch information
MaulingMonkey committed May 20, 2019
1 parent 26ab324 commit e33c441
Show file tree
Hide file tree
Showing 8 changed files with 251 additions and 31 deletions.
8 changes: 2 additions & 6 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
}
Expand Down
1 change: 1 addition & 0 deletions src/test/debuginfo/issue-13213.rs
Original file line number Diff line number Diff line change
@@ -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

Expand Down
53 changes: 51 additions & 2 deletions src/test/debuginfo/pretty-std.rs
Original file line number Diff line number Diff line change
@@ -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 // Test 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
Expand Down Expand Up @@ -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 10.0.18362.1

// 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!" } [Type: [...]::Option<[...]::String>]

#![allow(unused_variables)]
use std::ffi::OsString;

Expand Down
7 changes: 7 additions & 0 deletions src/test/debuginfo/should-fail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
16 changes: 12 additions & 4 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub use self::Mode::*;

use std::ffi::OsString;
use std::fmt;
use std::path::{Path, PathBuf};
use std::str::FromStr;
Expand All @@ -15,7 +16,8 @@ pub enum Mode {
RunPass,
RunPassValgrind,
Pretty,
DebugInfoBoth,
DebugInfoCdb,
DebugInfoGdbLldb,
DebugInfoGdb,
DebugInfoLldb,
Codegen,
Expand All @@ -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",
_ => "",
Expand All @@ -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),
Expand All @@ -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",
Expand Down Expand Up @@ -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>,

Expand Down
21 changes: 15 additions & 6 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand All @@ -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();
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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" {
Expand All @@ -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 {
Expand Down
66 changes: 59 additions & 7 deletions src/tools/compiletest/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![crate_name = "compiletest"]
#![feature(test)]
#![feature(path_buf_capacity)]
#![feature(vec_remove_item)]
#![deny(warnings, rust_2018_idioms)]

Expand All @@ -8,7 +9,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;
Expand Down Expand Up @@ -164,6 +165,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",
Expand Down Expand Up @@ -273,6 +280,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"));
Expand Down Expand Up @@ -319,6 +327,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,
Expand Down Expand Up @@ -421,7 +430,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",
Expand All @@ -440,8 +449,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!(
Expand Down Expand Up @@ -470,7 +479,8 @@ pub fn run_tests(config: &Config) {
return;
}
}
_ => { /* proceed */ }

DebugInfoCdb | _ => { /* proceed */ }
}

// FIXME(#33435) Avoid spurious failures in codegen-units/partitioning tests.
Expand Down Expand Up @@ -667,7 +677,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())
Expand Down Expand Up @@ -815,7 +825,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() {
Expand All @@ -841,6 +851,48 @@ 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) {
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::with_capacity(64);
path.push(pf86);
path.push(r"Windows Kits\10\Debuggers"); // We could check more known install locations (8.1?)
path.push(cdb_arch);
path.push(r"cdb.exe");

if path.exists() {
Some(path.into_os_string())
} else {
None
}
}
else {
None
}
}

/// 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) {
Expand Down
Loading

0 comments on commit e33c441

Please sign in to comment.