diff --git a/Justfile b/Justfile index 216dc30d3..aed82725d 100644 --- a/Justfile +++ b/Justfile @@ -140,7 +140,7 @@ like-ci config=default-target hypervisor="kvm": {{ if os() == "linux" { "just test-rust-tracing " + config + " " + if hypervisor == "mshv" { "mshv2" } else if hypervisor == "mshv3" { "mshv3" } else { "kvm" } } else { "" } }} @# Run benchmarks - {{ if config == "release" { "just bench-ci main " + config + " " + if hypervisor == "mshv" { "mshv2" } else if hypervisor == "mshv3" { "mshv3" } else { "kvm" } } else { "" } }} + {{ if config == "release" { "just bench-ci main " + if hypervisor == "mshv" { "mshv2" } else if hypervisor == "mshv3" { "mshv3" } else { "kvm" } } else { "" } }} # runs all tests test target=default-target features="": (test-unit target features) (test-isolated target features) (test-integration "rust" target features) (test-integration "c" target features) (test-seccomp target features) (test-doc target features) diff --git a/flake.nix b/flake.nix index 47cd32fcb..fb6de20dd 100644 --- a/flake.nix +++ b/flake.nix @@ -59,6 +59,11 @@ channel = "stable"; sha256 = "sha256-AJ6LX/Q/Er9kS15bn9iflkUwcgYqRQxiOIL2ToVAXaU="; }; + "1.86" = { + date = "2025-04-03"; + channel = "stable"; + sha256 = "sha256-X/4ZBHO3iW0fOenQ3foEvscgAPJYl2abspaBThDOukI="; + }; }; rust-platform = makeRustPlatform { diff --git a/src/hyperlight_component_util/src/emit.rs b/src/hyperlight_component_util/src/emit.rs index d075f973b..2cd14c9df 100644 --- a/src/hyperlight_component_util/src/emit.rs +++ b/src/hyperlight_component_util/src/emit.rs @@ -196,6 +196,28 @@ impl Mod { } } +/// Unlike [`tv::ResolvedTyvar`], which is mostly concerned with free +/// variables and leaves bound variables alone, this tells us the most +/// information that we have at codegen time for a top level bound +/// variable. +pub enum ResolvedBoundVar<'a> { + Definite { + /// The final variable offset (relative to s.var_offset) that + /// we followed to get to this definite type, used + /// occasionally to name things. + final_bound_var: u32, + /// The actual definite type that this resolved to + ty: Defined<'a>, + }, + Resource { + /// A resource-type index. Currently a resource-type index is + /// the same as the de Bruijn index of the tyvar that + /// introduced the resource type, but is never affected by + /// e.g. s.var_offset. + rtidx: u32, + }, +} + /// A whole grab-bag of useful state to have while emitting Rust #[derive(Debug)] pub struct State<'a, 'b> { @@ -260,6 +282,8 @@ pub struct State<'a, 'b> { /// wasmtime guest emit. When that is refactored to use the host /// guest emit, this can go away. pub is_wasmtime_guest: bool, + /// Are we working on an export or an import of the component type? + pub is_export: bool, } /// Create a State with all of its &mut references pointing to @@ -311,6 +335,7 @@ impl<'a, 'b> State<'a, 'b> { root_component_name: None, is_guest, is_wasmtime_guest, + is_export: false, } } pub fn clone<'c>(&'c mut self) -> State<'c, 'b> { @@ -331,6 +356,7 @@ impl<'a, 'b> State<'a, 'b> { root_component_name: self.root_component_name.clone(), is_guest: self.is_guest, is_wasmtime_guest: self.is_wasmtime_guest, + is_export: self.is_export, } } /// Obtain a reference to the [`Mod`] that we are currently @@ -508,9 +534,17 @@ impl<'a, 'b> State<'a, 'b> { } /// Add an import/export to [`State::origin`], reflecting that we are now /// looking at code underneath it - pub fn push_origin<'c>(&'c mut self, is_export: bool, name: &'b str) -> State<'c, 'b> { + /// + /// origin_was_export differs from s.is_export in that s.is_export + /// keeps track of whether the item overall was imported or exported + /// from the root component (taking into account positivity), whereas + /// origin_was_export just checks if this particular extern_decl was + /// imported or exported from its parent instance (and so e.g. an + /// export of an instance that is imported by the root component has + /// !s.is_export && origin_was_export) + pub fn push_origin<'c>(&'c mut self, origin_was_export: bool, name: &'b str) -> State<'c, 'b> { let mut s = self.clone(); - s.origin.push(if is_export { + s.origin.push(if origin_was_export { ImportExport::Export(name) } else { ImportExport::Import(name) @@ -588,15 +622,24 @@ impl<'a, 'b> State<'a, 'b> { /// up with a definition, in which case, let's get that, or it /// ends up with a resource type, in which case we return the /// resource index - pub fn resolve_tv(&self, n: u32) -> (u32, Option>) { - match &self.bound_vars[self.var_offset + n as usize].bound { + /// + /// Distinct from [`Ctx::resolve_tv`], which is mostly concerned + /// with free variables, because this is concerned entirely with + /// bound variables. + pub fn resolve_bound_var(&self, n: u32) -> ResolvedBoundVar<'b> { + let noff = self.var_offset as u32 + n; + match &self.bound_vars[noff as usize].bound { TypeBound::Eq(Defined::Handleable(Handleable::Var(Tyvar::Bound(nn)))) => { - self.resolve_tv(n + 1 + nn) + self.resolve_bound_var(n + 1 + nn) } - TypeBound::Eq(t) => (n, Some(t.clone())), - TypeBound::SubResource => (n, None), + TypeBound::Eq(t) => ResolvedBoundVar::Definite { + final_bound_var: n, + ty: t.clone(), + }, + TypeBound::SubResource => ResolvedBoundVar::Resource { rtidx: noff }, } } + /// Construct a namespace path referring to the resource trait for /// a resource with the given name pub fn resource_trait_path(&self, r: Ident) -> Vec { diff --git a/src/hyperlight_component_util/src/guest.rs b/src/hyperlight_component_util/src/guest.rs index 864f9cd0f..09e909034 100644 --- a/src/hyperlight_component_util/src/guest.rs +++ b/src/hyperlight_component_util/src/guest.rs @@ -18,8 +18,9 @@ use proc_macro2::TokenStream; use quote::{format_ident, quote}; use crate::emit::{ - FnName, ResourceItemName, State, WitName, kebab_to_exports_name, kebab_to_fn, kebab_to_getter, - kebab_to_imports_name, kebab_to_namespace, kebab_to_type, kebab_to_var, split_wit_name, + FnName, ResolvedBoundVar, ResourceItemName, State, WitName, kebab_to_exports_name, kebab_to_fn, + kebab_to_getter, kebab_to_imports_name, kebab_to_namespace, kebab_to_type, kebab_to_var, + split_wit_name, }; use crate::etypes::{Component, Defined, ExternDecl, ExternDesc, Handleable, Instance, Tyvar}; use crate::hl::{ @@ -98,10 +99,10 @@ fn emit_import_extern_decl<'a, 'b, 'c>( ExternDesc::Type(t) => match t { Defined::Handleable(Handleable::Var(Tyvar::Bound(b))) => { // only resources need something emitted - let (b, None) = s.resolve_tv(*b) else { + let ResolvedBoundVar::Resource { rtidx } = s.resolve_bound_var(*b) else { return quote! {}; }; - let rtid = format_ident!("HostResource{}", s.var_offset + b as usize); + let rtid = format_ident!("HostResource{}", rtidx as usize); let path = s.resource_trait_path(kebab_to_type(ed.kebab_name)); s.root_mod .r#impl(path, format_ident!("Host")) @@ -314,6 +315,8 @@ fn emit_component<'a, 'b, 'c>( s.var_offset = 0; + s.is_export = true; + let exports = ct .instance .unqualified diff --git a/src/hyperlight_component_util/src/hl.rs b/src/hyperlight_component_util/src/hl.rs index 719262f6e..d58eb7aaf 100644 --- a/src/hyperlight_component_util/src/hl.rs +++ b/src/hyperlight_component_util/src/hl.rs @@ -18,8 +18,8 @@ use itertools::Itertools; use proc_macro2::{Ident, TokenStream}; use quote::{format_ident, quote}; -use crate::emit::{State, kebab_to_cons, kebab_to_var}; -use crate::etypes::{self, Defined, Handleable, TypeBound, Tyvar, Value}; +use crate::emit::{ResolvedBoundVar, State, kebab_to_cons, kebab_to_var}; +use crate::etypes::{self, Defined, Handleable, Tyvar, Value}; use crate::rtypes; /// Construct a string that can be used "on the wire" to identify a @@ -151,25 +151,16 @@ pub fn emit_hl_unmarshal_toplevel_value( } } -/// Find the resource index that the given type variable refers to. -/// -/// Precondition: this type variable does refer to a resource type -fn resolve_tyvar_to_resource(s: &mut State, v: u32) -> u32 { - match s.bound_vars[v as usize].bound { - TypeBound::SubResource => v, - TypeBound::Eq(Defined::Handleable(Handleable::Var(Tyvar::Bound(vv)))) => { - resolve_tyvar_to_resource(s, v + vv + 1) - } - _ => panic!("impossible: resource var is not resource"), - } -} /// Find the resource index that the given Handleable refers to. /// /// Precondition: this type variable does refer to a resource type pub fn resolve_handleable_to_resource(s: &mut State, ht: &Handleable) -> u32 { match ht { Handleable::Var(Tyvar::Bound(vi)) => { - resolve_tyvar_to_resource(s, s.var_offset as u32 + *vi) + let ResolvedBoundVar::Resource { rtidx } = s.resolve_bound_var(*vi) else { + panic!("impossible: resource var is not resource"); + }; + rtidx } _ => panic!("impossible handleable in type"), } @@ -338,9 +329,29 @@ pub fn emit_hl_unmarshal_value(s: &mut State, id: Ident, vt: &Value) -> TokenStr log::debug!("resolved ht to r (2) {:?} {:?}", ht, vi); if s.is_guest { let rid = format_ident!("HostResource{}", vi); - quote! { - let i = u32::from_ne_bytes(#id[0..4].try_into().unwrap()); - (::wasmtime::component::Resource::<#rid>::new_borrow(i), 4) + if s.is_wasmtime_guest { + quote! { + let i = u32::from_ne_bytes(#id[0..4].try_into().unwrap()); + (::wasmtime::component::Resource::<#rid>::new_borrow(i), 4) + } + } else { + // TODO: When we add the Drop impl (#810), we need + // to make sure it does not get called here + // + // If we tried to actually return a reference + // here, rustc would get mad about the temporary + // constructed here not living long enough, so + // instead we return the temporary and construct + // the reference elsewhere. It might be a bit more + // principled to have a separate + // HostResourceXXBorrow struct that implements + // AsRef or something in the + // future... + quote! { + let i = u32::from_ne_bytes(#id[0..4].try_into().unwrap()); + + (#rid { rep: i }, 4) + } } } else { let rid = format_ident!("resource{}", vi); @@ -358,7 +369,11 @@ pub fn emit_hl_unmarshal_value(s: &mut State, id: Ident, vt: &Value) -> TokenStr let Some(Tyvar::Bound(n)) = tv else { panic!("impossible tyvar") }; - let (n, Some(Defined::Value(vt))) = s.resolve_tv(*n) else { + let ResolvedBoundVar::Definite { + final_bound_var: n, + ty: Defined::Value(vt), + } = s.resolve_bound_var(*n) + else { panic!("unresolvable tyvar (2)"); }; let vt = vt.clone(); @@ -644,7 +659,9 @@ pub fn emit_hl_marshal_value(s: &mut State, id: Ident, vt: &Value) -> TokenStrea let rid = format_ident!("resource{}", vi); quote! { let i = rts.#rid.len(); - rts.#rid.push_back(::hyperlight_common::resource::ResourceEntry::lend(#id)); + let (lrg, re) = ::hyperlight_common::resource::ResourceEntry::lend(#id); + to_cleanup.push(Box::new(lrg)); + rts.#rid.push_back(re); alloc::vec::Vec::from(u32::to_ne_bytes(i as u32)) } } @@ -653,7 +670,11 @@ pub fn emit_hl_marshal_value(s: &mut State, id: Ident, vt: &Value) -> TokenStrea let Some(Tyvar::Bound(n)) = tv else { panic!("impossible tyvar") }; - let (n, Some(Defined::Value(vt))) = s.resolve_tv(*n) else { + let ResolvedBoundVar::Definite { + final_bound_var: n, + ty: Defined::Value(vt), + } = s.resolve_bound_var(*n) + else { panic!("unresolvable tyvar (2)"); }; let vt = vt.clone(); @@ -668,7 +689,20 @@ pub fn emit_hl_marshal_value(s: &mut State, id: Ident, vt: &Value) -> TokenStrea /// [`crate::rtypes`] module) of the given value type. pub fn emit_hl_unmarshal_param(s: &mut State, id: Ident, pt: &Value) -> TokenStream { let toks = emit_hl_unmarshal_value(s, id, pt); - quote! { { #toks }.0 } + // Slight hack to avoid rust complaints about deserialised + // resource borrow lifetimes. + fn is_borrow(vt: &Value) -> bool { + match vt { + Value::Borrow(_) => true, + Value::Var(_, vt) => is_borrow(vt), + _ => false, + } + } + if s.is_guest && !s.is_wasmtime_guest && is_borrow(pt) { + quote! { &({ #toks }.0) } + } else { + quote! { { #toks }.0 } + } } /// Emit code to unmarshal the result of a function with result type diff --git a/src/hyperlight_component_util/src/host.rs b/src/hyperlight_component_util/src/host.rs index b8a194dd2..0f6bca65b 100644 --- a/src/hyperlight_component_util/src/host.rs +++ b/src/hyperlight_component_util/src/host.rs @@ -56,12 +56,20 @@ fn emit_export_extern_decl<'a, 'b, 'c>( let unmarshal = emit_hl_unmarshal_result(s, ret.clone(), &ft.result); quote! { fn #n(&mut self, #(#param_decls),*) -> #result_decl { + let mut to_cleanup = Vec::>::new(); + let marshalled = { + let mut rts = self.rt.lock().unwrap(); + #[allow(clippy::unused_unit)] + (#(#marshal,)*) + }; let #ret = ::hyperlight_host::sandbox::Callable::call::<::std::vec::Vec::>(&mut self.sb, #hln, - (#(#marshal,)*) + marshalled, ); let ::std::result::Result::Ok(#ret) = #ret else { panic!("bad return from guest {:?}", #ret) }; #[allow(clippy::unused_unit)] + let mut rts = self.rt.lock().unwrap(); + #[allow(clippy::unused_unit)] #unmarshal } } @@ -333,6 +341,8 @@ fn emit_component<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, ct: &'c Com s.root_component_name = Some((ns.clone(), wn.name)); s.cur_trait = Some(export_trait.clone()); s.import_param_var = Some(format_ident!("I")); + s.is_export = true; + let exports = ct .instance .unqualified diff --git a/src/hyperlight_component_util/src/rtypes.rs b/src/hyperlight_component_util/src/rtypes.rs index e9b8ded61..18665db31 100644 --- a/src/hyperlight_component_util/src/rtypes.rs +++ b/src/hyperlight_component_util/src/rtypes.rs @@ -345,7 +345,11 @@ pub fn emit_value(s: &mut State, vt: &Value) -> TokenStream { } } else { let vr = emit_var_ref(s, tv); - quote! { ::hyperlight_common::resource::BorrowedResourceGuard<#vr> } + if s.is_export { + quote! { &#vr } + } else { + quote! { ::hyperlight_common::resource::BorrowedResourceGuard<#vr> } + } } } }, @@ -607,8 +611,11 @@ fn emit_type_alias TokenStream>( /// Emit (via returning) a Rust trait item corresponding to this /// extern decl +/// +/// See note on emit.rs push_origin for the difference between +/// origin_was_export and s.is_export. fn emit_extern_decl<'a, 'b, 'c>( - is_export: bool, + origin_was_export: bool, s: &'c mut State<'a, 'b>, ed: &'c ExternDecl<'b>, ) -> TokenStream { @@ -616,7 +623,7 @@ fn emit_extern_decl<'a, 'b, 'c>( match &ed.desc { ExternDesc::CoreModule(_) => panic!("core module (im/ex)ports are not supported"), ExternDesc::Func(ft) => { - let mut s = s.push_origin(is_export, ed.kebab_name); + let mut s = s.push_origin(origin_was_export, ed.kebab_name); match kebab_to_fn(ed.kebab_name) { FnName::Plain(n) => { let params = ft @@ -681,7 +688,7 @@ fn emit_extern_decl<'a, 'b, 'c>( TokenStream::new() } let edn: &'b str = ed.kebab_name; - let mut s: State<'_, 'b> = s.push_origin(is_export, edn); + let mut s: State<'_, 'b> = s.push_origin(origin_was_export, edn); if let Some((n, bound)) = s.is_var_defn(t) { match bound { TypeBound::Eq(t) => { @@ -708,7 +715,7 @@ fn emit_extern_decl<'a, 'b, 'c>( } } ExternDesc::Instance(it) => { - let mut s = s.push_origin(is_export, ed.kebab_name); + let mut s = s.push_origin(origin_was_export, ed.kebab_name); let wn = split_wit_name(ed.kebab_name); emit_instance(&mut s, wn.clone(), it); @@ -831,8 +838,8 @@ fn emit_component<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, ct: &'c Com s.cur_trait().items.extend(quote! { #(#imports)* }); s.adjust_vars(ct.instance.evars.len() as u32); - s.import_param_var = Some(format_ident!("I")); + s.is_export = true; let export_name = kebab_to_exports_name(wn.name); *s.bound_vars = ct diff --git a/src/hyperlight_host/tests/wit_test.rs b/src/hyperlight_host/tests/wit_test.rs index 8a12e1433..e47faf6da 100644 --- a/src/hyperlight_host/tests/wit_test.rs +++ b/src/hyperlight_host/tests/wit_test.rs @@ -206,12 +206,21 @@ struct TestResource { x: String, last: char, } +impl TestResource { + fn new(x: String, last: char) -> Arc> { + Arc::new(Mutex::new(TestResource { + n_calls: 0, + x, + last, + })) + } +} use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering::Relaxed; -// We only have 1 test that uses this, and it isn't a proptest or -// anything, so it should only run once. If multiple tests using this -// could run in parallel, there would be problems. +// We use some care below in the tests that use HAS_BEEN_DROPPED to +// synchronise on this mutex to avoid them stepping on each other +static SERIALIZE_TEST_RESOURCE_TESTS: Mutex<()> = Mutex::new(()); static HAS_BEEN_DROPPED: AtomicBool = AtomicBool::new(false); impl Drop for TestResource { @@ -225,11 +234,7 @@ impl Drop for TestResource { impl test::wit::host_resource::Testresource for Host { type T = Arc>; fn new(&mut self, x: String, last: char) -> Self::T { - Arc::new(Mutex::new(TestResource { - n_calls: 0, - x, - last, - })) + TestResource::new(x, last) } fn append_char(&mut self, self_: BorrowedResourceGuard<'_, Self::T>, c: char) { let mut self_ = self_.lock().unwrap(); @@ -393,12 +398,31 @@ mod wit_test { sb().roundtrip().roundtrip_no_result(42); } + use std::sync::atomic::Ordering::Relaxed; + + #[test] + fn test_host_resource_uses_locally() { + let guard = crate::SERIALIZE_TEST_RESOURCE_TESTS.lock(); + crate::HAS_BEEN_DROPPED.store(false, Relaxed); + { + sb().test_host_resource().test_uses_locally(); + } + assert!(crate::HAS_BEEN_DROPPED.load(Relaxed)); + drop(guard); + } #[test] - fn test_host_resource() { + fn test_host_resource_passed_in_out() { + let guard = crate::SERIALIZE_TEST_RESOURCE_TESTS.lock(); + crate::HAS_BEEN_DROPPED.store(false, Relaxed); { - sb().test_host_resource().test(); + let mut sb = sb(); + let inst = sb.test_host_resource(); + let r = inst.test_makes(); + inst.test_accepts_borrow(&r); + inst.test_accepts_own(r); + inst.test_returns(); } - use std::sync::atomic::Ordering::Relaxed; assert!(crate::HAS_BEEN_DROPPED.load(Relaxed)); + drop(guard); } } diff --git a/src/tests/rust_guests/witguest/Cargo.lock b/src/tests/rust_guests/witguest/Cargo.lock index 9d018920a..5687337e0 100644 --- a/src/tests/rust_guests/witguest/Cargo.lock +++ b/src/tests/rust_guests/witguest/Cargo.lock @@ -620,4 +620,5 @@ dependencies = [ "hyperlight-component-macro", "hyperlight-guest", "hyperlight-guest-bin", + "spin 0.10.0", ] diff --git a/src/tests/rust_guests/witguest/Cargo.toml b/src/tests/rust_guests/witguest/Cargo.toml index 611b96b29..65762e639 100644 --- a/src/tests/rust_guests/witguest/Cargo.toml +++ b/src/tests/rust_guests/witguest/Cargo.toml @@ -8,6 +8,7 @@ hyperlight-guest = { path = "../../../hyperlight_guest" } hyperlight-guest-bin = { path = "../../../hyperlight_guest_bin" } hyperlight-common = { path = "../../../hyperlight_common", default-features = false } hyperlight-component-macro = { path = "../../../hyperlight_component_macro" } +spin = "0.10.0" [features] default = [] diff --git a/src/tests/rust_guests/witguest/guest.wit b/src/tests/rust_guests/witguest/guest.wit index d8f4c4a5f..9ed164b80 100644 --- a/src/tests/rust_guests/witguest/guest.wit +++ b/src/tests/rust_guests/witguest/guest.wit @@ -80,5 +80,13 @@ interface host-resource { } interface test-host-resource { - test: func() -> bool; + use host-resource.{testresource}; + + test-uses-locally: func() -> bool; + + test-makes: func() -> own; + + test-accepts-borrow: func(x: borrow); + test-accepts-own: func(x: own); + test-returns: func() -> own; } \ No newline at end of file diff --git a/src/tests/rust_guests/witguest/src/main.rs b/src/tests/rust_guests/witguest/src/main.rs index cef36b65d..307d9cca6 100644 --- a/src/tests/rust_guests/witguest/src/main.rs +++ b/src/tests/rust_guests/witguest/src/main.rs @@ -20,12 +20,16 @@ limitations under the License. extern crate alloc; extern crate hyperlight_guest; -mod bindings; use alloc::string::String; +use spin::Mutex; + +mod bindings; use bindings::*; -struct Guest {} +struct Guest { + host_resource: Option<::T>, +} impl test::wit::Roundtrip for Guest { fn roundtrip_bool(&mut self, x: bool) -> bool { @@ -162,11 +166,12 @@ impl test::wit::Roundtrip for Guest { } } -impl test::wit::TestHostResource for Guest { - fn test(&mut self) -> bool { - use test::wit::host_resource::Testresource; +use alloc::string::ToString; + +use test::wit::host_resource::Testresource; +impl test::wit::TestHostResource<::T> for Guest { + fn test_uses_locally(&mut self) -> bool { let mut host = Host {}; - use alloc::string::ToString; let r = ::new(&mut host, "str".to_string(), 'z'); ::append_char(&mut host, &r, 'a'); ::append_char(&mut host, &r, 'b'); @@ -176,6 +181,27 @@ impl test::wit::TestHostResource for Guest { ::return_own(&mut host, r); true } + fn test_makes(&mut self) -> ::T { + let mut host = Host {}; + ::new(&mut host, "str".to_string(), 'z') + } + fn test_accepts_borrow(&mut self, r: &::T) { + let mut host = Host {}; + ::append_char(&mut host, r, 'a'); + } + fn test_accepts_own(&mut self, r: ::T) { + let mut host = Host {}; + // TODO: add test about the old contents of this being + // dropped, when #810 is fixed. + ::append_char(&mut host, &r, 'b'); + self.host_resource = Some(r); + } + fn test_returns(&mut self) -> ::T { + let mut host = Host {}; + let r = self.host_resource.take().unwrap(); + ::append_char(&mut host, &r, 'c'); + r + } } #[allow(refining_impl_trait)] @@ -190,9 +216,13 @@ impl test::wit::TestExports for Guest { } } +static GUEST_STATE: Mutex = Mutex::new(Guest { + host_resource: None, +}); + impl bindings::Guest for Guest { fn with_guest_state R>(f: F) -> R { - let mut g = Guest {}; + let mut g = GUEST_STATE.lock(); f(&mut g) } }