From 45be3d375badca20d955f86b3407886237b0294b Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Wed, 4 Nov 2026 14:49:04 -0800 Subject: [PATCH 1/2] Debugging: edit breakpoints and watchpoints through `BreakpointState`. In order to prepare for record/replay and reversible debugging, we need a little more discipline about read-only vs. read-write access to the `Store`. The breakpoint/watchpoint state in the store is a logically separate thing that has more to do with execution strategy than the actual Wasm state (i.e., cannot affect Wasm state). Also, separately, breakpoints and watchpoints have a somewhat distributed API surface: breakpoints are edited via the existing `Store::edit_breakpoints` that returns a `BreakpointEdit` view struct, while watchpoints are edited via `Memory::debug_watch`. Watchpoints are really a kind of breakpoint (if you squint enough) and both should be managed in the same way. This PR introduces a `BreakpointState` view struct in the API returned from `Store::edit_breakpoints` that manages both breakpoints and watchpoints. This solves both problems: we can now treat `BreakpointState` as a thing that we can hand out mutably in future record/replay debug APIs (when the overall store is read-only), providing access to exactly and only the bit of execution-control state we want to be mutable. --- crates/debugger/src/host/opaque.rs | 8 +- crates/debugger/src/lib.rs | 13 +- crates/wasmtime/src/runtime/debug.rs | 211 +++++++++--------- .../wasmtime/src/runtime/module/registry.rs | 2 +- crates/wasmtime/src/runtime/store.rs | 8 +- tests/all/debug.rs | 33 ++- 6 files changed, 153 insertions(+), 122 deletions(-) diff --git a/crates/debugger/src/host/opaque.rs b/crates/debugger/src/host/opaque.rs index 46912eb1481d..d14cef482011 100644 --- a/crates/debugger/src/host/opaque.rs +++ b/crates/debugger/src/host/opaque.rs @@ -519,14 +519,16 @@ impl OpaqueDebugger for crate::Debuggee { #[cfg(feature = "threads")] Memory::Shared(_) => return Err(wit::Error::UnsupportedType.into()), }; - self.with_store(move |mut store| -> Result<()> { + self.with_store(move |store| -> Result<()> { let size = u64::try_from(memory.data_size(&store)).unwrap(); let end = addr .checked_add(len) .filter(|&end| end <= size) .ok_or(wit::Error::OutOfBounds)?; - memory - .debug_watch(&mut store, addr..end, watch) + store + .edit_breakpoints() + .ok_or(wit::Error::UnsupportedType)? + .watch(memory, addr..end, watch) .map_err(|_| wit::Error::OutOfBounds)?; Ok(()) }) diff --git a/crates/debugger/src/lib.rs b/crates/debugger/src/lib.rs index 8de98bd246dc..67d5b70f9e34 100644 --- a/crates/debugger/src/lib.rs +++ b/crates/debugger/src/lib.rs @@ -749,7 +749,10 @@ mod test { // Watch bytes [8, 12): the first store is just below the // range, the second overlaps it partially, the third is just // above it, and the fill covers all of it. - memory.debug_watch(&mut store, 8..12, true)?; + store + .edit_breakpoints() + .unwrap() + .watch(memory, 8..12, true)?; let mut debuggee = Debuggee::new(store, move |store| { Box::pin(async move { @@ -792,13 +795,17 @@ mod test { } ); debuggee - .with_store(move |mut store| { + .with_store(move |store| { // The previous write has now happened, but not the // fill. assert_eq!(&memory.data(&store)[6..10], &0x11223344_u32.to_le_bytes()); assert_eq!(memory.data(&store)[12], 2); // Stop watching; the rest of execution runs freely. - memory.debug_watch(&mut store, 0..16, false).unwrap(); + store + .edit_breakpoints() + .unwrap() + .watch(memory, 0..16, false) + .unwrap(); }) .await?; diff --git a/crates/wasmtime/src/runtime/debug.rs b/crates/wasmtime/src/runtime/debug.rs index 9df28c0f114a..eb7f659e0160 100644 --- a/crates/wasmtime/src/runtime/debug.rs +++ b/crates/wasmtime/src/runtime/debug.rs @@ -6,7 +6,7 @@ use crate::module::RegisterBreakpointState; use crate::store::StoreId; use crate::vm::{Activation, Backtrace}; use crate::{ - AnyRef, AsContextMut, CodeMemory, Engine, ExnRef, Extern, ExternRef, Func, Instance, Module, + AnyRef, AsContextMut, CodeMemory, ExnRef, Extern, ExternRef, Func, Instance, Module, OwnedRooted, StoreContext, StoreContextMut, Val, code::StoreCodePC, module::ModuleRegistry, @@ -47,8 +47,11 @@ impl Store { self.as_store_opaque().debug_exit_frames() } - /// Start an edit session to update breakpoints. - pub fn edit_breakpoints<'a>(&'a mut self) -> Option> { + /// Borrow this store's [`BreakpointState`], to edit its + /// breakpoints, single-stepping, or watchpoints. + /// + /// Returns `None` if guest debugging is not enabled. + pub fn edit_breakpoints<'a>(&'a mut self) -> Option> { self.as_store_opaque().edit_breakpoints() } @@ -80,8 +83,10 @@ impl<'a, T> StoreContextMut<'a, T> { self.0.as_store_opaque().debug_exit_frames() } - /// Start an edit session to update breakpoints. - pub fn edit_breakpoints(self) -> Option> { + /// Borrow this store's [`BreakpointState`]. + /// + /// See [`Store::edit_breakpoints`] for more details. + pub fn edit_breakpoints(self) -> Option> { self.0.as_store_opaque().edit_breakpoints() } @@ -111,8 +116,10 @@ impl<'a, T> Caller<'a, T> { self.store.0.as_store_opaque().debug_exit_frames() } - /// Start an edit session to update breakpoints. - pub fn edit_breakpoints<'b>(&'b mut self) -> Option> { + /// Borrow this store's [`BreakpointState`]. + /// + /// See [`Store::edit_breakpoints`] for more details. + pub fn edit_breakpoints<'b>(&'b mut self) -> Option> { self.store.0.as_store_opaque().edit_breakpoints() } @@ -134,6 +141,33 @@ impl<'a, T> Caller<'a, T> { } impl StoreOpaque { + /// Sets or clears the watch bits of the bytes in `range` of `memory`; see + /// [`BreakpointState::watch`]. + pub(crate) fn debug_set_watch( + &mut self, + memory: crate::Memory, + range: core::ops::Range, + watch: bool, + ) -> Result<()> { + let Some(shadow) = memory.vm_shadow_mut(self) else { + crate::error::bail!("watchpoints require guest debugging"); + }; + let bytes = shadow.bytes_mut(); + let start = usize::try_from(range.start)?; + let end = usize::try_from(range.end)?; + let Some(bytes) = bytes.get_mut(start..end) else { + crate::error::bail!("watchpoint range is out of bounds of the memory"); + }; + for byte in bytes { + if watch { + *byte |= crate::runtime::vm::WATCH_DEBUG; + } else { + *byte &= !crate::runtime::vm::WATCH_DEBUG; + } + } + Ok(()) + } + fn debug_exit_frames(&mut self) -> impl Iterator { let activations = if self.engine().tunables().debug_guest { Backtrace::activations(self) @@ -148,13 +182,15 @@ impl StoreOpaque { .filter_map(|act| unsafe { FrameHandle::exit_frame(self, act) }) } - fn edit_breakpoints<'a>(&'a mut self) -> Option> { + pub(crate) fn edit_breakpoints<'a>(&'a mut self) -> Option> { if !self.engine().tunables().debug_guest { return None; } - let (breakpoints, registry, engine) = self.breakpoints_and_registry_and_engine_mut(); - Some(breakpoints.edit(registry, engine)) + Some(BreakpointState { + store: self, + dirty_modules: BTreeSet::new(), + }) } fn debug_all_instances(&mut self) -> Vec { @@ -887,7 +923,7 @@ pub enum DebugEvent<'a> { /// An epoch yield occurred. EpochYield, /// Guest code is about to write watched bytes of a linear memory; see - /// [`Memory::debug_watch`](crate::Memory::debug_watch). The write happens + /// [`BreakpointState::watch`]. The write happens /// once the handler returns. Watchpoint(WatchpointHit), } @@ -917,42 +953,6 @@ impl PartialEq for WatchpointHit { impl Eq for WatchpointHit {} -impl crate::Memory { - /// Watches, or stops watching, guest writes to the bytes in `range` of this - /// memory. A guest store or bulk memory operation that writes a watched - /// byte raises [`DebugEvent::Watchpoint`] before writing. Whether a byte - /// is watched is a per-byte property, so unwatching a range unwatches its - /// bytes regardless of how they were watched. - /// - /// Watchpoints require guest debugging to be enabled; bytes beyond the - /// memory's current size cannot be watched. - pub fn debug_watch( - &self, - mut store: impl AsContextMut, - range: core::ops::Range, - watch: bool, - ) -> Result<()> { - let store = store.as_context_mut().0.as_store_opaque(); - let Some(shadow) = self.vm_shadow_mut(store) else { - crate::error::bail!("watchpoints require guest debugging"); - }; - let bytes = shadow.bytes_mut(); - let start = usize::try_from(range.start)?; - let end = usize::try_from(range.end)?; - let Some(bytes) = bytes.get_mut(start..end) else { - crate::error::bail!("watchpoint range is out of bounds of the memory"); - }; - for byte in bytes { - if watch { - *byte |= crate::runtime::vm::WATCH_DEBUG; - } else { - *byte &= !crate::runtime::vm::WATCH_DEBUG; - } - } - Ok(()) - } -} - /// A handler for debug events. /// /// This is an async callback that is invoked directly within the @@ -1014,7 +1014,7 @@ pub trait DebugHandler: Clone + Send + Sync + 'static { /// Breakpoint state for modules within a store. #[derive(Default)] -pub(crate) struct BreakpointState { +pub(crate) struct Breakpoints { /// Single-step mode. single_step: bool, /// Breakpoints added individually. Maps from the actual @@ -1054,19 +1054,17 @@ impl BreakpointKey { } } -/// A breakpoint-editing session. +/// A store's breakpoint state: its breakpoints, single-stepping, and +/// watchpoints, borrowed for editing. /// -/// This enables updating breakpoint state (setting or unsetting -/// individual breakpoints or the store-global single-step flag) in a -/// batch. It is more efficient to batch these updates because -/// "re-publishing" the newly patched code, with update breakpoint -/// settings, typically requires a syscall to re-enable execute -/// permissions. -pub struct BreakpointEdit<'a> { - state: &'a mut BreakpointState, - registry: &'a mut ModuleRegistry, - /// The engine that owns everything in `registry`. - engine: &'a Engine, +/// Obtained with [`Store::edit_breakpoints`] (or the same method on +/// [`StoreContextMut`] and [`Caller`]). +/// +/// Changes to breakpoints and single-stepping are batched for +/// efficiency, because code-patching has some overhead (e.g. syscalls +/// to write-protect/re-publish code). +pub struct BreakpointState<'a> { + store: &'a mut StoreOpaque, /// Modules that have been edited. /// /// Invariant: each of these modules' CodeMemory objects is @@ -1074,20 +1072,7 @@ pub struct BreakpointEdit<'a> { dirty_modules: BTreeSet, } -impl BreakpointState { - pub(crate) fn edit<'a>( - &'a mut self, - registry: &'a mut ModuleRegistry, - engine: &'a Engine, - ) -> BreakpointEdit<'a> { - BreakpointEdit { - state: self, - registry, - engine, - dirty_modules: BTreeSet::new(), - } - } - +impl Breakpoints { pub(crate) fn breakpoints<'a>( &'a self, registry: &'a ModuleRegistry, @@ -1110,14 +1095,14 @@ impl BreakpointState { if self.single_step { let mem = code.code_memory_mut().unwrap(); mem.unpublish()?; - BreakpointEdit::apply_single_step(mem, module, true, |_key| false)?; + BreakpointState::apply_single_step(mem, module, true, |_key| false)?; mem.publish()?; } Ok(()) } } -impl<'a> BreakpointEdit<'a> { +impl<'a> BreakpointState<'a> { /// Errors if `module` does not belong to the same engine as the store /// being edited. /// @@ -1127,14 +1112,14 @@ impl<'a> BreakpointEdit<'a> { /// at all. So this editing session checks up front as well. fn check_engine(&self, module: &Module) -> Result<()> { crate::ensure!( - crate::Engine::same(self.engine, module.engine()), + crate::Engine::same(self.store.engine(), module.engine()), "cross-`Engine` breakpoint editing is not supported" ); Ok(()) } fn get_code_memory<'b>( - breakpoints: &BreakpointState, + breakpoints: &Breakpoints, registry: &'b mut ModuleRegistry, dirty_modules: &mut BTreeSet, module: &Module, @@ -1191,19 +1176,17 @@ impl<'a> BreakpointEdit<'a> { let requested_key = BreakpointKey::from_raw(module, pc); let actual_key = BreakpointKey::from_raw(module, actual_pc); + let (state, registry, _) = self.store.breakpoints_and_registry_and_engine_mut(); if actual_pc != pc { log::trace!("slipping breakpoint from {requested_key:?} to {actual_key:?}"); - self.state - .breakpoint_redirects - .insert(requested_key, actual_key); + state.breakpoint_redirects.insert(requested_key, actual_key); } - let refcount = self.state.breakpoints.entry(actual_key).or_insert(0); + let refcount = state.breakpoints.entry(actual_key).or_insert(0); *refcount += 1; if *refcount == 1 { // First reference: actually patch the code. - let mem = - Self::get_code_memory(self.state, self.registry, &mut self.dirty_modules, module)?; + let mem = Self::get_code_memory(state, registry, &mut self.dirty_modules, module)?; let patches = frame_table.lookup_breakpoint_patches_by_pc(actual_pc); Self::patch(patches, mem, true); } @@ -1221,25 +1204,21 @@ impl<'a> BreakpointEdit<'a> { /// the store being edited. pub fn remove_breakpoint(&mut self, module: &Module, pc: ModulePC) -> Result<()> { self.check_engine(module)?; + let (state, registry, _) = self.store.breakpoints_and_registry_and_engine_mut(); let requested_key = BreakpointKey::from_raw(module, pc); - let actual_key = self - .state + let actual_key = state .breakpoint_redirects .remove(&requested_key) .unwrap_or(requested_key); let actual_pc = actual_key.1; - if let Some(refcount) = self.state.breakpoints.get_mut(&actual_key) { + if let Some(refcount) = state.breakpoints.get_mut(&actual_key) { *refcount -= 1; if *refcount == 0 { - self.state.breakpoints.remove(&actual_key); - if !self.state.single_step { - let mem = Self::get_code_memory( - self.state, - self.registry, - &mut self.dirty_modules, - module, - )?; + state.breakpoints.remove(&actual_key); + if !state.single_step { + let mem = + Self::get_code_memory(state, registry, &mut self.dirty_modules, module)?; let frame_table = module .frame_table() .expect("Frame table must be present when guest-debug is enabled"); @@ -1276,38 +1255,60 @@ impl<'a> BreakpointEdit<'a> { /// In single-step mode, a breakpoint event is emitted at every /// Wasm PC. pub fn single_step(&mut self, enabled: bool) -> Result<()> { + let (state, registry, _) = self.store.breakpoints_and_registry_and_engine_mut(); log::trace!( "single_step({enabled}) with breakpoint set {:?}", - self.state.breakpoints + state.breakpoints ); - if self.state.single_step == enabled { + if state.single_step == enabled { // No change to current state; don't go through the effort of re-patching and // re-publishing code. return Ok(()); } - let modules = self - .registry + let modules = registry .all_modules() .map(|(_, m)| m.clone()) .collect::>(); for module in modules { - let mem = - Self::get_code_memory(self.state, self.registry, &mut self.dirty_modules, &module)?; + let mem = Self::get_code_memory(state, registry, &mut self.dirty_modules, &module)?; Self::apply_single_step(mem, &module, enabled, |key| { - self.state.breakpoints.contains_key(key) + state.breakpoints.contains_key(key) })?; } - self.state.single_step = enabled; + state.single_step = enabled; Ok(()) } + + /// Watches, or stops watching, guest writes to the bytes in `range` of + /// `memory`. A guest store or bulk memory operation that writes a watched + /// byte raises [`DebugEvent::Watchpoint`] before writing. Whether a byte + /// is watched is a per-byte property, so unwatching a range unwatches its + /// bytes regardless of how they were watched. + /// + /// # Errors + /// + /// Returns an error if `range` extends beyond the memory's current size. + /// + /// # Panics + /// + /// Panics if `memory` does not belong to this store. + pub fn watch( + &mut self, + memory: crate::Memory, + range: core::ops::Range, + watch: bool, + ) -> Result<()> { + self.store.debug_set_watch(memory, range, watch) + } } -impl<'a> Drop for BreakpointEdit<'a> { +impl<'a> Drop for BreakpointState<'a> { fn drop(&mut self) { + let (_, registry, _) = self.store.breakpoints_and_registry_and_engine_mut(); for &store_code_base in &self.dirty_modules { - let store_code = self.registry.store_code_mut(store_code_base).unwrap(); + let store_code = registry.store_code_mut(store_code_base).unwrap(); if let Err(e) = store_code .code_memory_mut() .expect("Must have unique ownership of StoreCode in guest-debug mode") diff --git a/crates/wasmtime/src/runtime/module/registry.rs b/crates/wasmtime/src/runtime/module/registry.rs index e5bd81c774f2..17d05ce6110a 100644 --- a/crates/wasmtime/src/runtime/module/registry.rs +++ b/crates/wasmtime/src/runtime/module/registry.rs @@ -118,7 +118,7 @@ fn assert_no_overlap( } #[cfg(feature = "debug")] -pub struct RegisterBreakpointState<'a>(pub(crate) &'a crate::runtime::debug::BreakpointState); +pub struct RegisterBreakpointState<'a>(pub(crate) &'a crate::runtime::debug::Breakpoints); #[cfg(not(feature = "debug"))] pub struct RegisterBreakpointState<'a>(pub(crate) PhantomData<&'a ()>); diff --git a/crates/wasmtime/src/runtime/store.rs b/crates/wasmtime/src/runtime/store.rs index 592eb0e2d6a1..36f878e16251 100644 --- a/crates/wasmtime/src/runtime/store.rs +++ b/crates/wasmtime/src/runtime/store.rs @@ -92,7 +92,7 @@ use crate::runtime::vm::{ }; use crate::trampoline::VMHostGlobalContext; #[cfg(feature = "debug")] -use crate::{BreakpointState, DebugHandler, FrameDataCache}; +use crate::{Breakpoints, DebugHandler, FrameDataCache}; use crate::{Engine, Module, Val, ValRaw, module::ModuleRegistry}; use crate::{Global, Instance, Table}; use core::convert::Infallible; @@ -532,7 +532,7 @@ pub struct StoreOpaque { /// handler defined above, if any, has a chance to perform some /// logic before returning to allow execution to resume. #[cfg(feature = "debug")] - breakpoints: BreakpointState, + breakpoints: Breakpoints, /// The debug PC-to-FrameData cache for this store. /// @@ -1611,12 +1611,12 @@ impl StoreOpaque { #[cfg(feature = "debug")] pub(crate) fn breakpoints_and_registry_and_engine_mut( &mut self, - ) -> (&mut BreakpointState, &mut ModuleRegistry, &Engine) { + ) -> (&mut Breakpoints, &mut ModuleRegistry, &Engine) { (&mut self.breakpoints, &mut self.modules, &self.engine) } #[cfg(feature = "debug")] - pub(crate) fn breakpoints_and_registry(&self) -> (&BreakpointState, &ModuleRegistry) { + pub(crate) fn breakpoints_and_registry(&self) -> (&Breakpoints, &ModuleRegistry) { (&self.breakpoints, &self.modules) } diff --git a/tests/all/debug.rs b/tests/all/debug.rs index 88885fb03208..affc718035fd 100644 --- a/tests/all/debug.rs +++ b/tests/all/debug.rs @@ -2027,7 +2027,10 @@ async fn watchpoints_report_writes_before_they_happen() -> wasmtime::Result<()> // Watch [17, 52): the i32 store overlaps it partially, and the store to // byte 100 not at all. - memory.debug_watch(&mut store, 17..52, true)?; + store + .edit_breakpoints() + .unwrap() + .watch(memory, 17..52, true)?; main.call_async(&mut store, ()).await?; { let hits = recorder.0.lock().unwrap(); @@ -2054,12 +2057,21 @@ async fn watchpoints_report_writes_before_they_happen() -> wasmtime::Result<()> // Unwatched bytes no longer report writes. recorder.0.lock().unwrap().clear(); - memory.debug_watch(&mut store, 0..64, false)?; + store + .edit_breakpoints() + .unwrap() + .watch(memory, 0..64, false)?; main.call_async(&mut store, ()).await?; assert!(recorder.0.lock().unwrap().is_empty()); // Watches must be within the memory. - assert!(memory.debug_watch(&mut store, 0..65537, true).is_err()); + assert!( + store + .edit_breakpoints() + .unwrap() + .watch(memory, 0..65537, true) + .is_err() + ); Ok(()) } @@ -2096,10 +2108,16 @@ async fn watchpoints_on_imported_memory_follow_growth() -> wasmtime::Result<()> // Watch a byte, then grow the memory, and watch a byte in the new pages // too. - memory.debug_watch(&mut store, 10..11, true)?; + store + .edit_breakpoints() + .unwrap() + .watch(memory, 10..11, true)?; assert_eq!(grow.call_async(&mut store, 7).await?, 1); let high = 7 * 65536 + 3; - memory.debug_watch(&mut store, high..high + 1, true)?; + store + .edit_breakpoints() + .unwrap() + .watch(memory, high..high + 1, true)?; // Writes through the importing instance see both watches, and only // those. @@ -2147,7 +2165,10 @@ async fn watchpoints_out_of_bounds_stores_trap() -> wasmtime::Result<()> { let instance = Instance::new_async(&mut store, &module, &[]).await?; let memory = instance.get_memory(&mut store, "memory").unwrap(); let store_i64 = instance.get_typed_func::(&mut store, "store")?; - memory.debug_watch(&mut store, 65528..65536, true)?; + store + .edit_breakpoints() + .unwrap() + .watch(memory, 65528..65536, true)?; // A store that is partially or entirely out of bounds traps before // checking for watchpoints. From ad7f9a786dbf6d4b487d1f8430ea550542ccd189 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Fri, 6 Nov 2026 14:30:00 -0800 Subject: [PATCH 2/2] Review feedback. --- crates/debugger/src/host/opaque.rs | 12 ++++++--- crates/debugger/src/lib.rs | 7 ++--- crates/wasmtime/src/runtime/debug.rs | 38 +++++++++++++++++++--------- tests/all/debug.rs | 21 +++++---------- 4 files changed, 42 insertions(+), 36 deletions(-) diff --git a/crates/debugger/src/host/opaque.rs b/crates/debugger/src/host/opaque.rs index d14cef482011..c264f46f1327 100644 --- a/crates/debugger/src/host/opaque.rs +++ b/crates/debugger/src/host/opaque.rs @@ -525,11 +525,15 @@ impl OpaqueDebugger for crate::Debuggee { .checked_add(len) .filter(|&end| end <= size) .ok_or(wit::Error::OutOfBounds)?; - store + let mut breakpoints = store .edit_breakpoints() - .ok_or(wit::Error::UnsupportedType)? - .watch(memory, addr..end, watch) - .map_err(|_| wit::Error::OutOfBounds)?; + .ok_or(wit::Error::UnsupportedType)?; + let result = if watch { + breakpoints.watch(memory, addr..end) + } else { + breakpoints.unwatch(memory, addr..end) + }; + result.map_err(|_| wit::Error::OutOfBounds)?; Ok(()) }) .await? diff --git a/crates/debugger/src/lib.rs b/crates/debugger/src/lib.rs index 67d5b70f9e34..3b57d3d87c53 100644 --- a/crates/debugger/src/lib.rs +++ b/crates/debugger/src/lib.rs @@ -749,10 +749,7 @@ mod test { // Watch bytes [8, 12): the first store is just below the // range, the second overlaps it partially, the third is just // above it, and the fill covers all of it. - store - .edit_breakpoints() - .unwrap() - .watch(memory, 8..12, true)?; + store.edit_breakpoints().unwrap().watch(memory, 8..12)?; let mut debuggee = Debuggee::new(store, move |store| { Box::pin(async move { @@ -804,7 +801,7 @@ mod test { store .edit_breakpoints() .unwrap() - .watch(memory, 0..16, false) + .unwatch(memory, 0..16) .unwrap(); }) .await?; diff --git a/crates/wasmtime/src/runtime/debug.rs b/crates/wasmtime/src/runtime/debug.rs index eb7f659e0160..cb5234a54c23 100644 --- a/crates/wasmtime/src/runtime/debug.rs +++ b/crates/wasmtime/src/runtime/debug.rs @@ -1281,11 +1281,13 @@ impl<'a> BreakpointState<'a> { Ok(()) } - /// Watches, or stops watching, guest writes to the bytes in `range` of - /// `memory`. A guest store or bulk memory operation that writes a watched - /// byte raises [`DebugEvent::Watchpoint`] before writing. Whether a byte - /// is watched is a per-byte property, so unwatching a range unwatches its - /// bytes regardless of how they were watched. + /// Watches guest writes to the bytes in `range` of `memory`. A guest + /// store or bulk memory operation that writes a watched byte raises + /// [`DebugEvent::Watchpoint`] before writing. + /// + /// Whether a byte is watched is a per-byte property: watching an + /// already-watched byte has no further effect. See + /// [`BreakpointState::unwatch`]. /// /// # Errors /// @@ -1294,13 +1296,25 @@ impl<'a> BreakpointState<'a> { /// # Panics /// /// Panics if `memory` does not belong to this store. - pub fn watch( - &mut self, - memory: crate::Memory, - range: core::ops::Range, - watch: bool, - ) -> Result<()> { - self.store.debug_set_watch(memory, range, watch) + pub fn watch(&mut self, memory: crate::Memory, range: core::ops::Range) -> Result<()> { + self.store.debug_set_watch(memory, range, true) + } + + /// Stops watching guest writes to the bytes in `range` of `memory`. + /// + /// Whether a byte is watched is a per-byte property, so this unwatches + /// every byte in `range`, regardless of how many (possibly overlapping) + /// calls to [`BreakpointState::watch`] watched it. + /// + /// # Errors + /// + /// Returns an error if `range` extends beyond the memory's current size. + /// + /// # Panics + /// + /// Panics if `memory` does not belong to this store. + pub fn unwatch(&mut self, memory: crate::Memory, range: core::ops::Range) -> Result<()> { + self.store.debug_set_watch(memory, range, false) } } diff --git a/tests/all/debug.rs b/tests/all/debug.rs index affc718035fd..1a8c439ce62f 100644 --- a/tests/all/debug.rs +++ b/tests/all/debug.rs @@ -2027,10 +2027,7 @@ async fn watchpoints_report_writes_before_they_happen() -> wasmtime::Result<()> // Watch [17, 52): the i32 store overlaps it partially, and the store to // byte 100 not at all. - store - .edit_breakpoints() - .unwrap() - .watch(memory, 17..52, true)?; + store.edit_breakpoints().unwrap().watch(memory, 17..52)?; main.call_async(&mut store, ()).await?; { let hits = recorder.0.lock().unwrap(); @@ -2057,10 +2054,7 @@ async fn watchpoints_report_writes_before_they_happen() -> wasmtime::Result<()> // Unwatched bytes no longer report writes. recorder.0.lock().unwrap().clear(); - store - .edit_breakpoints() - .unwrap() - .watch(memory, 0..64, false)?; + store.edit_breakpoints().unwrap().unwatch(memory, 0..64)?; main.call_async(&mut store, ()).await?; assert!(recorder.0.lock().unwrap().is_empty()); @@ -2069,7 +2063,7 @@ async fn watchpoints_report_writes_before_they_happen() -> wasmtime::Result<()> store .edit_breakpoints() .unwrap() - .watch(memory, 0..65537, true) + .watch(memory, 0..65537) .is_err() ); Ok(()) @@ -2108,16 +2102,13 @@ async fn watchpoints_on_imported_memory_follow_growth() -> wasmtime::Result<()> // Watch a byte, then grow the memory, and watch a byte in the new pages // too. - store - .edit_breakpoints() - .unwrap() - .watch(memory, 10..11, true)?; + store.edit_breakpoints().unwrap().watch(memory, 10..11)?; assert_eq!(grow.call_async(&mut store, 7).await?, 1); let high = 7 * 65536 + 3; store .edit_breakpoints() .unwrap() - .watch(memory, high..high + 1, true)?; + .watch(memory, high..high + 1)?; // Writes through the importing instance see both watches, and only // those. @@ -2168,7 +2159,7 @@ async fn watchpoints_out_of_bounds_stores_trap() -> wasmtime::Result<()> { store .edit_breakpoints() .unwrap() - .watch(memory, 65528..65536, true)?; + .watch(memory, 65528..65536)?; // A store that is partially or entirely out of bounds traps before // checking for watchpoints.