From e33c4415b3a504bd723328a38bce0f14786db61b Mon Sep 17 00:00:00 2001 From: MaulingMonkey Date: Sun, 19 May 2019 17:10:48 -0700 Subject: [PATCH 1/3] 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 --- src/bootstrap/test.rs | 8 +- src/test/debuginfo/issue-13213.rs | 1 + src/test/debuginfo/pretty-std.rs | 53 ++++++++++++- src/test/debuginfo/should-fail.rs | 7 ++ src/tools/compiletest/src/common.rs | 16 +++- src/tools/compiletest/src/header.rs | 21 +++-- src/tools/compiletest/src/main.rs | 66 ++++++++++++++-- src/tools/compiletest/src/runtest.rs | 110 +++++++++++++++++++++++++-- 8 files changed, 251 insertions(+), 31 deletions(-) diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 7826ac9471806..40105e0911738 100644 --- a/src/bootstrap/test.rs +++ b/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 }); } diff --git a/src/test/debuginfo/issue-13213.rs b/src/test/debuginfo/issue-13213.rs index 3c9a365fd4f7b..393478460d48f 100644 --- a/src/test/debuginfo/issue-13213.rs +++ b/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 diff --git a/src/test/debuginfo/pretty-std.rs b/src/test/debuginfo/pretty-std.rs index 82802eff08abb..f568371d68a1a 100644 --- a/src/test/debuginfo/pretty-std.rs +++ b/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 // 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 @@ -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] +// 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: [] [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] +// cdb-command: dx none +// cdb-check:none : { None } [Type: [...]::Option] +// cdb-command: dx some_string +// cdb-check:some_string : { Some "IAMA optional string!" } [Type: [...]::Option<[...]::String>] + #![allow(unused_variables)] use std::ffi::OsString; diff --git a/src/test/debuginfo/should-fail.rs b/src/test/debuginfo/should-fail.rs index 8765c018b1022..1e0d22cbce404 100644 --- a/src/test/debuginfo/should-fail.rs +++ b/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; diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index 4699dee1716a9..722979c3c1402 100644 --- a/src/tools/compiletest/src/common.rs +++ b/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, + /// Path to / name of the GDB executable pub gdb: Option, diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index dbc477585cbfc..ab5594f36050d 100644 --- a/src/tools/compiletest/src/header.rs +++ b/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 { diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 442e58bfd74e1..631ed067c341a 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -1,5 +1,6 @@ #![crate_name = "compiletest"] #![feature(test)] +#![feature(path_buf_capacity)] #![feature(vec_remove_item)] #![deny(warnings, rust_2018_idioms)] @@ -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; @@ -164,6 +165,12 @@ pub fn parse_config(args: Vec) -> 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 +280,7 @@ pub fn parse_config(args: Vec) -> 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 +327,7 @@ pub fn parse_config(args: Vec) -> 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 +430,7 @@ pub fn opt_str2(maybestr: Option) -> 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 +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!( @@ -470,7 +479,8 @@ pub fn run_tests(config: &Config) { return; } } - _ => { /* proceed */ } + + DebugInfoCdb | _ => { /* proceed */ } } // FIXME(#33435) Avoid spurious failures in codegen-units/partitioning tests. @@ -667,7 +677,7 @@ pub fn make_test(config: &Config, testpaths: &TestPaths) -> Vec, ) -> 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 +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 { + 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, target: &String) -> Option { + 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, target: &String, android_cross_path: &PathBuf) -> (Option, Option, bool) { diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 2082de7cbce34..c0dedccdff8a3 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3,7 +3,7 @@ use crate::common::CompareMode; use crate::common::{expected_output_path, UI_EXTENSIONS, UI_FIXED, UI_STDERR, UI_STDOUT}; use crate::common::{output_base_dir, output_base_name, output_testname_unique}; -use crate::common::{Codegen, CodegenUnits, DebugInfoBoth, DebugInfoGdb, DebugInfoLldb, Rustdoc}; +use crate::common::{Codegen, CodegenUnits, DebugInfoCdb, DebugInfoGdbLldb, DebugInfoGdb, DebugInfoLldb, Rustdoc}; use crate::common::{CompileFail, Pretty, RunFail, RunPass, RunPassValgrind}; use crate::common::{Config, TestPaths}; use crate::common::{Incremental, MirOpt, RunMake, Ui, JsDocTest, Assembly}; @@ -242,7 +242,15 @@ pub fn compute_stamp_hash(config: &Config) -> String { let mut hash = DefaultHasher::new(); config.stage_id.hash(&mut hash); - if config.mode == DebugInfoGdb || config.mode == DebugInfoBoth { + if config.mode == DebugInfoCdb { + match config.cdb { + None => env::var_os("ProgramFiles(x86)").hash(&mut hash), + Some(ref s) if s.is_empty() => env::var_os("ProgramFiles(x86)").hash(&mut hash), + Some(ref s) => s.hash(&mut hash), + } + } + + if config.mode == DebugInfoGdb || config.mode == DebugInfoGdbLldb { match config.gdb { None => env::var_os("PATH").hash(&mut hash), Some(ref s) if s.is_empty() => env::var_os("PATH").hash(&mut hash), @@ -250,7 +258,7 @@ pub fn compute_stamp_hash(config: &Config) -> String { }; } - if config.mode == DebugInfoLldb || config.mode == DebugInfoBoth { + if config.mode == DebugInfoLldb || config.mode == DebugInfoGdbLldb { env::var_os("PATH").hash(&mut hash); env::var_os("PYTHONPATH").hash(&mut hash); } @@ -285,10 +293,11 @@ impl<'test> TestCx<'test> { RunFail => self.run_rfail_test(), RunPassValgrind => self.run_valgrind_test(), Pretty => self.run_pretty_test(), - DebugInfoBoth => { + DebugInfoGdbLldb => { self.run_debuginfo_gdb_test(); self.run_debuginfo_lldb_test(); }, + DebugInfoCdb => self.run_debuginfo_cdb_test(), DebugInfoGdb => self.run_debuginfo_gdb_test(), DebugInfoLldb => self.run_debuginfo_lldb_test(), Codegen => self.run_codegen_test(), @@ -656,6 +665,95 @@ impl<'test> TestCx<'test> { self.compose_and_run_compiler(rustc, Some(src)) } + fn run_debuginfo_cdb_test(&self) { + assert!(self.revision.is_none(), "revisions not relevant here"); + + let config = Config { + target_rustcflags: self.cleanup_debug_info_options(&self.config.target_rustcflags), + host_rustcflags: self.cleanup_debug_info_options(&self.config.host_rustcflags), + mode: DebugInfoCdb, + ..self.config.clone() + }; + + let test_cx = TestCx { + config: &config, + ..*self + }; + + test_cx.run_debuginfo_cdb_test_no_opt(); + } + + fn run_debuginfo_cdb_test_no_opt(&self) { + // compile test file (it should have 'compile-flags:-g' in the header) + let compile_result = self.compile_test(); + if !compile_result.status.success() { + self.fatal_proc_rec("compilation failed!", &compile_result); + } + + let exe_file = self.make_exe_name(); + + let prefixes = { + static PREFIXES: &'static [&'static str] = &["cdb", "cdbg"]; + // No "native rust support" variation for CDB yet. + PREFIXES + }; + + // Parse debugger commands etc from test files + let DebuggerCommands { + commands, + check_lines, + breakpoint_lines, + .. + } = self.parse_debugger_commands(prefixes); + + // https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/debugger-commands + let mut script_str = String::with_capacity(2048); + script_str.push_str("version\n"); // List CDB (and more) version info in test output + script_str.push_str(".nvlist\n"); // List loaded `*.natvis` files (bulk of MSVC debugger customizations) + + // Set breakpoints on every line that contains the string "#break" + let source_file_name = self.testpaths.file.file_name().unwrap().to_string_lossy(); + for line in &breakpoint_lines { + script_str.push_str(&format!( + "bp `{}:{}`\n", + source_file_name, line + )); + } + + // Append the other `cdb-command:`s + for line in &commands { + script_str.push_str(line); + script_str.push_str("\n"); + } + + script_str.push_str("\nqq\n"); // Quit the debugger (including remote debugger, if any) + + // Write the script into a file + debug!("script_str = {}", script_str); + self.dump_output_file(&script_str, "debugger.script"); + let debugger_script = self.make_out_name("debugger.script"); + + let cdb_path = &self.config.cdb.as_ref().unwrap(); + let mut cdb = Command::new(cdb_path); + cdb + .arg("-lines") // Enable source line debugging. + .arg("-cf").arg(&debugger_script) + .arg(&exe_file); + + let debugger_run_result = self.compose_and_run( + cdb, + self.config.run_lib_path.to_str().unwrap(), + None, // aux_path + None // input + ); + + if !debugger_run_result.status.success() { + self.fatal_proc_rec("Error while running CDB", &debugger_run_result); + } + + self.check_debugger_output(&debugger_run_result, &check_lines); + } + fn run_debuginfo_gdb_test(&self) { assert!(self.revision.is_none(), "revisions not relevant here"); @@ -1429,7 +1527,7 @@ impl<'test> TestCx<'test> { RunPass | Ui => self.should_run_successfully(), Incremental => self.revision.unwrap().starts_with("r"), RunFail | RunPassValgrind | MirOpt | - DebugInfoBoth | DebugInfoGdb | DebugInfoLldb => true, + DebugInfoCdb | DebugInfoGdbLldb | DebugInfoGdb | DebugInfoLldb => true, _ => false, }; let output_file = if will_execute { @@ -1870,7 +1968,7 @@ impl<'test> TestCx<'test> { rustc.arg(dir_opt); } - RunFail | RunPassValgrind | Pretty | DebugInfoBoth | DebugInfoGdb | DebugInfoLldb + RunFail | RunPassValgrind | Pretty | DebugInfoCdb | DebugInfoGdbLldb | DebugInfoGdb | DebugInfoLldb | Codegen | Rustdoc | RunMake | CodegenUnits | JsDocTest | Assembly => { // do not use JSON output } From 0a423a70bb303195f09753dfdc5a7c4e149e29ff Mon Sep 17 00:00:00 2001 From: MaulingMonkey Date: Mon, 20 May 2019 02:44:26 -0700 Subject: [PATCH 2/3] Fix CDB support tidy check line length failures. --- src/test/debuginfo/pretty-std.rs | 6 +++--- src/tools/compiletest/src/main.rs | 2 +- src/tools/compiletest/src/runtest.rs | 9 +++++---- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/test/debuginfo/pretty-std.rs b/src/test/debuginfo/pretty-std.rs index f568371d68a1a..a684d3b88fd07 100644 --- a/src/test/debuginfo/pretty-std.rs +++ b/src/test/debuginfo/pretty-std.rs @@ -1,5 +1,5 @@ // ignore-freebsd: gdb package too new -// only-cdb // Test temporarily ignored on GDB/LLDB 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 @@ -81,7 +81,7 @@ // 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 +// 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] @@ -110,7 +110,7 @@ // cdb-command: dx none // cdb-check:none : { None } [Type: [...]::Option] // cdb-command: dx some_string -// cdb-check:some_string : { Some "IAMA optional string!" } [Type: [...]::Option<[...]::String>] +// cdb-check:some_string : { Some "IAMA optional string!" } [[...]::Option<[...]::String>] #![allow(unused_variables)] use std::ffi::OsString; diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 631ed067c341a..48ecd5a914223 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -873,7 +873,7 @@ fn find_cdb(target: &String) -> Option { 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(r"Windows Kits\10\Debuggers"); // We could check 8.1 etc. too? path.push(cdb_arch); path.push(r"cdb.exe"); diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index c0dedccdff8a3..c940e2de665ab 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3,7 +3,8 @@ use crate::common::CompareMode; use crate::common::{expected_output_path, UI_EXTENSIONS, UI_FIXED, UI_STDERR, UI_STDOUT}; use crate::common::{output_base_dir, output_base_name, output_testname_unique}; -use crate::common::{Codegen, CodegenUnits, DebugInfoCdb, DebugInfoGdbLldb, DebugInfoGdb, DebugInfoLldb, Rustdoc}; +use crate::common::{Codegen, CodegenUnits, Rustdoc}; +use crate::common::{DebugInfoCdb, DebugInfoGdbLldb, DebugInfoGdb, DebugInfoLldb}; use crate::common::{CompileFail, Pretty, RunFail, RunPass, RunPassValgrind}; use crate::common::{Config, TestPaths}; use crate::common::{Incremental, MirOpt, RunMake, Ui, JsDocTest, Assembly}; @@ -709,7 +710,7 @@ impl<'test> TestCx<'test> { // https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/debugger-commands let mut script_str = String::with_capacity(2048); script_str.push_str("version\n"); // List CDB (and more) version info in test output - script_str.push_str(".nvlist\n"); // List loaded `*.natvis` files (bulk of MSVC debugger customizations) + script_str.push_str(".nvlist\n"); // List loaded `*.natvis` files, bulk of custom MSVC debug // Set breakpoints on every line that contains the string "#break" let source_file_name = self.testpaths.file.file_name().unwrap().to_string_lossy(); @@ -1968,8 +1969,8 @@ impl<'test> TestCx<'test> { rustc.arg(dir_opt); } - RunFail | RunPassValgrind | Pretty | DebugInfoCdb | DebugInfoGdbLldb | DebugInfoGdb | DebugInfoLldb - | Codegen | Rustdoc | RunMake | CodegenUnits | JsDocTest | Assembly => { + RunFail | RunPassValgrind | Pretty | DebugInfoCdb | DebugInfoGdbLldb | DebugInfoGdb + | DebugInfoLldb | Codegen | Rustdoc | RunMake | CodegenUnits | JsDocTest | Assembly => { // do not use JSON output } } From 56b18ce6370a7e3376fce2d1c404532a887eb5b6 Mon Sep 17 00:00:00 2001 From: MaulingMonkey Date: Mon, 20 May 2019 15:00:36 -0700 Subject: [PATCH 3/3] Address CDB review feedback - Don't add path_buf_capacity feature. - Convert `find_cdb` to early return style, reducing indentation - Simplify `compute_stamp_hash` for CDB to just hash it's path, if any. --- src/tools/compiletest/src/main.rs | 52 +++++++++++++--------------- src/tools/compiletest/src/runtest.rs | 6 +--- 2 files changed, 26 insertions(+), 32 deletions(-) diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 48ecd5a914223..d0dc9d11d3963 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -1,6 +1,5 @@ #![crate_name = "compiletest"] #![feature(test)] -#![feature(path_buf_capacity)] #![feature(vec_remove_item)] #![deny(warnings, rust_2018_idioms)] @@ -857,35 +856,34 @@ fn is_pc_windows_msvc_target(target: &String) -> bool { } fn find_cdb(target: &String) -> Option { - 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 - }; + 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::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"); + 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() { - Some(path.into_os_string()) - } else { - None - } - } - else { - None + if !path.exists() { + return None; } + + Some(path.into_os_string()) } /// Returns Path to CDB diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index c940e2de665ab..d87bd66a1ac70 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -244,11 +244,7 @@ pub fn compute_stamp_hash(config: &Config) -> String { config.stage_id.hash(&mut hash); if config.mode == DebugInfoCdb { - match config.cdb { - None => env::var_os("ProgramFiles(x86)").hash(&mut hash), - Some(ref s) if s.is_empty() => env::var_os("ProgramFiles(x86)").hash(&mut hash), - Some(ref s) => s.hash(&mut hash), - } + config.cdb.hash(&mut hash); } if config.mode == DebugInfoGdb || config.mode == DebugInfoGdbLldb {