Refactor debugger breakpoint handling

src/agent/coverage/examples/block_coverage.rs

@@ -14,6 +14,9 @@ struct Opt {
 
     #[structopt(min_values = 1)]
     cmd: Vec<String>,
+
+    #[structopt(short, long, default_value = "5")]
+    timeout: u64,
 }
 
 impl Opt {
@@ -40,7 +43,8 @@ fn main() -> Result<()> {
     let mut cmd = Command::new(&opt.cmd[0]);
     cmd.args(&opt.cmd[1..]);
 
-    let coverage = coverage::block::windows::record(cmd, filter)?;
+    let timeout = std::time::Duration::from_secs(opt.timeout);
+    let coverage = coverage::block::windows::record(cmd, filter, timeout)?;
 
     for (module, cov) in coverage.iter() {
         let total = cov.blocks.len();
@@ -63,7 +67,7 @@ fn main() -> Result<()> {
     let filter = opt.load_filter_or_default()?;
 
     let mut cmd = Command::new(&opt.cmd[0]);
-    cmd.stdin(Stdio::null()).args(&opt.cmd[1..]);
+    cmd.stdin(std::process::Stdio::null()).args(&opt.cmd[1..]);
 
     let mut cache = ModuleCache::default();
     let mut recorder = Recorder::new(&mut cache, filter);

src/agent/coverage/src/block/windows.rs

@@ -6,19 +6,15 @@ use std::process::Command;
 use std::time::{Duration, Instant};
 
 use anyhow::Result;
-use debugger::{
-    debugger::{BreakpointId, BreakpointType, DebugEventHandler, Debugger},
-    target::Module,
-};
+use debugger::{BreakpointId, BreakpointType, DebugEventHandler, Debugger, ModuleLoadInfo};
 
 use crate::block::CommandBlockCov;
 use crate::cache::ModuleCache;
 use crate::code::{CmdFilter, ModulePath};
 
-pub fn record(cmd: Command, filter: CmdFilter) -> Result<CommandBlockCov> {
+pub fn record(cmd: Command, filter: CmdFilter, timeout: Duration) -> Result<CommandBlockCov> {
     let mut cache = ModuleCache::default();
     let mut recorder = Recorder::new(&mut cache, filter);
-    let timeout = Duration::from_secs(5);
     let mut handler = RecorderEventHandler::new(&mut recorder, timeout);
     handler.run(cmd)?;
     Ok(recorder.into_coverage())
@@ -107,10 +103,11 @@ impl<'c> Recorder<'c> {
         self.coverage
     }
 
-    pub fn on_create_process(&mut self, dbg: &mut Debugger, module: &Module) -> Result<()> {
+    pub fn on_create_process(&mut self, dbg: &mut Debugger, module: &ModuleLoadInfo) -> Result<()> {
         log::debug!("process created: {}", module.path().display());
 
-        if let Err(err) = dbg.target().sym_initialize() {
+        // TODO: we should avoid loading symbols if the module is in the cache.
+        if let Err(err) = dbg.target().maybe_sym_initialize() {
             log::error!(
                 "unable to initialize symbol handler for new process {}: {:?}",
                 module.path().display(),
@@ -121,9 +118,11 @@ impl<'c> Recorder<'c> {
         self.insert_module(dbg, module)
     }
 
-    pub fn on_load_dll(&mut self, dbg: &mut Debugger, module: &Module) -> Result<()> {
+    pub fn on_load_dll(&mut self, dbg: &mut Debugger, module: &ModuleLoadInfo) -> Result<()> {
         log::debug!("DLL loaded: {}", module.path().display());
 
+        // TODO: we should load symbols if the module is not in the cache (see on_create_process).
+
         self.insert_module(dbg, module)
     }
 
@@ -163,7 +162,7 @@ impl<'c> Recorder<'c> {
         Ok(())
     }
 
-    fn insert_module(&mut self, dbg: &mut Debugger, module: &Module) -> Result<()> {
+    fn insert_module(&mut self, dbg: &mut Debugger, module: &ModuleLoadInfo) -> Result<()> {
         let path = ModulePath::new(module.path().to_owned())?;
 
         if !self.filter.includes_module(&path) {
@@ -197,13 +196,13 @@ impl<'c> Recorder<'c> {
 }
 
 impl<'r, 'c> DebugEventHandler for RecorderEventHandler<'r, 'c> {
-    fn on_create_process(&mut self, dbg: &mut Debugger, module: &Module) {
+    fn on_create_process(&mut self, dbg: &mut Debugger, module: &ModuleLoadInfo) {
         if self.recorder.on_create_process(dbg, module).is_err() {
             self.stop(dbg);
         }
     }
 
-    fn on_load_dll(&mut self, dbg: &mut Debugger, module: &Module) {
+    fn on_load_dll(&mut self, dbg: &mut Debugger, module: &ModuleLoadInfo) {
         if self.recorder.on_load_dll(dbg, module).is_err() {
             self.stop(dbg);
         }
@@ -244,17 +243,18 @@ impl Breakpoints {
     pub fn set(
         &mut self,
         dbg: &mut Debugger,
-        module: &Module,
+        module: &ModuleLoadInfo,
         offsets: impl Iterator<Item = u32>,
     ) -> Result<()> {
-        // From the `target::Module`, create and save a `ModulePath`.
+        // From the `debugger::ModuleLoadInfo`, create and save a `ModulePath`.
         let module_path = ModulePath::new(module.path().to_owned())?;
         let module_index = self.modules.len();
         self.modules.push(module_path);
 
         for offset in offsets {
             // Register the breakpoint in the running target address space.
-            let id = dbg.register_breakpoint(module.name(), offset as u64, BreakpointType::OneTime);
+            let id =
+                dbg.new_rva_breakpoint(module.name(), offset as u64, BreakpointType::OneTime)?;
 
             // Associate the opaque `BreakpointId` with the module and offset.
             self.registered.insert(id, (module_index, offset));