Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 23, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 // "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

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

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)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this use existing stable methods instead of adding a new unstable feature?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Can't reserve capacity without it, but that's an unimportant optimization at best.

#![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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be inverted to reduce indentation? e.g.:

if !current_condition {
    return None;
}

// ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

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 8.1 etc. too?
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