Fix some panics compiling components and resources (#11798)

In working on bytecodealliance/wasm-tools#2335 I found that there's a
few test cases in wasm-tools which Wasmtime was panicking to compile.
The issues were all related to resource types and how information wasn't
registered ahead of time before it was translated from wasmparser's
representation to Wasmtime's representation. The high-level cause for
this had to do with how component and instance types are handled, as
opposed to concrete components or instances themselves. This was
effectively a hole in Wasmtime's translation process for components
which has never been filled out since the original implementation of
resources. The reason that this never came up before is:

* Most components don't currently import or export a component itself.
* Most components don't currently import or export component or instance
  types (as opposed to values).

One of these was required to trigger this issue. The solution
implemented in this commit is to plumb the concept of an "abstract
resource" which is part of a type but not actually ever used at runtime
except for type equality during type reflection. This is expected to
have little-to-no impact on real-world components given that these
situations are rarely occurring.

Author: alexcrichton

crates/cranelift/src/compiler/component.rs

@@ -1058,7 +1058,7 @@ impl<'a> TrampolineCompiler<'a> {
         let should_run_destructor =
             self.raise_if_negative_one(self.builder.func.dfg.inst_results(call)[0]);
 
-        let resource_ty = self.types[resource].ty;
+        let resource_ty = self.types[resource].unwrap_concrete_ty();
         let resource_def = self
             .component
             .defined_resource_index(resource_ty)
@@ -1136,7 +1136,7 @@ impl<'a> TrampolineCompiler<'a> {
         // the same component instance that defined the resource as the
         // component is calling itself.
         if let Some(def) = resource_def {
-            if self.types[resource].instance != def.instance {
+            if self.types[resource].unwrap_concrete_instance() != def.instance {
                 let flags = self.builder.ins().load(
                     ir::types::I32,
                     trusted,
@@ -1156,7 +1156,7 @@ impl<'a> TrampolineCompiler<'a> {
         if has_destructor {
             let rep = self.builder.ins().ushr_imm(should_run_destructor, 1);
             let rep = self.builder.ins().ireduce(ir::types::I32, rep);
-            let index = self.types[resource].ty;
+            let index = self.types[resource].unwrap_concrete_ty();
             // NB: despite the vmcontext storing nullable funcrefs for function
             // pointers we know this is statically never null due to the
             // `has_destructor` check above.

crates/environ/src/component/translate/inline.rs

@@ -1854,7 +1854,7 @@ impl<'a> ComponentItemDef<'a> {
         // Once `path` has been iterated over it must be the case that the final
         // item is a resource type, in which case a lookup can be performed.
         match cur {
-            ComponentItemDef::Type(TypeDef::Resource(idx)) => types[idx].ty,
+            ComponentItemDef::Type(TypeDef::Resource(idx)) => types[idx].unwrap_concrete_ty(),
             _ => unreachable!(),
         }
     }

crates/environ/src/component/types.rs

@@ -243,6 +243,14 @@ indices! {
 
     /// An index into `Component::options` at the end of compilation.
     pub struct OptionsIndex(u32);
+
+    /// An index that doesn't actually index into a list but instead represents
+    /// a unique counter.
+    ///
+    /// This is used for "abstract" resources which aren't actually instantiated
+    /// in the component model. For example this represents a resource in a
+    /// component or instance type, but not an actual concrete instance.
+    pub struct AbstractResourceIndex(u32);
 }
 
 // Reexport for convenience some core-wasm indices which are also used in the
@@ -1106,15 +1114,56 @@ pub struct TypeErrorContextTable {
 
 /// Metadata about a resource table added to a component.
 #[derive(Serialize, Deserialize, Clone, Hash, Eq, PartialEq, Debug)]
-pub struct TypeResourceTable {
-    /// The original resource that this table contains.
+pub enum TypeResourceTable {
+    /// This resource is for an actual concrete resource which has runtime state
+    /// associated with it.
     ///
-    /// This is used when destroying resources within this table since this
-    /// original definition will know how to execute destructors.
-    pub ty: ResourceIndex,
+    /// This is used for any resource which might actually enter a component.
+    /// For example when a resource is either imported or defined in a component
+    /// it'll get this case.
+    Concrete {
+        /// The original resource that this table contains.
+        ///
+        /// This is used when destroying resources within this table since this
+        /// original definition will know how to execute destructors.
+        ty: ResourceIndex,
+
+        /// The component instance that contains this resource table.
+        instance: RuntimeComponentInstanceIndex,
+    },
+
+    /// This table does not actually exist at runtime but instead represents
+    /// type information for an uninstantiable resource. This tracks, for
+    /// example, resources in component and instance types.
+    Abstract(AbstractResourceIndex),
+}
 
-    /// The component instance that contains this resource table.
-    pub instance: RuntimeComponentInstanceIndex,
+impl TypeResourceTable {
+    /// Asserts that this is `TypeResourceTable::Concrete` and returns the `ty`
+    /// field.
+    ///
+    /// # Panics
+    ///
+    /// Panics if this is `TypeResourceTable::Abstract`.
+    pub fn unwrap_concrete_ty(&self) -> ResourceIndex {
+        match self {
+            TypeResourceTable::Concrete { ty, .. } => *ty,
+            TypeResourceTable::Abstract(_) => panic!("not a concrete resource table"),
+        }
+    }
+
+    /// Asserts that this is `TypeResourceTable::Concrete` and returns the
+    /// `instance` field.
+    ///
+    /// # Panics
+    ///
+    /// Panics if this is `TypeResourceTable::Abstract`.
+    pub fn unwrap_concrete_instance(&self) -> RuntimeComponentInstanceIndex {
+        match self {
+            TypeResourceTable::Concrete { instance, .. } => *instance,
+            TypeResourceTable::Abstract(_) => panic!("not a concrete resource table"),
+        }
+    }
 }
 
 /// Shape of a "list" interface type.

crates/environ/src/component/types_builder.rs

@@ -60,6 +60,12 @@ pub struct ComponentTypesBuilder {
     type_info: TypeInformationCache,
 
     resources: ResourcesBuilder,
+
+    // Total number of abstract resources allocated.
+    //
+    // These are only allocated within component and instance types when
+    // translating them.
+    abstract_resources: u32,
 }
 
 impl<T> Index<T> for ComponentTypesBuilder
@@ -111,6 +117,7 @@ impl ComponentTypesBuilder {
             component_types: ComponentTypes::default(),
             type_info: TypeInformationCache::default(),
             resources: ResourcesBuilder::default(),
+            abstract_resources: 0,
         }
     }
 
@@ -313,12 +320,14 @@ impl ComponentTypesBuilder {
         let ty = &types[id];
         let mut result = TypeComponent::default();
         for (name, ty) in ty.imports.iter() {
+            self.register_abstract_component_entity_type(types, *ty);
             result.imports.insert(
                 name.clone(),
                 self.convert_component_entity_type(types, *ty)?,
             );
         }
         for (name, ty) in ty.exports.iter() {
+            self.register_abstract_component_entity_type(types, *ty);
             result.exports.insert(
                 name.clone(),
                 self.convert_component_entity_type(types, *ty)?,
@@ -336,6 +345,7 @@ impl ComponentTypesBuilder {
         let ty = &types[id];
         let mut result = TypeComponentInstance::default();
         for (name, ty) in ty.exports.iter() {
+            self.register_abstract_component_entity_type(types, *ty);
             result.exports.insert(
                 name.clone(),
                 self.convert_component_entity_type(types, *ty)?,
@@ -344,6 +354,23 @@ impl ComponentTypesBuilder {
         Ok(self.component_types.component_instances.push(result))
     }
 
+    fn register_abstract_component_entity_type(
+        &mut self,
+        types: TypesRef<'_>,
+        ty: ComponentEntityType,
+    ) {
+        let mut path = Vec::new();
+        self.resources.register_abstract_component_entity_type(
+            &types,
+            ty,
+            &mut path,
+            &mut |_path| {
+                self.abstract_resources += 1;
+                AbstractResourceIndex::from_u32(self.abstract_resources)
+            },
+        );
+    }
+
     pub(crate) fn convert_module(
         &mut self,
         types: TypesRef<'_>,

crates/environ/src/component/types_builder/resources.rs

@@ -68,8 +68,8 @@
 //! methods below.
 
 use crate::component::{
-    ComponentTypes, ResourceIndex, RuntimeComponentInstanceIndex, TypeResourceTable,
-    TypeResourceTableIndex,
+    AbstractResourceIndex, ComponentTypes, ResourceIndex, RuntimeComponentInstanceIndex,
+    TypeResourceTable, TypeResourceTableIndex,
 };
 use crate::prelude::*;
 use std::collections::HashMap;
@@ -122,13 +122,27 @@ pub struct ResourcesBuilder {
     /// phase. This is used to record the actual underlying type of a resource
     /// and where it originally comes from. When a resource is later referred to
     /// then a table is injected to be referred to.
-    resource_id_to_resource_index: HashMap<ResourceId, ResourceIndex>,
+    resource_id_to_resource_index: HashMap<ResourceId, ResourceIndexKind>,
 
     /// The current instance index that's being visited. This is updated as
     /// inliner frames are processed and components are instantiated.
     current_instance: Option<RuntimeComponentInstanceIndex>,
 }
 
+/// Resources are considered either "concrete" or "abstract" depending on where
+/// the resource type is defined.
+///
+/// Resources defined in a component, or imported into a component, are
+/// considered "concrete" and may actually be instantiated/have a value at
+/// runtime. Resources defined in instance types or component types are
+/// considered "abstract" meaning that they won't ever actually exist at runtime
+/// so only an integer identifier is tracked for them.
+#[derive(Clone, Copy, Debug)]
+enum ResourceIndexKind {
+    Concrete(ResourceIndex),
+    Abstract(AbstractResourceIndex),
+}
+
 impl ResourcesBuilder {
     /// Converts the `id` provided into a `TypeResourceTableIndex`.
     ///
@@ -152,9 +166,14 @@ impl ResourcesBuilder {
             .resource_id_to_table_index
             .entry(id)
             .or_insert_with(|| {
-                let ty = self.resource_id_to_resource_index[&id];
-                let instance = self.current_instance.expect("current instance not set");
-                types.push_resource_table(TypeResourceTable { ty, instance })
+                let table_ty = match self.resource_id_to_resource_index[&id] {
+                    ResourceIndexKind::Concrete(ty) => {
+                        let instance = self.current_instance.expect("current instance not set");
+                        TypeResourceTable::Concrete { ty, instance }
+                    }
+                    ResourceIndexKind::Abstract(i) => TypeResourceTable::Abstract(i),
+                };
+                types.push_resource_table(table_ty)
             })
     }
 
@@ -179,6 +198,32 @@ impl ResourcesBuilder {
         ty: ComponentEntityType,
         path: &mut Vec<&'a str>,
         register: &mut dyn FnMut(&[&'a str]) -> ResourceIndex,
+    ) {
+        self.register_component_entity_type_(types, ty, path, &mut |path| {
+            ResourceIndexKind::Concrete(register(path))
+        })
+    }
+
+    /// Same as [`Self::register_component_entity_type`], but for when an
+    /// [`AbstractResourceIndex`] is created for all resources.
+    pub fn register_abstract_component_entity_type<'a>(
+        &mut self,
+        types: &'a TypesRef<'_>,
+        ty: ComponentEntityType,
+        path: &mut Vec<&'a str>,
+        register: &mut dyn FnMut(&[&'a str]) -> AbstractResourceIndex,
+    ) {
+        self.register_component_entity_type_(types, ty, path, &mut |path| {
+            ResourceIndexKind::Abstract(register(path))
+        })
+    }
+
+    fn register_component_entity_type_<'a>(
+        &mut self,
+        types: &'a TypesRef<'_>,
+        ty: ComponentEntityType,
+        path: &mut Vec<&'a str>,
+        register: &mut dyn FnMut(&[&'a str]) -> ResourceIndexKind,
     ) {
         match ty {
             // If `ty` is itself a type, and that's a resource type, then this
@@ -202,7 +247,7 @@ impl ResourcesBuilder {
                 let ty = &types[id];
                 for (name, ty) in ty.exports.iter() {
                     path.push(name);
-                    self.register_component_entity_type(types, *ty, path, register);
+                    self.register_component_entity_type_(types, *ty, path, register);
                     path.pop();
                 }
             }
@@ -216,13 +261,14 @@ impl ResourcesBuilder {
             | ComponentEntityType::Value(_) => {}
         }
     }
-
     /// Declares that the wasmparser `id`, which must point to a resource, is
     /// defined by the `ty` provided.
     ///
     /// This is used when a local resource is defined within a component for example.
     pub fn register_resource(&mut self, id: ResourceId, ty: ResourceIndex) {
-        let prev = self.resource_id_to_resource_index.insert(id, ty);
+        let prev = self
+            .resource_id_to_resource_index
+            .insert(id, ResourceIndexKind::Concrete(ty));
         assert!(prev.is_none());
     }
 

crates/wasmtime/src/runtime/component/matching.rs

@@ -10,7 +10,8 @@ use alloc::sync::Arc;
 use wasmtime_environ::PrimaryMap;
 use wasmtime_environ::component::{
     ComponentTypes, NameMap, ResourceIndex, TypeComponentInstance, TypeDef, TypeFuncIndex,
-    TypeFutureTableIndex, TypeModule, TypeResourceTableIndex, TypeStreamTableIndex,
+    TypeFutureTableIndex, TypeModule, TypeResourceTable, TypeResourceTableIndex,
+    TypeStreamTableIndex,
 };
 
 pub struct TypeChecker<'a> {
@@ -59,7 +60,7 @@ impl TypeChecker<'_> {
             },
 
             TypeDef::Resource(i) => {
-                let i = self.types[i].ty;
+                let i = self.types[i].unwrap_concrete_ty();
                 let actual = match actual {
                     Some(Definition::Resource(actual, _dtor)) => actual,
 
@@ -193,11 +194,14 @@ impl<'a> InstanceType<'a> {
     }
 
     pub fn resource_type(&self, index: TypeResourceTableIndex) -> ResourceType {
-        let index = self.types[index].ty;
-        self.resources
-            .get(index)
-            .copied()
-            .unwrap_or_else(|| ResourceType::uninstantiated(&self.types, index))
+        match self.types[index] {
+            TypeResourceTable::Concrete { ty, .. } => self
+                .resources
+                .get(ty)
+                .copied()
+                .unwrap_or_else(|| ResourceType::uninstantiated(&self.types, ty)),
+            TypeResourceTable::Abstract(ty) => ResourceType::abstract_(&self.types, ty),
+        }
     }
 
     pub fn future_type(&self, index: TypeFutureTableIndex) -> FutureType {

crates/wasmtime/src/runtime/component/resources.rs

@@ -15,8 +15,8 @@ use core::mem::MaybeUninit;
 use core::ptr::NonNull;
 use core::sync::atomic::{AtomicU32, Ordering::Relaxed};
 use wasmtime_environ::component::{
-    CanonicalAbiInfo, ComponentTypes, DefinedResourceIndex, InterfaceType, ResourceIndex,
-    TypeResourceTableIndex,
+    AbstractResourceIndex, CanonicalAbiInfo, ComponentTypes, DefinedResourceIndex, InterfaceType,
+    ResourceIndex, TypeResourceTableIndex,
 };
 
 /// Representation of a resource type in the component model.
@@ -80,6 +80,15 @@ impl ResourceType {
             },
         }
     }
+
+    pub(crate) fn abstract_(types: &ComponentTypes, index: AbstractResourceIndex) -> ResourceType {
+        ResourceType {
+            kind: ResourceTypeKind::Abstract {
+                component: types as *const _ as usize,
+                index,
+            },
+        }
+    }
 }
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
@@ -101,6 +110,13 @@ enum ResourceTypeKind {
         component: usize,
         index: ResourceIndex,
     },
+    /// The type of this resource is considered "abstract" meaning that it
+    /// doesn't actually correspond to anything at runtime but instead it just
+    /// needs to be kept distinct from everything but itself.
+    Abstract {
+        component: usize,
+        index: AbstractResourceIndex,
+    },
 }
 
 /// A host-defined resource in the component model.