From e545dc9e2c1fe9852032ee753c0006b25e164d38 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Fri, 28 Sep 2018 10:32:59 -0600 Subject: [PATCH] Compute Android gdb version in compiletest compiletest has special code for running gdb for Android targets. In particular it computes a different path to gdb. However, this gdb is not used for the version test, which results in some tests being run when they should not be. You can see this in #54004. This patch moves the special case to analyze_gdb and a new helper function to decide whether the case applies. This causes the version check to work properly. Note that the bulk of the runtest.rs change is just reindentation caused by moving from a "match" to an "if" -- but there is a (small) change buried in there. --- src/tools/compiletest/src/main.rs | 44 ++- src/tools/compiletest/src/runtest.rs | 384 +++++++++++++-------------- 2 files changed, 225 insertions(+), 203 deletions(-) diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index a5cf45baa653a..2fa459bec9455 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -278,7 +278,10 @@ pub fn parse_config(args: Vec) -> Config { } } - let (gdb, gdb_version, gdb_native_rust) = analyze_gdb(matches.opt_str("gdb")); + let target = opt_str2(matches.opt_str("target")); + let android_cross_path = opt_path(matches, "android-cross-path"); + let (gdb, gdb_version, gdb_native_rust) = analyze_gdb(matches.opt_str("gdb"), &target, + &android_cross_path); let color = match matches.opt_str("color").as_ref().map(|x| &**x) { Some("auto") | None => ColorConfig::AutoColor, @@ -318,7 +321,7 @@ pub fn parse_config(args: Vec) -> Config { runtool: matches.opt_str("runtool"), host_rustcflags: matches.opt_str("host-rustcflags"), target_rustcflags: matches.opt_str("target-rustcflags"), - target: opt_str2(matches.opt_str("target")), + target: target, host: opt_str2(matches.opt_str("host")), gdb, gdb_version, @@ -326,7 +329,7 @@ pub fn parse_config(args: Vec) -> Config { lldb_version: extract_lldb_version(matches.opt_str("lldb-version")), llvm_version: matches.opt_str("llvm-version"), system_llvm: matches.opt_present("system-llvm"), - android_cross_path: opt_path(matches, "android-cross-path"), + android_cross_path: android_cross_path, adb_path: opt_str2(matches.opt_str("adb-path")), adb_test_dir: opt_str2(matches.opt_str("adb-test-dir")), adb_device_status: opt_str2(matches.opt_str("target")).contains("android") @@ -780,8 +783,18 @@ fn make_test_closure( })) } +/// Returns true if the given target is an Android target for the +/// purposes of GDB testing. +fn is_android_gdb_target(target: &String) -> bool { + match &target[..] { + "arm-linux-androideabi" | "armv7-linux-androideabi" | "aarch64-linux-android" => true, + _ => false, + } +} + /// Returns (Path to GDB, GDB Version, GDB has Rust Support) -fn analyze_gdb(gdb: Option) -> (Option, Option, bool) { +fn analyze_gdb(gdb: Option, target: &String, android_cross_path: &PathBuf) + -> (Option, Option, bool) { #[cfg(not(windows))] const GDB_FALLBACK: &str = "gdb"; #[cfg(windows)] @@ -789,14 +802,27 @@ fn analyze_gdb(gdb: Option) -> (Option, Option, bool) { const MIN_GDB_WITH_RUST: u32 = 7011010; + let fallback_gdb = || { + if is_android_gdb_target(target) { + let mut gdb_path = match android_cross_path.to_str() { + Some(x) => x.to_owned(), + None => panic!("cannot find android cross path"), + }; + gdb_path.push_str("/bin/gdb"); + gdb_path + } else { + GDB_FALLBACK.to_owned() + } + }; + let gdb = match gdb { - None => GDB_FALLBACK, - Some(ref s) if s.is_empty() => GDB_FALLBACK, // may be empty if configure found no gdb - Some(ref s) => s, + None => fallback_gdb(), + Some(ref s) if s.is_empty() => fallback_gdb(), // may be empty if configure found no gdb + Some(ref s) => s.to_owned(), }; let mut version_line = None; - if let Ok(output) = Command::new(gdb).arg("--version").output() { + if let Ok(output) = Command::new(&gdb).arg("--version").output() { if let Some(first_line) = String::from_utf8_lossy(&output.stdout).lines().next() { version_line = Some(first_line.to_string()); } @@ -809,7 +835,7 @@ fn analyze_gdb(gdb: Option) -> (Option, Option, bool) { let gdb_native_rust = version.map_or(false, |v| v >= MIN_GDB_WITH_RUST); - (Some(gdb.to_owned()), version, gdb_native_rust) + (Some(gdb), version, gdb_native_rust) } fn extract_gdb_version(full_version_line: &str) -> Option { diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 306d9f8d852b3..de3db96155bb6 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -38,6 +38,7 @@ use std::process::{Child, Command, ExitStatus, Output, Stdio}; use std::str; use extract_gdb_version; +use is_android_gdb_target; #[cfg(windows)] fn disable_error_reporting R, R>(f: F) -> R { @@ -666,222 +667,217 @@ impl<'test> TestCx<'test> { let exe_file = self.make_exe_name(); let debugger_run_result; - match &*self.config.target { - "arm-linux-androideabi" | "armv7-linux-androideabi" | "aarch64-linux-android" => { - cmds = cmds.replace("run", "continue"); + if is_android_gdb_target(&self.config.target) { + cmds = cmds.replace("run", "continue"); - let tool_path = match self.config.android_cross_path.to_str() { - Some(x) => x.to_owned(), - None => self.fatal("cannot find android cross path"), - }; + let tool_path = match self.config.android_cross_path.to_str() { + Some(x) => x.to_owned(), + None => self.fatal("cannot find android cross path"), + }; - // write debugger script - let mut script_str = String::with_capacity(2048); - script_str.push_str(&format!("set charset {}\n", Self::charset())); - script_str.push_str(&format!("set sysroot {}\n", tool_path)); - script_str.push_str(&format!("file {}\n", exe_file.to_str().unwrap())); - script_str.push_str("target remote :5039\n"); - script_str.push_str(&format!( - "set solib-search-path \ - ./{}/stage2/lib/rustlib/{}/lib/\n", - self.config.host, self.config.target - )); - for line in &breakpoint_lines { - script_str.push_str( - &format!( - "break {:?}:{}\n", - self.testpaths.file.file_name().unwrap().to_string_lossy(), - *line - )[..], - ); - } - script_str.push_str(&cmds); - script_str.push_str("\nquit\n"); - - debug!("script_str = {}", script_str); - self.dump_output_file(&script_str, "debugger.script"); - - let adb_path = &self.config.adb_path; - - Command::new(adb_path) - .arg("push") - .arg(&exe_file) - .arg(&self.config.adb_test_dir) - .status() - .expect(&format!("failed to exec `{:?}`", adb_path)); - - Command::new(adb_path) - .args(&["forward", "tcp:5039", "tcp:5039"]) - .status() - .expect(&format!("failed to exec `{:?}`", adb_path)); - - let adb_arg = format!( - "export LD_LIBRARY_PATH={}; \ - gdbserver{} :5039 {}/{}", - self.config.adb_test_dir.clone(), - if self.config.target.contains("aarch64") { - "64" - } else { - "" - }, - self.config.adb_test_dir.clone(), - exe_file.file_name().unwrap().to_str().unwrap() + // write debugger script + let mut script_str = String::with_capacity(2048); + script_str.push_str(&format!("set charset {}\n", Self::charset())); + script_str.push_str(&format!("set sysroot {}\n", tool_path)); + script_str.push_str(&format!("file {}\n", exe_file.to_str().unwrap())); + script_str.push_str("target remote :5039\n"); + script_str.push_str(&format!( + "set solib-search-path \ + ./{}/stage2/lib/rustlib/{}/lib/\n", + self.config.host, self.config.target + )); + for line in &breakpoint_lines { + script_str.push_str( + &format!( + "break {:?}:{}\n", + self.testpaths.file.file_name().unwrap().to_string_lossy(), + *line + )[..], ); + } + script_str.push_str(&cmds); + script_str.push_str("\nquit\n"); - debug!("adb arg: {}", adb_arg); - let mut adb = Command::new(adb_path) - .args(&["shell", &adb_arg]) - .stdout(Stdio::piped()) - .stderr(Stdio::inherit()) - .spawn() - .expect(&format!("failed to exec `{:?}`", adb_path)); - - // Wait for the gdbserver to print out "Listening on port ..." - // at which point we know that it's started and then we can - // execute the debugger below. - let mut stdout = BufReader::new(adb.stdout.take().unwrap()); - let mut line = String::new(); - loop { - line.truncate(0); - stdout.read_line(&mut line).unwrap(); - if line.starts_with("Listening on port 5039") { - break; - } - } - drop(stdout); - - let debugger_script = self.make_out_name("debugger.script"); - // FIXME (#9639): This needs to handle non-utf8 paths - let debugger_opts = vec![ - "-quiet".to_owned(), - "-batch".to_owned(), - "-nx".to_owned(), - format!("-command={}", debugger_script.to_str().unwrap()), - ]; - - let mut gdb_path = tool_path; - gdb_path.push_str("/bin/gdb"); - let Output { - status, - stdout, - stderr, - } = Command::new(&gdb_path) - .args(&debugger_opts) - .output() - .expect(&format!("failed to exec `{:?}`", gdb_path)); - let cmdline = { - let mut gdb = Command::new(&format!("{}-gdb", self.config.target)); - gdb.args(&debugger_opts); - let cmdline = self.make_cmdline(&gdb, ""); - logv(self.config, format!("executing {}", cmdline)); - cmdline - }; + debug!("script_str = {}", script_str); + self.dump_output_file(&script_str, "debugger.script"); - debugger_run_result = ProcRes { - status, - stdout: String::from_utf8(stdout).unwrap(), - stderr: String::from_utf8(stderr).unwrap(), - cmdline, - }; - if adb.kill().is_err() { - println!("Adb process is already finished."); + let adb_path = &self.config.adb_path; + + Command::new(adb_path) + .arg("push") + .arg(&exe_file) + .arg(&self.config.adb_test_dir) + .status() + .expect(&format!("failed to exec `{:?}`", adb_path)); + + Command::new(adb_path) + .args(&["forward", "tcp:5039", "tcp:5039"]) + .status() + .expect(&format!("failed to exec `{:?}`", adb_path)); + + let adb_arg = format!( + "export LD_LIBRARY_PATH={}; \ + gdbserver{} :5039 {}/{}", + self.config.adb_test_dir.clone(), + if self.config.target.contains("aarch64") { + "64" + } else { + "" + }, + self.config.adb_test_dir.clone(), + exe_file.file_name().unwrap().to_str().unwrap() + ); + + debug!("adb arg: {}", adb_arg); + let mut adb = Command::new(adb_path) + .args(&["shell", &adb_arg]) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) + .spawn() + .expect(&format!("failed to exec `{:?}`", adb_path)); + + // Wait for the gdbserver to print out "Listening on port ..." + // at which point we know that it's started and then we can + // execute the debugger below. + let mut stdout = BufReader::new(adb.stdout.take().unwrap()); + let mut line = String::new(); + loop { + line.truncate(0); + stdout.read_line(&mut line).unwrap(); + if line.starts_with("Listening on port 5039") { + break; } } + drop(stdout); - _ => { - let rust_src_root = self - .config - .find_rust_src_root() - .expect("Could not find Rust source root"); - let rust_pp_module_rel_path = Path::new("./src/etc"); - let rust_pp_module_abs_path = rust_src_root - .join(rust_pp_module_rel_path) - .to_str() - .unwrap() - .to_owned(); - // write debugger script - let mut script_str = String::with_capacity(2048); - script_str.push_str(&format!("set charset {}\n", Self::charset())); - script_str.push_str("show version\n"); - - match self.config.gdb_version { - Some(version) => { - println!( - "NOTE: compiletest thinks it is using GDB version {}", - version - ); + let debugger_script = self.make_out_name("debugger.script"); + // FIXME (#9639): This needs to handle non-utf8 paths + let debugger_opts = vec![ + "-quiet".to_owned(), + "-batch".to_owned(), + "-nx".to_owned(), + format!("-command={}", debugger_script.to_str().unwrap()), + ]; - if version > extract_gdb_version("7.4").unwrap() { - // Add the directory containing the pretty printers to - // GDB's script auto loading safe path - script_str.push_str(&format!( - "add-auto-load-safe-path {}\n", - rust_pp_module_abs_path.replace(r"\", r"\\") - )); - } - } - _ => { - println!( - "NOTE: compiletest does not know which version of \ - GDB it is using" - ); + let gdb_path = self.config.gdb.as_ref().unwrap(); + let Output { + status, + stdout, + stderr, + } = Command::new(&gdb_path) + .args(&debugger_opts) + .output() + .expect(&format!("failed to exec `{:?}`", gdb_path)); + let cmdline = { + let mut gdb = Command::new(&format!("{}-gdb", self.config.target)); + gdb.args(&debugger_opts); + let cmdline = self.make_cmdline(&gdb, ""); + logv(self.config, format!("executing {}", cmdline)); + cmdline + }; + + debugger_run_result = ProcRes { + status, + stdout: String::from_utf8(stdout).unwrap(), + stderr: String::from_utf8(stderr).unwrap(), + cmdline, + }; + if adb.kill().is_err() { + println!("Adb process is already finished."); + } + } else { + let rust_src_root = self + .config + .find_rust_src_root() + .expect("Could not find Rust source root"); + let rust_pp_module_rel_path = Path::new("./src/etc"); + let rust_pp_module_abs_path = rust_src_root + .join(rust_pp_module_rel_path) + .to_str() + .unwrap() + .to_owned(); + // write debugger script + let mut script_str = String::with_capacity(2048); + script_str.push_str(&format!("set charset {}\n", Self::charset())); + script_str.push_str("show version\n"); + + match self.config.gdb_version { + Some(version) => { + println!( + "NOTE: compiletest thinks it is using GDB version {}", + version + ); + + if version > extract_gdb_version("7.4").unwrap() { + // Add the directory containing the pretty printers to + // GDB's script auto loading safe path + script_str.push_str(&format!( + "add-auto-load-safe-path {}\n", + rust_pp_module_abs_path.replace(r"\", r"\\") + )); } } + _ => { + println!( + "NOTE: compiletest does not know which version of \ + GDB it is using" + ); + } + } - // The following line actually doesn't have to do anything with - // pretty printing, it just tells GDB to print values on one line: - script_str.push_str("set print pretty off\n"); + // The following line actually doesn't have to do anything with + // pretty printing, it just tells GDB to print values on one line: + script_str.push_str("set print pretty off\n"); - // Add the pretty printer directory to GDB's source-file search path - script_str.push_str(&format!("directory {}\n", rust_pp_module_abs_path)); + // Add the pretty printer directory to GDB's source-file search path + script_str.push_str(&format!("directory {}\n", rust_pp_module_abs_path)); - // Load the target executable - script_str.push_str(&format!( - "file {}\n", - exe_file.to_str().unwrap().replace(r"\", r"\\") - )); + // Load the target executable + script_str.push_str(&format!( + "file {}\n", + exe_file.to_str().unwrap().replace(r"\", r"\\") + )); - // Force GDB to print values in the Rust format. - if self.config.gdb_native_rust { - script_str.push_str("set language rust\n"); - } + // Force GDB to print values in the Rust format. + if self.config.gdb_native_rust { + script_str.push_str("set language rust\n"); + } - // Add line breakpoints - for line in &breakpoint_lines { - script_str.push_str(&format!( - "break '{}':{}\n", - self.testpaths.file.file_name().unwrap().to_string_lossy(), - *line - )); - } + // Add line breakpoints + for line in &breakpoint_lines { + script_str.push_str(&format!( + "break '{}':{}\n", + self.testpaths.file.file_name().unwrap().to_string_lossy(), + *line + )); + } - script_str.push_str(&cmds); - script_str.push_str("\nquit\n"); + script_str.push_str(&cmds); + script_str.push_str("\nquit\n"); - debug!("script_str = {}", script_str); - self.dump_output_file(&script_str, "debugger.script"); + debug!("script_str = {}", script_str); + self.dump_output_file(&script_str, "debugger.script"); - let debugger_script = self.make_out_name("debugger.script"); + let debugger_script = self.make_out_name("debugger.script"); - // FIXME (#9639): This needs to handle non-utf8 paths - let debugger_opts = vec![ - "-quiet".to_owned(), - "-batch".to_owned(), - "-nx".to_owned(), - format!("-command={}", debugger_script.to_str().unwrap()), - ]; + // FIXME (#9639): This needs to handle non-utf8 paths + let debugger_opts = vec![ + "-quiet".to_owned(), + "-batch".to_owned(), + "-nx".to_owned(), + format!("-command={}", debugger_script.to_str().unwrap()), + ]; - let mut gdb = Command::new(self.config.gdb.as_ref().unwrap()); - gdb.args(&debugger_opts) - .env("PYTHONPATH", rust_pp_module_abs_path); + let mut gdb = Command::new(self.config.gdb.as_ref().unwrap()); + gdb.args(&debugger_opts) + .env("PYTHONPATH", rust_pp_module_abs_path); - debugger_run_result = self.compose_and_run( - gdb, - self.config.run_lib_path.to_str().unwrap(), - None, - None, - ); - } + debugger_run_result = self.compose_and_run( + gdb, + self.config.run_lib_path.to_str().unwrap(), + None, + None, + ); } if !debugger_run_result.status.success() {