From 829a8bf797658feb6b4eee9790b7a56f51772eba Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Wed, 9 Jun 2021 23:27:40 +0000 Subject: [PATCH 1/5] Set context on errors --- src/agent/coverage/src/block/linux.rs | 50 +++++++++++++++++++-------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/src/agent/coverage/src/block/linux.rs b/src/agent/coverage/src/block/linux.rs index 1b09a23532..60f9505d73 100644 --- a/src/agent/coverage/src/block/linux.rs +++ b/src/agent/coverage/src/block/linux.rs @@ -71,21 +71,28 @@ impl<'c> Recorder<'c> { options.remove(Options::PTRACE_O_TRACEFORK); options.remove(Options::PTRACE_O_TRACEVFORK); options.remove(Options::PTRACE_O_TRACEEXEC); - tracee.set_options(options)?; + tracee + .set_options(options) + .context("setting tracee options")?; self.images = Some(Images::new(tracee.pid.as_raw())); - self.update_images(&mut tracee)?; + self.update_images(&mut tracee) + .context("initial update of module images")?; - self.tracer.restart(tracee, Restart::Syscall)?; + self.tracer + .restart(tracee, Restart::Syscall) + .context("initial tracer restart")?; - while let Some(mut tracee) = self.tracer.wait()? { + while let Some(mut tracee) = self.tracer.wait().context("main tracing loop")? { match tracee.stop { Stop::SyscallEnterStop(..) => log::trace!("syscall-enter: {:?}", tracee.stop), Stop::SyscallExitStop(..) => { - self.update_images(&mut tracee)?; + self.update_images(&mut tracee) + .context("updating module images after syscall-stop")?; } Stop::SignalDeliveryStop(_pid, Signal::SIGTRAP) => { - self.on_breakpoint(&mut tracee)?; + self.on_breakpoint(&mut tracee) + .context("calling breakpoint handler")?; } Stop::Clone(pid, tid) => { // Only seen when the `VM_CLONE` flag is set, as of Linux 4.15. @@ -113,7 +120,8 @@ impl<'c> Recorder<'c> { for (_base, image) in &events.loaded { if self.filter.includes_module(image.path()) { - self.on_module_load(tracee, image)?; + self.on_module_load(tracee, image) + .context("module load callback")?; } } @@ -137,12 +145,16 @@ 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) + .context("converting PC to module offset")?; self.coverage.increment(image.path(), offset); // Execute clobbered instruction on restart. regs.rip = pc; - tracee.set_registers(regs)?; + tracee + .set_registers(regs) + .context("resetting PC in breakpoint handler")?; } else { // Assume the tracee concurrently executed an `int3` that we restored // in another handler. @@ -151,7 +163,9 @@ impl<'c> Recorder<'c> { // clearing, but making their value a state. log::debug!("no breakpoint at {:x}, assuming race", pc); regs.rip = pc; - tracee.set_registers(regs)?; + tracee + .set_registers(regs) + .context("resetting PC after ignoring spurious breakpoint")?; } Ok(()) @@ -233,11 +247,11 @@ impl Images { } pub fn update(&mut self) -> Result { - let proc = Process::new(self.pid)?; + let proc = Process::new(self.pid).context("getting procinfo")?; let mut new = BTreeMap::default(); - for map in proc.maps()? { + for map in proc.maps().context("getting maps for process")? { if let Ok(image) = ModuleImage::new(map) { new.insert(image.base(), image); } @@ -374,7 +388,9 @@ impl Breakpoints { let mut data = [0u8]; tracee.read_memory_mut(va, &mut data)?; self.saved.insert(va, data[0]); - tracee.write_memory(va, &[0xcc])?; + tracee + .write_memory(va, &[0xcc]) + .context("setting breakpoint, writing int3")?; Ok(()) } @@ -383,7 +399,9 @@ impl Breakpoints { let data = self.saved.remove(&va); let cleared = if let Some(data) = data { - tracee.write_memory(va, &[data])?; + tracee + .write_memory(va, &[data]) + .context("clearing breakpoint, restoring byte")?; true } else { false @@ -399,7 +417,9 @@ fn continue_to_init_execve(tracer: &mut Ptracer) -> Result { return Ok(tracee); } - tracer.restart(tracee, Restart::Continue)?; + tracer + .restart(tracee, Restart::Continue) + .context("restarting tracee pre-execve()")?; } anyhow::bail!("did not see initial execve() in tracee while recording coverage"); From ad32f79b00be07edac03509aa9dd3dd057a0d93b Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Wed, 9 Jun 2021 23:50:58 +0000 Subject: [PATCH 2/5] Add Windows context --- src/agent/coverage/src/block/windows.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/agent/coverage/src/block/windows.rs b/src/agent/coverage/src/block/windows.rs index 0ede357dea..7ba89bac49 100644 --- a/src/agent/coverage/src/block/windows.rs +++ b/src/agent/coverage/src/block/windows.rs @@ -5,7 +5,7 @@ use std::collections::BTreeMap; use std::process::Command; use std::time::{Duration, Instant}; -use anyhow::Result; +use anyhow::{Context, Result}; use debugger::{BreakpointId, BreakpointType, DebugEventHandler, Debugger, ModuleLoadInfo}; use crate::block::CommandBlockCov; @@ -50,8 +50,8 @@ impl<'r, 'c> RecorderEventHandler<'r, 'c> { } pub fn run(&mut self, cmd: Command) -> Result<()> { - let (mut dbg, _child) = Debugger::init(cmd, self)?; - dbg.run(self)?; + let (mut dbg, _child) = Debugger::init(cmd, self).context("initializing debugger")?; + dbg.run(self).context("running debuggee")?; Ok(()) } @@ -129,7 +129,7 @@ impl<'c> Recorder<'c> { if log::max_level() == log::Level::Trace { let name = breakpoint.module.name().to_string_lossy(); let offset = breakpoint.offset; - let pc = dbg.read_program_counter()?; + let pc = dbg.read_program_counter().context("reading PC on breakpoint")?; if let Ok(sym) = dbg.get_symbol(pc) { log::trace!( @@ -161,7 +161,7 @@ impl<'c> Recorder<'c> { } fn insert_module(&mut self, dbg: &mut Debugger, module: &ModuleLoadInfo) -> Result<()> { - let path = ModulePath::new(module.path().to_owned())?; + let path = ModulePath::new(module.path().to_owned()).context("parsing module path")?; if !self.filter.includes_module(&path) { log::debug!("skipping module: {}", path); @@ -182,7 +182,7 @@ impl<'c> Recorder<'c> { } self.breakpoints - .set(dbg, module, info.blocks.iter().copied())?; + .set(dbg, module, info.blocks.iter().copied()).context("setting breakpoints for module")?; log::debug!("set {} breakpoints for module {}", info.blocks.len(), path); } @@ -238,7 +238,7 @@ struct Breakpoints { impl Breakpoints { pub fn get(&self, id: BreakpointId) -> Option> { - let (module_index, offset) = self.registered.get(&id).copied()?; + let (module_index, offset) = self.registered.get(&id).copied().context("looking up breakpoint")?; let module = self.modules.get(module_index)?; Some(BreakpointData { module, offset }) } From 0933e9e78dfc21c9e6ccc95779418590f4bbb5de Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Wed, 9 Jun 2021 23:58:59 +0000 Subject: [PATCH 3/5] Format --- src/agent/coverage/src/block/windows.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/agent/coverage/src/block/windows.rs b/src/agent/coverage/src/block/windows.rs index 7ba89bac49..c1ec42632b 100644 --- a/src/agent/coverage/src/block/windows.rs +++ b/src/agent/coverage/src/block/windows.rs @@ -129,7 +129,9 @@ impl<'c> Recorder<'c> { if log::max_level() == log::Level::Trace { let name = breakpoint.module.name().to_string_lossy(); let offset = breakpoint.offset; - let pc = dbg.read_program_counter().context("reading PC on breakpoint")?; + let pc = dbg + .read_program_counter() + .context("reading PC on breakpoint")?; if let Ok(sym) = dbg.get_symbol(pc) { log::trace!( @@ -182,7 +184,8 @@ impl<'c> Recorder<'c> { } self.breakpoints - .set(dbg, module, info.blocks.iter().copied()).context("setting breakpoints for module")?; + .set(dbg, module, info.blocks.iter().copied()) + .context("setting breakpoints for module")?; log::debug!("set {} breakpoints for module {}", info.blocks.len(), path); } @@ -238,7 +241,11 @@ struct Breakpoints { impl Breakpoints { pub fn get(&self, id: BreakpointId) -> Option> { - let (module_index, offset) = self.registered.get(&id).copied().context("looking up breakpoint")?; + let (module_index, offset) = self + .registered + .get(&id) + .copied() + .context("looking up breakpoint")?; let module = self.modules.get(module_index)?; Some(BreakpointData { module, offset }) } From 1b2658548f1bf0f60a606b991a7ae0df5f5f714d Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Thu, 10 Jun 2021 00:49:49 +0000 Subject: [PATCH 4/5] Remove invalid `context()` --- src/agent/coverage/src/block/windows.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/agent/coverage/src/block/windows.rs b/src/agent/coverage/src/block/windows.rs index c1ec42632b..fb3ab3d814 100644 --- a/src/agent/coverage/src/block/windows.rs +++ b/src/agent/coverage/src/block/windows.rs @@ -244,8 +244,7 @@ impl Breakpoints { let (module_index, offset) = self .registered .get(&id) - .copied() - .context("looking up breakpoint")?; + .copied()?; let module = self.modules.get(module_index)?; Some(BreakpointData { module, offset }) } From a2014b72422eb9897c3e24e90ed210eccf6e6435 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Thu, 10 Jun 2021 00:52:27 +0000 Subject: [PATCH 5/5] Format --- src/agent/coverage/src/block/windows.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/agent/coverage/src/block/windows.rs b/src/agent/coverage/src/block/windows.rs index fb3ab3d814..5029c27bff 100644 --- a/src/agent/coverage/src/block/windows.rs +++ b/src/agent/coverage/src/block/windows.rs @@ -241,10 +241,7 @@ struct Breakpoints { impl Breakpoints { pub fn get(&self, id: BreakpointId) -> Option> { - let (module_index, offset) = self - .registered - .get(&id) - .copied()?; + let (module_index, offset) = self.registered.get(&id).copied()?; let module = self.modules.get(module_index)?; Some(BreakpointData { module, offset }) }