Revamped public API for record; replay still broken

Author: arjunr2

crates/wasmtime/src/config.rs

@@ -179,7 +179,7 @@ pub struct Config {
     pub(crate) macos_use_mach_ports: bool,
     pub(crate) detect_host_feature: Option<fn(&str) -> Option<bool>>,
     #[cfg(feature = "rr")]
-    pub(crate) rr: Option<RRConfig>,
+    pub(crate) record_support: bool,
 }
 
 /// User-provided configuration for the compiler.
@@ -395,7 +395,7 @@ impl Config {
             #[cfg(not(feature = "std"))]
             detect_host_feature: None,
             #[cfg(feature = "rr")]
-            rr: None,
+            record_support: false,
         };
         #[cfg(any(feature = "cranelift", feature = "winch"))]
         {
@@ -2834,66 +2834,30 @@ impl Config {
         self
     }
 
-    /// Evaluates to true if current configuration must respect
-    /// deterministic execution in its configuration.
-    ///
-    /// Required for faithful record/replay execution.
-    #[cfg(feature = "rr")]
-    #[inline]
-    pub fn is_determinism_enforced(&mut self) -> bool {
-        self.rr.is_some()
-    }
-
     /// Enable execution trace recording with the provided configuration.
     ///
     /// This method implicitly enforces determinism (see [`Config::enforce_determinism`]
     /// for details).
-    ///
-    /// ## Errors
-    ///
-    /// Errors if record/replay are simultaneously enabled.
     #[cfg(feature = "rr")]
-    pub fn enable_record(&mut self, record: RecordConfig) -> Result<&mut Self> {
-        self.enforce_determinism();
-        if let Some(cfg) = &self.rr {
-            if let RRConfig::Replay(_) = cfg {
-                bail!("Cannot enable recording when replay is already enabled");
-            }
-        }
-        self.rr = Some(RRConfig::from(record));
-        Ok(self)
-    }
-
-    /// Enable replay execution based on the provided configuration.
-    ///
-    /// This method implicitly enforces determinism (see [`Config::enforce_determinism`]
-    /// for details).
-    ///
-    /// ## Errors
-    ///
-    /// Errors if record/replay are simultaneously enabled.
-    #[cfg(feature = "rr")]
-    pub fn enable_replay(&mut self, replay: ReplayConfig) -> Result<&mut Self> {
-        self.enforce_determinism();
-        if let Some(cfg) = &self.rr {
-            if let RRConfig::Record(_) = cfg {
-                bail!("Cannot enable replay when recording is already enabled");
-            }
+    #[inline]
+    pub fn recording(&mut self, enable: bool) -> &mut Self {
+        if enable {
+            self.enforce_determinism();
+        } else {
+            self.remove_determinism_enforcement();
         }
-        self.rr = Some(RRConfig::from(replay));
-        Ok(self)
+        self.record_support = enable;
+        self
     }
 
-    /// Disable the currently active record/replay configuration, and remove
-    /// any determinism enforcement it introduced as side-effects.
+    /// Evaluates to true if current configuration must respect
+    /// deterministic execution in its configuration.
     ///
-    /// A common option is used for both record/replay here
-    /// since record and replay can never be set simultaneously/
+    /// Required for faithful record/replay execution.
     #[cfg(feature = "rr")]
-    pub fn disable_record_replay(&mut self) -> &mut Self {
-        self.remove_determinism_enforcement();
-        self.rr = None;
-        self
+    #[inline]
+    pub fn is_determinism_enforced(&mut self) -> bool {
+        self.record_support
     }
 }
 

crates/wasmtime/src/engine.rs

@@ -1,6 +1,4 @@
 use crate::Config;
-#[cfg(feature = "rr")]
-use crate::RRConfig;
 use crate::prelude::*;
 #[cfg(feature = "runtime")]
 pub use crate::runtime::code_memory::CustomCodeMemory;
@@ -256,12 +254,11 @@ impl Engine {
         self.config().async_support
     }
 
-    /// Returns an immutable reference to the record/replay configuration settings
-    /// used by the engine
+    /// Returns whether the engine is configured to support execution recording
     #[cfg(feature = "rr")]
     #[inline]
-    pub fn rr(&self) -> Option<&RRConfig> {
-        self.config().rr.as_ref()
+    pub fn is_recording(&self) -> bool {
+        self.config().record_support
     }
 
     /// Detects whether the bytes provided are a precompiled object produced by

crates/wasmtime/src/runtime.rs

@@ -88,6 +88,8 @@ pub use linker::*;
 pub use memory::*;
 pub use module::{Module, ModuleExport};
 pub use resources::*;
+#[cfg(feature = "rr")]
+pub use rr::{RecordWriter, ReplayReader};
 #[cfg(all(feature = "async", feature = "call-hook"))]
 pub use store::CallHookHandler;
 pub use store::{

crates/wasmtime/src/runtime/instance.rs

@@ -933,7 +933,7 @@ fn pre_instantiate_raw(
     }
 
     #[cfg(feature = "rr")]
-    if module.engine().rr().is_some()
+    if module.engine().is_recording()
         && module.exports().any(|export| {
             use crate::ExternType;
             if let ExternType::Memory(_) = export.ty() {
@@ -943,7 +943,9 @@ fn pre_instantiate_raw(
             }
         })
     {
-        bail!("Cannot support record/replay for core wasm modules when a memory is exported");
+        bail!(
+            "Cannot support recording for core wasm modules when a memory is exported; consider using components"
+        );
     }
 
     Ok(imports)

crates/wasmtime/src/runtime/rr/io.rs

@@ -36,7 +36,7 @@ cfg_if::cfg_if! {
 /// Serialize and write `value` to a `RecordWriter`
 ///
 /// Currently uses `postcard` serializer
-pub fn to_record_writer<T, W>(value: &T, writer: W) -> Result<()>
+pub(super) fn to_record_writer<T, W>(value: &T, writer: W) -> Result<()>
 where
     T: Serialize + ?Sized,
     W: RecordWriter,
@@ -55,7 +55,7 @@ where
 ///
 /// Currently uses `postcard` deserializer, with optional scratch
 /// buffer to deserialize into
-pub fn from_replay_reader<'a, T, R>(reader: R, scratch: &'a mut [u8]) -> Result<T>
+pub(super) fn from_replay_reader<'a, T, R>(reader: R, scratch: &'a mut [u8]) -> Result<T>
 where
     T: Deserialize<'a>,
     R: ReplayReader + 'a,

crates/wasmtime/src/runtime/rr/mod.rs

@@ -118,13 +118,14 @@ rr_event! {
     ComponentBuiltinReturn(__component_events::BuiltinReturnEvent),
 
     // OPTIONAL events for replay validation
-    //
-    // ReallocReturn is optional because we can assume the realloc is deterministic
-    // and the error message is subsumed by the containing LowerReturn/LowerStoreReturn
 
     /// Return from a Wasm component function back to host
     ComponentWasmFuncReturn(__component_events::WasmFuncReturnEvent),
     /// Return from Component ABI realloc call
+    ///
+    /// Since realloc is deterministic, ReallocReturn is optional.
+    /// Any error is subsumed by the containing LowerReturn/LowerStoreReturn
+    /// that triggered realloc
     ComponentReallocReturn(__component_events::ReallocReturnEvent),
     /// Call into host function from component
     ComponentHostFuncEntry(__component_events::HostFuncEntryEvent),
@@ -193,7 +194,7 @@ impl From<EventActionError> for ReplayError {
 /// This trait provides the interface for a FIFO recorder
 pub trait Recorder {
     /// Construct a recorder with the writer backend
-    fn new_recorder(writer: Box<dyn RecordWriter>, settings: RecordSettings) -> Result<Self>
+    fn new_recorder(writer: impl RecordWriter + 'static, settings: RecordSettings) -> Result<Self>
     where
         Self: Sized;
 
@@ -242,19 +243,9 @@ pub trait Replayer: Iterator<Item = RREvent> {
         Self: Sized;
 
     /// Get settings associated with the replay process
-    #[allow(
-        unused,
-        reason = "currently used only for validation resulting in \
-    many unnecessary feature gates. will expand in the future to more features and this attribute can be removed"
-    )]
     fn settings(&self) -> &ReplaySettings;
 
     /// Get the settings (embedded within the trace) during recording
-    #[allow(
-        unused,
-        reason = "currently used only for validation resulting in \
-    many unnecessary feature gates. will expand in the future to more features and this attribute can be removed"
-    )]
     fn trace_settings(&self) -> &RecordSettings;
 
     // Provided Methods
@@ -364,13 +355,16 @@ impl Drop for RecordBuffer {
 }
 
 impl Recorder for RecordBuffer {
-    fn new_recorder(mut writer: Box<dyn RecordWriter>, settings: RecordSettings) -> Result<Self> {
+    fn new_recorder(
+        mut writer: impl RecordWriter + 'static,
+        settings: RecordSettings,
+    ) -> Result<Self> {
         // Replay requires the Module version and record settings
         io::to_record_writer(ModuleVersionStrategy::WasmtimeVersion.as_str(), &mut writer)?;
         io::to_record_writer(&settings, &mut writer)?;
         Ok(RecordBuffer {
             buf: Vec::new(),
-            writer: writer,
+            writer: Box::new(writer),
             settings: settings,
         })
     }
@@ -405,18 +399,8 @@ pub struct ReplayBuffer {
     /// Reader to read replay trace from
     reader: Box<dyn ReplayReader>,
     /// Settings in replay configuration
-    #[allow(
-        unused,
-        reason = "currently used only for validation resulting in \
-    many unnecessary feature gates. will expand in the future to more features and this attribute can be removed"
-    )]
     settings: ReplaySettings,
     /// Settings for record configuration (encoded in the trace)
-    #[allow(
-        unused,
-        reason = "currently used only for validation resulting in \
-    many unnecessary feature gates. will expand in the future to more features and this attribute can be removed"
-    )]
     trace_settings: RecordSettings,
     /// Intermediate static buffer for deserialization
     deser_buffer: Vec<u8>,
@@ -454,7 +438,8 @@ impl Drop for ReplayBuffer {
             if let RREvent::Eof = event {
             } else {
                 log::warn!(
-                    "Replay buffer is dropped with {} remaining events, and is likely an invalid execution",
+                    "Replay buffer is dropped with {} remaining events, 
+                    and is likely an invalid/incomplete execution",
                     self.count()
                 );
             }

crates/wasmtime/src/runtime/store.rs

@@ -76,6 +76,8 @@
 //! contents of `StoreOpaque`. This is an invariant that we, as the authors of
 //! `wasmtime`, must uphold for the public interface to be safe.
 
+#[cfg(feature = "rr")]
+use crate::RecordSettings;
 use crate::RootSet;
 #[cfg(feature = "component-model-async")]
 use crate::component::ComponentStoreData;
@@ -88,7 +90,9 @@ use crate::prelude::*;
 #[cfg(feature = "rr-validate")]
 use crate::rr::Validate;
 #[cfg(feature = "rr")]
-use crate::rr::{RREvent, RecordBuffer, Recorder, ReplayBuffer, ReplayError, Replayer};
+use crate::rr::{
+    RREvent, RecordBuffer, RecordWriter, Recorder, ReplayBuffer, ReplayError, Replayer,
+};
 #[cfg(feature = "gc")]
 use crate::runtime::vm::GcRootsList;
 #[cfg(feature = "stack-switching")]
@@ -617,29 +621,9 @@ impl<T> Store<T> {
             #[cfg(feature = "component-model-async")]
             concurrent_async_state: Default::default(),
             #[cfg(feature = "rr")]
-            record_buffer: engine.rr().and_then(|v| {
-                v.record().and_then(|record| {
-                    Some(
-                        RecordBuffer::new_recorder(
-                            (record.writer_initializer)(),
-                            record.settings.clone(),
-                        )
-                        .unwrap(),
-                    )
-                })
-            }),
+            record_buffer: None,
             #[cfg(feature = "rr")]
-            replay_buffer: engine.rr().and_then(|v| {
-                v.replay().and_then(|replay| {
-                    Some(
-                        ReplayBuffer::new_replayer(
-                            (replay.reader_initializer)(),
-                            replay.settings.clone(),
-                        )
-                        .unwrap(),
-                    )
-                })
-            }),
+            replay_buffer: None,
         };
         let mut inner = Box::new(StoreInner {
             inner,
@@ -1028,6 +1012,20 @@ impl<T> Store<T> {
     ) {
         self.inner.epoch_deadline_callback(Box::new(callback));
     }
+
+    /// Configure a [`Store`] to enable execution recording
+    ///
+    /// This feature must be initialized before instantiating any module within
+    /// the Store. Recording of events is performed according to provided settings, and
+    /// written to the provided writer.
+    #[cfg(feature = "rr")]
+    pub fn init_recording(
+        &mut self,
+        recorder: impl RecordWriter + 'static,
+        settings: RecordSettings,
+    ) -> Result<()> {
+        self.inner.init_recording(recorder, settings)
+    }
 }
 
 impl<'a, T> StoreContext<'a, T> {
@@ -1458,6 +1456,24 @@ impl StoreOpaque {
         &mut self.vm_store_context
     }
 
+    #[cfg(feature = "rr")]
+    pub fn init_recording(
+        &mut self,
+        recorder: impl RecordWriter + 'static,
+        settings: RecordSettings,
+    ) -> Result<()> {
+        ensure!(
+            self.instance_count == 0,
+            "store recording must be initialized before instantiating any modules or components"
+        );
+        ensure!(
+            self.engine().is_recording(),
+            "store recording requires recording enabled on config"
+        );
+        self.record_buffer = Some(RecordBuffer::new_recorder(recorder, settings)?);
+        Ok(())
+    }
+
     #[cfg(feature = "rr")]
     #[inline(always)]
     pub fn record_buffer_mut(&mut self) -> Option<&mut RecordBuffer> {