Skip to content

Commit

Permalink
Add support for resources to wit-component (#1084)
Browse files Browse the repository at this point in the history 
* Add support for resources to `wit-component`

This commit fully integrates resource types in the component model with
the `wit-component` crate, implementing features such as:

* A WIT package using `resource` types can now be round-tripped through
  its WebAssembly encoding. This enables use cases such as `wit-bindgen`
  which embed type information in custom sections of core wasm binaries
  produced by native compilers.

* Core modules can be lifted into components and the component can use
  resources. This provides a target for `wit-bindgen` and other code
  generators to use when generating guest code. Resource intrinsics and
  destructors are all available to the core wasm as desired.

* WIT can be inferred from components using resources, where functions
  are represented as `resource`-related functions in WIT.

* The `roundtrip-wit` fuzzer is extended with resources support meaning
  all of the above support will be fuzzed on OSS-Fuzz.

This required a number of refactorings in `wit-component` especially
around how type information was handled. Previous processing was a bit
fast-and-loose because where exactly a type was defined didn't really
matter since everything was nominal. With resource types, however,
definition locations are significant and this required some fixes to
previous processing. One example of this is that
WebAssembly/component-model#208 was discovered through this work and the
fixes required were implemented previously and further handled here in
`wit-component`.

Overall this PR has been almost exclusively fuzz-driven in its
development. I started out with the bare bones of getting simple
components working with resources being imported and exported, then
added fuzzing support to `wit-smith`, then let the fuzzer go wild. Quite
a few issues were discovered which led to all of the refactorings and
processing here in this PR. I definitely won't claim that this is a
simplification at all to `wit-component` by any measure. Rather it's
taking a complicated codebase and making it more complicated. In my
mind though the "saving grace" is that I'm pretty confident in the
testing/fuzzing story here. It's relatively easy to isolate issues and
add test cases for the various things that can crop up and the fuzzer
has quite good coverage of all the various paths through
`wit-component`. All that's to say that this is surely not the "best" or
easiest to understand implementation of resources, but it's intended to
be sufficient for now.

* add basic ABI/bindgen support for resources

These additions are needed for `wit-bindgen` guest binding generation.

Signed-off-by: Joel Dice <[email protected]>

* Update expected fuzz error message

---------

Signed-off-by: Joel Dice <[email protected]>
Co-authored-by: Joel Dice <[email protected]>
  • Loading branch information
@alexcrichton @dicej
alexcrichton and dicej authored Jul 5, 2023
1 parent d4be472 commit 65395c8
Show file tree
Hide file tree
Showing 82 changed files with 2,809 additions and 533 deletions.
6 changes: 6 additions & 0 deletions crates/wasm-encoder/src/component/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,12 @@ impl ComponentTypeSection {
pub fn defined_type(&mut self) -> ComponentDefinedTypeEncoder<'_> {
self.ty().defined_type()
}

/// Defines a new resource type.
pub fn resource(&mut self, rep: ValType, dtor: Option<u32>) -> &mut Self {
self.ty().resource(rep, dtor);
self
}
}

impl Encode for ComponentTypeSection {
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmprinter/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1785,7 +1785,7 @@ impl Printer {
self.print_valtype(rep)?;
self.result.push_str(")");
if let Some(dtor) = dtor {
self.result.push_str("(dtor (func ");
self.result.push_str(" (dtor (func ");
self.print_idx(&states.last().unwrap().core.func_names, dtor)?;
self.result.push_str("))");
}
Expand Down
20 changes: 20 additions & 0 deletions crates/wit-component/src/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,11 @@ impl ComponentBuilder {
(inc(&mut self.types), self.types().function())
}

pub fn resource(&mut self, rep: ValType, dtor: Option<u32>) -> u32 {
self.types().resource(rep, dtor);
inc(&mut self.types)
}

pub fn alias_type_export(&mut self, instance: u32, name: &str) -> u32 {
self.aliases().alias(Alias::InstanceExport {
instance,
Expand Down Expand Up @@ -200,6 +205,21 @@ impl ComponentBuilder {
pub fn add_producers(&mut self, producers: &Producers) {
self.producers.merge(producers)
}

pub fn resource_drop(&mut self, ty: ComponentValType) -> u32 {
self.canonical_functions().resource_drop(ty);
inc(&mut self.core_funcs)
}

pub fn resource_new(&mut self, ty: u32) -> u32 {
self.canonical_functions().resource_new(ty);
inc(&mut self.core_funcs)
}

pub fn resource_rep(&mut self, ty: u32) -> u32 {
self.canonical_functions().resource_rep(ty);
inc(&mut self.core_funcs)
}
}

// Helper macro to generate methods on `ComponentBuilder` to get specific
Expand Down
124 changes: 86 additions & 38 deletions crates/wit-component/src/decoding.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ impl<'a> ComponentInfo<'a> {
foreign_packages: Default::default(),
iface_to_package_index: Default::default(),
named_interfaces: Default::default(),
resources: Default::default(),
};

let mut pkg = None;
Expand Down Expand Up @@ -150,6 +151,7 @@ impl<'a> ComponentInfo<'a> {
foreign_packages: Default::default(),
iface_to_package_index: Default::default(),
named_interfaces: Default::default(),
resources: Default::default(),
};
let mut package = Package {
// Similar to `world_name` above this is arbitrarily chosen as it's
Expand Down Expand Up @@ -237,6 +239,15 @@ struct WitPackageDecoder<'a> {
iface_to_package_index: HashMap<InterfaceId, usize>,
named_interfaces: HashMap<String, InterfaceId>,

/// A map which tracks named resources to what their corresponding `TypeId`
/// is. This first layer of key in this map is the owner scope of a
/// resource, more-or-less the `world` or `interface` that it's defined
/// within. The second layer of this map is keyed by name of the resource
/// and points to the actual ID of the resource.
///
/// This map is populated in `register_type_export`.
resources: HashMap<TypeOwner, HashMap<String, TypeId>>,

/// A map from a type id to what it's been translated to.
type_map: HashMap<types::TypeId, TypeId>,
}
Expand Down Expand Up @@ -316,6 +327,7 @@ impl WitPackageDecoder<'_> {
.types
.component_entity_type_of_import(import.name.as_str())
.unwrap();
let owner = TypeOwner::World(world);
let (name, item) = match ty {
types::ComponentEntityType::Instance(i) => {
let ty = match self.info.types.type_from_id(i) {
Expand All @@ -337,7 +349,7 @@ impl WitPackageDecoder<'_> {
_ => unreachable!(),
};
let func = self
.convert_function(name, ty)
.convert_function(name, ty, owner)
.with_context(|| format!("failed to decode function from import `{name}`"))?;
(WorldKey::Name(name.to_string()), WorldItem::Function(func))
}
Expand All @@ -346,7 +358,7 @@ impl WitPackageDecoder<'_> {
created,
} => {
let id = self
.register_type_export(name, TypeOwner::World(world), referenced, created)
.register_type_export(name, owner, referenced, created)
.with_context(|| format!("failed to decode type from export `{name}`"))?;
(WorldKey::Name(name.to_string()), WorldItem::Type(id))
}
Expand All @@ -373,7 +385,7 @@ impl WitPackageDecoder<'_> {
_ => unreachable!(),
};
let func = self
.convert_function(name, ty)
.convert_function(name, ty, TypeOwner::World(world))
.with_context(|| format!("failed to decode function from export `{name}`"))?;

(WorldKey::Name(name.to_string()), WorldItem::Function(func))
Expand Down Expand Up @@ -418,18 +430,13 @@ impl WitPackageDecoder<'_> {
Some(id) => (true, *id),
None => (false, self.extract_dep_interface(name)?),
};

let owner = TypeOwner::Interface(interface);
for (name, ty) in ty.exports.iter() {
match *ty {
types::ComponentEntityType::Type {
referenced,
created,
} => {
let def = match self.info.types.type_from_id(referenced) {
Some(types::Type::Defined(ty)) => ty,
_ => unreachable!(),
};

match self.resolve.interfaces[interface]
.types
.get(name.as_str())
Expand All @@ -453,7 +460,11 @@ impl WitPackageDecoder<'_> {
// is not strictly necessary but assists with
// roundtripping assertions during fuzzing.
Some(id) => {
self.register_defined(id, def)?;
match self.info.types.type_from_id(referenced) {
Some(types::Type::Defined(ty)) => self.register_defined(id, ty)?,
Some(types::Type::Resource(_)) => {}
_ => unreachable!(),
}
let prev = self.type_map.insert(created, id);
assert!(prev.is_none());
}
Expand All @@ -477,7 +488,7 @@ impl WitPackageDecoder<'_> {
}
let id = self.register_type_export(
name.as_str(),
TypeOwner::Interface(interface),
owner,
referenced,
created,
)?;
Expand Down Expand Up @@ -508,7 +519,7 @@ impl WitPackageDecoder<'_> {
if is_local {
bail!("instance function export `{name}` not defined in interface");
}
let func = self.convert_function(name.as_str(), def)?;
let func = self.convert_function(name.as_str(), def, owner)?;
let prev = self.resolve.interfaces[interface]
.functions
.insert(name.to_string(), func);
Expand Down Expand Up @@ -639,19 +650,15 @@ impl WitPackageDecoder<'_> {
package: None,
};

let owner = TypeOwner::Interface(self.resolve.interfaces.next_id());
for (name, ty) in ty.exports.iter() {
match *ty {
types::ComponentEntityType::Type {
referenced,
created,
} => {
let ty = self
.register_type_export(
name.as_str(),
TypeOwner::Interface(self.resolve.interfaces.next_id()),
referenced,
created,
)
.register_type_export(name.as_str(), owner, referenced, created)
.with_context(|| format!("failed to register type export '{name}'"))?;
let prev = interface.types.insert(name.to_string(), ty);
assert!(prev.is_none());
Expand All @@ -663,7 +670,7 @@ impl WitPackageDecoder<'_> {
_ => unreachable!(),
};
let func = self
.convert_function(name.as_str(), ty)
.convert_function(name.as_str(), ty, owner)
.with_context(|| format!("failed to convert function '{name}'"))?;
let prev = interface.functions.insert(name.to_string(), func);
assert!(prev.is_none());
Expand Down Expand Up @@ -712,10 +719,6 @@ impl WitPackageDecoder<'_> {
referenced: types::TypeId,
created: types::TypeId,
) -> Result<TypeId> {
let ty = match self.info.types.type_from_id(referenced) {
Some(types::Type::Defined(ty)) => ty,
_ => unreachable!(),
};
let kind = match self.find_alias(referenced) {
// If this `TypeId` points to a type which has
// previously been defined, meaning we're aliasing a
Expand All @@ -724,9 +727,13 @@ impl WitPackageDecoder<'_> {

// ... or this `TypeId`'s source definition has never
// been seen before, so declare the full type.
None => self
.convert_defined(ty)
.context("failed to convert unaliased type")?,
None => match self.info.types.type_from_id(referenced) {
Some(types::Type::Defined(ty)) => self
.convert_defined(ty)
.context("failed to convert unaliased type")?,
Some(types::Type::Resource(_)) => TypeDefKind::Resource,
_ => unreachable!(),
},
};
let ty = self.resolve.types.alloc(TypeDef {
name: Some(name.to_string()),
Expand All @@ -735,6 +742,18 @@ impl WitPackageDecoder<'_> {
owner,
});

// If this is a resource then doubly-register it in `self.resources` so
// the ID allocated here can be looked up via name later on during
// `convert_function`.
if let TypeDefKind::Resource = self.resolve.types[ty].kind {
let prev = self
.resources
.entry(owner)
.or_insert(HashMap::new())
.insert(name.to_string(), ty);
assert!(prev.is_none());
}

let prev = self.type_map.insert(created, ty);
assert!(prev.is_none());
Ok(ty)
Expand All @@ -759,6 +778,7 @@ impl WitPackageDecoder<'_> {
package: None,
};

let owner = TypeOwner::World(self.resolve.worlds.next_id());
for (name, ty) in ty.imports.iter() {
let (name, item) = match ty {
types::ComponentEntityType::Instance(idx) => {
Expand All @@ -785,20 +805,16 @@ impl WitPackageDecoder<'_> {
created,
referenced,
} => {
let ty = self.register_type_export(
name.as_str(),
TypeOwner::World(self.resolve.worlds.next_id()),
*referenced,
*created,
)?;
let ty =
self.register_type_export(name.as_str(), owner, *referenced, *created)?;
(WorldKey::Name(name.to_string()), WorldItem::Type(ty))
}
types::ComponentEntityType::Func(idx) => {
let ty = match self.info.types.type_from_id(*idx) {
Some(types::Type::ComponentFunc(ty)) => ty,
_ => unreachable!(),
};
let func = self.convert_function(name.as_str(), ty)?;
let func = self.convert_function(name.as_str(), ty, owner)?;
(WorldKey::Name(name.to_string()), WorldItem::Function(func))
}
_ => bail!("component import `{name}` is not an instance, func, or type"),
Expand Down Expand Up @@ -833,7 +849,7 @@ impl WitPackageDecoder<'_> {
Some(types::Type::ComponentFunc(ty)) => ty,
_ => unreachable!(),
};
let func = self.convert_function(name.as_str(), ty)?;
let func = self.convert_function(name.as_str(), ty, owner)?;
(WorldKey::Name(name.to_string()), WorldItem::Function(func))
}

Expand All @@ -847,7 +863,13 @@ impl WitPackageDecoder<'_> {
Ok(id)
}

fn convert_function(&mut self, name: &str, ty: &types::ComponentFuncType) -> Result<Function> {
fn convert_function(
&mut self,
name: &str,
ty: &types::ComponentFuncType,
owner: TypeOwner,
) -> Result<Function> {
let name = KebabName::new(ComponentExternName::Kebab(name), 0).unwrap();
let params = ty
.params
.iter()
Expand Down Expand Up @@ -875,7 +897,26 @@ impl WitPackageDecoder<'_> {
};
Ok(Function {
docs: Default::default(),
kind: FunctionKind::Freestanding,
kind: match name.kind() {
KebabNameKind::Normal(_) => FunctionKind::Freestanding,
KebabNameKind::Constructor(resource) => {
FunctionKind::Constructor(self.resources[&owner][resource.as_str()])
}
KebabNameKind::Method { resource, .. } => {
FunctionKind::Method(self.resources[&owner][resource.as_str()])
}
KebabNameKind::Static { resource, .. } => {
FunctionKind::Static(self.resources[&owner][resource.as_str()])
}

// Functions shouldn't have ID-based names at this time.
KebabNameKind::Id { .. } => unreachable!(),
},

// Note that this name includes "name mangling" such as
// `[method]foo.bar` which is intentional. The `FunctionKind`
// discriminant calculated above indicates how to interpret this
// name.
name: name.to_string(),
params,
results,
Expand Down Expand Up @@ -1049,8 +1090,15 @@ impl WitPackageDecoder<'_> {
Ok(TypeDefKind::Enum(Enum { cases }))
}

types::ComponentDefinedType::Own(_) => unimplemented!(),
types::ComponentDefinedType::Borrow(_) => unimplemented!(),
types::ComponentDefinedType::Own(id) => {
let id = self.type_map[id];
Ok(TypeDefKind::Handle(Handle::Own(id)))
}

types::ComponentDefinedType::Borrow(id) => {
let id = self.type_map[id];
Ok(TypeDefKind::Handle(Handle::Borrow(id)))
}
}
}

Expand Down
Loading

0 comments on commit 65395c8

Please sign in to comment.