Skip to content

Commit

Permalink
compiletest: Simplify multi-debugger support
Browse files Browse the repository at this point in the history 
Previous implementation used a single mode type to store various pieces
of otherwise loosely related information:

* Whether debuginfo mode is in use or not.
* Which debuggers should run in general.
* Which debuggers are enabled for particular test case.

The new implementation introduces a separation between those aspects.
There is a single debuginfo mode parametrized by a debugger type.
The debugger detection is performed first and a separate configuration
is created for each detected debugger. The test cases are gathered
independently for each debugger which makes it trivial to implement
support for `ignore` / `only` conditions.

Functional changes:

* A single `debuginfo` entry point (rather than `debuginfo-cdb`, `debuginfo-gdb+lldb`, etc.).
* Debugger name is included in the test name.
* Test outputs are placed in per-debugger directory.
* Fixed spurious hash mismatch. Previously, the config mode would change
  from `DebugInfoGdbLldb` (when collecting tests) to `DebugInfoGdb` or
  `DebugInfoLldb` (when running them) which would affect hash computation.
* PYTHONPATH is additionally included in gdb hash.
* lldb-python and lldb-python-dir are additionally included in lldb hash.
  • Loading branch information
@tmiasko
tmiasko committed Jan 20, 2020
1 parent 900811e commit 6d218db
Show file tree
Hide file tree
Showing 5 changed files with 211 additions and 286 deletions.
8 changes: 0 additions & 8 deletions src/bootstrap/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -957,14 +957,6 @@ impl Step for Compiletest {
}

if suite == "debuginfo" {
let msvc = builder.config.build.contains("msvc");
if mode == "debuginfo" {
return builder.ensure(Compiletest {
mode: if msvc { "debuginfo-cdb" } else { "debuginfo-gdb+lldb" },
..self
});
}

builder
.ensure(dist::DebuggerScripts { sysroot: builder.sysroot(compiler), host: target });
}
Expand Down
49 changes: 32 additions & 17 deletions src/tools/compiletest/src/common.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@ pub enum Mode {
RunFail,
RunPassValgrind,
Pretty,
DebugInfoCdb,
DebugInfoGdbLldb,
DebugInfoGdb,
DebugInfoLldb,
DebugInfo,
Codegen,
Rustdoc,
CodegenUnits,
Expand All @@ -32,13 +29,9 @@ pub enum Mode {
impl Mode {
pub fn disambiguator(self) -> &'static str {
// Pretty-printing 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 on cdb, gdb, and lldb.
// they need to keep their output segregated.
match self {
Pretty => ".pretty",
DebugInfoCdb => ".cdb",
DebugInfoGdb => ".gdb",
DebugInfoLldb => ".lldb",
_ => "",
}
}
Expand All @@ -52,10 +45,7 @@ impl FromStr for Mode {
"run-fail" => Ok(RunFail),
"run-pass-valgrind" => Ok(RunPassValgrind),
"pretty" => Ok(Pretty),
"debuginfo-cdb" => Ok(DebugInfoCdb),
"debuginfo-gdb+lldb" => Ok(DebugInfoGdbLldb),
"debuginfo-lldb" => Ok(DebugInfoLldb),
"debuginfo-gdb" => Ok(DebugInfoGdb),
"debuginfo" => Ok(DebugInfo),
"codegen" => Ok(Codegen),
"rustdoc" => Ok(Rustdoc),
"codegen-units" => Ok(CodegenUnits),
Expand All @@ -77,10 +67,7 @@ impl fmt::Display for Mode {
RunFail => "run-fail",
RunPassValgrind => "run-pass-valgrind",
Pretty => "pretty",
DebugInfoCdb => "debuginfo-cdb",
DebugInfoGdbLldb => "debuginfo-gdb+lldb",
DebugInfoGdb => "debuginfo-gdb",
DebugInfoLldb => "debuginfo-lldb",
DebugInfo => "debuginfo",
Codegen => "codegen",
Rustdoc => "rustdoc",
CodegenUnits => "codegen-units",
Expand Down Expand Up @@ -155,6 +142,29 @@ impl CompareMode {
}
}

#[derive(Clone, Copy, Debug, PartialEq)]
pub enum Debugger {
Cdb,
Gdb,
Lldb,
}

impl Debugger {
fn to_str(&self) -> &'static str {
match self {
Debugger::Cdb => "cdb",
Debugger::Gdb => "gdb",
Debugger::Lldb => "lldb",
}
}
}

impl fmt::Display for Debugger {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt(self.to_str(), f)
}
}

/// Configuration for compiletest
#[derive(Clone)]
pub struct Config {
Expand Down Expand Up @@ -208,6 +218,9 @@ pub struct Config {
/// The test mode, compile-fail, run-fail, ui
pub mode: Mode,

/// The debugger to use in debuginfo mode. Unset otherwise.
pub debugger: Option<Debugger>,

/// Run ignored tests
pub run_ignored: bool,

Expand Down Expand Up @@ -362,9 +375,11 @@ pub fn output_testname_unique(
revision: Option<&str>,
) -> PathBuf {
let mode = config.compare_mode.as_ref().map_or("", |m| m.to_str());
let debugger = config.debugger.as_ref().map_or("", |m| m.to_str());
PathBuf::from(&testpaths.file.file_stem().unwrap())
.with_extra_extension(revision.unwrap_or(""))
.with_extra_extension(mode)
.with_extra_extension(debugger)
}

/// Absolute path to the directory where all output for the given
Expand Down
193 changes: 47 additions & 146 deletions src/tools/compiletest/src/header.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,69 +6,26 @@ use std::path::{Path, PathBuf};

use log::*;

use crate::common::{self, CompareMode, Config, FailMode, Mode, PassMode};
use crate::common::{CompareMode, Config, Debugger, FailMode, Mode, PassMode};
use crate::extract_gdb_version;
use crate::util;

#[cfg(test)]
mod tests;

/// Whether to ignore the test.
#[derive(Clone, Copy, PartialEq, Debug)]
pub enum Ignore {
/// Runs it.
Run,
/// Ignore it totally.
Ignore,
/// Ignore only the gdb test, but run the lldb test.
IgnoreGdb,
/// Ignore only the lldb test, but run the gdb test.
IgnoreLldb,
}

impl Ignore {
pub fn can_run_gdb(&self) -> bool {
*self == Ignore::Run || *self == Ignore::IgnoreLldb
}

pub fn can_run_lldb(&self) -> bool {
*self == Ignore::Run || *self == Ignore::IgnoreGdb
}

pub fn no_gdb(&self) -> Ignore {
match *self {
Ignore::Run => Ignore::IgnoreGdb,
Ignore::IgnoreGdb => Ignore::IgnoreGdb,
_ => Ignore::Ignore,
}
}

pub fn no_lldb(&self) -> Ignore {
match *self {
Ignore::Run => Ignore::IgnoreLldb,
Ignore::IgnoreLldb => Ignore::IgnoreLldb,
_ => Ignore::Ignore,
}
}
}

/// The result of parse_cfg_name_directive.
#[derive(Clone, Copy, PartialEq, Debug)]
enum ParsedNameDirective {
/// No match.
NoMatch,
/// Match.
Match,
/// Mode was DebugInfoGdbLldb and this matched gdb.
MatchGdb,
/// Mode was DebugInfoGdbLldb and this matched lldb.
MatchLldb,
}

/// Properties which must be known very early, before actually running
/// the test.
pub struct EarlyProps {
pub ignore: Ignore,
pub ignore: bool,
pub should_fail: bool,
pub aux: Vec<String>,
pub aux_crate: Vec<(String, String)>,
Expand All @@ -78,84 +35,61 @@ pub struct EarlyProps {
impl EarlyProps {
pub fn from_file(config: &Config, testfile: &Path) -> Self {
let mut props = EarlyProps {
ignore: Ignore::Run,
ignore: false,
should_fail: false,
aux: Vec::new(),
aux_crate: Vec::new(),
revisions: vec![],
};

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();
let rustc_has_sanitizer_support = env::var_os("RUSTC_SANITIZER_SUPPORT").is_some();

iter_header(testfile, None, &mut |ln| {
// we should check if any only-<platform> exists and if it exists
// and does not matches the current platform, skip the test
if props.ignore != Ignore::Ignore {
if !props.ignore {
props.ignore = match config.parse_cfg_name_directive(ln, "ignore") {
ParsedNameDirective::Match => Ignore::Ignore,
ParsedNameDirective::Match => true,
ParsedNameDirective::NoMatch => props.ignore,
ParsedNameDirective::MatchGdb => props.ignore.no_gdb(),
ParsedNameDirective::MatchLldb => props.ignore.no_lldb(),
};

if config.has_cfg_prefix(ln, "only") {
props.ignore = match config.parse_cfg_name_directive(ln, "only") {
ParsedNameDirective::Match => props.ignore,
ParsedNameDirective::NoMatch => Ignore::Ignore,
ParsedNameDirective::MatchLldb => props.ignore.no_gdb(),
ParsedNameDirective::MatchGdb => props.ignore.no_lldb(),
ParsedNameDirective::NoMatch => true,
};
}

if ignore_llvm(config, ln) {
props.ignore = Ignore::Ignore;
props.ignore = true;
}

if config.run_clang_based_tests_with.is_none()
&& config.parse_needs_matching_clang(ln)
{
props.ignore = Ignore::Ignore;
props.ignore = true;
}

if !rustc_has_profiler_support && config.parse_needs_profiler_support(ln) {
props.ignore = Ignore::Ignore;
props.ignore = true;
}

if !rustc_has_sanitizer_support && config.parse_needs_sanitizer_support(ln) {
props.ignore = Ignore::Ignore;
props.ignore = true;
}

if config.target == "wasm32-unknown-unknown" && config.parse_check_run_results(ln) {
props.ignore = Ignore::Ignore;
props.ignore = true;
}
}

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.debugger == Some(Debugger::Gdb) && ignore_gdb(config, ln) {
props.ignore = true;
}

if (config.mode == common::DebugInfoLldb || config.mode == common::DebugInfoGdbLldb)
&& props.ignore.can_run_lldb()
&& ignore_lldb(config, ln)
{
props.ignore = props.ignore.no_lldb();
if config.debugger == Some(Debugger::Lldb) && ignore_lldb(config, ln) {
props.ignore = true;
}
}

if let Some(s) = config.parse_aux_build(ln) {
Expand Down Expand Up @@ -881,70 +815,37 @@ impl Config {
/// Parses a name-value directive which contains config-specific information, e.g., `ignore-x86`
/// or `normalize-stderr-32bit`.
fn parse_cfg_name_directive(&self, line: &str, prefix: &str) -> ParsedNameDirective {
if line.starts_with(prefix) && line.as_bytes().get(prefix.len()) == Some(&b'-') {
let name = line[prefix.len() + 1..].split(&[':', ' '][..]).next().unwrap();

if name == "test" ||
&self.target == name || // triple
util::matches_os(&self.target, name) || // target
util::matches_env(&self.target, name) || // env
name == util::get_arch(&self.target) || // architecture
name == util::get_pointer_width(&self.target) || // pointer width
name == self.stage_id.split('-').next().unwrap() || // stage
(self.target != self.host && name == "cross-compile") ||
match self.compare_mode {
Some(CompareMode::Nll) => name == "compare-mode-nll",
Some(CompareMode::Polonius) => name == "compare-mode-polonius",
None => false,
} ||
(cfg!(debug_assertions) && name == "debug")
{
ParsedNameDirective::Match
} else {
match self.mode {
common::DebugInfoGdbLldb => {
if name == "gdb" {
ParsedNameDirective::MatchGdb
} else if name == "lldb" {
ParsedNameDirective::MatchLldb
} else {
ParsedNameDirective::NoMatch
}
}
common::DebugInfoCdb => {
if name == "cdb" {
ParsedNameDirective::Match
} else {
ParsedNameDirective::NoMatch
}
}
common::DebugInfoGdb => {
if name == "gdb" {
ParsedNameDirective::Match
} else {
ParsedNameDirective::NoMatch
}
}
common::DebugInfoLldb => {
if name == "lldb" {
ParsedNameDirective::Match
} else {
ParsedNameDirective::NoMatch
}
}
common::Pretty => {
if name == "pretty" {
ParsedNameDirective::Match
} else {
ParsedNameDirective::NoMatch
}
}
_ => ParsedNameDirective::NoMatch,
}
}
} else {
ParsedNameDirective::NoMatch
if !line.as_bytes().starts_with(prefix.as_bytes()) {
return ParsedNameDirective::NoMatch;
}
if line.as_bytes().get(prefix.len()) != Some(&b'-') {
return ParsedNameDirective::NoMatch;
}

let name = line[prefix.len() + 1..].split(&[':', ' '][..]).next().unwrap();

let is_match = name == "test" ||
&self.target == name || // triple
util::matches_os(&self.target, name) || // target
util::matches_env(&self.target, name) || // env
name == util::get_arch(&self.target) || // architecture
name == util::get_pointer_width(&self.target) || // pointer width
name == self.stage_id.split('-').next().unwrap() || // stage
(self.target != self.host && name == "cross-compile") ||
match self.compare_mode {
Some(CompareMode::Nll) => name == "compare-mode-nll",
Some(CompareMode::Polonius) => name == "compare-mode-polonius",
None => false,
} ||
(cfg!(debug_assertions) && name == "debug") ||
match self.debugger {
Some(Debugger::Cdb) => name == "cdb",
Some(Debugger::Gdb) => name == "gdb",
Some(Debugger::Lldb) => name == "lldb",
None => false,
};

if is_match { ParsedNameDirective::Match } else { ParsedNameDirective::NoMatch }
}

fn has_cfg_prefix(&self, line: &str, prefix: &str) -> bool {
Expand Down
Loading

0 comments on commit 6d218db

Please sign in to comment.