From 7aec6de687ce587f3b8909103a15dc26663a142b Mon Sep 17 00:00:00 2001 From: George Pollard Date: Mon, 31 Jul 2023 22:24:54 +0000 Subject: [PATCH 1/9] Start to write a test --- src/agent/Cargo.lock | 2 + src/agent/srcview/Cargo.toml | 4 ++ src/agent/srcview/tests/srcview.rs | 42 +++++++++++++++++++++ src/agent/srcview/tests/windows/inlinee.cpp | 12 ++++++ src/proxy-manager/Cargo.lock | 4 +- 5 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 src/agent/srcview/tests/windows/inlinee.cpp diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock index 40f8e42f8f..4bec6d4273 100644 --- a/src/agent/Cargo.lock +++ b/src/agent/Cargo.lock @@ -3353,7 +3353,9 @@ version = "0.1.2" dependencies = [ "anyhow", "clap", + "coverage", "env_logger", + "insta", "log", "nom", "pdb", diff --git a/src/agent/srcview/Cargo.toml b/src/agent/srcview/Cargo.toml index 2eccef59f7..267e2ac73d 100644 --- a/src/agent/srcview/Cargo.toml +++ b/src/agent/srcview/Cargo.toml @@ -19,3 +19,7 @@ quick-xml = "0.30" anyhow = "1.0" env_logger = "0.10" clap = { version = "4.3.0", features = ["derive"] } + +[dev-dependencies] +insta = { version = "1.31.0", features = ["glob", "json"] } +coverage = { path = "../coverage" } diff --git a/src/agent/srcview/tests/srcview.rs b/src/agent/srcview/tests/srcview.rs index 29ba90f64f..de440d7aa5 100644 --- a/src/agent/srcview/tests/srcview.rs +++ b/src/agent/srcview/tests/srcview.rs @@ -75,3 +75,45 @@ fn path() { assert!(srcview.path_lines("z:\\does\\not\\exist.c").is_none()); } + +#[test] +//#[cfg(target_os = "windows")] +fn windows_snapshot_tests() { + insta::glob!("testdata", "*.cpp", |path| { + let output = std::process::Command::new("cl.exe") + .args("/EHsc") + .arg("/Zi") + .arg(path) + .spawn() + .expect("launching compiler") + .wait_with_output() + .expect("waiting for compiler to finish"); + + assert!(output.status.success()); + + let exe_name = PathBuf::from(path.file_name().unwrap()).with_extension("exe"); + let pdb_name = exe_name.with_extension("pdb"); + + let mut srcview = SrcView::new(); + srcview + .insert(exe_name.to_string_lossy().as_ref(), &pdb_name) + .unwrap(); + + let exe_cmd = std::process::Command::new(exe_name); + let recorded = coverage::CoverageRecorder::new(exe_cmd).record().unwrap(); + + let mut srclines_hit: Vec = Vec::new(); + for (module, coverage) in &recorded.coverage.modules { + for offset in coverage.as_ref().keys() { + if let Some(line) = srcview.modoff(&ModOff { + module: module.to_string(), + offset: offset.0 as usize, + }) { + srclines_hit.push(line); + } + } + } + + insta::assert_json_snapshot!(srclines_hit); + }); +} diff --git a/src/agent/srcview/tests/windows/inlinee.cpp b/src/agent/srcview/tests/windows/inlinee.cpp new file mode 100644 index 0000000000..df398b23c3 --- /dev/null +++ b/src/agent/srcview/tests/windows/inlinee.cpp @@ -0,0 +1,12 @@ +#include + +__declspec(dllexport) void test(); + +int main() +{ + test(); +} + +__declspec(dllexport) void test() { + std::cout << "Hello World!\n"; +} diff --git a/src/proxy-manager/Cargo.lock b/src/proxy-manager/Cargo.lock index 666cd23c9c..a5205a47ba 100644 --- a/src/proxy-manager/Cargo.lock +++ b/src/proxy-manager/Cargo.lock @@ -1064,9 +1064,9 @@ dependencies = [ [[package]] name = "quick-xml" -version = "0.29.0" +version = "0.30.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "81b9228215d82c7b61490fec1de287136b5de6f5700f6e58ea9ad61a7964ca51" +checksum = "eff6510e86862b57b210fd8cbe8ed3f0d7d600b9c2863cd4549a2e033c66e956" dependencies = [ "memchr", "serde", From 5e3543cd8458c44f799b38e54154271fad64269d Mon Sep 17 00:00:00 2001 From: George Pollard Date: Tue, 1 Aug 2023 12:22:47 +1200 Subject: [PATCH 2/9] Finish impl --- src/agent/Cargo.lock | 3 + src/agent/coverage/src/record.rs | 27 +++++--- src/agent/srcview/Cargo.toml | 3 + .../srcview__windows_snapshot_tests.snap | 11 +++ src/agent/srcview/tests/srcview.rs | 67 ++++++++++++++----- src/agent/srcview/tests/windows/inlinee.cpp | 2 + 6 files changed, 86 insertions(+), 27 deletions(-) create mode 100644 src/agent/srcview/tests/snapshots/srcview__windows_snapshot_tests.snap diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock index 4bec6d4273..bbc4d8d711 100644 --- a/src/agent/Cargo.lock +++ b/src/agent/Cargo.lock @@ -3352,8 +3352,10 @@ name = "srcview" version = "0.1.2" dependencies = [ "anyhow", + "cc", "clap", "coverage", + "dunce", "env_logger", "insta", "log", @@ -3362,6 +3364,7 @@ dependencies = [ "quick-xml 0.30.0", "regex", "serde", + "tempfile", ] [[package]] diff --git a/src/agent/coverage/src/record.rs b/src/agent/coverage/src/record.rs index a53b81e1c4..45cf62e582 100644 --- a/src/agent/coverage/src/record.rs +++ b/src/agent/coverage/src/record.rs @@ -18,8 +18,8 @@ pub mod linux; pub mod windows; pub struct CoverageRecorder { - allowlist: TargetAllowList, - cache: Arc, + allowlist: Option, + cache: Option>, cmd: Command, loader: Arc, timeout: Duration, @@ -30,14 +30,12 @@ impl CoverageRecorder { cmd.stdout(Stdio::piped()); cmd.stderr(Stdio::piped()); - let allowlist = TargetAllowList::default(); - let cache = Arc::new(DebugInfoCache::new(allowlist.source_files.clone())); let loader = Arc::new(Loader::new()); let timeout = Duration::from_secs(5); Self { - allowlist, - cache, + allowlist: None, + cache: None, cmd, loader, timeout, @@ -45,7 +43,7 @@ impl CoverageRecorder { } pub fn allowlist(mut self, allowlist: TargetAllowList) -> Self { - self.allowlist = allowlist; + self.allowlist = Some(allowlist); self } @@ -55,7 +53,7 @@ impl CoverageRecorder { } pub fn debuginfo_cache(mut self, cache: impl Into>) -> Self { - self.cache = cache.into(); + self.cache = Some(cache.into()); self } @@ -75,6 +73,10 @@ impl CoverageRecorder { use linux::LinuxRecorder; let loader = self.loader.clone(); + let allowlist = self.allowlist.unwrap_or_default(); + let cache = self + .cache + .unwrap_or_else(|| Arc::new(DebugInfoCache::new(allowlist.source_files.clone()))); let child_pid: Arc>> = Arc::new(Mutex::new(None)); @@ -82,7 +84,7 @@ impl CoverageRecorder { let child_pid = child_pid.clone(); timer::timed(self.timeout, move || { - let mut recorder = LinuxRecorder::new(&loader, self.allowlist, &self.cache); + let mut recorder = LinuxRecorder::new(&loader, allowlist, &cache); let mut dbg = Debugger::new(&mut recorder); let child = dbg.spawn(self.cmd)?; @@ -127,8 +129,13 @@ impl CoverageRecorder { let loader = self.loader.clone(); + let allowlist = self.allowlist.unwrap_or_default(); + let cache = self + .cache + .unwrap_or_else(|| Arc::new(DebugInfoCache::new(allowlist.source_files.clone()))); + crate::timer::timed(self.timeout, move || { - let mut recorder = WindowsRecorder::new(&loader, self.allowlist, &self.cache); + let mut recorder = WindowsRecorder::new(&loader, allowlist, cache.as_ref()); let (mut dbg, child) = Debugger::init(self.cmd, &mut recorder)?; dbg.run(&mut recorder)?; diff --git a/src/agent/srcview/Cargo.toml b/src/agent/srcview/Cargo.toml index 267e2ac73d..af4888a5b6 100644 --- a/src/agent/srcview/Cargo.toml +++ b/src/agent/srcview/Cargo.toml @@ -23,3 +23,6 @@ clap = { version = "4.3.0", features = ["derive"] } [dev-dependencies] insta = { version = "1.31.0", features = ["glob", "json"] } coverage = { path = "../coverage" } +cc = "1.0" +tempfile = "3.7.0" +dunce = "1.0" diff --git a/src/agent/srcview/tests/snapshots/srcview__windows_snapshot_tests.snap b/src/agent/srcview/tests/snapshots/srcview__windows_snapshot_tests.snap new file mode 100644 index 0000000000..6da8e9e069 --- /dev/null +++ b/src/agent/srcview/tests/snapshots/srcview__windows_snapshot_tests.snap @@ -0,0 +1,11 @@ +--- +source: srcview/tests/srcview.rs +expression: srclines_hit +input_file: srcview/tests/windows/inlinee.cpp +--- +[ + { + "path": "D:\\git\\onefuzz\\src\\agent\\srcview\\tests\\windows\\inlinee.cpp", + "line": 6 + } +] diff --git a/src/agent/srcview/tests/srcview.rs b/src/agent/srcview/tests/srcview.rs index de440d7aa5..e618d0352f 100644 --- a/src/agent/srcview/tests/srcview.rs +++ b/src/agent/srcview/tests/srcview.rs @@ -5,9 +5,11 @@ // $ sha256sum example.pdb // ecc4214d687c97e9c8afd0c84b4b75383eaa0a237f8a8ca5049478f63b2c98b9 example.pdb -use std::env; use std::path::PathBuf; +use std::{env, time::Duration}; +use coverage::binary::DebugInfoCache; +use coverage::{AllowList, TargetAllowList}; use srcview::{ModOff, SrcLine, SrcView}; fn test_srcview() -> SrcView { @@ -77,39 +79,70 @@ fn path() { } #[test] -//#[cfg(target_os = "windows")] +#[cfg(target_os = "windows")] fn windows_snapshot_tests() { - insta::glob!("testdata", "*.cpp", |path| { - let output = std::process::Command::new("cl.exe") - .args("/EHsc") + insta::glob!("windows", "*.cpp", |path| { + let file_name = path.file_name().unwrap(); + let build_in = tempfile::tempdir().expect("creating tempdir"); + + let mut cl_exe = + cc::windows_registry::find("x86_64-pc-windows-msvc", "cl.exe").expect("finding cl.exe"); + + let input_path = dunce::canonicalize(path).unwrap(); + + let output = cl_exe + .arg("/EHsc") .arg("/Zi") - .arg(path) + .arg("/O2") + .arg(&input_path) + .current_dir(build_in.path()) .spawn() .expect("launching compiler") .wait_with_output() .expect("waiting for compiler to finish"); - assert!(output.status.success()); + assert!(output.status.success(), "cl.exe failed: {:?}", output); + + let exe_name = { + let mut cwd = build_in.path().to_path_buf(); + cwd.push(file_name); + cwd.set_extension("exe"); + cwd + }; - let exe_name = PathBuf::from(path.file_name().unwrap()).with_extension("exe"); - let pdb_name = exe_name.with_extension("pdb"); + println!("Recording coverage for: {:?}", exe_name); + let target_file_name = exe_name.file_name().unwrap().to_string_lossy().into_owned(); let mut srcview = SrcView::new(); srcview - .insert(exe_name.to_string_lossy().as_ref(), &pdb_name) + .insert(&target_file_name, &exe_name.with_extension("pdb")) .unwrap(); + let allowlist = TargetAllowList { + source_files: AllowList::parse(&input_path.to_string_lossy()).unwrap(), + modules: AllowList::default(), + }; + + println!("allowed: {:?}", allowlist); + let exe_cmd = std::process::Command::new(exe_name); - let recorded = coverage::CoverageRecorder::new(exe_cmd).record().unwrap(); + let recorded = coverage::CoverageRecorder::new(exe_cmd) + .timeout(Duration::from_secs(120)) + .allowlist(allowlist) + .record() + .unwrap(); let mut srclines_hit: Vec = Vec::new(); for (module, coverage) in &recorded.coverage.modules { - for offset in coverage.as_ref().keys() { - if let Some(line) = srcview.modoff(&ModOff { - module: module.to_string(), - offset: offset.0 as usize, - }) { - srclines_hit.push(line); + let module_name = module.file_name(); + for (offset, count) in &coverage.offsets { + if count.0 > 0 { + if let Some(line) = srcview.modoff(&ModOff { + module: module_name.to_string(), + offset: offset.0 as usize, + }) { + srclines_hit.push(line); + }; } } } diff --git a/src/agent/srcview/tests/windows/inlinee.cpp b/src/agent/srcview/tests/windows/inlinee.cpp index df398b23c3..e0167f4afb 100644 --- a/src/agent/srcview/tests/windows/inlinee.cpp +++ b/src/agent/srcview/tests/windows/inlinee.cpp @@ -4,7 +4,9 @@ __declspec(dllexport) void test(); int main() { + std::cout << "Before\n"; test(); + std::cout << "After\n"; } __declspec(dllexport) void test() { From ece3917c59400e92ec0e66a55db3553457ed03bb Mon Sep 17 00:00:00 2001 From: George Pollard Date: Tue, 1 Aug 2023 12:32:54 +1200 Subject: [PATCH 3/9] Filter output for stability --- src/agent/Cargo.lock | 1 + src/agent/srcview/Cargo.toml | 2 +- .../srcview__windows_snapshot_tests.snap | 2 +- src/agent/srcview/tests/srcview.rs | 31 +++++++++++-------- 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock index bbc4d8d711..ee3b7231d0 100644 --- a/src/agent/Cargo.lock +++ b/src/agent/Cargo.lock @@ -1640,6 +1640,7 @@ dependencies = [ "globset", "lazy_static", "linked-hash-map", + "regex", "serde", "similar", "walkdir", diff --git a/src/agent/srcview/Cargo.toml b/src/agent/srcview/Cargo.toml index af4888a5b6..c8467fc9ea 100644 --- a/src/agent/srcview/Cargo.toml +++ b/src/agent/srcview/Cargo.toml @@ -21,7 +21,7 @@ env_logger = "0.10" clap = { version = "4.3.0", features = ["derive"] } [dev-dependencies] -insta = { version = "1.31.0", features = ["glob", "json"] } +insta = { version = "1.31.0", features = ["glob", "json", "filters"] } coverage = { path = "../coverage" } cc = "1.0" tempfile = "3.7.0" diff --git a/src/agent/srcview/tests/snapshots/srcview__windows_snapshot_tests.snap b/src/agent/srcview/tests/snapshots/srcview__windows_snapshot_tests.snap index 6da8e9e069..21c0497c6b 100644 --- a/src/agent/srcview/tests/snapshots/srcview__windows_snapshot_tests.snap +++ b/src/agent/srcview/tests/snapshots/srcview__windows_snapshot_tests.snap @@ -5,7 +5,7 @@ input_file: srcview/tests/windows/inlinee.cpp --- [ { - "path": "D:\\git\\onefuzz\\src\\agent\\srcview\\tests\\windows\\inlinee.cpp", + "path": "…\\inlinee.cpp", "line": 6 } ] diff --git a/src/agent/srcview/tests/srcview.rs b/src/agent/srcview/tests/srcview.rs index e618d0352f..62e700c275 100644 --- a/src/agent/srcview/tests/srcview.rs +++ b/src/agent/srcview/tests/srcview.rs @@ -83,13 +83,17 @@ fn path() { fn windows_snapshot_tests() { insta::glob!("windows", "*.cpp", |path| { let file_name = path.file_name().unwrap(); - let build_in = tempfile::tempdir().expect("creating tempdir"); + // locate appropriate compiler let mut cl_exe = cc::windows_registry::find("x86_64-pc-windows-msvc", "cl.exe").expect("finding cl.exe"); + // path will have \\?\ prefix, cl.exe doesn't like it let input_path = dunce::canonicalize(path).unwrap(); + // directory to compile into + let build_in = tempfile::tempdir().expect("creating tempdir"); + let output = cl_exe .arg("/EHsc") .arg("/Zi") @@ -110,28 +114,26 @@ fn windows_snapshot_tests() { cwd }; - println!("Recording coverage for: {:?}", exe_name); - - let target_file_name = exe_name.file_name().unwrap().to_string_lossy().into_owned(); - let mut srcview = SrcView::new(); - srcview - .insert(&target_file_name, &exe_name.with_extension("pdb")) - .unwrap(); - + // filter to just the input test file: let allowlist = TargetAllowList { source_files: AllowList::parse(&input_path.to_string_lossy()).unwrap(), modules: AllowList::default(), }; - println!("allowed: {:?}", allowlist); - - let exe_cmd = std::process::Command::new(exe_name); + let exe_cmd = std::process::Command::new(&exe_name); let recorded = coverage::CoverageRecorder::new(exe_cmd) .timeout(Duration::from_secs(120)) .allowlist(allowlist) .record() .unwrap(); + // generate information with srcview + let target_file_name = exe_name.file_name().unwrap().to_string_lossy().into_owned(); + let mut srcview = SrcView::new(); + srcview + .insert(&target_file_name, &exe_name.with_extension("pdb")) + .unwrap(); + let mut srclines_hit: Vec = Vec::new(); for (module, coverage) in &recorded.coverage.modules { let module_name = module.file_name(); @@ -147,6 +149,9 @@ fn windows_snapshot_tests() { } } - insta::assert_json_snapshot!(srclines_hit); + // apply filter to make output stable, replace prefix of path with "…" + insta::with_settings!({filters => vec![(r"[A-Z]:.*windows", "…")]}, { + insta::assert_json_snapshot!(srclines_hit); + }); }); } From 7bba275d4d95382a402c5ab5c23e97bbd2bc6e33 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Tue, 1 Aug 2023 02:25:51 +0000 Subject: [PATCH 4/9] Fix imports --- src/agent/srcview/tests/srcview.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/agent/srcview/tests/srcview.rs b/src/agent/srcview/tests/srcview.rs index 62e700c275..aa4a27ca28 100644 --- a/src/agent/srcview/tests/srcview.rs +++ b/src/agent/srcview/tests/srcview.rs @@ -5,11 +5,9 @@ // $ sha256sum example.pdb // ecc4214d687c97e9c8afd0c84b4b75383eaa0a237f8a8ca5049478f63b2c98b9 example.pdb +use std::env; use std::path::PathBuf; -use std::{env, time::Duration}; -use coverage::binary::DebugInfoCache; -use coverage::{AllowList, TargetAllowList}; use srcview::{ModOff, SrcLine, SrcView}; fn test_srcview() -> SrcView { @@ -81,6 +79,9 @@ fn path() { #[test] #[cfg(target_os = "windows")] fn windows_snapshot_tests() { + use coverage::{AllowList, TargetAllowList}; + use std::time::Duration; + insta::glob!("windows", "*.cpp", |path| { let file_name = path.file_name().unwrap(); From ca84b910608dd93773dbbfd75c07bc1959691a20 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Wed, 2 Aug 2023 08:45:04 +1200 Subject: [PATCH 5/9] Mostly remove TargetAllowList --- src/agent/coverage/examples/record.rs | 32 ++++++++++--------- src/agent/coverage/src/allowlist.rs | 25 --------------- src/agent/coverage/src/binary.rs | 8 ++--- src/agent/coverage/src/lib.rs | 2 +- src/agent/coverage/src/record.rs | 30 +++++++---------- src/agent/coverage/src/record/linux.rs | 8 ++--- src/agent/coverage/src/record/windows.rs | 18 +++++------ .../src/tasks/coverage/generic.rs | 22 +++++++++---- src/agent/srcview/tests/srcview.rs | 9 ++---- 9 files changed, 64 insertions(+), 90 deletions(-) diff --git a/src/agent/coverage/examples/record.rs b/src/agent/coverage/examples/record.rs index 0edae5645d..4d2ec46922 100644 --- a/src/agent/coverage/examples/record.rs +++ b/src/agent/coverage/examples/record.rs @@ -2,10 +2,10 @@ use std::process::Command; use std::sync::Arc; use std::time::Duration; -use anyhow::{bail, Result}; +use anyhow::{bail, Context, Result}; use clap::Parser; use cobertura::CoberturaCoverage; -use coverage::allowlist::{AllowList, TargetAllowList}; +use coverage::allowlist::AllowList; use coverage::binary::{BinaryCoverage, DebugInfoCache}; use coverage::record::{CoverageRecorder, Recorded}; use debuggable_module::load_module::LoadModule; @@ -57,19 +57,21 @@ fn main() -> Result<()> { .map(Duration::from_millis) .unwrap_or(DEFAULT_TIMEOUT); - let mut allowlist = TargetAllowList::default(); + let module_allowlist = args + .module_allowlist + .map(AllowList::load) + .unwrap_or_else(|| Ok(AllowList::default())) + .context("loading module allowlist")?; - if let Some(path) = &args.module_allowlist { - allowlist.modules = AllowList::load(path)?; - } - - if let Some(path) = &args.source_allowlist { - allowlist.source_files = AllowList::load(path)?; - } + let source_allowlist = args + .source_allowlist + .map(AllowList::load) + .unwrap_or_else(|| Ok(AllowList::default())) + .context("loading source allowlist")?; let mut coverage = BinaryCoverage::default(); let loader = Arc::new(Loader::new()); - let cache = Arc::new(DebugInfoCache::new(allowlist.source_files.clone())); + let cache = Arc::new(DebugInfoCache::new(source_allowlist.clone())); let t = std::time::Instant::now(); precache_target(&args.command[0], &loader, &cache)?; @@ -84,7 +86,7 @@ fn main() -> Result<()> { let t = std::time::Instant::now(); let recorded = CoverageRecorder::new(cmd) - .allowlist(allowlist.clone()) + .module_allowlist(module_allowlist.clone()) .loader(loader.clone()) .debuginfo_cache(cache.clone()) .timeout(timeout) @@ -102,7 +104,7 @@ fn main() -> Result<()> { let t = std::time::Instant::now(); let recorded = CoverageRecorder::new(cmd) - .allowlist(allowlist.clone()) + .module_allowlist(module_allowlist) .loader(loader) .debuginfo_cache(cache) .timeout(timeout) @@ -118,8 +120,8 @@ fn main() -> Result<()> { match args.output { OutputFormat::ModOff => dump_modoff(&coverage)?, - OutputFormat::Source => dump_source_line(&coverage, allowlist.source_files)?, - OutputFormat::Cobertura => dump_cobertura(&coverage, allowlist.source_files)?, + OutputFormat::Source => dump_source_line(&coverage, source_allowlist)?, + OutputFormat::Cobertura => dump_cobertura(&coverage, source_allowlist)?, } Ok(()) diff --git a/src/agent/coverage/src/allowlist.rs b/src/agent/coverage/src/allowlist.rs index 45520cde40..079f415004 100644 --- a/src/agent/coverage/src/allowlist.rs +++ b/src/agent/coverage/src/allowlist.rs @@ -5,31 +5,6 @@ use anyhow::Result; use regex::{Regex, RegexSet}; use std::path::Path; -#[derive(Clone, Debug, Default)] -pub struct TargetAllowList { - pub modules: AllowList, - pub source_files: AllowList, -} - -impl TargetAllowList { - pub fn new(modules: AllowList, source_files: AllowList) -> Self { - Self { - modules, - source_files, - } - } - - #[allow(clippy::field_reassign_with_default)] - pub fn extend(&self, other: &Self) -> Self { - let mut new = Self::default(); - - new.modules = self.modules.extend(&other.modules); - new.source_files = self.source_files.extend(&other.source_files); - - new - } -} - #[derive(Clone, Debug)] pub struct AllowList { allow: RegexSet, diff --git a/src/agent/coverage/src/binary.rs b/src/agent/coverage/src/binary.rs index c773d6cf3c..61284bd6f0 100644 --- a/src/agent/coverage/src/binary.rs +++ b/src/agent/coverage/src/binary.rs @@ -11,7 +11,7 @@ pub use debuggable_module::{block, path::FilePath, Offset}; use symbolic::debuginfo::Object; use symbolic::symcache::{SymCache, SymCacheConverter}; -use crate::allowlist::{AllowList, TargetAllowList}; +use crate::allowlist::AllowList; #[derive(Clone, Debug, Default, Eq, PartialEq)] pub struct BinaryCoverage { @@ -214,7 +214,7 @@ impl CachedDebugInfo { pub fn find_coverage_sites( module: &dyn Module, - allowlist: &TargetAllowList, + source_allowlist: &AllowList, ) -> Result { let debuginfo = module.debuginfo()?; @@ -232,7 +232,7 @@ pub fn find_coverage_sites( for function in debuginfo.functions() { if let Some(location) = symcache.lookup(function.offset.0).next() { if let Some(file) = location.file() { - if !allowlist.source_files.is_allowed(file.full_path()) { + if !source_allowlist.is_allowed(file.full_path()) { debug!( "skipping sweep of `{}:{}` due to excluded source path `{}`", module.executable_path(), @@ -253,7 +253,7 @@ pub fn find_coverage_sites( // Apply allowlists per block, to account for inlining. The `location` values // here describe the top of the inline-inclusive call stack. - if !allowlist.source_files.is_allowed(path) { + if !source_allowlist.is_allowed(path) { continue; } diff --git a/src/agent/coverage/src/lib.rs b/src/agent/coverage/src/lib.rs index 499e82a924..771b0986fd 100644 --- a/src/agent/coverage/src/lib.rs +++ b/src/agent/coverage/src/lib.rs @@ -12,7 +12,7 @@ pub mod source; mod timer; #[doc(inline)] -pub use allowlist::{AllowList, TargetAllowList}; +pub use allowlist::AllowList; #[doc(inline)] pub use record::{CoverageRecorder, Recorded}; diff --git a/src/agent/coverage/src/record.rs b/src/agent/coverage/src/record.rs index 45cf62e582..4a1b2c9e45 100644 --- a/src/agent/coverage/src/record.rs +++ b/src/agent/coverage/src/record.rs @@ -8,8 +8,8 @@ use std::time::Duration; use anyhow::Result; use debuggable_module::loader::Loader; -use crate::allowlist::TargetAllowList; use crate::binary::{BinaryCoverage, DebugInfoCache}; +use crate::AllowList; #[cfg(target_os = "linux")] pub mod linux; @@ -18,8 +18,8 @@ pub mod linux; pub mod windows; pub struct CoverageRecorder { - allowlist: Option, - cache: Option>, + module_allowlist: AllowList, + cache: Arc, cmd: Command, loader: Arc, timeout: Duration, @@ -34,16 +34,16 @@ impl CoverageRecorder { let timeout = Duration::from_secs(5); Self { - allowlist: None, - cache: None, + module_allowlist: AllowList::default(), + cache: Arc::new(DebugInfoCache::new(AllowList::default())), cmd, loader, timeout, } } - pub fn allowlist(mut self, allowlist: TargetAllowList) -> Self { - self.allowlist = Some(allowlist); + pub fn module_allowlist(mut self, module_allowlist: AllowList) -> Self { + self.module_allowlist = module_allowlist; self } @@ -53,7 +53,7 @@ impl CoverageRecorder { } pub fn debuginfo_cache(mut self, cache: impl Into>) -> Self { - self.cache = Some(cache.into()); + self.cache = cache.into(); self } @@ -73,10 +73,6 @@ impl CoverageRecorder { use linux::LinuxRecorder; let loader = self.loader.clone(); - let allowlist = self.allowlist.unwrap_or_default(); - let cache = self - .cache - .unwrap_or_else(|| Arc::new(DebugInfoCache::new(allowlist.source_files.clone()))); let child_pid: Arc>> = Arc::new(Mutex::new(None)); @@ -84,7 +80,7 @@ impl CoverageRecorder { let child_pid = child_pid.clone(); timer::timed(self.timeout, move || { - let mut recorder = LinuxRecorder::new(&loader, allowlist, &cache); + let mut recorder = LinuxRecorder::new(&loader, self.allowlist, &self.cache); let mut dbg = Debugger::new(&mut recorder); let child = dbg.spawn(self.cmd)?; @@ -129,13 +125,9 @@ impl CoverageRecorder { let loader = self.loader.clone(); - let allowlist = self.allowlist.unwrap_or_default(); - let cache = self - .cache - .unwrap_or_else(|| Arc::new(DebugInfoCache::new(allowlist.source_files.clone()))); - crate::timer::timed(self.timeout, move || { - let mut recorder = WindowsRecorder::new(&loader, allowlist, cache.as_ref()); + let mut recorder = + WindowsRecorder::new(&loader, self.module_allowlist, self.cache.as_ref()); let (mut dbg, child) = Debugger::init(self.cmd, &mut recorder)?; dbg.run(&mut recorder)?; diff --git a/src/agent/coverage/src/record/linux.rs b/src/agent/coverage/src/record/linux.rs index 5eadf579f0..9becaf9ee0 100644 --- a/src/agent/coverage/src/record/linux.rs +++ b/src/agent/coverage/src/record/linux.rs @@ -14,11 +14,11 @@ use pete::Tracee; pub mod debugger; use debugger::{DebugEventHandler, DebuggerContext, ModuleImage}; -use crate::allowlist::TargetAllowList; +use crate::allowlist::AllowList; use crate::binary::{BinaryCoverage, DebugInfoCache}; pub struct LinuxRecorder<'cache, 'data> { - allowlist: TargetAllowList, + module_allowlist: AllowList, cache: &'cache DebugInfoCache, pub coverage: BinaryCoverage, loader: &'data Loader, @@ -28,7 +28,7 @@ pub struct LinuxRecorder<'cache, 'data> { impl<'cache, 'data> LinuxRecorder<'cache, 'data> { pub fn new( loader: &'data Loader, - allowlist: TargetAllowList, + module_allowlist: AllowList, cache: &'cache DebugInfoCache, ) -> Self { let coverage = BinaryCoverage::default(); @@ -80,7 +80,7 @@ impl<'cache, 'data> LinuxRecorder<'cache, 'data> { let path = image.path(); - if !self.allowlist.modules.is_allowed(path) { + if !self.module_allowlist.is_allowed(path) { debug!("not inserting denylisted module: {path}"); return Ok(()); } diff --git a/src/agent/coverage/src/record/windows.rs b/src/agent/coverage/src/record/windows.rs index 79170a50e3..076aa70d37 100644 --- a/src/agent/coverage/src/record/windows.rs +++ b/src/agent/coverage/src/record/windows.rs @@ -13,8 +13,8 @@ use debuggable_module::windows::WindowsModule; use debuggable_module::{Module, Offset}; use debugger::{BreakpointId, BreakpointType, DebugEventHandler, Debugger, ModuleLoadInfo}; -use crate::allowlist::TargetAllowList; use crate::binary::{BinaryCoverage, DebugInfoCache}; +use crate::AllowList; // For a new module image, we defer setting coverage breakpoints until exit from one of these // functions (when present). This avoids breaking hotpatching routines in the ASan interceptor @@ -26,7 +26,7 @@ const PROCESS_IMAGE_DEFERRAL_TRIGGER: &str = "__asan::AsanInitInternal("; const LIBRARY_IMAGE_DEFERRAL_TRIGGER: &str = "DllMain("; pub struct WindowsRecorder<'cache, 'data> { - allowlist: TargetAllowList, + module_allowlist: AllowList, breakpoints: Breakpoints, cache: &'cache DebugInfoCache, deferred_breakpoints: BTreeMap, @@ -39,7 +39,7 @@ pub struct WindowsRecorder<'cache, 'data> { impl<'cache, 'data> WindowsRecorder<'cache, 'data> { pub fn new( loader: &'data Loader, - allowlist: TargetAllowList, + module_allowlist: AllowList, cache: &'cache DebugInfoCache, ) -> Self { let breakpoints = Breakpoints::default(); @@ -49,7 +49,7 @@ impl<'cache, 'data> WindowsRecorder<'cache, 'data> { let stop_error = None; Self { - allowlist, + module_allowlist, breakpoints, cache, deferred_breakpoints, @@ -60,12 +60,12 @@ impl<'cache, 'data> WindowsRecorder<'cache, 'data> { } } - pub fn allowlist(&self) -> &TargetAllowList { - &self.allowlist + pub fn module_allowlist(&self) -> &AllowList { + &self.module_allowlist } - pub fn allowlist_mut(&mut self) -> &mut TargetAllowList { - &mut self.allowlist + pub fn module_allowlist_mut(&mut self) -> &mut AllowList { + &mut self.module_allowlist } fn try_on_create_process(&mut self, dbg: &mut Debugger, module: &ModuleLoadInfo) -> Result<()> { @@ -163,7 +163,7 @@ impl<'cache, 'data> WindowsRecorder<'cache, 'data> { return Ok(()); } - if !self.allowlist.modules.is_allowed(&path) { + if !self.module_allowlist.is_allowed(&path) { debug!("not inserting denylisted module: {path}"); return Ok(()); } diff --git a/src/agent/onefuzz-task/src/tasks/coverage/generic.rs b/src/agent/onefuzz-task/src/tasks/coverage/generic.rs index b4d56b4579..17ece8c017 100644 --- a/src/agent/onefuzz-task/src/tasks/coverage/generic.rs +++ b/src/agent/onefuzz-task/src/tasks/coverage/generic.rs @@ -11,7 +11,7 @@ use std::time::Duration; use anyhow::{bail, Context, Result}; use async_trait::async_trait; use cobertura::CoberturaCoverage; -use coverage::allowlist::{AllowList, TargetAllowList}; +use coverage::allowlist::AllowList; use coverage::binary::{BinaryCoverage, DebugInfoCache}; use coverage::record::CoverageRecorder; use coverage::source::{binary_to_source_coverage, SourceCoverage}; @@ -206,10 +206,17 @@ impl CoverageTask { } } +#[derive(Clone, Debug, Default)] +struct TargetAllowList { + modules: AllowList, + source_files: AllowList, +} + struct TaskContext<'a> { config: &'a Config, coverage: BinaryCoverage, - allowlist: TargetAllowList, + module_allowlist: AllowList, + source_allowlist: AllowList, heartbeat: Option, cache: Arc, } @@ -236,7 +243,8 @@ impl<'a> TaskContext<'a> { Ok(Self { config, coverage, - allowlist, + module_allowlist: allowlist.modules, + source_allowlist: allowlist.source_files, heartbeat, cache: Arc::new(cache), }) @@ -285,14 +293,14 @@ impl<'a> TaskContext<'a> { } async fn record_impl(&mut self, input: &Path) -> Result { - let allowlist = self.allowlist.clone(); + let module_allowlist = self.module_allowlist.clone(); let cmd = self.command_for_input(input).await?; let timeout = self.config.timeout(); let cache = self.cache.clone(); let recorded = spawn_blocking(move || { CoverageRecorder::new(cmd) .debuginfo_cache(cache) - .allowlist(allowlist) + .module_allowlist(module_allowlist) .timeout(timeout) .record() }) @@ -487,11 +495,11 @@ impl<'a> TaskContext<'a> { async fn source_coverage(&self) -> Result { // Must be owned due to `spawn_blocking()` lifetimes. - let allowlist = self.allowlist.clone(); + let allowlist = self.source_allowlist.clone(); let binary = self.coverage.clone(); // Conversion to source coverage heavy on blocking I/O. - spawn_blocking(move || binary_to_source_coverage(&binary, allowlist.source_files)).await? + spawn_blocking(move || binary_to_source_coverage(&binary, allowlist)).await? } } diff --git a/src/agent/srcview/tests/srcview.rs b/src/agent/srcview/tests/srcview.rs index aa4a27ca28..4371592836 100644 --- a/src/agent/srcview/tests/srcview.rs +++ b/src/agent/srcview/tests/srcview.rs @@ -79,7 +79,7 @@ fn path() { #[test] #[cfg(target_os = "windows")] fn windows_snapshot_tests() { - use coverage::{AllowList, TargetAllowList}; + use coverage::{binary::DebugInfoCache, AllowList}; use std::time::Duration; insta::glob!("windows", "*.cpp", |path| { @@ -116,15 +116,12 @@ fn windows_snapshot_tests() { }; // filter to just the input test file: - let allowlist = TargetAllowList { - source_files: AllowList::parse(&input_path.to_string_lossy()).unwrap(), - modules: AllowList::default(), - }; + let source_filter = AllowList::parse(&input_path.to_string_lossy()).unwrap(); let exe_cmd = std::process::Command::new(&exe_name); let recorded = coverage::CoverageRecorder::new(exe_cmd) .timeout(Duration::from_secs(120)) - .allowlist(allowlist) + .debuginfo_cache(DebugInfoCache::new(source_filter)) .record() .unwrap(); From 5e03d4957c60379487352429db667bce7a488b21 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Wed, 2 Aug 2023 09:10:45 +1200 Subject: [PATCH 6/9] Migrate test to new coverage --- src/agent/Cargo.lock | 11 ++- src/agent/coverage/Cargo.toml | 5 ++ src/agent/coverage/src/source.rs | 6 +- src/agent/coverage/tests/snapshot.rs | 81 +++++++++++++++++++ .../snapshot__windows_snapshot_tests.snap | 20 +++++ .../tests/windows/inlinee.cpp | 0 src/agent/srcview/Cargo.toml | 7 -- .../srcview__windows_snapshot_tests.snap | 11 --- 8 files changed, 114 insertions(+), 27 deletions(-) create mode 100644 src/agent/coverage/tests/snapshot.rs create mode 100644 src/agent/coverage/tests/snapshots/snapshot__windows_snapshot_tests.snap rename src/agent/{srcview => coverage}/tests/windows/inlinee.cpp (100%) delete mode 100644 src/agent/srcview/tests/snapshots/srcview__windows_snapshot_tests.snap diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock index ee3b7231d0..53ae43494f 100644 --- a/src/agent/Cargo.lock +++ b/src/agent/Cargo.lock @@ -590,12 +590,16 @@ name = "coverage" version = "0.1.0" dependencies = [ "anyhow", + "cc", "clap", "cobertura", + "coverage", "debuggable-module", "debugger", + "dunce", "env_logger", "iced-x86", + "insta", "log", "nix", "pete", @@ -603,6 +607,7 @@ dependencies = [ "procfs", "regex", "symbolic", + "tempfile", "thiserror", ] @@ -1640,7 +1645,6 @@ dependencies = [ "globset", "lazy_static", "linked-hash-map", - "regex", "serde", "similar", "walkdir", @@ -3353,19 +3357,14 @@ name = "srcview" version = "0.1.2" dependencies = [ "anyhow", - "cc", "clap", - "coverage", - "dunce", "env_logger", - "insta", "log", "nom", "pdb", "quick-xml 0.30.0", "regex", "serde", - "tempfile", ] [[package]] diff --git a/src/agent/coverage/Cargo.toml b/src/agent/coverage/Cargo.toml index 0695885b43..f66df17c5a 100644 --- a/src/agent/coverage/Cargo.toml +++ b/src/agent/coverage/Cargo.toml @@ -32,3 +32,8 @@ procfs = { version = "0.12", default-features = false, features = ["flate2"] } clap = { version = "4.3", features = ["derive"] } env_logger = "0.10.0" pretty_assertions = "1.4.0" +insta = { version = "1.31.0", features = ["glob"] } +coverage = { path = "../coverage" } +cc = "1.0" +tempfile = "3.7.0" +dunce = "1.0" diff --git a/src/agent/coverage/src/source.rs b/src/agent/coverage/src/source.rs index 07e35d5aea..735e7c6b75 100644 --- a/src/agent/coverage/src/source.rs +++ b/src/agent/coverage/src/source.rs @@ -51,14 +51,14 @@ impl From for u32 { pub fn binary_to_source_coverage( binary: &BinaryCoverage, - allowlist: impl Into>, + source_allowlist: impl Into>, ) -> Result { use std::collections::btree_map::Entry; use symbolic::debuginfo::Object; use symbolic::symcache::{SymCache, SymCacheConverter}; - let allowlist = allowlist.into().unwrap_or_default(); + let source_allowlist = source_allowlist.into().unwrap_or_default(); let loader = Loader::new(); let mut source = SourceCoverage::default(); @@ -106,7 +106,7 @@ pub fn binary_to_source_coverage( if let Some(file) = location.file() { // Only include relevant inlinees. - if !allowlist.is_allowed(&file.full_path()) { + if !source_allowlist.is_allowed(&file.full_path()) { continue; } diff --git a/src/agent/coverage/tests/snapshot.rs b/src/agent/coverage/tests/snapshot.rs new file mode 100644 index 0000000000..96cd8ae251 --- /dev/null +++ b/src/agent/coverage/tests/snapshot.rs @@ -0,0 +1,81 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#[test] +#[cfg(target_os = "windows")] +fn windows_snapshot_tests() { + use coverage::{binary::DebugInfoCache, source::Line, AllowList}; + use debuggable_module::path::FilePath; + use std::fmt::Write; + use std::time::Duration; + + insta::glob!("windows", "*.cpp", |path| { + let file_name = path.file_name().unwrap(); + + // locate appropriate compiler + let mut cl_exe = + cc::windows_registry::find("x86_64-pc-windows-msvc", "cl.exe").expect("finding cl.exe"); + + // path will have \\?\ prefix, cl.exe doesn't like it + let input_path = dunce::canonicalize(path).unwrap(); + + // directory to compile into + let build_in = tempfile::tempdir().expect("creating tempdir"); + + let output = cl_exe + .arg("/EHsc") + .arg("/Zi") + .arg("/O2") + .arg(&input_path) + .current_dir(build_in.path()) + .spawn() + .expect("launching compiler") + .wait_with_output() + .expect("waiting for compiler to finish"); + + assert!(output.status.success(), "cl.exe failed: {:?}", output); + + let exe_name = { + let mut cwd = build_in.path().to_path_buf(); + cwd.push(file_name); + cwd.set_extension("exe"); + cwd + }; + + // filter to just the input test file: + let source_allowlist = AllowList::parse(&input_path.to_string_lossy()).unwrap(); + + let exe_cmd = std::process::Command::new(&exe_name); + let recorded = coverage::CoverageRecorder::new(exe_cmd) + .timeout(Duration::from_secs(120)) + .debuginfo_cache(DebugInfoCache::new(source_allowlist.clone())) + .record() + .unwrap(); + + // generate source-line coverage info + let source = + coverage::source::binary_to_source_coverage(&recorded.coverage, source_allowlist) + .expect("binary_to_source_coverage"); + + let file_coverage = source + .files + .get(&FilePath::new(input_path.to_string_lossy()).unwrap()) + .expect("coverage for input"); + + let mut result = String::new(); + + let file_source = std::fs::read_to_string(input_path).expect("reading source file"); + for (ix, content) in file_source.lines().enumerate() { + let line = Line::new((ix + 1) as u32).unwrap(); + let prefix = if file_coverage.lines.contains_key(&line) { + "[✔]" + } else { + "[ ]" + }; + + writeln!(result, "{prefix} {content}").unwrap(); + } + + insta::assert_snapshot!(result); + }); +} diff --git a/src/agent/coverage/tests/snapshots/snapshot__windows_snapshot_tests.snap b/src/agent/coverage/tests/snapshots/snapshot__windows_snapshot_tests.snap new file mode 100644 index 0000000000..016717f8ab --- /dev/null +++ b/src/agent/coverage/tests/snapshots/snapshot__windows_snapshot_tests.snap @@ -0,0 +1,20 @@ +--- +source: coverage/tests/snapshot.rs +expression: result +input_file: coverage/tests/windows/inlinee.cpp +--- +[ ] #include +[ ] +[ ] __declspec(dllexport) void test(); +[ ] +[ ] int main() +[✔] { +[✔] std::cout << "Before\n"; +[✔] test(); +[✔] std::cout << "After\n"; +[✔] } +[ ] +[ ] __declspec(dllexport) void test() { +[✔] std::cout << "Hello World!\n"; +[ ] } + diff --git a/src/agent/srcview/tests/windows/inlinee.cpp b/src/agent/coverage/tests/windows/inlinee.cpp similarity index 100% rename from src/agent/srcview/tests/windows/inlinee.cpp rename to src/agent/coverage/tests/windows/inlinee.cpp diff --git a/src/agent/srcview/Cargo.toml b/src/agent/srcview/Cargo.toml index c8467fc9ea..2eccef59f7 100644 --- a/src/agent/srcview/Cargo.toml +++ b/src/agent/srcview/Cargo.toml @@ -19,10 +19,3 @@ quick-xml = "0.30" anyhow = "1.0" env_logger = "0.10" clap = { version = "4.3.0", features = ["derive"] } - -[dev-dependencies] -insta = { version = "1.31.0", features = ["glob", "json", "filters"] } -coverage = { path = "../coverage" } -cc = "1.0" -tempfile = "3.7.0" -dunce = "1.0" diff --git a/src/agent/srcview/tests/snapshots/srcview__windows_snapshot_tests.snap b/src/agent/srcview/tests/snapshots/srcview__windows_snapshot_tests.snap deleted file mode 100644 index 21c0497c6b..0000000000 --- a/src/agent/srcview/tests/snapshots/srcview__windows_snapshot_tests.snap +++ /dev/null @@ -1,11 +0,0 @@ ---- -source: srcview/tests/srcview.rs -expression: srclines_hit -input_file: srcview/tests/windows/inlinee.cpp ---- -[ - { - "path": "…\\inlinee.cpp", - "line": 6 - } -] From f4499f83ab5c0d6dfa991061f4c44047e44f01a3 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Wed, 2 Aug 2023 09:15:50 +1200 Subject: [PATCH 7/9] Remove extra test --- src/agent/srcview/tests/srcview.rs | 78 ------------------------------ 1 file changed, 78 deletions(-) diff --git a/src/agent/srcview/tests/srcview.rs b/src/agent/srcview/tests/srcview.rs index 4371592836..29ba90f64f 100644 --- a/src/agent/srcview/tests/srcview.rs +++ b/src/agent/srcview/tests/srcview.rs @@ -75,81 +75,3 @@ fn path() { assert!(srcview.path_lines("z:\\does\\not\\exist.c").is_none()); } - -#[test] -#[cfg(target_os = "windows")] -fn windows_snapshot_tests() { - use coverage::{binary::DebugInfoCache, AllowList}; - use std::time::Duration; - - insta::glob!("windows", "*.cpp", |path| { - let file_name = path.file_name().unwrap(); - - // locate appropriate compiler - let mut cl_exe = - cc::windows_registry::find("x86_64-pc-windows-msvc", "cl.exe").expect("finding cl.exe"); - - // path will have \\?\ prefix, cl.exe doesn't like it - let input_path = dunce::canonicalize(path).unwrap(); - - // directory to compile into - let build_in = tempfile::tempdir().expect("creating tempdir"); - - let output = cl_exe - .arg("/EHsc") - .arg("/Zi") - .arg("/O2") - .arg(&input_path) - .current_dir(build_in.path()) - .spawn() - .expect("launching compiler") - .wait_with_output() - .expect("waiting for compiler to finish"); - - assert!(output.status.success(), "cl.exe failed: {:?}", output); - - let exe_name = { - let mut cwd = build_in.path().to_path_buf(); - cwd.push(file_name); - cwd.set_extension("exe"); - cwd - }; - - // filter to just the input test file: - let source_filter = AllowList::parse(&input_path.to_string_lossy()).unwrap(); - - let exe_cmd = std::process::Command::new(&exe_name); - let recorded = coverage::CoverageRecorder::new(exe_cmd) - .timeout(Duration::from_secs(120)) - .debuginfo_cache(DebugInfoCache::new(source_filter)) - .record() - .unwrap(); - - // generate information with srcview - let target_file_name = exe_name.file_name().unwrap().to_string_lossy().into_owned(); - let mut srcview = SrcView::new(); - srcview - .insert(&target_file_name, &exe_name.with_extension("pdb")) - .unwrap(); - - let mut srclines_hit: Vec = Vec::new(); - for (module, coverage) in &recorded.coverage.modules { - let module_name = module.file_name(); - for (offset, count) in &coverage.offsets { - if count.0 > 0 { - if let Some(line) = srcview.modoff(&ModOff { - module: module_name.to_string(), - offset: offset.0 as usize, - }) { - srclines_hit.push(line); - }; - } - } - } - - // apply filter to make output stable, replace prefix of path with "…" - insta::with_settings!({filters => vec![(r"[A-Z]:.*windows", "…")]}, { - insta::assert_json_snapshot!(srclines_hit); - }); - }); -} From 26c3b7dfef2a415e681a30b57608e522475ecd81 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Tue, 1 Aug 2023 21:26:45 +0000 Subject: [PATCH 8/9] Fixup Linux build --- src/agent/coverage/src/record.rs | 2 +- src/agent/coverage/src/record/linux.rs | 2 +- src/agent/srcview/src/modoff.rs | 21 +-------------------- 3 files changed, 3 insertions(+), 22 deletions(-) diff --git a/src/agent/coverage/src/record.rs b/src/agent/coverage/src/record.rs index 4a1b2c9e45..534d1d4d63 100644 --- a/src/agent/coverage/src/record.rs +++ b/src/agent/coverage/src/record.rs @@ -80,7 +80,7 @@ impl CoverageRecorder { let child_pid = child_pid.clone(); timer::timed(self.timeout, move || { - let mut recorder = LinuxRecorder::new(&loader, self.allowlist, &self.cache); + let mut recorder = LinuxRecorder::new(&loader, self.module_allowlist, &self.cache); let mut dbg = Debugger::new(&mut recorder); let child = dbg.spawn(self.cmd)?; diff --git a/src/agent/coverage/src/record/linux.rs b/src/agent/coverage/src/record/linux.rs index 9becaf9ee0..7ed47de7d5 100644 --- a/src/agent/coverage/src/record/linux.rs +++ b/src/agent/coverage/src/record/linux.rs @@ -35,7 +35,7 @@ impl<'cache, 'data> LinuxRecorder<'cache, 'data> { let modules = BTreeMap::new(); Self { - allowlist, + module_allowlist, cache, coverage, loader, diff --git a/src/agent/srcview/src/modoff.rs b/src/agent/srcview/src/modoff.rs index c067588af6..696380c73a 100644 --- a/src/agent/srcview/src/modoff.rs +++ b/src/agent/srcview/src/modoff.rs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use std::cmp::Ordering; use std::error::Error; use std::fmt; @@ -14,7 +13,7 @@ use nom::multi::many0; use nom::IResult; /// A module name and an offset -#[derive(Clone, Eq, PartialEq, Hash)] +#[derive(Clone, Eq, PartialEq, Hash, PartialOrd, Ord)] pub struct ModOff { pub module: String, pub offset: usize, @@ -59,24 +58,6 @@ impl fmt::Display for ModOff { } } -impl Ord for ModOff { - fn cmp(&self, other: &Self) -> Ordering { - let path_cmp = self.module.cmp(&other.module); - - if path_cmp != Ordering::Equal { - return path_cmp; - } - - self.offset.cmp(&other.offset) - } -} - -impl PartialOrd for ModOff { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - impl ModOff { pub fn new(module: &str, offset: usize) -> Self { Self { From f8c230d2fd673a64ba8b77ccb3138e1937512d78 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Tue, 8 Aug 2023 11:13:09 +1200 Subject: [PATCH 9/9] Add support for slow-tests --- src/agent/coverage/Cargo.toml | 3 +++ src/agent/coverage/tests/snapshot.rs | 2 +- src/ci/agent.sh | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/agent/coverage/Cargo.toml b/src/agent/coverage/Cargo.toml index f66df17c5a..ad87a12273 100644 --- a/src/agent/coverage/Cargo.toml +++ b/src/agent/coverage/Cargo.toml @@ -4,6 +4,9 @@ version = "0.1.0" edition = "2021" license = "MIT" +[features] +slow-tests = [] + [dependencies] anyhow = { version = "1.0", features = ["backtrace"] } cobertura = { path = "../cobertura" } diff --git a/src/agent/coverage/tests/snapshot.rs b/src/agent/coverage/tests/snapshot.rs index 96cd8ae251..0b44bf1dd6 100644 --- a/src/agent/coverage/tests/snapshot.rs +++ b/src/agent/coverage/tests/snapshot.rs @@ -2,7 +2,7 @@ // Licensed under the MIT License. #[test] -#[cfg(target_os = "windows")] +#[cfg(all(target_os = "windows", feature = "slow-tests"))] fn windows_snapshot_tests() { use coverage::{binary::DebugInfoCache, source::Line, AllowList}; use debuggable_module::path::FilePath; diff --git a/src/ci/agent.sh b/src/ci/agent.sh index 092d1c5a72..4a49c975b3 100755 --- a/src/ci/agent.sh +++ b/src/ci/agent.sh @@ -37,7 +37,7 @@ export RUST_BACKTRACE=full # Run tests and collect coverage # https://github.com/taiki-e/cargo-llvm-cov -cargo llvm-cov nextest --all-targets --locked --workspace --lcov --output-path "$output_dir/lcov.info" +cargo llvm-cov nextest --all-targets --features slow-tests --locked --workspace --lcov --output-path "$output_dir/lcov.info" # TODO: re-enable integration tests. # cargo test --release --manifest-path ./onefuzz-task/Cargo.toml --features integration_test -- --nocapture