From 9f52e10fe228386856a76893d98f4b908616cc7e Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Thu, 15 Apr 2021 13:27:20 -0700 Subject: [PATCH 01/12] Represent offsets as `u32` --- src/agent/coverage/src/block.rs | 31 ++++++++++++------------- src/agent/coverage/src/block/windows.rs | 8 +++---- src/agent/coverage/src/cache.rs | 4 ++-- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/agent/coverage/src/block.rs b/src/agent/coverage/src/block.rs index 752c2b5f84..91328cec42 100644 --- a/src/agent/coverage/src/block.rs +++ b/src/agent/coverage/src/block.rs @@ -26,7 +26,7 @@ impl CommandBlockCov { /// Returns `true` if the module was newly-inserted (which initializes its /// block coverage map). Otherwise, returns `false`, and no re-computation /// is performed. - pub fn insert(&mut self, path: &ModulePath, offsets: impl Iterator) -> bool { + pub fn insert(&mut self, path: &ModulePath, offsets: impl Iterator) -> bool { use std::collections::btree_map::Entry; match self.modules.entry(path.clone()) { @@ -38,7 +38,7 @@ impl CommandBlockCov { } } - pub fn increment(&mut self, path: &ModulePath, offset: u64) { + pub fn increment(&mut self, path: &ModulePath, offset: u32) { let entry = self.modules.entry(path.clone()); if let btree_map::Entry::Vacant(_) = entry { @@ -69,16 +69,16 @@ impl CommandBlockCov { #[serde(transparent)] pub struct ModuleCov { #[serde(with = "array")] - pub blocks: BTreeMap, + pub blocks: BTreeMap, } impl ModuleCov { - pub fn new(offsets: impl Iterator) -> Self { + pub fn new(offsets: impl Iterator) -> Self { let blocks = offsets.map(|o| (o, BlockCov::new(o))).collect(); Self { blocks } } - pub fn increment(&mut self, offset: u64) { + pub fn increment(&mut self, offset: u32) { let block = self .blocks .entry(offset) @@ -102,13 +102,12 @@ impl ModuleCov { pub struct BlockCov { /// Offset of the block, relative to the module base load address. // - // These offsets are represented as `u64` values, but since they are offsets - // within an assumed well-formed module, they should never exceed a `u32`. - // We thus assume that they can be losslessly serialized to an `f64`. + // These offsets come from well-formed executable modules, so we assume they + // can be represented as `u32` values and losslessly serialized to an `f64`. // // If we need to handle malformed binaries or arbitrary addresses, then this // will need revision. - pub offset: u64, + pub offset: u32, /// Number of times a block was seen to be executed, relative to some input /// or corpus. @@ -124,7 +123,7 @@ pub struct BlockCov { } impl BlockCov { - pub fn new(offset: u64) -> Self { + pub fn new(offset: u32) -> Self { Self { offset, count: 0 } } } @@ -138,7 +137,7 @@ mod array { use super::BlockCov; - type BlockCovMap = BTreeMap; + type BlockCovMap = BTreeMap; pub fn serialize(data: &BlockCovMap, ser: S) -> Result where @@ -189,8 +188,8 @@ mod tests { use super::*; - // Builds a `ModuleCov` from a vec of `(offset, counts)` tuples. - fn from_vec(data: Vec<(u64, u32)>) -> ModuleCov { + // Builds a `ModuleCov` from a vec of `(offset, count)` tuples. + fn from_vec(data: Vec<(u32, u32)>) -> ModuleCov { let offsets = data.iter().map(|(o, _)| *o); let mut cov = ModuleCov::new(offsets); for (offset, count) in data { @@ -201,8 +200,8 @@ mod tests { cov } - // Builds a vec of `(count, offset)` tuples from a `ModuleCov`. - fn to_vec(cov: &ModuleCov) -> Vec<(u64, u32)> { + // Builds a vec of `(offset, count)` tuples from a `ModuleCov`. + fn to_vec(cov: &ModuleCov) -> Vec<(u32, u32)> { cov.blocks.iter().map(|(o, b)| (*o, b.count)).collect() } @@ -276,7 +275,7 @@ mod tests { ModulePath::new(p) } - fn cmd_cov_from_vec(data: Vec<(&ModulePath, Vec<(u64, u32)>)>) -> CommandBlockCov { + fn cmd_cov_from_vec(data: Vec<(&ModulePath, Vec<(u32, u32)>)>) -> CommandBlockCov { let mut cov = CommandBlockCov::default(); for (path, module_data) in data { diff --git a/src/agent/coverage/src/block/windows.rs b/src/agent/coverage/src/block/windows.rs index 519fa4f266..e97b880bd4 100644 --- a/src/agent/coverage/src/block/windows.rs +++ b/src/agent/coverage/src/block/windows.rs @@ -231,7 +231,7 @@ struct Breakpoints { // Map of breakpoint IDs to data which pick out an code location. For a // value `(module, offset)`, `module` is an index into `self.modules`, and // `offset` is a VA offset relative to the module base. - registered: BTreeMap, + registered: BTreeMap, } impl Breakpoints { @@ -245,7 +245,7 @@ impl Breakpoints { &mut self, dbg: &mut Debugger, module: &Module, - offsets: impl Iterator, + offsets: impl Iterator, ) -> Result<()> { // From the `target::Module`, create and save a `ModulePath`. let module_path = ModulePath::new(module.path().to_owned())?; @@ -254,7 +254,7 @@ impl Breakpoints { for offset in offsets { // Register the breakpoint in the running target address space. - let id = dbg.register_breakpoint(module.name(), offset, BreakpointType::OneTime); + let id = dbg.register_breakpoint(module.name(), offset as u64, BreakpointType::OneTime); // Associate the opaque `BreakpointId` with the module and offset. self.registered.insert(id, (module_index, offset)); @@ -271,5 +271,5 @@ impl Breakpoints { #[derive(Clone, Copy, Debug)] pub struct BreakpointData<'a> { pub module: &'a ModulePath, - pub offset: u64, + pub offset: u32, } diff --git a/src/agent/coverage/src/cache.rs b/src/agent/coverage/src/cache.rs index 8c8085ca7e..53128f4adb 100644 --- a/src/agent/coverage/src/cache.rs +++ b/src/agent/coverage/src/cache.rs @@ -66,7 +66,7 @@ pub struct ModuleInfo { pub module: ModuleIndex, /// Set of image offsets of basic blocks. - pub blocks: BTreeSet, + pub blocks: BTreeSet, } impl ModuleInfo { @@ -89,7 +89,7 @@ impl ModuleInfo { let pe = goblin::pe::PE::parse(&data)?; let module = ModuleIndex::index_pe(path.clone(), &pe); let offsets = crate::pe::process_module(path, &data, &pe, false, handle.into())?; - let blocks = offsets.ones().map(|off| off as u64).collect(); + let blocks = offsets.ones().map(|off| off as u32).collect(); Ok(Self { module, blocks }) } From 1abefc76e10e24a2fc70b99a30cc955308767a73 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Thu, 15 Apr 2021 21:51:32 +0000 Subject: [PATCH 02/12] Also represent ELF offsets as `u32` --- src/agent/coverage/src/block/linux.rs | 25 +++++++++++++++++++------ src/agent/coverage/src/disasm.rs | 19 +++++++++++++------ 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/src/agent/coverage/src/block/linux.rs b/src/agent/coverage/src/block/linux.rs index 78b382eceb..506a06ec21 100644 --- a/src/agent/coverage/src/block/linux.rs +++ b/src/agent/coverage/src/block/linux.rs @@ -2,6 +2,7 @@ // Licensed under the MIT License. use std::collections::BTreeMap; +use std::convert::TryInto; use std::ffi::OsStr; use std::process::Command; @@ -140,7 +141,7 @@ impl<'c> Recorder<'c> { .find_va_image(pc) .ok_or_else(|| format_err!("unable to find image for va = {:x}", pc))?; - let offset = image.va_to_offset(pc); + let offset = image.va_to_offset(pc)?; self.coverage.increment(image.path(), offset); // Execute clobbered instruction on restart. @@ -182,7 +183,15 @@ impl<'c> Recorder<'c> { // Check the maybe-demangled against the coverage filter. if self.filter.includes_symbol(&info.module.path, symbol_name) { - for offset in info.blocks.range(symbol.range()) { + // Convert range bounds to an `offset`-sized type. + let range = { + let range = symbol.range(); + let lo: u32 = range.start.try_into()?; + let hi: u32 = range.end.try_into()?; + lo..hi + }; + + for offset in info.blocks.range(range) { allowed_blocks.push(*offset); } } @@ -307,12 +316,16 @@ impl ModuleImage { (self.map.address.0)..(self.map.address.1) } - pub fn va_to_offset(&self, va: u64) -> u64 { - va - self.base() + pub fn va_to_offset(&self, va: u64) -> Result { + if let Some(offset) = va.checked_sub(self.base()) { + Ok(offset.try_into()?) + } else { + anyhow::bail!("underflow converting VA to image offset") + } } - pub fn offset_to_va(&self, offset: u64) -> u64 { - self.base() + offset + pub fn offset_to_va(&self, offset: u32) -> u64 { + self.base() + (offset as u64) } } diff --git a/src/agent/coverage/src/disasm.rs b/src/agent/coverage/src/disasm.rs index 10dc30e953..024e991699 100644 --- a/src/agent/coverage/src/disasm.rs +++ b/src/agent/coverage/src/disasm.rs @@ -2,8 +2,9 @@ // Licensed under the MIT License. use std::collections::BTreeSet; +use std::convert::TryInto; -use anyhow::{bail, format_err, Result}; +use anyhow::{bail, format_err, Context, Result}; use iced_x86::{Decoder, DecoderOptions, Instruction}; use crate::code::{ModuleIndex, Symbol}; @@ -19,7 +20,7 @@ impl<'a> ModuleDisassembler<'a> { } /// Find block entry points for every symbol in the module. - pub fn find_blocks(&self) -> BTreeSet { + pub fn find_blocks(&self) -> BTreeSet { let mut blocks = BTreeSet::new(); for symbol in self.module.symbols.iter() { @@ -36,7 +37,7 @@ impl<'a> ModuleDisassembler<'a> { } /// Find all entry points for blocks contained within the region of `symbol`. - fn insert_symbol_blocks(&self, blocks: &mut BTreeSet, symbol: &Symbol) -> Result<()> { + fn insert_symbol_blocks(&self, blocks: &mut BTreeSet, symbol: &Symbol) -> Result<()> { // Slice the symbol's instruction data from the module file data. let data = if let Some(data) = self.data.get(symbol.file_range_usize()) { data @@ -56,7 +57,7 @@ impl<'a> ModuleDisassembler<'a> { decoder.set_ip(va); // Function entry is a leader. - blocks.insert(symbol.image_offset); + blocks.insert(symbol.image_offset.try_into()?); let mut inst = Instruction::default(); while decoder.can_decode() { @@ -67,7 +68,7 @@ impl<'a> ModuleDisassembler<'a> { // The branch target is a leader, if it is intra-procedural. if symbol.contains_image_offset(offset) { - blocks.insert(offset); + blocks.insert(offset.try_into().context("ELF offset overflows `u32`")?); } // Only mark the fallthrough instruction as a leader if the branch is conditional. @@ -84,7 +85,13 @@ impl<'a> ModuleDisassembler<'a> { // We decoded the current instruction, so the decoder offset is // set to the next instruction. let next = decoder.ip() as u64; - let next_offset = next - self.module.base_va; + let next_offset = + if let Some(offset) = next.checked_sub(self.module.base_va) { + offset.try_into().context("ELF offset overflows `u32`")? + } else { + anyhow::bail!("underflow converting ELF VA to offset") + }; + blocks.insert(next_offset); } } From f29504f5b1053e46f131c94d6f734194f876c0ce Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Thu, 15 Apr 2021 11:16:53 -0700 Subject: [PATCH 03/12] Add covered, features, difference --- src/agent/coverage/src/block.rs | 106 ++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/src/agent/coverage/src/block.rs b/src/agent/coverage/src/block.rs index 91328cec42..ef9ffca858 100644 --- a/src/agent/coverage/src/block.rs +++ b/src/agent/coverage/src/block.rs @@ -57,12 +57,35 @@ impl CommandBlockCov { self.modules.iter() } + pub fn covered(&self) -> u64 { + self.modules.values().map(|m| m.covered()).sum() + } + + pub fn features(&self) -> u64 { + self.modules.values().map(|m| m.features()).sum() + } + pub fn merge_max(&mut self, other: &Self) { for (module, cov) in other.iter() { let entry = self.modules.entry(module.clone()).or_default(); entry.merge_max(cov); } } + + // Count of blocks covered by `self` but not `other`. + pub fn difference(&self, other: &Self) -> u64 { + let mut total = 0; + + for (module, cov) in &self.modules { + if let Some(other_cov) = other.modules.get(module) { + total += cov.difference(other_cov); + } else { + total += cov.covered(); + } + } + + total + } } #[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] @@ -78,6 +101,33 @@ impl ModuleCov { Self { blocks } } + pub fn covered(&self) -> u64 { + self.blocks + .values() + .map(|b| u64::min(1, b.count as u64)) + .sum() + } + + pub fn features(&self) -> u64 { + self.blocks.len() as u64 + } + + pub fn difference(&self, other: &Self) -> u64 { + let mut total = 0; + + for (offset, block) in &self.blocks { + if let Some(other_block) = other.blocks.get(offset) { + if other_block.count == 0 { + total += u64::min(1, block.count as u64); + } + } else { + total += u64::min(1, block.count as u64); + } + } + + total + } + pub fn increment(&mut self, offset: u32) { let block = self .blocks @@ -399,4 +449,60 @@ mod tests { Ok(()) } + + #[test] + fn test_cmd_cov_stats() -> Result<()> { + let main_exe = module_path("/onefuzz/main.exe")?; + let some_dll = module_path("/common/some.dll")?; + + let mut total: CommandBlockCov = serde_json::from_value(json!({ + some_dll.to_string(): [ + { "offset": 2, "count": 0 }, + { "offset": 30, "count": 1 }, + { "offset": 400, "count": 0 }, + ], + main_exe.to_string(): [ + { "offset": 1, "count": 2 }, + { "offset": 20, "count": 0 }, + { "offset": 300, "count": 3 }, + ], + }))?; + + assert_eq!(total.features(), 6); + assert_eq!(total.covered(), 3); + assert_eq!( + total.covered(), + total.difference(&CommandBlockCov::default()) + ); + assert_eq!(total.difference(&total), 0); + + let new: CommandBlockCov = serde_json::from_value(json!({ + some_dll.to_string(): [ + { "offset": 2, "count": 0 }, + { "offset": 22, "count": 4 }, + { "offset": 30, "count": 5 }, + { "offset": 400, "count": 6 }, + ], + main_exe.to_string(): [ + { "offset": 1, "count": 0 }, + { "offset": 300, "count": 1 }, + { "offset": 5000, "count": 0 }, + ], + }))?; + + assert_eq!(new.features(), 7); + assert_eq!(new.covered(), 4); + assert_eq!(new.covered(), new.difference(&CommandBlockCov::default())); + assert_eq!(new.difference(&new), 0); + + assert_eq!(new.difference(&total), 2); + assert_eq!(total.difference(&new), 1); + + total.merge_max(&new); + + assert_eq!(total.features(), 8); + assert_eq!(total.covered(), 5); + + Ok(()) + } } From c73088c91491c52b5dc60d589e0adae2dd2409e2 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Thu, 15 Apr 2021 16:37:29 -0700 Subject: [PATCH 04/12] Add doc comments --- src/agent/coverage/src/block.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/agent/coverage/src/block.rs b/src/agent/coverage/src/block.rs index ef9ffca858..04fc798c84 100644 --- a/src/agent/coverage/src/block.rs +++ b/src/agent/coverage/src/block.rs @@ -57,10 +57,12 @@ impl CommandBlockCov { self.modules.iter() } + /// Total count of covered blocks across all modules. pub fn covered(&self) -> u64 { self.modules.values().map(|m| m.covered()).sum() } + /// Total count of known blocks across all modules. pub fn features(&self) -> u64 { self.modules.values().map(|m| m.features()).sum() } @@ -72,7 +74,7 @@ impl CommandBlockCov { } } - // Count of blocks covered by `self` but not `other`. + /// Total count of blocks covered by `self` but not `other`. pub fn difference(&self, other: &Self) -> u64 { let mut total = 0; @@ -101,6 +103,7 @@ impl ModuleCov { Self { blocks } } + /// Total count of blocks that have been reached (have a nonzero count). pub fn covered(&self) -> u64 { self.blocks .values() @@ -108,6 +111,7 @@ impl ModuleCov { .sum() } + /// Total count of known blocks. pub fn features(&self) -> u64 { self.blocks.len() as u64 } From 1519d75a4ce0ac2bb513804bbd4ce6d36c2503a4 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Thu, 15 Apr 2021 18:20:34 -0700 Subject: [PATCH 05/12] Add comments --- src/agent/coverage/src/block.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/agent/coverage/src/block.rs b/src/agent/coverage/src/block.rs index 04fc798c84..a0a2751987 100644 --- a/src/agent/coverage/src/block.rs +++ b/src/agent/coverage/src/block.rs @@ -74,7 +74,9 @@ impl CommandBlockCov { } } - /// Total count of blocks covered by `self` but not `other`. + /// Total count of blocks covered by modules in `self` but not `other`. + /// + /// Counts modules absent in `self`. pub fn difference(&self, other: &Self) -> u64 { let mut total = 0; @@ -116,6 +118,7 @@ impl ModuleCov { self.blocks.len() as u64 } + /// Total count of blocks covered by `self` but not `other`. pub fn difference(&self, other: &Self) -> u64 { let mut total = 0; From dc25af348820c547582e94ad974b3471e870a082 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Thu, 15 Apr 2021 18:22:45 -0700 Subject: [PATCH 06/12] Test case of newly-discovered module --- src/agent/coverage/src/block.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/agent/coverage/src/block.rs b/src/agent/coverage/src/block.rs index a0a2751987..1408beb7f6 100644 --- a/src/agent/coverage/src/block.rs +++ b/src/agent/coverage/src/block.rs @@ -461,6 +461,7 @@ mod tests { fn test_cmd_cov_stats() -> Result<()> { let main_exe = module_path("/onefuzz/main.exe")?; let some_dll = module_path("/common/some.dll")?; + let other_dll = module_path("/common/other.dll")?; let mut total: CommandBlockCov = serde_json::from_value(json!({ some_dll.to_string(): [ @@ -495,20 +496,24 @@ mod tests { { "offset": 300, "count": 1 }, { "offset": 5000, "count": 0 }, ], + other_dll.to_string(): [ + { "offset": 123, "count": 0 }, + { "offset": 456, "count": 10 }, + ], }))?; - assert_eq!(new.features(), 7); - assert_eq!(new.covered(), 4); + assert_eq!(new.features(), 9); + assert_eq!(new.covered(), 5); assert_eq!(new.covered(), new.difference(&CommandBlockCov::default())); assert_eq!(new.difference(&new), 0); - assert_eq!(new.difference(&total), 2); + assert_eq!(new.difference(&total), 3); assert_eq!(total.difference(&new), 1); total.merge_max(&new); - assert_eq!(total.features(), 8); - assert_eq!(total.covered(), 5); + assert_eq!(total.features(), 10); + assert_eq!(total.covered(), 6); Ok(()) } From f77fe7141a5a0dd409a2e9f9b6f316c4e00384e6 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Fri, 16 Apr 2021 14:11:59 -0700 Subject: [PATCH 07/12] Use less general method names --- src/agent/coverage/src/block.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/agent/coverage/src/block.rs b/src/agent/coverage/src/block.rs index 1408beb7f6..5a7e80925e 100644 --- a/src/agent/coverage/src/block.rs +++ b/src/agent/coverage/src/block.rs @@ -58,13 +58,13 @@ impl CommandBlockCov { } /// Total count of covered blocks across all modules. - pub fn covered(&self) -> u64 { - self.modules.values().map(|m| m.covered()).sum() + pub fn covered_blocks(&self) -> u64 { + self.modules.values().map(|m| m.covered_blocks()).sum() } /// Total count of known blocks across all modules. - pub fn features(&self) -> u64 { - self.modules.values().map(|m| m.features()).sum() + pub fn known_blocks(&self) -> u64 { + self.modules.values().map(|m| m.known_blocks()).sum() } pub fn merge_max(&mut self, other: &Self) { @@ -84,7 +84,7 @@ impl CommandBlockCov { if let Some(other_cov) = other.modules.get(module) { total += cov.difference(other_cov); } else { - total += cov.covered(); + total += cov.covered_blocks(); } } @@ -106,7 +106,7 @@ impl ModuleCov { } /// Total count of blocks that have been reached (have a nonzero count). - pub fn covered(&self) -> u64 { + pub fn covered_blocks(&self) -> u64 { self.blocks .values() .map(|b| u64::min(1, b.count as u64)) @@ -114,7 +114,7 @@ impl ModuleCov { } /// Total count of known blocks. - pub fn features(&self) -> u64 { + pub fn known_blocks(&self) -> u64 { self.blocks.len() as u64 } @@ -476,10 +476,10 @@ mod tests { ], }))?; - assert_eq!(total.features(), 6); - assert_eq!(total.covered(), 3); + assert_eq!(total.known_blocks(), 6); + assert_eq!(total.covered_blocks(), 3); assert_eq!( - total.covered(), + total.covered_blocks(), total.difference(&CommandBlockCov::default()) ); assert_eq!(total.difference(&total), 0); @@ -502,9 +502,9 @@ mod tests { ], }))?; - assert_eq!(new.features(), 9); - assert_eq!(new.covered(), 5); - assert_eq!(new.covered(), new.difference(&CommandBlockCov::default())); + assert_eq!(new.known_blocks(), 9); + assert_eq!(new.covered_blocks(), 5); + assert_eq!(new.covered_blocks(), new.difference(&CommandBlockCov::default())); assert_eq!(new.difference(&new), 0); assert_eq!(new.difference(&total), 3); @@ -512,8 +512,8 @@ mod tests { total.merge_max(&new); - assert_eq!(total.features(), 10); - assert_eq!(total.covered(), 6); + assert_eq!(total.known_blocks(), 10); + assert_eq!(total.covered_blocks(), 6); Ok(()) } From 2a9d7e6e35633ce3c7b8bda6b07fa4067e307ac1 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Fri, 16 Apr 2021 14:14:13 -0700 Subject: [PATCH 08/12] Filter then count --- src/agent/coverage/src/block.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/agent/coverage/src/block.rs b/src/agent/coverage/src/block.rs index 5a7e80925e..a786180035 100644 --- a/src/agent/coverage/src/block.rs +++ b/src/agent/coverage/src/block.rs @@ -107,10 +107,7 @@ impl ModuleCov { /// Total count of blocks that have been reached (have a nonzero count). pub fn covered_blocks(&self) -> u64 { - self.blocks - .values() - .map(|b| u64::min(1, b.count as u64)) - .sum() + self.blocks.values().filter(|b| b.count > 0).count() as u64 } /// Total count of known blocks. From 76573554c04d38daa2133322e8a642952587ee0c Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Fri, 16 Apr 2021 14:19:18 -0700 Subject: [PATCH 09/12] Emphasize corner case in comment --- src/agent/coverage/src/block.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/agent/coverage/src/block.rs b/src/agent/coverage/src/block.rs index a786180035..2ed57af771 100644 --- a/src/agent/coverage/src/block.rs +++ b/src/agent/coverage/src/block.rs @@ -116,6 +116,9 @@ impl ModuleCov { } /// Total count of blocks covered by `self` but not `other`. + /// + /// A difference of 0 does not imply identical coverage, and a nonzero + /// difference does not imply that `self` covers every block in `other`. pub fn difference(&self, other: &Self) -> u64 { let mut total = 0; From b047bf9afa55d7a7b15203f26fa9f1cd8ca2bc24 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Fri, 16 Apr 2021 14:19:57 -0700 Subject: [PATCH 10/12] Format --- src/agent/coverage/src/block.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/agent/coverage/src/block.rs b/src/agent/coverage/src/block.rs index 2ed57af771..8e3b2dc33c 100644 --- a/src/agent/coverage/src/block.rs +++ b/src/agent/coverage/src/block.rs @@ -504,7 +504,10 @@ mod tests { assert_eq!(new.known_blocks(), 9); assert_eq!(new.covered_blocks(), 5); - assert_eq!(new.covered_blocks(), new.difference(&CommandBlockCov::default())); + assert_eq!( + new.covered_blocks(), + new.difference(&CommandBlockCov::default()) + ); assert_eq!(new.difference(&new), 0); assert_eq!(new.difference(&total), 3); From 9c2c8ea4998e0b69a5621e3151515db9bb7346da Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Fri, 16 Apr 2021 14:20:47 -0700 Subject: [PATCH 11/12] Use local for empty coverage --- src/agent/coverage/src/block.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/agent/coverage/src/block.rs b/src/agent/coverage/src/block.rs index 8e3b2dc33c..e91fd3b6f0 100644 --- a/src/agent/coverage/src/block.rs +++ b/src/agent/coverage/src/block.rs @@ -463,6 +463,8 @@ mod tests { let some_dll = module_path("/common/some.dll")?; let other_dll = module_path("/common/other.dll")?; + let empty = CommandBlockCov::default(); + let mut total: CommandBlockCov = serde_json::from_value(json!({ some_dll.to_string(): [ { "offset": 2, "count": 0 }, @@ -478,10 +480,7 @@ mod tests { assert_eq!(total.known_blocks(), 6); assert_eq!(total.covered_blocks(), 3); - assert_eq!( - total.covered_blocks(), - total.difference(&CommandBlockCov::default()) - ); + assert_eq!(total.covered_blocks(), total.difference(&empty)); assert_eq!(total.difference(&total), 0); let new: CommandBlockCov = serde_json::from_value(json!({ @@ -504,10 +503,7 @@ mod tests { assert_eq!(new.known_blocks(), 9); assert_eq!(new.covered_blocks(), 5); - assert_eq!( - new.covered_blocks(), - new.difference(&CommandBlockCov::default()) - ); + assert_eq!(new.covered_blocks(), new.difference(&empty)); assert_eq!(new.difference(&new), 0); assert_eq!(new.difference(&total), 3); From b9821ef7465be1d96dfd710b927a5043ed79f274 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Fri, 16 Apr 2021 14:24:52 -0700 Subject: [PATCH 12/12] Improve word choice --- src/agent/coverage/src/block.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/agent/coverage/src/block.rs b/src/agent/coverage/src/block.rs index e91fd3b6f0..7ac6f0a36e 100644 --- a/src/agent/coverage/src/block.rs +++ b/src/agent/coverage/src/block.rs @@ -105,7 +105,7 @@ impl ModuleCov { Self { blocks } } - /// Total count of blocks that have been reached (have a nonzero count). + /// Total count of blocks that have been reached (have a positive count). pub fn covered_blocks(&self) -> u64 { self.blocks.values().filter(|b| b.count > 0).count() as u64 } @@ -117,7 +117,7 @@ impl ModuleCov { /// Total count of blocks covered by `self` but not `other`. /// - /// A difference of 0 does not imply identical coverage, and a nonzero + /// A difference of 0 does not imply identical coverage, and a positive /// difference does not imply that `self` covers every block in `other`. pub fn difference(&self, other: &Self) -> u64 { let mut total = 0;