diff --git a/src/hyperlight_host/examples/guest-debugging/main.rs b/src/hyperlight_host/examples/guest-debugging/main.rs index dc42b0aea..4ced2f8cf 100644 --- a/src/hyperlight_host/examples/guest-debugging/main.rs +++ b/src/hyperlight_host/examples/guest-debugging/main.rs @@ -115,6 +115,76 @@ mod tests { #[cfg(windows)] const GDB_COMMAND: &str = "gdb"; + /// Construct the (out_file_path, cmd_file_path, manifest_dir) + /// triple every gdb test needs. + fn gdb_test_paths(name: &str) -> (String, String, String) { + let out_dir = std::env::var("OUT_DIR").expect("Failed to get out dir"); + let manifest_dir = std::env::var("CARGO_MANIFEST_DIR") + .expect("Failed to get manifest dir") + .replace('\\', "/"); + let out_file_path = format!("{out_dir}/{name}.output"); + let cmd_file_path = format!("{out_dir}/{name}-commands.txt"); + (out_file_path, cmd_file_path, manifest_dir) + } + + /// Build a gdb script that connects to `port`, sets a single + /// breakpoint at `breakpoint`, prints `echo_msg` when hit, and + /// detaches before quitting. + /// + /// The breakpoint commands end with `detach` + `quit` instead of + /// `continue`. The previous "inner continue, outer continue, quit" + /// shape races with the inferior exit. After the breakpoint hits + /// and the inner `continue` resumes the guest, the guest may run + /// to completion and the gdb stub may close the remote before gdb + /// has dispatched the outer `continue`, producing a non-zero exit + /// with `Remote connection closed`. Detaching from the breakpoint + /// commands removes that window. The host process keeps running + /// the guest call to completion on its own after detach. + fn single_breakpoint_script( + manifest_dir: &str, + port: u16, + out_file_path: &str, + breakpoint: &str, + echo_msg: &str, + ) -> String { + let cmd = format!( + "file {manifest_dir}/../tests/rust_guests/bin/debug/simpleguest + target remote :{port} + + set pagination off + set logging file {out_file_path} + set logging enabled on + + break {breakpoint} + commands + echo \"{echo_msg}\\n\" + backtrace + + set logging enabled off + detach + quit + end + + continue + " + ); + #[cfg(windows)] + let cmd = format!("set osabi none\n{cmd}"); + cmd + } + + /// Spawn the gdb client to execute the script in `cmd_file_path`. + fn spawn_gdb_client(cmd_file_path: &str) -> std::process::Child { + Command::new(GDB_COMMAND) + .arg("-nx") + .arg("--nw") + .arg("--batch") + .arg("-x") + .arg(cmd_file_path) + .spawn() + .expect("Failed to start gdb") + } + fn write_cmds_file(cmd_file_path: &str, cmd: &str) -> io::Result<()> { let file = File::create(cmd_file_path)?; let mut writer = BufWriter::new(file); @@ -163,14 +233,7 @@ mod tests { // wait 3 seconds for the gdb to connect thread::sleep(Duration::from_secs(3)); - let mut gdb = Command::new(GDB_COMMAND) - .arg("-nx") // Don't load any .gdbinit files - .arg("--nw") - .arg("--batch") - .arg("-x") - .arg(cmd_file_path) - .spawn() - .map_err(|e| new_error!("Failed to start gdb process: {}", e))?; + let mut gdb = spawn_gdb_client(cmd_file_path); // wait 3 seconds for the gdb to connect thread::sleep(Duration::from_secs(10)); @@ -245,38 +308,16 @@ mod tests { #[test] #[serial] fn test_gdb_end_to_end() { - let out_dir = std::env::var("OUT_DIR").expect("Failed to get out dir"); - let manifest_dir = std::env::var("CARGO_MANIFEST_DIR") - .expect("Failed to get manifest dir") - .replace('\\', "/"); - let out_file_path = format!("{out_dir}/gdb.output"); - let cmd_file_path = format!("{out_dir}/gdb-commands.txt"); - - let cmd = format!( - "file {manifest_dir}/../tests/rust_guests/bin/debug/simpleguest - target remote :8080 - - set pagination off - set logging file {out_file_path} - set logging enabled on - - break hyperlight_main - commands - echo \"Stopped at hyperlight_main breakpoint\\n\" - backtrace - - set logging enabled off - detach - quit - end - - continue - " + let (out_file_path, cmd_file_path, manifest_dir) = gdb_test_paths("gdb"); + + let cmd = single_breakpoint_script( + &manifest_dir, + 8080, + &out_file_path, + "hyperlight_main", + "Stopped at hyperlight_main breakpoint", ); - #[cfg(windows)] - let cmd = format!("set osabi none\n{}", cmd); - let checker = |contents: String| contents.contains("Stopped at hyperlight_main breakpoint"); let result = run_guest_and_gdb(&cmd_file_path, &out_file_path, &cmd, checker); @@ -288,13 +329,8 @@ mod tests { #[test] #[serial] fn test_gdb_sse_check() { - let out_dir = std::env::var("OUT_DIR").expect("Failed to get out dir"); - let manifest_dir = std::env::var("CARGO_MANIFEST_DIR") - .expect("Failed to get manifest dir") - .replace('\\', "/"); + let (out_file_path, cmd_file_path, manifest_dir) = gdb_test_paths("gdb-sse"); println!("manifest dir {manifest_dir}"); - let out_file_path = format!("{out_dir}/gdb-sse.output"); - let cmd_file_path = format!("{out_dir}/gdb-sse--commands.txt"); let cmd = format!( "file {manifest_dir}/../tests/rust_guests/bin/debug/simpleguest @@ -330,4 +366,74 @@ mod tests { cleanup(&out_file_path, &cmd_file_path); assert!(result.is_ok(), "{}", result.unwrap_err()); } + + #[test] + #[serial] + fn test_gdb_from_snapshot() { + use hyperlight_host::HostFunctions; + + const PORT: u16 = 8081; + + let (out_file_path, cmd_file_path, manifest_dir) = gdb_test_paths("gdb-from-snapshot"); + + // Build a sandbox the normal way and snapshot it in-memory. + let mut producer: MultiUseSandbox = UninitializedSandbox::new( + hyperlight_host::GuestBinary::FilePath( + hyperlight_testing::simple_guest_as_string().unwrap(), + ), + None, + ) + .unwrap() + .evolve() + .unwrap(); + let snap = producer.snapshot().unwrap(); + + // Order matters. The gdb stub event loop must enter (i.e. + // `VcpuStopped` must be sent on the channel) before the gdb + // client connects, otherwise the wire protocol desyncs. The + // evolve case gets this for free because `evolve()` runs + // `vm.initialise()` which trips the entry breakpoint + // immediately. For a `Call` snapshot `vm.initialise` is a + // no-op, so we trigger the breakpoint by running `sbox.call` + // here before the client is launched below. + let snap_thread = snap.clone(); + let sandbox_thread = thread::spawn(move || -> Result<()> { + let mut cfg = SandboxConfiguration::default(); + cfg.set_guest_debug_info(DebugInfo { port: PORT }); + + let mut sbox = + MultiUseSandbox::from_snapshot(snap_thread, HostFunctions::default(), Some(cfg))?; + sbox.call::( + "PrintOutput", + "Hello from a from_snapshot sandbox\n".to_string(), + )?; + Ok(()) + }); + + // Wait for the sandbox thread to bind the listener, install + // the one-shot breakpoint, and trip it. + thread::sleep(Duration::from_secs(3)); + + let cmd = single_breakpoint_script( + &manifest_dir, + PORT, + &out_file_path, + "main.rs:simpleguest::print_output", + "Stopped at print_output breakpoint", + ); + write_cmds_file(&cmd_file_path, &cmd).expect("Failed to write gdb commands"); + + let mut gdb = spawn_gdb_client(&cmd_file_path); + let _ = gdb.wait(); + let sandbox_result = sandbox_thread + .join() + .expect("from_snapshot sandbox thread panicked"); + + let checker = |contents: String| contents.contains("Stopped at print_output breakpoint"); + let result = check_output(&out_file_path, checker); + + cleanup(&out_file_path, &cmd_file_path); + sandbox_result.expect("from_snapshot sandbox returned error"); + result.expect("gdb output missing expected breakpoint hit"); + } } diff --git a/src/hyperlight_host/src/func/host_functions.rs b/src/hyperlight_host/src/func/host_functions.rs index e87fa70b0..9ccb98f05 100644 --- a/src/hyperlight_host/src/func/host_functions.rs +++ b/src/hyperlight_host/src/func/host_functions.rs @@ -52,7 +52,8 @@ impl Registerable for UninitializedSandbox { return_type: Output::TYPE, }; - (*hfs).register_host_function(name.to_string(), entry) + (*hfs).register_host_function(name.to_string(), entry); + Ok(()) } } @@ -92,7 +93,26 @@ impl Registerable for crate::MultiUseSandbox { return_type: Output::TYPE, }; - (*hfs).register_host_function(name.to_string(), entry) + (*hfs).register_host_function(name.to_string(), entry); + Ok(()) + } +} + +impl Registerable for crate::HostFunctions { + fn register_host_function( + &mut self, + name: &str, + hf: impl Into>, + ) -> Result<()> { + let entry = FunctionEntry { + function: hf.into().into(), + parameter_types: Args::TYPE, + return_type: Output::TYPE, + }; + + self.inner_mut() + .register_host_function(name.to_string(), entry); + Ok(()) } } @@ -236,7 +256,7 @@ pub(crate) fn register_host_function std::result::Result { let CommonRegisters { rip, .. } = vm.regs()?; @@ -81,10 +80,6 @@ pub(crate) fn vcpu_stop_reason( // Check page 19-4 Vol. 3B of Intel 64 and IA-32 // Architectures Software Developer's Manual if DR6_HW_BP_FLAGS_MASK & dr6 != 0 { - if rip == entrypoint { - vm.remove_hw_breakpoint(entrypoint)?; - return Ok(VcpuStopReason::EntryPointBp); - } return Ok(VcpuStopReason::HwBp); } } @@ -98,12 +93,10 @@ pub(crate) fn vcpu_stop_reason( r"The vCPU exited because of an unknown reason: rip: {:?} dr6: {:?} - entrypoint: {:?} exception: {:?} ", rip, dr6, - entrypoint, exception, ); diff --git a/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs b/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs index bc7c9fd14..5edd81b50 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs @@ -59,7 +59,6 @@ impl run_blocking::BlockingEventLoop for GdbBlockingEventLoop { // Resume execution if unknown reason for stop let stop_response = match stop_reason { VcpuStopReason::DoneStep => BaseStopReason::DoneStep, - VcpuStopReason::EntryPointBp => BaseStopReason::HwBreak(()), VcpuStopReason::SwBp => BaseStopReason::SwBreak(()), VcpuStopReason::HwBp => BaseStopReason::HwBreak(()), // This is a consequence of the GDB client sending an interrupt signal diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 94396e5ae..0a0685f71 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -171,10 +171,6 @@ impl DebugMemoryAccess { pub enum VcpuStopReason { Crash, DoneStep, - /// Hardware breakpoint inserted by the hypervisor so the guest can be stopped - /// at the entry point. This is used to avoid the guest from executing - /// the entry point code before the debugger is connected - EntryPointBp, HwBp, SwBp, Interrupt, diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs index 830b856c0..afb332a66 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs @@ -389,6 +389,12 @@ pub(crate) struct HyperlightVm { pub(super) gdb_conn: Option>, #[cfg(gdb)] pub(super) sw_breakpoints: HashMap, // addr -> original instruction + /// One-shot hw breakpoint installed at the entry address when gdb is + /// enabled, so the gdb stub gets a `VcpuStopped` to enter its event + /// loop on the first vCPU run after construction. Cleared on first + /// hit by `handle_debug`. + #[cfg(gdb)] + pub(super) one_shot_entry_bp: Option, #[cfg(feature = "mem_profile")] pub(super) trace_info: MemTraceInfo, #[cfg(crashdump)] @@ -598,17 +604,28 @@ impl HyperlightVm { match exit_reason { #[cfg(gdb)] Ok(VmExit::Debug { dr6, exception }) => { - let initialise = match self.entrypoint { - NextAction::Initialise(initialise) => initialise, - _ => 0, - }; - // Handle debug event (breakpoints) + // Classify the debug exit. `vcpu_stop_reason` is a + // pure classifier and has no side effects on the VM. let stop_reason = crate::hypervisor::gdb::arch::vcpu_stop_reason( - self.vm.as_mut(), + self.vm.as_ref(), dr6, - initialise, exception, )?; + // Remove the one-shot entry breakpoint installed by + // `HyperlightVm::new` the first time it fires so it + // does not interfere with later user-installed + // breakpoints at the same address. + if matches!(stop_reason, VcpuStopReason::HwBp) + && let Some(entry_addr) = self.one_shot_entry_bp + { + let rip = self.vm.regs().map_err(VcpuStopReasonError::GetRegs)?.rip; + if rip == entry_addr { + self.vm + .remove_hw_breakpoint(entry_addr) + .map_err(VcpuStopReasonError::RemoveHwBreakpoint)?; + self.one_shot_entry_bp = None; + } + } if let Err(e) = self.handle_debug(dbg_mem_access_fn.clone(), stop_reason) { break Err(e.into()); } diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs index 16ac55ad3..6feff13f1 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs @@ -168,6 +168,8 @@ impl HyperlightVm { gdb_conn, #[cfg(gdb)] sw_breakpoints: HashMap::new(), + #[cfg(gdb)] + one_shot_entry_bp: None, #[cfg(feature = "mem_profile")] trace_info, #[cfg(crashdump)] @@ -182,12 +184,21 @@ impl HyperlightVm { #[cfg(gdb)] if ret.gdb_conn.is_some() { ret.send_dbg_msg(DebugResponse::InterruptHandle(ret.interrupt_handle.clone()))?; - // Add breakpoint to the entry point address, if we are going to initialise + // Add breakpoint at the entry point address. The breakpoint + // is removed on first hit by the run loop. Tracked via + // `one_shot_entry_bp` so it does not interfere with later + // user-installed breakpoints at the same address. ret.vm.set_debug(true).map_err(VmError::Debug)?; - if let NextAction::Initialise(initialise) = entrypoint { + let entry_addr = match entrypoint { + NextAction::Initialise(addr) | NextAction::Call(addr) => Some(addr), + #[cfg(test)] + NextAction::None => None, + }; + if let Some(addr) = entry_addr { ret.vm - .add_hw_breakpoint(initialise) + .add_hw_breakpoint(addr) .map_err(CreateHyperlightVmError::AddHwBreakpoint)?; + ret.one_shot_entry_bp = Some(addr); } } diff --git a/src/hyperlight_host/src/lib.rs b/src/hyperlight_host/src/lib.rs index b21f413e3..c1d51cd5b 100644 --- a/src/hyperlight_host/src/lib.rs +++ b/src/hyperlight_host/src/lib.rs @@ -91,6 +91,9 @@ pub use hypervisor::virtual_machine::is_hypervisor_present; pub use sandbox::MultiUseSandbox; /// The re-export for the `UninitializedSandbox` type pub use sandbox::UninitializedSandbox; +/// A collection of host functions that can be supplied to a sandbox +/// constructor (e.g. [`MultiUseSandbox::from_snapshot`]). +pub use sandbox::host_funcs::HostFunctions; /// The re-export for the `GuestBinary` type pub use sandbox::uninitialized::GuestBinary; /// The re-export for the `GuestCounter` type diff --git a/src/hyperlight_host/src/mem/layout.rs b/src/hyperlight_host/src/mem/layout.rs index 07f82a829..8f1790055 100644 --- a/src/hyperlight_host/src/mem/layout.rs +++ b/src/hyperlight_host/src/mem/layout.rs @@ -226,16 +226,16 @@ pub(crate) struct SandboxMemoryLayout { /// The size of the guest code section. pub(crate) code_size: usize, /// The size of the init data section (guest blob). - init_data_size: usize, + pub(crate) init_data_size: usize, /// Permission flags for the init data region. #[cfg_attr(feature = "i686-guest", allow(unused))] - init_data_permissions: Option, + pub(crate) init_data_permissions: Option, /// The size of the scratch region in physical memory. - scratch_size: usize, + pub(crate) scratch_size: usize, /// The size of the snapshot region in physical memory. - snapshot_size: usize, + pub(crate) snapshot_size: usize, /// The size of the page tables (None if not yet set). - pt_size: Option, + pub(crate) pt_size: Option, } impl Debug for SandboxMemoryLayout { @@ -295,7 +295,7 @@ impl SandboxMemoryLayout { /// Both the scratch region and the snapshot region are bounded by /// this size. The value is arbitrary but chosen to be large enough /// for most workloads while preventing accidental resource exhaustion. - const MAX_MEMORY_SIZE: usize = (16 * 1024 * 1024 * 1024) - Self::BASE_ADDRESS; // 16 GiB - BASE_ADDRESS + pub(crate) const MAX_MEMORY_SIZE: usize = (16 * 1024 * 1024 * 1024) - Self::BASE_ADDRESS; // 16 GiB - BASE_ADDRESS /// The base address of the sandbox's memory. pub(crate) const BASE_ADDRESS: usize = 0x1000; diff --git a/src/hyperlight_host/src/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index 9e5d843d1..d77779ce3 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -22,6 +22,7 @@ use hyperlight_common::flatbuffer_wrappers::function_call::{ }; use hyperlight_common::flatbuffer_wrappers::function_types::FunctionCallResult; use hyperlight_common::flatbuffer_wrappers::guest_log_data::GuestLogData; +use hyperlight_common::flatbuffer_wrappers::host_function_details::HostFunctionDetails; use hyperlight_common::vmem::{self, PAGE_TABLE_SIZE}; #[cfg(all(feature = "crashdump", not(feature = "i686-guest")))] use hyperlight_common::vmem::{BasicMapping, MappingKind}; @@ -298,6 +299,7 @@ where } /// Create a snapshot with the given mapped regions + #[allow(clippy::too_many_arguments)] pub(crate) fn snapshot( &mut self, sandbox_id: u64, @@ -306,6 +308,7 @@ where rsp_gva: u64, sregs: CommonSpecialRegisters, entrypoint: NextAction, + host_functions: HostFunctionDetails, ) -> Result { self.snapshot_count += 1; Snapshot::new( @@ -320,6 +323,7 @@ where sregs, entrypoint, self.snapshot_count, + host_functions, ) } } @@ -330,7 +334,13 @@ impl SandboxMemoryManager { let shared_mem = s.memory().to_mgr_snapshot_mem()?; let scratch_mem = ExclusiveSharedMemory::new(s.layout().get_scratch_size())?; let entrypoint = s.entrypoint(); - Ok(Self::new(layout, shared_mem, scratch_mem, entrypoint)) + let mut mgr = Self::new(layout, shared_mem, scratch_mem, entrypoint); + // Inherit the snapshot's generation number for the same + // reason `restore_snapshot` does: the guest-visible counter + // reflects "which snapshot is the sandbox currently a clone + // of", not "how many snapshots this partition has taken". + mgr.snapshot_count = s.snapshot_generation(); + Ok(mgr) } /// Wraps ExclusiveSharedMemory::build diff --git a/src/hyperlight_host/src/sandbox/host_funcs.rs b/src/hyperlight_host/src/sandbox/host_funcs.rs index a1430338b..c271e6c17 100644 --- a/src/hyperlight_host/src/sandbox/host_funcs.rs +++ b/src/hyperlight_host/src/sandbox/host_funcs.rs @@ -35,8 +35,80 @@ pub struct FunctionRegistry { functions_map: HashMap, } -impl From<&mut FunctionRegistry> for HostFunctionDetails { - fn from(registry: &mut FunctionRegistry) -> Self { +/// A collection of host functions that can be supplied to a sandbox +/// constructor (e.g. [`crate::MultiUseSandbox::from_snapshot`]) to +/// expose host-side functionality to the guest. +/// +/// Use [`HostFunctions::default`] to start with the standard +/// `HostPrint` function pre-registered (matches the registry that the +/// regular `UninitializedSandbox` → `evolve()` path constructs), or +/// [`HostFunctions::empty`] to start with an empty registry. +/// +/// Add additional host functions via the +/// [`crate::func::Registerable`] trait, just as you would on an +/// `UninitializedSandbox`. +/// +/// ```no_run +/// # use hyperlight_host::{HostFunctions, Result}; +/// # use hyperlight_host::func::Registerable; +/// # fn example() -> Result<()> { +/// // Default: HostPrint already registered. +/// let mut funcs = HostFunctions::default(); +/// funcs.register_host_function("Add", |a: i32, b: i32| Ok(a + b))?; +/// # Ok(()) +/// # } +/// ``` +pub struct HostFunctions(FunctionRegistry); + +impl HostFunctions { + /// Create an empty `HostFunctions` with no host functions + /// registered. + /// + /// Most callers want [`HostFunctions::default`] instead, which + /// pre-registers the standard `HostPrint` function. An empty + /// registry will fail snapshot validation against any snapshot + /// that captured `HostPrint`, and any guest code that tries to + /// `printf` into an empty registry will get an EIO from + /// `write(2)`. + pub fn empty() -> Self { + Self(FunctionRegistry::default()) + } + + /// Consume this `HostFunctions` and return the inner registry. + pub(crate) fn into_inner(self) -> FunctionRegistry { + self.0 + } + + /// Borrow the inner registry mutably. + pub(crate) fn inner_mut(&mut self) -> &mut FunctionRegistry { + &mut self.0 + } + + /// Borrow the inner registry immutably. + pub(crate) fn inner(&self) -> &FunctionRegistry { + &self.0 + } +} + +impl Default for HostFunctions { + /// Create a `HostFunctions` pre-populated with the standard + /// `HostPrint` function (writes UTF-8 strings to the host's + /// stdout in green). + /// + /// This matches the default registry installed by + /// `UninitializedSandbox::new()`, so a snapshot taken from a + /// regular sandbox can be loaded with + /// `MultiUseSandbox::from_snapshot(snap, HostFunctions::default(), None)` + /// without registering anything else. + /// + /// Use [`HostFunctions::empty`] for an empty registry. + fn default() -> Self { + Self(FunctionRegistry::with_default_host_print()) + } +} + +impl From<&FunctionRegistry> for HostFunctionDetails { + fn from(registry: &FunctionRegistry) -> Self { let host_functions = registry .functions_map .iter() @@ -61,15 +133,26 @@ pub struct FunctionEntry { impl FunctionRegistry { /// Register a host function with the sandbox. - #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] - pub(crate) fn register_host_function( - &mut self, - name: String, - func: FunctionEntry, - ) -> Result<()> { + #[instrument(skip_all, parent = Span::current(), level = "Trace")] + pub(crate) fn register_host_function(&mut self, name: String, func: FunctionEntry) { self.functions_map.insert(name, func); + } - Ok(()) + /// Create a `FunctionRegistry` pre-populated with the default + /// `HostPrint` function (writes to stdout with green text). + pub(crate) fn with_default_host_print() -> Self { + use crate::func::host_functions::HostFunction; + use crate::func::{ParameterTuple, SupportedReturnType}; + + let mut registry = Self::default(); + let hf: HostFunction = default_writer_func.into(); + let entry = FunctionEntry { + function: hf.into(), + parameter_types: <(String,)>::TYPE, + return_type: ::TYPE, + }; + registry.register_host_function("HostPrint".to_string(), entry); + registry } /// Assuming a host function called `"HostPrint"` exists, and takes a @@ -118,7 +201,7 @@ impl FunctionRegistry { /// The default writer function is to write to stdout with green text. #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] -pub(super) fn default_writer_func(s: String) -> Result { +fn default_writer_func(s: String) -> Result { match std::io::stdout().is_terminal() { false => { print!("{}", s); diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index 241622cab..b459d7507 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -145,6 +145,179 @@ impl MultiUseSandbox { self.pt_root_finder = Some(finder); } + /// Create a `MultiUseSandbox` directly from a [`Snapshot`], + /// bypassing [`UninitializedSandbox`](crate::UninitializedSandbox) + /// and [`evolve()`](crate::UninitializedSandbox::evolve). + /// + /// This is useful for fast sandbox creation when a snapshot of + /// an already-initialized guest is available, either saved to disk + /// or captured in memory from another sandbox. + /// + /// The provided [`HostFunctions`] must include every host function + /// that was registered on the sandbox at the time the snapshot was + /// taken (matched by name and signature). Additional host functions + /// not present in the snapshot are allowed. + /// + /// An optional [`SandboxConfiguration`](crate::sandbox::SandboxConfiguration) + /// can be supplied to override runtime settings such as timeouts and + /// interrupt behavior. Memory layout fields + /// (`input_data_size`, `output_data_size`, `heap_size`, `scratch_size`) + /// are always taken from the snapshot. Any values supplied in + /// `config` for those fields are ignored. + /// + /// # Examples + /// + /// From a snapshot taken on another sandbox: + /// + /// ```no_run + /// # use std::sync::Arc; + /// # use hyperlight_host::{HostFunctions, MultiUseSandbox, UninitializedSandbox, GuestBinary}; + /// # fn example() -> Result<(), Box> { + /// // Create and initialize a sandbox the normal way + /// let mut sandbox: MultiUseSandbox = UninitializedSandbox::new( + /// GuestBinary::FilePath("guest.bin".into()), + /// None, + /// )?.evolve()?; + /// + /// // Capture a snapshot of the initialized state + /// let snapshot = sandbox.snapshot()?; + /// + /// // Create a new sandbox directly from the snapshot + /// let mut sandbox2 = MultiUseSandbox::from_snapshot(snapshot, HostFunctions::default(), None)?; + /// let result: i32 = sandbox2.call("GetValue", ())?; + /// # Ok(()) + /// # } + /// ``` + #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] + pub fn from_snapshot( + snapshot: Arc, + host_funcs: crate::HostFunctions, + config: Option, + ) -> Result { + use rand::RngExt; + + use crate::mem::ptr::RawPtr; + use crate::sandbox::uninitialized_evolve::set_up_hypervisor_partition; + + // Validate that the provided host functions are a superset of + // those required by the snapshot. + snapshot.validate_host_functions(&host_funcs)?; + + let host_funcs = Arc::new(Mutex::new(host_funcs.into_inner())); + + let stack_top_gva = snapshot.stack_top_gva(); + // Start from the caller's config (if any) so runtime fields + // such as timeouts and interrupt knobs are honored, then + // overwrite the layout fields from the snapshot. The on-disk + // layout is fixed, so any layout values supplied by the + // caller are silently ignored. Warn if the caller passed a + // config whose layout fields disagree with the snapshot, so + // the override is at least visible. + let caller_supplied_config = config.is_some(); + let mut config = config.unwrap_or_default(); + if caller_supplied_config { + warn_on_layout_override(&config, snapshot.layout()); + } + config.set_input_data_size(snapshot.layout().input_data_size); + config.set_output_data_size(snapshot.layout().output_data_size); + config.set_heap_size(snapshot.layout().heap_size as u64); + config.set_scratch_size(snapshot.layout().get_scratch_size()); + let load_info = snapshot.load_info(); + + let mgr = crate::mem::mgr::SandboxMemoryManager::from_snapshot(&snapshot)?; + let (mut hshm, gshm) = mgr.build()?; + + let page_size = u32::try_from(page_size::get())? as usize; + + #[cfg(target_os = "linux")] + crate::signal_handlers::setup_signal_handlers(&config)?; + + // Build the runtime config from the caller's `SandboxConfiguration` + // so that `guest_core_dump` (crashdump) and `guest_debug_info` (gdb) + // take effect just like they do in the normal evolve path. + // `binary_path` and `entry_point` are not available from a snapshot + // and are left unset. This only affects metadata in core dumps. + #[cfg(any(crashdump, gdb))] + let rt_cfg = crate::sandbox::uninitialized::SandboxRuntimeConfig { + #[cfg(crashdump)] + binary_path: None, + #[cfg(gdb)] + debug_info: config.get_guest_debug_info(), + #[cfg(crashdump)] + guest_core_dump: config.get_guest_core_dump(), + #[cfg(crashdump)] + entry_point: None, + }; + + let mut vm = set_up_hypervisor_partition( + gshm, + &config, + stack_top_gva, + page_size, + #[cfg(any(crashdump, gdb))] + rt_cfg, + load_info, + )?; + + let seed = { + let mut rng = rand::rng(); + rng.random::() + }; + let peb_addr = RawPtr::from(u64::try_from(hshm.layout.peb_address())?); + + #[cfg(gdb)] + let dbg_mem_access_hdl = Arc::new(Mutex::new(hshm.clone())); + + vm.initialise( + peb_addr, + seed, + page_size as u32, + &mut hshm, + &host_funcs, + None, + #[cfg(gdb)] + dbg_mem_access_hdl, + ) + .map_err(crate::hypervisor::hyperlight_vm::HyperlightVmError::Initialize)?; + + // If the snapshot was taken from an already-initialized guest + // (NextAction::Call), apply the captured special registers so + // the guest resumes in the correct CPU state. + #[cfg(not(feature = "i686-guest"))] + if matches!(snapshot.entrypoint(), super::snapshot::NextAction::Call(_)) { + let sregs = snapshot.sregs().ok_or_else(|| { + crate::new_error!("snapshot with NextAction::Call must have captured sregs") + })?; + vm.apply_sregs(hshm.layout.get_pt_base_gpa(), sregs) + .map_err(|e| { + crate::HyperlightError::HyperlightVmError( + crate::hypervisor::hyperlight_vm::HyperlightVmError::Restore(e), + ) + })?; + } + + #[cfg(gdb)] + let dbg_mem_wrapper = Arc::new(Mutex::new(hshm.clone())); + + let mut sbox = MultiUseSandbox::from_uninit( + host_funcs, + hshm, + vm, + #[cfg(gdb)] + dbg_mem_wrapper, + ); + // Use the snapshot's sandbox_id so that restore() back to this + // snapshot is permitted. The id is process-local and never + // persisted to disk: `Snapshot::from_oci` assigns a fresh id + // on every load, so two `from_file` calls of the same path + // yield restore-incompatible sandboxes (which is the intended + // safer default). Sandboxes built from clones of the same + // in-memory `Arc` share the id and are mutually + // restore-compatible. + sbox.id = snapshot.sandbox_id(); + Ok(sbox) + } + /// Creates a snapshot of the sandbox's current memory state. /// /// The snapshot is tied to this specific sandbox instance and can only be @@ -207,6 +380,11 @@ impl MultiUseSandbox { .get_snapshot_sregs() .map_err(|e| HyperlightError::HyperlightVmError(e.into()))?; let entrypoint = self.vm.get_entrypoint(); + let host_functions = (&*self.host_funcs.try_lock().map_err(|e| { + crate::new_error!("Error locking host_funcs at {}:{}: {}", file!(), line!(), e) + })?) + .into(); + let memory_snapshot = self.mem_mgr.snapshot( self.id, mapped_regions_vec, @@ -214,6 +392,7 @@ impl MultiUseSandbox { stack_top_gpa, sregs, entrypoint, + host_functions, )?; let snapshot = Arc::new(memory_snapshot); self.snapshot = Some(snapshot.clone()); @@ -943,6 +1122,48 @@ impl std::fmt::Debug for MultiUseSandbox { } } +/// Emit a warning for each memory-layout field in `caller` that +/// disagrees with `snapshot`. Used by [`MultiUseSandbox::from_snapshot`] +/// to surface ignored caller-supplied layout values, since those +/// fields are always taken from the snapshot. +fn warn_on_layout_override( + caller: &crate::sandbox::SandboxConfiguration, + snapshot: &crate::mem::layout::SandboxMemoryLayout, +) { + let mismatches: &[(&str, u64, u64)] = &[ + ( + "input_data_size", + caller.get_input_data_size() as u64, + snapshot.input_data_size as u64, + ), + ( + "output_data_size", + caller.get_output_data_size() as u64, + snapshot.output_data_size as u64, + ), + ( + "heap_size", + caller.get_heap_size(), + snapshot.heap_size as u64, + ), + ( + "scratch_size", + caller.get_scratch_size() as u64, + snapshot.get_scratch_size() as u64, + ), + ]; + for (name, supplied, snap) in mismatches { + if supplied != snap { + tracing::warn!( + "from_snapshot ignoring caller-supplied {} ({}); using snapshot value ({})", + name, + supplied, + snap + ); + } + } +} + #[cfg(test)] mod tests { use std::sync::{Arc, Barrier}; @@ -2588,4 +2809,222 @@ mod tests { } let _ = std::fs::remove_file(&path); } + + /// Tests for [`MultiUseSandbox::from_snapshot`] in-memory. + mod from_snapshot { + use std::sync::Arc; + + use hyperlight_testing::simple_guest_as_string; + + use crate::func::Registerable; + use crate::sandbox::SandboxConfiguration; + use crate::sandbox::snapshot::Snapshot; + use crate::{GuestBinary, HostFunctions, MultiUseSandbox, UninitializedSandbox}; + + fn make_sandbox() -> MultiUseSandbox { + let path = simple_guest_as_string().unwrap(); + UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap() + } + + /// Sandbox with an extra `Add(i32, i32) -> i32` host function. + fn make_sandbox_with_add() -> MultiUseSandbox { + let path = simple_guest_as_string().unwrap(); + let mut u = UninitializedSandbox::new(GuestBinary::FilePath(path), None).unwrap(); + u.register_host_function("Add", |a: i32, b: i32| Ok(a + b)) + .unwrap(); + u.evolve().unwrap() + } + + fn host_funcs_with_matching_add() -> HostFunctions { + let mut hf = HostFunctions::default(); + hf.register_host_function("Add", |a: i32, b: i32| Ok(a + b)) + .unwrap(); + hf + } + + #[test] + fn round_trip_running_sandbox() { + let mut sbox = make_sandbox(); + sbox.call::("AddToStatic", 11i32).unwrap(); + let snapshot = sbox.snapshot().unwrap(); + let mut sbox2 = + MultiUseSandbox::from_snapshot(snapshot, HostFunctions::default(), None).unwrap(); + assert_eq!(sbox2.call::("GetStatic", ()).unwrap(), 11); + let echoed: String = sbox2.call("Echo", "hi".to_string()).unwrap(); + assert_eq!(echoed, "hi"); + } + + #[test] + fn round_trip_pre_init_snapshot() { + let path = simple_guest_as_string().unwrap(); + let snap = + Snapshot::from_env(GuestBinary::FilePath(path), SandboxConfiguration::default()) + .unwrap(); + let mut sbox = + MultiUseSandbox::from_snapshot(Arc::new(snap), HostFunctions::default(), None) + .unwrap(); + assert_eq!(sbox.call::("GetStatic", ()).unwrap(), 0); + } + + /// Sandboxes built from clones of one `Arc` share + /// `sandbox_id` (so both can `restore` to it) but are + /// memory-isolated from each other. + #[test] + fn arc_clone_isolation_and_restore_compat() { + let mut sbox = make_sandbox(); + sbox.call::("AddToStatic", 3i32).unwrap(); + let snapshot = sbox.snapshot().unwrap(); + + let mut a = + MultiUseSandbox::from_snapshot(snapshot.clone(), HostFunctions::default(), None) + .unwrap(); + let mut b = + MultiUseSandbox::from_snapshot(snapshot.clone(), HostFunctions::default(), None) + .unwrap(); + assert_eq!(a.call::("GetStatic", ()).unwrap(), 3); + assert_eq!(b.call::("GetStatic", ()).unwrap(), 3); + + a.call::("AddToStatic", 7i32).unwrap(); + assert_eq!(a.call::("GetStatic", ()).unwrap(), 10); + assert_eq!(b.call::("GetStatic", ()).unwrap(), 3); + + a.restore(snapshot.clone()).unwrap(); + b.restore(snapshot).unwrap(); + assert_eq!(a.call::("GetStatic", ()).unwrap(), 3); + assert_eq!(b.call::("GetStatic", ()).unwrap(), 3); + } + + #[test] + fn accepts_matching_host_functions() { + let mut sbox = make_sandbox_with_add(); + sbox.call::("AddToStatic", 5i32).unwrap(); + let snap = sbox.snapshot().unwrap(); + let mut sbox2 = + MultiUseSandbox::from_snapshot(snap, host_funcs_with_matching_add(), None).unwrap(); + assert_eq!(sbox2.call::("GetStatic", ()).unwrap(), 5); + } + + #[test] + fn rejects_missing_host_function() { + let mut sbox = make_sandbox_with_add(); + let snap = sbox.snapshot().unwrap(); + let err = MultiUseSandbox::from_snapshot(snap, HostFunctions::default(), None) + .expect_err("missing `Add` must be rejected"); + let msg = format!("{}", err); + assert!(msg.contains("Add"), "got: {}", msg); + } + + #[test] + fn rejects_signature_mismatch() { + let mut sbox = make_sandbox_with_add(); + let snap = sbox.snapshot().unwrap(); + let mut hf = HostFunctions::default(); + hf.register_host_function("Add", |a: String, b: String| Ok(format!("{a}{b}"))) + .unwrap(); + let err = MultiUseSandbox::from_snapshot(snap, hf, None) + .expect_err("signature mismatch on `Add` must be rejected"); + let msg = format!("{}", err); + assert!(msg.contains("Add"), "got: {}", msg); + } + + /// Supplied host-function set may be a strict superset of the + /// snapshot's required set. + #[test] + fn accepts_extra_host_functions() { + let mut sbox = make_sandbox_with_add(); + sbox.call::("AddToStatic", 9i32).unwrap(); + let snap = sbox.snapshot().unwrap(); + let mut hf = host_funcs_with_matching_add(); + hf.register_host_function("Mul", |a: i32, b: i32| Ok(a * b)) + .unwrap(); + let mut sbox2 = MultiUseSandbox::from_snapshot(snap, hf, None).unwrap(); + assert_eq!(sbox2.call::("GetStatic", ()).unwrap(), 9); + } + + /// A sandbox built via `from_snapshot` can itself be snapshotted + /// and restored, and its snapshots are restore-compatible with it. + #[test] + fn re_snapshot_after_from_snapshot() { + let mut sbox = make_sandbox(); + sbox.call::("AddToStatic", 4i32).unwrap(); + let snap1 = sbox.snapshot().unwrap(); + + let mut sbox2 = + MultiUseSandbox::from_snapshot(snap1, HostFunctions::default(), None).unwrap(); + sbox2.call::("AddToStatic", 6i32).unwrap(); + let snap2 = sbox2.snapshot().unwrap(); + + sbox2.call::("AddToStatic", 100i32).unwrap(); + assert_eq!(sbox2.call::("GetStatic", ()).unwrap(), 110); + + sbox2.restore(snap2.clone()).unwrap(); + assert_eq!(sbox2.call::("GetStatic", ()).unwrap(), 10); + + let mut sbox3 = + MultiUseSandbox::from_snapshot(snap2, HostFunctions::default(), None).unwrap(); + assert_eq!(sbox3.call::("GetStatic", ()).unwrap(), 10); + } + + /// The host function closure supplied to `from_snapshot` (not the + /// original sandbox's closure) is the one invoked at runtime. + #[test] + fn supplied_host_function_is_callable() { + let path = simple_guest_as_string().unwrap(); + let mut u = UninitializedSandbox::new(GuestBinary::FilePath(path), None).unwrap(); + u.register_host_function("Echo42", || Ok(1i64)).unwrap(); + let mut sbox = u.evolve().unwrap(); + let snap = sbox.snapshot().unwrap(); + + let mut hf = HostFunctions::default(); + hf.register_host_function("Echo42", || Ok(42i64)).unwrap(); + let mut sbox2 = MultiUseSandbox::from_snapshot(snap, hf, None).unwrap(); + + let got: i64 = sbox2 + .call( + "CallGivenParamlessHostFuncThatReturnsI64", + "Echo42".to_string(), + ) + .unwrap(); + assert_eq!(got, 42); + } + + /// Pre-init snapshots record no required host functions, so any + /// `HostFunctions` set is accepted. + #[test] + fn pre_init_snapshot_accepts_arbitrary_host_functions() { + let path = simple_guest_as_string().unwrap(); + let snap = + Snapshot::from_env(GuestBinary::FilePath(path), SandboxConfiguration::default()) + .unwrap(); + let mut hf = HostFunctions::default(); + hf.register_host_function("Unrelated", |a: i32| Ok(a + 1)) + .unwrap(); + let mut sbox = MultiUseSandbox::from_snapshot(Arc::new(snap), hf, None).unwrap(); + assert_eq!(sbox.call::("GetStatic", ()).unwrap(), 0); + } + + /// Snapshots taken from a sandbox built via `from_snapshot` + /// must continue the generation counter of the snapshot they + /// were constructed from, matching `restore`. + #[test] + fn snapshot_generation_propagates() { + let mut sbox = make_sandbox(); + sbox.call::("AddToStatic", 1i32).unwrap(); + let snap1 = sbox.snapshot().unwrap(); + let gen1 = snap1.snapshot_generation(); + sbox.call::("AddToStatic", 1i32).unwrap(); + let snap2 = sbox.snapshot().unwrap(); + let gen2 = snap2.snapshot_generation(); + assert_eq!(gen2, gen1 + 1); + + let mut sbox2 = + MultiUseSandbox::from_snapshot(snap2, HostFunctions::default(), None).unwrap(); + sbox2.call::("AddToStatic", 1i32).unwrap(); + let snap3 = sbox2.snapshot().unwrap(); + assert_eq!(snap3.snapshot_generation(), gen2 + 1); + } + } } diff --git a/src/hyperlight_host/src/sandbox/snapshot/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/mod.rs index e4c7b1133..4954645b5 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/mod.rs @@ -17,6 +17,7 @@ limitations under the License. use std::collections::{BTreeMap, HashMap}; use std::sync::atomic::{AtomicU64, Ordering}; +use hyperlight_common::flatbuffer_wrappers::host_function_details::HostFunctionDetails; use hyperlight_common::layout::{scratch_base_gpa, scratch_base_gva}; use hyperlight_common::vmem; use hyperlight_common::vmem::{ @@ -115,6 +116,13 @@ pub struct Snapshot { /// restored sandbox's guest-visible counter so the guest can tell /// which snapshot it is currently a clone of. snapshot_generation: u64, + + /// Names and signatures of host functions registered on the + /// sandbox at the time this snapshot was taken. Used by + /// [`crate::MultiUseSandbox::from_snapshot`] to reject a + /// `HostFunctions` set that is missing required functions or + /// has mismatched signatures. + host_functions: HostFunctionDetails, } impl core::convert::AsRef for Snapshot { fn as_ref(&self) -> &Self { @@ -423,6 +431,9 @@ impl Snapshot { sregs: None, entrypoint: NextAction::Initialise(load_addr + entrypoint_va - base_va), snapshot_generation: 0, + host_functions: HostFunctionDetails { + host_functions: None, + }, }) } @@ -447,6 +458,7 @@ impl Snapshot { sregs: CommonSpecialRegisters, entrypoint: NextAction, snapshot_generation: u64, + host_functions: HostFunctionDetails, ) -> Result { let mut phys_seen = HashMap::::new(); let scratch_gva = scratch_base_gva(layout.get_scratch_size()); @@ -610,6 +622,7 @@ impl Snapshot { sregs: Some(sregs), entrypoint, snapshot_generation, + host_functions, }) } @@ -663,6 +676,65 @@ impl Snapshot { pub(crate) fn entrypoint(&self) -> NextAction { self.entrypoint } + + /// Validate that `provided` is a superset of the host functions + /// recorded in this snapshot: every function that was registered + /// at snapshot time must also be present in `provided` with a + /// matching signature. Extras in `provided` are allowed. + /// + /// A snapshot with no recorded host functions (e.g. one + /// produced by a test-only constructor) accepts any `provided` + /// set. + pub(crate) fn validate_host_functions(&self, provided: &crate::HostFunctions) -> Result<()> { + let required = match &self.host_functions.host_functions { + Some(v) => v, + None => return Ok(()), + }; + if required.is_empty() { + return Ok(()); + } + + // Build a HostFunctionDetails view of the provided registry + // using the existing `From<&FunctionRegistry>` impl. + let provided_details: HostFunctionDetails = provided.inner().into(); + let provided_funcs = provided_details.host_functions.unwrap_or_default(); + + let mut missing: Vec = Vec::new(); + let mut signature_mismatches: Vec = Vec::new(); + + for req in required { + match provided_funcs + .iter() + .find(|f| f.function_name == req.function_name) + { + None => missing.push(req.function_name.clone()), + Some(found) + if found.parameter_types != req.parameter_types + || found.return_type != req.return_type => + { + signature_mismatches.push(format!( + "{}: snapshot has {:?} -> {:?}, registered {:?} -> {:?}", + req.function_name, + req.parameter_types, + req.return_type, + found.parameter_types, + found.return_type, + )); + } + Some(_) => {} + } + } + + if missing.is_empty() && signature_mismatches.is_empty() { + return Ok(()); + } + + Err(crate::new_error!( + "snapshot host function mismatch: missing={:?}, signature_mismatches={:?}", + missing, + signature_mismatches + )) + } } impl PartialEq for Snapshot { @@ -674,6 +746,7 @@ impl PartialEq for Snapshot { #[cfg(test)] #[cfg(not(feature = "i686-guest"))] mod tests { + use hyperlight_common::flatbuffer_wrappers::host_function_details::HostFunctionDetails; use hyperlight_common::vmem::{self, BasicMapping, Mapping, MappingKind, PAGE_SIZE}; use crate::hypervisor::regs::CommonSpecialRegisters; @@ -747,6 +820,7 @@ mod tests { default_sregs(), super::NextAction::None, 1, + HostFunctionDetails::default(), ) .unwrap(); @@ -764,6 +838,7 @@ mod tests { default_sregs(), super::NextAction::None, 2, + HostFunctionDetails::default(), ) .unwrap(); diff --git a/src/hyperlight_host/src/sandbox/uninitialized.rs b/src/hyperlight_host/src/sandbox/uninitialized.rs index 23c01be28..b65b1e655 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized.rs @@ -22,7 +22,7 @@ use std::sync::{Arc, Mutex}; use tracing::{Span, instrument}; use tracing_core::LevelFilter; -use super::host_funcs::{FunctionRegistry, default_writer_func}; +use super::host_funcs::FunctionRegistry; use super::snapshot::Snapshot; use super::uninitialized_evolve::evolve_impl_multi_use; use crate::func::host_functions::{HostFunction, register_host_function}; @@ -365,9 +365,9 @@ impl UninitializedSandbox { let mem_mgr_wrapper = SandboxMemoryManager::::from_snapshot(snapshot.as_ref())?; - let host_funcs = Arc::new(Mutex::new(FunctionRegistry::default())); + let host_funcs = Arc::new(Mutex::new(FunctionRegistry::with_default_host_print())); - let mut sandbox = Self { + let sandbox = Self { host_funcs, mgr: mem_mgr_wrapper, max_guest_log_level: None, @@ -383,9 +383,6 @@ impl UninitializedSandbox { pending_file_mappings: Vec::new(), }; - // If we were passed a writer for host print register it otherwise use the default. - sandbox.register_print(default_writer_func)?; - crate::debug!("Sandbox created: {:#?}", sandbox); Ok(sandbox)