Skip to content

Commit e33c441

Browse files
MaulingMonkeyMaulingMonkey
committed
Add basic CDB support to debuginfo compiletest s, to help catch *.natvis regressions, like those fixed in #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
1 parent 26ab324 commit e33c441

File tree

8 files changed

+251
-31
lines changed

8 files changed

+251
-31
lines changed

‎src/bootstrap/test.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -976,14 +976,10 @@ impl Step for Compiletest {
976976
}
977977

978978
if suite == "debuginfo" {
979-
// Skip debuginfo tests on MSVC
980-
if builder.config.build.contains("msvc") {
981-
return;
982-
}
983-
979+
let msvc = builder.config.build.contains("msvc");
984980
if mode == "debuginfo" {
985981
return builder.ensure(Compiletest {
986-
mode: "debuginfo-both",
982+
mode: if msvc { "debuginfo-cdb" } else { "debuginfo-gdb+lldb" },
987983
..self
988984
});
989985
}

‎src/test/debuginfo/issue-13213.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// min-lldb-version: 310
2+
// ignore-cdb: Fails with exit code 0xc0000135 ("the application failed to initialize properly")
23

34
// aux-build:issue-13213-aux.rs
45

‎src/test/debuginfo/pretty-std.rs

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
// ignore-windows failing on win32 bot
21
// ignore-freebsd: gdb package too new
3-
// ignore-test // Test temporarily ignored due to debuginfo tests being disabled, see PR 47155
2+
// only-cdb // Test temporarily ignored on GDB/LLDB due to debuginfo tests being disabled, see PR 47155
43
// ignore-android: FIXME(#10381)
54
// compile-flags:-g
65
// min-gdb-version 7.7
@@ -63,6 +62,56 @@
6362
// lldb-check:[...]$5 = None
6463

6564

65+
// === CDB TESTS ==================================================================================
66+
67+
// cdb-command: g
68+
69+
// cdb-command: dx slice,d
70+
// cdb-check:slice,d [...]
71+
// NOTE: While slices have a .natvis entry that works in VS & VS Code, it fails in CDB 10.0.18362.1
72+
73+
// cdb-command: dx vec,d
74+
// cdb-check:vec,d [...] : { size=4 } [Type: [...]::Vec<u64>]
75+
// cdb-check: [size] : 4 [Type: [...]]
76+
// cdb-check: [capacity] : [...] [Type: [...]]
77+
// cdb-check: [0] : 4 [Type: unsigned __int64]
78+
// cdb-check: [1] : 5 [Type: unsigned __int64]
79+
// cdb-check: [2] : 6 [Type: unsigned __int64]
80+
// cdb-check: [3] : 7 [Type: unsigned __int64]
81+
82+
// cdb-command: dx str_slice
83+
// cdb-check:str_slice [...]
84+
// NOTE: While string slices have a .natvis entry that works in VS & VS Code, it fails in CDB 10.0.18362.1
85+
86+
// cdb-command: dx string
87+
// cdb-check:string : "IAMA string!" [Type: [...]::String]
88+
// cdb-check: [<Raw View>] [Type: [...]::String]
89+
// cdb-check: [size] : 0xc [Type: [...]]
90+
// cdb-check: [capacity] : 0xc [Type: [...]]
91+
// cdb-check: [0] : 73 'I' [Type: char]
92+
// cdb-check: [1] : 65 'A' [Type: char]
93+
// cdb-check: [2] : 77 'M' [Type: char]
94+
// cdb-check: [3] : 65 'A' [Type: char]
95+
// cdb-check: [4] : 32 ' ' [Type: char]
96+
// cdb-check: [5] : 115 's' [Type: char]
97+
// cdb-check: [6] : 116 't' [Type: char]
98+
// cdb-check: [7] : 114 'r' [Type: char]
99+
// cdb-check: [8] : 105 'i' [Type: char]
100+
// cdb-check: [9] : 110 'n' [Type: char]
101+
// cdb-check: [10] : 103 'g' [Type: char]
102+
// cdb-check: [11] : 33 '!' [Type: char]
103+
104+
// cdb-command: dx os_string
105+
// cdb-check:os_string [Type: [...]::OsString]
106+
// NOTE: OsString doesn't have a .natvis entry yet.
107+
108+
// cdb-command: dx some
109+
// cdb-check:some : { Some 8 } [Type: [...]::Option<i16>]
110+
// cdb-command: dx none
111+
// cdb-check:none : { None } [Type: [...]::Option<i64>]
112+
// cdb-command: dx some_string
113+
// cdb-check:some_string : { Some "IAMA optional string!" } [Type: [...]::Option<[...]::String>]
114+
66115
#![allow(unused_variables)]
67116
use std::ffi::OsString;
68117

‎src/test/debuginfo/should-fail.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@
1818
// lldb-command:print x
1919
// lldb-check:[...]$0 = 5
2020

21+
// === CDB TESTS ==================================================================================
22+
23+
// cdb-command:g
24+
25+
// cdb-command:dx x
26+
// cdb-check:string [...] : 5 [Type: [...]]
27+
2128
fn main() {
2229
let x = 1;
2330

‎src/tools/compiletest/src/common.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
pub use self::Mode::*;
22

3+
use std::ffi::OsString;
34
use std::fmt;
45
use std::path::{Path, PathBuf};
56
use std::str::FromStr;
@@ -15,7 +16,8 @@ pub enum Mode {
1516
RunPass,
1617
RunPassValgrind,
1718
Pretty,
18-
DebugInfoBoth,
19+
DebugInfoCdb,
20+
DebugInfoGdbLldb,
1921
DebugInfoGdb,
2022
DebugInfoLldb,
2123
Codegen,
@@ -33,9 +35,10 @@ impl Mode {
3335
pub fn disambiguator(self) -> &'static str {
3436
// Run-pass and pretty run-pass tests could run concurrently, and if they do,
3537
// they need to keep their output segregated. Same is true for debuginfo tests that
36-
// can be run both on gdb and lldb.
38+
// can be run on cdb, gdb, and lldb.
3739
match self {
3840
Pretty => ".pretty",
41+
DebugInfoCdb => ".cdb",
3942
DebugInfoGdb => ".gdb",
4043
DebugInfoLldb => ".lldb",
4144
_ => "",
@@ -52,7 +55,8 @@ impl FromStr for Mode {
5255
"run-pass" => Ok(RunPass),
5356
"run-pass-valgrind" => Ok(RunPassValgrind),
5457
"pretty" => Ok(Pretty),
55-
"debuginfo-both" => Ok(DebugInfoBoth),
58+
"debuginfo-cdb" => Ok(DebugInfoCdb),
59+
"debuginfo-gdb+lldb" => Ok(DebugInfoGdbLldb),
5660
"debuginfo-lldb" => Ok(DebugInfoLldb),
5761
"debuginfo-gdb" => Ok(DebugInfoGdb),
5862
"codegen" => Ok(Codegen),
@@ -77,7 +81,8 @@ impl fmt::Display for Mode {
7781
RunPass => "run-pass",
7882
RunPassValgrind => "run-pass-valgrind",
7983
Pretty => "pretty",
80-
DebugInfoBoth => "debuginfo-both",
84+
DebugInfoCdb => "debuginfo-cdb",
85+
DebugInfoGdbLldb => "debuginfo-gdb+lldb",
8186
DebugInfoGdb => "debuginfo-gdb",
8287
DebugInfoLldb => "debuginfo-lldb",
8388
Codegen => "codegen",
@@ -198,6 +203,9 @@ pub struct Config {
198203
/// Host triple for the compiler being invoked
199204
pub host: String,
200205

206+
/// Path to / name of the Microsoft Console Debugger (CDB) executable
207+
pub cdb: Option<OsString>,
208+
201209
/// Path to / name of the GDB executable
202210
pub gdb: Option<String>,
203211

‎src/tools/compiletest/src/header.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,9 @@ enum ParsedNameDirective {
5757
NoMatch,
5858
/// Match.
5959
Match,
60-
/// Mode was DebugInfoBoth and this matched gdb.
60+
/// Mode was DebugInfoGdbLldb and this matched gdb.
6161
MatchGdb,
62-
/// Mode was DebugInfoBoth and this matched lldb.
62+
/// Mode was DebugInfoGdbLldb and this matched lldb.
6363
MatchLldb,
6464
}
6565

@@ -81,13 +81,17 @@ impl EarlyProps {
8181
revisions: vec![],
8282
};
8383

84-
if config.mode == common::DebugInfoBoth {
84+
if config.mode == common::DebugInfoGdbLldb {
8585
if config.lldb_python_dir.is_none() {
8686
props.ignore = props.ignore.no_lldb();
8787
}
8888
if config.gdb_version.is_none() {
8989
props.ignore = props.ignore.no_gdb();
9090
}
91+
} else if config.mode == common::DebugInfoCdb {
92+
if config.cdb.is_none() {
93+
props.ignore = Ignore::Ignore;
94+
}
9195
}
9296

9397
let rustc_has_profiler_support = env::var_os("RUSTC_PROFILER_SUPPORT").is_some();
@@ -133,12 +137,12 @@ impl EarlyProps {
133137
}
134138
}
135139

136-
if (config.mode == common::DebugInfoGdb || config.mode == common::DebugInfoBoth) &&
140+
if (config.mode == common::DebugInfoGdb || config.mode == common::DebugInfoGdbLldb) &&
137141
props.ignore.can_run_gdb() && ignore_gdb(config, ln) {
138142
props.ignore = props.ignore.no_gdb();
139143
}
140144

141-
if (config.mode == common::DebugInfoLldb || config.mode == common::DebugInfoBoth) &&
145+
if (config.mode == common::DebugInfoLldb || config.mode == common::DebugInfoGdbLldb) &&
142146
props.ignore.can_run_lldb() && ignore_lldb(config, ln) {
143147
props.ignore = props.ignore.no_lldb();
144148
}
@@ -804,7 +808,7 @@ impl Config {
804808
ParsedNameDirective::Match
805809
} else {
806810
match self.mode {
807-
common::DebugInfoBoth => {
811+
common::DebugInfoGdbLldb => {
808812
if name == "gdb" {
809813
ParsedNameDirective::MatchGdb
810814
} else if name == "lldb" {
@@ -813,6 +817,11 @@ impl Config {
813817
ParsedNameDirective::NoMatch
814818
}
815819
},
820+
common::DebugInfoCdb => if name == "cdb" {
821+
ParsedNameDirective::Match
822+
} else {
823+
ParsedNameDirective::NoMatch
824+
},
816825
common::DebugInfoGdb => if name == "gdb" {
817826
ParsedNameDirective::Match
818827
} else {

‎src/tools/compiletest/src/main.rs

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#![crate_name = "compiletest"]
22
#![feature(test)]
3+
#![feature(path_buf_capacity)]
34
#![feature(vec_remove_item)]
45
#![deny(warnings, rust_2018_idioms)]
56

@@ -8,7 +9,7 @@ extern crate test;
89
use crate::common::CompareMode;
910
use crate::common::{expected_output_path, output_base_dir, output_relative_path, UI_EXTENSIONS};
1011
use crate::common::{Config, TestPaths};
11-
use crate::common::{DebugInfoBoth, DebugInfoGdb, DebugInfoLldb, Mode, Pretty};
12+
use crate::common::{DebugInfoCdb, DebugInfoGdbLldb, DebugInfoGdb, DebugInfoLldb, Mode, Pretty};
1213
use getopts::Options;
1314
use std::env;
1415
use std::ffi::OsString;
@@ -164,6 +165,12 @@ pub fn parse_config(args: Vec<String>) -> Config {
164165
.optopt("", "logfile", "file to log test execution to", "FILE")
165166
.optopt("", "target", "the target to build for", "TARGET")
166167
.optopt("", "host", "the host to build for", "HOST")
168+
.optopt(
169+
"",
170+
"cdb",
171+
"path to CDB to use for CDB debuginfo tests",
172+
"PATH",
173+
)
167174
.optopt(
168175
"",
169176
"gdb",
@@ -273,6 +280,7 @@ pub fn parse_config(args: Vec<String>) -> Config {
273280

274281
let target = opt_str2(matches.opt_str("target"));
275282
let android_cross_path = opt_path(matches, "android-cross-path");
283+
let cdb = analyze_cdb(matches.opt_str("cdb"), &target);
276284
let (gdb, gdb_version, gdb_native_rust) = analyze_gdb(matches.opt_str("gdb"), &target,
277285
&android_cross_path);
278286
let (lldb_version, lldb_native_rust) = extract_lldb_version(matches.opt_str("lldb-version"));
@@ -319,6 +327,7 @@ pub fn parse_config(args: Vec<String>) -> Config {
319327
target_rustcflags: matches.opt_str("target-rustcflags"),
320328
target: target,
321329
host: opt_str2(matches.opt_str("host")),
330+
cdb,
322331
gdb,
323332
gdb_version,
324333
gdb_native_rust,
@@ -421,7 +430,7 @@ pub fn opt_str2(maybestr: Option<String>) -> String {
421430

422431
pub fn run_tests(config: &Config) {
423432
if config.target.contains("android") {
424-
if config.mode == DebugInfoGdb || config.mode == DebugInfoBoth {
433+
if config.mode == DebugInfoGdb || config.mode == DebugInfoGdbLldb {
425434
println!(
426435
"{} debug-info test uses tcp 5039 port.\
427436
please reserve it",
@@ -440,8 +449,8 @@ pub fn run_tests(config: &Config) {
440449

441450
match config.mode {
442451
// Note that we don't need to emit the gdb warning when
443-
// DebugInfoBoth, so it is ok to list that here.
444-
DebugInfoBoth | DebugInfoLldb => {
452+
// DebugInfoGdbLldb, so it is ok to list that here.
453+
DebugInfoGdbLldb | DebugInfoLldb => {
445454
if let Some(lldb_version) = config.lldb_version.as_ref() {
446455
if is_blacklisted_lldb_version(&lldb_version[..]) {
447456
println!(
@@ -470,7 +479,8 @@ pub fn run_tests(config: &Config) {
470479
return;
471480
}
472481
}
473-
_ => { /* proceed */ }
482+
483+
DebugInfoCdb | _ => { /* proceed */ }
474484
}
475485

476486
// FIXME(#33435) Avoid spurious failures in codegen-units/partitioning tests.
@@ -667,7 +677,7 @@ pub fn make_test(config: &Config, testpaths: &TestPaths) -> Vec<test::TestDescAn
667677
&early_props,
668678
revision.map(|s| s.as_str()),
669679
)
670-
|| ((config.mode == DebugInfoBoth ||
680+
|| ((config.mode == DebugInfoGdbLldb || config.mode == DebugInfoCdb ||
671681
config.mode == DebugInfoGdb || config.mode == DebugInfoLldb)
672682
&& config.target.contains("emscripten"))
673683
|| (config.mode == DebugInfoGdb && !early_props.ignore.can_run_gdb())
@@ -815,7 +825,7 @@ fn make_test_closure(
815825
revision: Option<&String>,
816826
) -> test::TestFn {
817827
let mut config = config.clone();
818-
if config.mode == DebugInfoBoth {
828+
if config.mode == DebugInfoGdbLldb {
819829
// If both gdb and lldb were ignored, then the test as a whole
820830
// would be ignored.
821831
if !ignore.can_run_gdb() {
@@ -841,6 +851,48 @@ fn is_android_gdb_target(target: &String) -> bool {
841851
}
842852
}
843853

854+
/// Returns `true` if the given target is a MSVC target for the purpouses of CDB testing.
855+
fn is_pc_windows_msvc_target(target: &String) -> bool {
856+
target.ends_with("-pc-windows-msvc")
857+
}
858+
859+
fn find_cdb(target: &String) -> Option<OsString> {
860+
if cfg!(windows) && is_pc_windows_msvc_target(target) {
861+
let pf86 = env::var_os("ProgramFiles(x86)").or(env::var_os("ProgramFiles"))?;
862+
let cdb_arch = if cfg!(target_arch="x86") {
863+
"x86"
864+
} else if cfg!(target_arch="x86_64") {
865+
"x64"
866+
} else if cfg!(target_arch="aarch64") {
867+
"arm64"
868+
} else if cfg!(target_arch="arm") {
869+
"arm"
870+
} else {
871+
return None; // No compatible CDB.exe in the Windows 10 SDK
872+
};
873+
874+
let mut path = PathBuf::with_capacity(64);
875+
path.push(pf86);
876+
path.push(r"Windows Kits\10\Debuggers"); // We could check more known install locations (8.1?)
877+
path.push(cdb_arch);
878+
path.push(r"cdb.exe");
879+
880+
if path.exists() {
881+
Some(path.into_os_string())
882+
} else {
883+
None
884+
}
885+
}
886+
else {
887+
None
888+
}
889+
}
890+
891+
/// Returns Path to CDB
892+
fn analyze_cdb(cdb: Option<String>, target: &String) -> Option<OsString> {
893+
cdb.map(|s| OsString::from(s)).or(find_cdb(target))
894+
}
895+
844896
/// Returns (Path to GDB, GDB Version, GDB has Rust Support)
845897
fn analyze_gdb(gdb: Option<String>, target: &String, android_cross_path: &PathBuf)
846898
-> (Option<String>, Option<u32>, bool) {

0 commit comments

Comments
 (0)