diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock index 28a3ca63e..916183743 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", ] diff --git a/src/agent/coverage/Cargo.toml b/src/agent/coverage/Cargo.toml index 0695885b4..ad87a1227 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" } @@ -32,3 +35,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/examples/record.rs b/src/agent/coverage/examples/record.rs index 0edae5645..4d2ec4692 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 45520cde4..079f41500 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 c773d6cf3..61284bd6f 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 499e82a92..771b0986f 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 a53b81e1c..534d1d4d6 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,7 +18,7 @@ pub mod linux; pub mod windows; pub struct CoverageRecorder { - allowlist: TargetAllowList, + module_allowlist: AllowList, cache: Arc, cmd: Command, loader: Arc, @@ -30,22 +30,20 @@ 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, + module_allowlist: AllowList::default(), + cache: Arc::new(DebugInfoCache::new(AllowList::default())), cmd, loader, timeout, } } - pub fn allowlist(mut self, allowlist: TargetAllowList) -> Self { - self.allowlist = allowlist; + pub fn module_allowlist(mut self, module_allowlist: AllowList) -> Self { + self.module_allowlist = module_allowlist; self } @@ -82,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)?; @@ -128,7 +126,8 @@ impl CoverageRecorder { let loader = self.loader.clone(); crate::timer::timed(self.timeout, move || { - let mut recorder = WindowsRecorder::new(&loader, self.allowlist, &self.cache); + 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 5eadf579f..7ed47de7d 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,14 +28,14 @@ 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(); let modules = BTreeMap::new(); Self { - allowlist, + module_allowlist, cache, coverage, loader, @@ -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 79170a50e..076aa70d3 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/coverage/src/source.rs b/src/agent/coverage/src/source.rs index 07e35d5ae..735e7c6b7 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 000000000..0b44bf1dd --- /dev/null +++ b/src/agent/coverage/tests/snapshot.rs @@ -0,0 +1,81 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#[test] +#[cfg(all(target_os = "windows", feature = "slow-tests"))] +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 000000000..016717f8a --- /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/coverage/tests/windows/inlinee.cpp b/src/agent/coverage/tests/windows/inlinee.cpp new file mode 100644 index 000000000..e0167f4af --- /dev/null +++ b/src/agent/coverage/tests/windows/inlinee.cpp @@ -0,0 +1,14 @@ +#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/onefuzz-task/src/tasks/coverage/generic.rs b/src/agent/onefuzz-task/src/tasks/coverage/generic.rs index b4d56b457..17ece8c01 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/ci/agent.sh b/src/ci/agent.sh index 092d1c5a7..4a49c975b 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 diff --git a/src/proxy-manager/Cargo.lock b/src/proxy-manager/Cargo.lock index 666cd23c9..a5205a47b 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",