From 4d44120f4e4bce7f6dae569d3a9de324f0a067b5 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Mon, 13 Feb 2023 18:15:02 -0800 Subject: [PATCH 01/16] Derive common traits for `Function` --- src/agent/debuggable-module/src/debuginfo.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/agent/debuggable-module/src/debuginfo.rs b/src/agent/debuggable-module/src/debuginfo.rs index 7411d8bb06..f5f3a9b9d7 100644 --- a/src/agent/debuggable-module/src/debuginfo.rs +++ b/src/agent/debuggable-module/src/debuginfo.rs @@ -40,6 +40,7 @@ impl DebugInfo { } } +#[derive(Clone, Debug)] pub struct Function { pub name: String, pub offset: Offset, From be7fbac2c4dccd9eda726e39c6d5afd2bd33f02d Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Mon, 13 Feb 2023 18:19:34 -0800 Subject: [PATCH 02/16] Log count of new breakpoints --- src/agent/coverage/src/record/windows.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/agent/coverage/src/record/windows.rs b/src/agent/coverage/src/record/windows.rs index 5f7ec95d9f..f79943bed3 100644 --- a/src/agent/coverage/src/record/windows.rs +++ b/src/agent/coverage/src/record/windows.rs @@ -110,7 +110,8 @@ impl<'data> WindowsRecorder<'data> { self.breakpoints.set(dbg, breakpoint)?; } - self.coverage.modules.insert(path.clone(), coverage); + let count = coverage.offsets.len(); + debug!("set {} breakpoints for module {}", count, path); self.modules.insert(path, module); From b29b6930305ee6b0e26fef67fdfde2ba0dc507d3 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Mon, 13 Feb 2023 18:19:53 -0800 Subject: [PATCH 03/16] Log call stack on dangling breakpoints --- src/agent/coverage/src/record/windows.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/agent/coverage/src/record/windows.rs b/src/agent/coverage/src/record/windows.rs index f79943bed3..1c383694c8 100644 --- a/src/agent/coverage/src/record/windows.rs +++ b/src/agent/coverage/src/record/windows.rs @@ -69,8 +69,12 @@ impl<'data> WindowsRecorder<'data> { fn try_on_breakpoint(&mut self, _dbg: &mut Debugger, id: BreakpointId) -> Result<()> { let breakpoint = self .breakpoints - .remove(id) - .ok_or_else(|| anyhow!("stopped on dangling breakpoint"))?; + .remove(id); + + let Some(breakpoint) = breakpoint else { + let stack = dbg.get_current_stack()?; + bail!("stopped on dangling breakpoint, debuggee stack:\n{}", stack); + }; let coverage = self .coverage From 73df13e1171d5fc91e2bee267d76a78caa304e7c Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Mon, 13 Feb 2023 18:22:02 -0800 Subject: [PATCH 04/16] Relax `Breakpoint` field visibility --- src/agent/coverage/src/record/windows.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/agent/coverage/src/record/windows.rs b/src/agent/coverage/src/record/windows.rs index 1c383694c8..b946b598d6 100644 --- a/src/agent/coverage/src/record/windows.rs +++ b/src/agent/coverage/src/record/windows.rs @@ -166,8 +166,8 @@ impl Breakpoints { #[derive(Clone, Debug)] struct Breakpoint { - module: FilePath, - offset: Offset, + pub module: FilePath, + pub offset: Offset, } impl Breakpoint { From fc1782c2a1aa08ed4a14b59780ed6831613ee29c Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Mon, 13 Feb 2023 18:22:18 -0800 Subject: [PATCH 05/16] Impl breakpoint setting via `Breakpoint` --- src/agent/coverage/src/record/windows.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/agent/coverage/src/record/windows.rs b/src/agent/coverage/src/record/windows.rs index b946b598d6..fd78b2fd70 100644 --- a/src/agent/coverage/src/record/windows.rs +++ b/src/agent/coverage/src/record/windows.rs @@ -174,6 +174,12 @@ impl Breakpoint { pub fn new(module: FilePath, offset: Offset) -> Self { Self { module, offset } } + + pub fn set(&self, dbg: &mut Debugger) -> Result { + let name = Path::new(self.module.base_name()); + let id = dbg.new_rva_breakpoint(name, self.offset.0, BreakpointType::OneTime)?; + Ok(id) + } } impl<'data> DebugEventHandler for WindowsRecorder<'data> { From ceadfc150399a2000545dc5083f826856fd1f2e6 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Mon, 13 Feb 2023 18:23:49 -0800 Subject: [PATCH 06/16] Remove broken redundancy check, simplify breakpoint registry --- src/agent/coverage/src/record/windows.rs | 36 ++++++------------------ 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/src/agent/coverage/src/record/windows.rs b/src/agent/coverage/src/record/windows.rs index fd78b2fd70..7c69810209 100644 --- a/src/agent/coverage/src/record/windows.rs +++ b/src/agent/coverage/src/record/windows.rs @@ -125,42 +125,22 @@ impl<'data> WindowsRecorder<'data> { #[derive(Debug, Default)] struct Breakpoints { - id_to_offset: BTreeMap, - offset_to_breakpoint: BTreeMap, + id_to_breakpoint: BTreeMap, } impl Breakpoints { pub fn set(&mut self, dbg: &mut Debugger, breakpoint: Breakpoint) -> Result<()> { - if self.is_set(&breakpoint) { - return Ok(()); - } - - self.write(dbg, breakpoint) - } - - // Unguarded action that ovewrites both the target process address space and our index - // of known breakpoints. Callers must use `set()`, which avoids redundant breakpoint - // setting. - fn write(&mut self, dbg: &mut Debugger, breakpoint: Breakpoint) -> Result<()> { - // The `debugger` crates tracks loaded modules by base name. If a path or file - // name is used, software breakpoints will not be written. - let name = Path::new(breakpoint.module.base_name()); - let id = dbg.new_rva_breakpoint(name, breakpoint.offset.0, BreakpointType::OneTime)?; - - self.id_to_offset.insert(id, breakpoint.offset); - self.offset_to_breakpoint - .insert(breakpoint.offset, breakpoint); - + let id = breakpoint.set(dbg)?; + self.id_to_breakpoint.insert(id, breakpoint); Ok(()) } - pub fn is_set(&self, breakpoint: &Breakpoint) -> bool { - self.offset_to_breakpoint.contains_key(&breakpoint.offset) - } - + /// Remove a registered breakpoint from the index. + /// + /// This method should be called when a breakpoint is hit to retrieve its associated data. + /// It does NOT clear the actual breakpoint in the debugger engine. pub fn remove(&mut self, id: BreakpointId) -> Option { - let offset = self.id_to_offset.remove(&id)?; - self.offset_to_breakpoint.remove(&offset) + self.id_to_breakpoint.remove(&id) } } From eaa1e283ef40864812bf61990816a1d6d3e483b9 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Mon, 13 Feb 2023 18:25:56 -0800 Subject: [PATCH 07/16] Defer coverage breakpoint setting --- src/agent/coverage/src/record/windows.rs | 115 +++++++++++++++++++++-- 1 file changed, 109 insertions(+), 6 deletions(-) diff --git a/src/agent/coverage/src/record/windows.rs b/src/agent/coverage/src/record/windows.rs index 7c69810209..577111a5f5 100644 --- a/src/agent/coverage/src/record/windows.rs +++ b/src/agent/coverage/src/record/windows.rs @@ -4,12 +4,13 @@ use std::collections::BTreeMap; use std::path::Path; -use anyhow::{anyhow, Error, Result}; +use anyhow::{anyhow, bail, Error, Result}; +use debuggable_module::debuginfo::{DebugInfo, Function}; use debuggable_module::load_module::LoadModule; use debuggable_module::loader::Loader; use debuggable_module::path::FilePath; use debuggable_module::windows::WindowsModule; -use debuggable_module::Offset; +use debuggable_module::{Module, Offset}; use debugger::{BreakpointId, BreakpointType, DebugEventHandler, Debugger, ModuleLoadInfo}; use crate::allowlist::TargetAllowList; @@ -18,15 +19,17 @@ use crate::binary::{self, BinaryCoverage}; pub struct WindowsRecorder<'data> { allowlist: TargetAllowList, breakpoints: Breakpoints, + deferred_breakpoints: BTreeMap, pub coverage: BinaryCoverage, loader: &'data Loader, - modules: BTreeMap>, + modules: BTreeMap, DebugInfo)>, pub stop_error: Option, } impl<'data> WindowsRecorder<'data> { pub fn new(loader: &'data Loader, allowlist: TargetAllowList) -> Self { let breakpoints = Breakpoints::default(); + let deferred_breakpoints = BTreeMap::new(); let coverage = BinaryCoverage::default(); let modules = BTreeMap::new(); let stop_error = None; @@ -34,6 +37,7 @@ impl<'data> WindowsRecorder<'data> { Self { allowlist, breakpoints, + deferred_breakpoints, coverage, loader, modules, @@ -66,7 +70,46 @@ impl<'data> WindowsRecorder<'data> { self.insert_module(dbg, module) } - fn try_on_breakpoint(&mut self, _dbg: &mut Debugger, id: BreakpointId) -> Result<()> { + fn try_on_breakpoint(&mut self, dbg: &mut Debugger, id: BreakpointId) -> Result<()> { + let pc = dbg.read_program_counter()?; + let tid = dbg.get_current_thread_id(); + + if let Some((trigger, state)) = self.deferred_breakpoints.remove(&id) { + match state { + DeferralState::NotEntered => { + // Find the return address. + let frame = dbg.get_current_frame()?; + let ret = frame.return_address(); + let id = dbg.new_address_breakpoint(ret, BreakpointType::OneTime)?; + + // Update the state for this deferral to set module coverage breakpoints on ret. + let thread_id = dbg.get_current_thread_id(); + let state = DeferralState::PendingReturn { thread_id }; + self.deferred_breakpoints.insert(id, (trigger, state)); + // return Ok(()); + }, + DeferralState::PendingReturn { thread_id } => { + if dbg.get_current_thread_id() == thread_id { + // We've returned from the trigger function, and on the same thread. + // + // It's safe to set coverage breakpoints. + self.set_module_breakpoints(dbg, trigger.module)?; + } else { + // Hit a ret breakpoint, but on the wrong thread. Reset it so the correct + // thread has a chance to see it. + // + // We only defer breakpoints in image initialization code, so we don't + // expect to reach this code in practice. + let id = trigger.set(dbg)?; + self.deferred_breakpoints.insert(id, (trigger, state)); + // return Ok(()); + } + }, + } + + return Ok(()); + } + let breakpoint = self .breakpoints .remove(id); @@ -107,7 +150,62 @@ impl<'data> WindowsRecorder<'data> { return Ok(()); }; - let coverage = binary::find_coverage_sites(&module, &self.allowlist)?; + let debuginfo = module.debuginfo()?; + self.modules.insert(path.clone(), (module, debuginfo)); + + self.set_or_defer_module_breakpoints(dbg, path)?; + + Ok(()) + } + + fn set_or_defer_module_breakpoints(&mut self, dbg: &mut Debugger, path: FilePath) -> Result<()> { + let (_module, debuginfo) = &self.modules[&path]; + + // For borrocwk. + let mut trigger = None; + + for function in debuginfo.functions() { + // Called on process startup. + if function.name.contains("__asan::AsanInitInternal") { + trigger = Some(function.clone()); + break; + } + + // Called on shared library load. + if function.name.contains("DllMain(") { + trigger = Some(function.clone()); + break; + } + } + + if let Some(trigger) = trigger { + debug!("deferring module breakpoints for {}", path); + self.defer_module_breakpoints(dbg, path, trigger) + } else { + debug!("immediately setting module breakpoints for {}", path); + self.set_module_breakpoints(dbg, path) + } + } + + fn defer_module_breakpoints( + &mut self, + dbg: &mut Debugger, + path: FilePath, + trigger: Function, + ) -> Result<()> { + let state = DeferralState::NotEntered; + let entry_breakpoint = Breakpoint::new(path, trigger.offset); + let id = entry_breakpoint.set(dbg)?; + let thread_id = dbg.get_current_thread_id(); + + self.deferred_breakpoints.insert(id, (entry_breakpoint, state)); + + Ok(()) + } + + fn set_module_breakpoints(&mut self, dbg: &mut Debugger, path: FilePath) -> Result<()> { + let (module, _) = &self.modules[&path]; + let coverage = binary::find_coverage_sites(module, &self.allowlist)?; for offset in coverage.as_ref().keys().copied() { let breakpoint = Breakpoint::new(path.clone(), offset); @@ -117,7 +215,7 @@ impl<'data> WindowsRecorder<'data> { let count = coverage.offsets.len(); debug!("set {} breakpoints for module {}", count, path); - self.modules.insert(path, module); + self.coverage.modules.insert(path, coverage); Ok(()) } @@ -184,3 +282,8 @@ impl<'data> DebugEventHandler for WindowsRecorder<'data> { } } } + +enum DeferralState { + NotEntered, + PendingReturn { thread_id: u64 }, +} From 9be94f0172a5a4f542ed7e6f37ee335d19cf8fd0 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Mon, 13 Feb 2023 20:02:15 -0800 Subject: [PATCH 08/16] Fix typo --- src/agent/coverage/src/record/windows.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/agent/coverage/src/record/windows.rs b/src/agent/coverage/src/record/windows.rs index 577111a5f5..eeea113e5a 100644 --- a/src/agent/coverage/src/record/windows.rs +++ b/src/agent/coverage/src/record/windows.rs @@ -161,7 +161,7 @@ impl<'data> WindowsRecorder<'data> { fn set_or_defer_module_breakpoints(&mut self, dbg: &mut Debugger, path: FilePath) -> Result<()> { let (_module, debuginfo) = &self.modules[&path]; - // For borrocwk. + // For borrowck. let mut trigger = None; for function in debuginfo.functions() { From 8eff37ba34e826f442e836e4591db7b2fbc540c0 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Mon, 13 Feb 2023 20:03:17 -0800 Subject: [PATCH 09/16] Reword log messages --- src/agent/coverage/src/record/windows.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/agent/coverage/src/record/windows.rs b/src/agent/coverage/src/record/windows.rs index eeea113e5a..7dfb43e797 100644 --- a/src/agent/coverage/src/record/windows.rs +++ b/src/agent/coverage/src/record/windows.rs @@ -179,10 +179,10 @@ impl<'data> WindowsRecorder<'data> { } if let Some(trigger) = trigger { - debug!("deferring module breakpoints for {}", path); + debug!("deferring coverage breakpoints for module {}", path); self.defer_module_breakpoints(dbg, path, trigger) } else { - debug!("immediately setting module breakpoints for {}", path); + debug!("immediately setting coverage breakpoints for module {}", path); self.set_module_breakpoints(dbg, path) } } From ea4bb7e5ab3cb0be38e377933dca3970c772de4f Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Mon, 13 Feb 2023 20:04:09 -0800 Subject: [PATCH 10/16] Remove unused locals --- src/agent/coverage/src/record/windows.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/agent/coverage/src/record/windows.rs b/src/agent/coverage/src/record/windows.rs index 7dfb43e797..acbfaef9c6 100644 --- a/src/agent/coverage/src/record/windows.rs +++ b/src/agent/coverage/src/record/windows.rs @@ -71,9 +71,6 @@ impl<'data> WindowsRecorder<'data> { } fn try_on_breakpoint(&mut self, dbg: &mut Debugger, id: BreakpointId) -> Result<()> { - let pc = dbg.read_program_counter()?; - let tid = dbg.get_current_thread_id(); - if let Some((trigger, state)) = self.deferred_breakpoints.remove(&id) { match state { DeferralState::NotEntered => { @@ -196,7 +193,6 @@ impl<'data> WindowsRecorder<'data> { let state = DeferralState::NotEntered; let entry_breakpoint = Breakpoint::new(path, trigger.offset); let id = entry_breakpoint.set(dbg)?; - let thread_id = dbg.get_current_thread_id(); self.deferred_breakpoints.insert(id, (entry_breakpoint, state)); From 3388501e26209558235bdef97092ed7299451ec9 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Mon, 13 Feb 2023 20:12:51 -0800 Subject: [PATCH 11/16] Remove commented code --- src/agent/coverage/src/record/windows.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/agent/coverage/src/record/windows.rs b/src/agent/coverage/src/record/windows.rs index acbfaef9c6..8c4264b025 100644 --- a/src/agent/coverage/src/record/windows.rs +++ b/src/agent/coverage/src/record/windows.rs @@ -83,7 +83,6 @@ impl<'data> WindowsRecorder<'data> { let thread_id = dbg.get_current_thread_id(); let state = DeferralState::PendingReturn { thread_id }; self.deferred_breakpoints.insert(id, (trigger, state)); - // return Ok(()); }, DeferralState::PendingReturn { thread_id } => { if dbg.get_current_thread_id() == thread_id { @@ -99,7 +98,6 @@ impl<'data> WindowsRecorder<'data> { // expect to reach this code in practice. let id = trigger.set(dbg)?; self.deferred_breakpoints.insert(id, (trigger, state)); - // return Ok(()); } }, } From 0b0f2503912f35eecabe56faa840b28182d899e4 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Mon, 13 Feb 2023 20:15:32 -0800 Subject: [PATCH 12/16] Format --- src/agent/coverage/src/record/windows.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/agent/coverage/src/record/windows.rs b/src/agent/coverage/src/record/windows.rs index 8c4264b025..2628db5830 100644 --- a/src/agent/coverage/src/record/windows.rs +++ b/src/agent/coverage/src/record/windows.rs @@ -83,7 +83,7 @@ impl<'data> WindowsRecorder<'data> { let thread_id = dbg.get_current_thread_id(); let state = DeferralState::PendingReturn { thread_id }; self.deferred_breakpoints.insert(id, (trigger, state)); - }, + } DeferralState::PendingReturn { thread_id } => { if dbg.get_current_thread_id() == thread_id { // We've returned from the trigger function, and on the same thread. @@ -99,15 +99,13 @@ impl<'data> WindowsRecorder<'data> { let id = trigger.set(dbg)?; self.deferred_breakpoints.insert(id, (trigger, state)); } - }, + } } return Ok(()); } - let breakpoint = self - .breakpoints - .remove(id); + let breakpoint = self.breakpoints.remove(id); let Some(breakpoint) = breakpoint else { let stack = dbg.get_current_stack()?; @@ -153,7 +151,11 @@ impl<'data> WindowsRecorder<'data> { Ok(()) } - fn set_or_defer_module_breakpoints(&mut self, dbg: &mut Debugger, path: FilePath) -> Result<()> { + fn set_or_defer_module_breakpoints( + &mut self, + dbg: &mut Debugger, + path: FilePath, + ) -> Result<()> { let (_module, debuginfo) = &self.modules[&path]; // For borrowck. @@ -177,7 +179,10 @@ impl<'data> WindowsRecorder<'data> { debug!("deferring coverage breakpoints for module {}", path); self.defer_module_breakpoints(dbg, path, trigger) } else { - debug!("immediately setting coverage breakpoints for module {}", path); + debug!( + "immediately setting coverage breakpoints for module {}", + path + ); self.set_module_breakpoints(dbg, path) } } @@ -192,7 +197,8 @@ impl<'data> WindowsRecorder<'data> { let entry_breakpoint = Breakpoint::new(path, trigger.offset); let id = entry_breakpoint.set(dbg)?; - self.deferred_breakpoints.insert(id, (entry_breakpoint, state)); + self.deferred_breakpoints + .insert(id, (entry_breakpoint, state)); Ok(()) } From 2fa4f9331fd819fa045818e811b679fb63e12a3c Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Tue, 14 Feb 2023 08:22:26 -0800 Subject: [PATCH 13/16] Add logging --- src/agent/coverage/src/record/windows.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/agent/coverage/src/record/windows.rs b/src/agent/coverage/src/record/windows.rs index 2628db5830..9922622cb2 100644 --- a/src/agent/coverage/src/record/windows.rs +++ b/src/agent/coverage/src/record/windows.rs @@ -74,6 +74,10 @@ impl<'data> WindowsRecorder<'data> { if let Some((trigger, state)) = self.deferred_breakpoints.remove(&id) { match state { DeferralState::NotEntered => { + debug!( + "hit trigger breakpoint (not-entered) for deferred coverage breakpoints" + ); + // Find the return address. let frame = dbg.get_current_frame()?; let ret = frame.return_address(); @@ -85,12 +89,20 @@ impl<'data> WindowsRecorder<'data> { self.deferred_breakpoints.insert(id, (trigger, state)); } DeferralState::PendingReturn { thread_id } => { + debug!( + "hit trigger breakpoint (pending-return) for deferred coverage breakpoints" + ); + if dbg.get_current_thread_id() == thread_id { + debug!("correct thread hit pending-return trigger breakpoint"); + // We've returned from the trigger function, and on the same thread. // // It's safe to set coverage breakpoints. self.set_module_breakpoints(dbg, trigger.module)?; } else { + warn!("wrong thread saw pending-return trigger breakpoint, resetting"); + // Hit a ret breakpoint, but on the wrong thread. Reset it so the correct // thread has a chance to see it. // From fe47d377265af98de975e09c4c22baed4d8291ae Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Tue, 14 Feb 2023 08:44:19 -0800 Subject: [PATCH 14/16] Add constants for deferral triggers, check prefix --- src/agent/coverage/src/record/windows.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/agent/coverage/src/record/windows.rs b/src/agent/coverage/src/record/windows.rs index 9922622cb2..72b73db655 100644 --- a/src/agent/coverage/src/record/windows.rs +++ b/src/agent/coverage/src/record/windows.rs @@ -16,6 +16,15 @@ use debugger::{BreakpointId, BreakpointType, DebugEventHandler, Debugger, Module use crate::allowlist::TargetAllowList; use crate::binary::{self, BinaryCoverage}; +// 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 +// initializers. +// +// We will use these constants with a prefix comparison. Only check up to the first open paren to +// avoid false negatives if the demangled representation of parameters ever varies. +const PROCESS_IMAGE_DEFERRAL_TRIGGER: &str = "__asan::AsanInitInternal("; +const LIBRARY_IMAGE_DEFERRAL_TRIGGER: &str = "DllMain("; + pub struct WindowsRecorder<'data> { allowlist: TargetAllowList, breakpoints: Breakpoints, @@ -175,13 +184,13 @@ impl<'data> WindowsRecorder<'data> { for function in debuginfo.functions() { // Called on process startup. - if function.name.contains("__asan::AsanInitInternal") { + if function.name.starts_with(PROCESS_IMAGE_DEFERRAL_TRIGGER) { trigger = Some(function.clone()); break; } // Called on shared library load. - if function.name.contains("DllMain(") { + if function.name.starts_with(LIBRARY_IMAGE_DEFERRAL_TRIGGER) { trigger = Some(function.clone()); break; } From 1a45c51edb2f025d033b5bc2b9907478d5ec87a0 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Tue, 14 Feb 2023 08:49:30 -0800 Subject: [PATCH 15/16] Add comment about base name usage --- src/agent/coverage/src/record/windows.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/agent/coverage/src/record/windows.rs b/src/agent/coverage/src/record/windows.rs index 72b73db655..7689c651f4 100644 --- a/src/agent/coverage/src/record/windows.rs +++ b/src/agent/coverage/src/record/windows.rs @@ -275,6 +275,8 @@ impl Breakpoint { } pub fn set(&self, dbg: &mut Debugger) -> Result { + // The `debugger` crates tracks loaded modules by base name. If a path or file + // name is used, software breakpoints will not be written. let name = Path::new(self.module.base_name()); let id = dbg.new_rva_breakpoint(name, self.offset.0, BreakpointType::OneTime)?; Ok(id) From c69edb9ecb9f7a8b6749f57eb2f94de5a1d51147 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Tue, 14 Feb 2023 08:53:49 -0800 Subject: [PATCH 16/16] Don't double-instrument modules --- src/agent/coverage/src/record/windows.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/agent/coverage/src/record/windows.rs b/src/agent/coverage/src/record/windows.rs index 7689c651f4..eef34fc8ac 100644 --- a/src/agent/coverage/src/record/windows.rs +++ b/src/agent/coverage/src/record/windows.rs @@ -152,6 +152,11 @@ impl<'data> WindowsRecorder<'data> { fn insert_module(&mut self, dbg: &mut Debugger, module: &ModuleLoadInfo) -> Result<()> { let path = FilePath::new(module.path().to_string_lossy())?; + if self.modules.contains_key(&path) { + warn!("module coverage already initialized, skipping"); + return Ok(()); + } + if !self.allowlist.modules.is_allowed(&path) { debug!("not inserting denylisted module: {path}"); return Ok(());