From e7da16a8875fa2d443ad77babc157a644cf2281e Mon Sep 17 00:00:00 2001 From: Peter Huene Date: Wed, 29 Nov 2023 16:38:58 -0800 Subject: [PATCH 1/2] Fix merge of disjoint interfaces. Currently, a bug exists in merging of interfaces such that if one interface exports a function that references an exported (i.e. "named") type and another interface exports a different function that references the same exported type, the merged interface would have references to function types with types that are not present in the "encoded type index" map. This results in the encoder re-encoding the same type and using that newly encoded type index instead of the correct type index to the named type. Ultimately this resulted in a validation error of the encoded output where the interface wasn't valid for import because of the function not using what should be a named value type. The fix is for the merged interface to maintain a mapping between the new type and the old types so that the "encoded type index" map is updated accordingly. --- crates/wac-parser/src/resolution/ast.rs | 10 +++- crates/wac-parser/src/resolution/encoding.rs | 12 ++++- crates/wac-parser/src/resolution/package.rs | 1 + crates/wac-parser/src/resolution/types.rs | 9 ++++ .../tests/encoding/merged-functions.wac | 9 ++++ .../encoding/merged-functions.wat.result | 52 +++++++++++++++++++ .../encoding/merged-functions/foo/bar.wat | 7 +++ .../encoding/merged-functions/foo/baz.wat | 7 +++ 8 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 crates/wac-parser/tests/encoding/merged-functions.wac create mode 100644 crates/wac-parser/tests/encoding/merged-functions.wat.result create mode 100644 crates/wac-parser/tests/encoding/merged-functions/foo/bar.wat create mode 100644 crates/wac-parser/tests/encoding/merged-functions/foo/baz.wat diff --git a/crates/wac-parser/src/resolution/ast.rs b/crates/wac-parser/src/resolution/ast.rs index a097412..2c717b6 100644 --- a/crates/wac-parser/src/resolution/ast.rs +++ b/crates/wac-parser/src/resolution/ast.rs @@ -445,6 +445,7 @@ impl<'a> AstResolver<'a> { let mut ty = Interface { id: None, + remapped_types: Default::default(), uses: Default::default(), exports: Default::default(), }; @@ -483,6 +484,7 @@ impl<'a> AstResolver<'a> { let mut ty = Interface { id: Some(self.id(decl.id.string)), + remapped_types: Default::default(), uses: Default::default(), exports: Default::default(), }; @@ -1898,7 +1900,13 @@ impl<'a> AstResolver<'a> { .with_context(|| format!("mismatched type for export `{name}`")), ) { (Ok(_), Ok(_)) => { - // The two are compatible, so do nothing + // The two are compatible, check for remapped type + match (*target_kind, *source_kind) { + (ItemKind::Type(new), ItemKind::Type(old)) if new != old => { + target.remapped_types.entry(new).or_default().insert(old); + } + _ => {} + } } (Err(e), _) | (_, Err(e)) => { // Neither is a subtype of the other, so error diff --git a/crates/wac-parser/src/resolution/encoding.rs b/crates/wac-parser/src/resolution/encoding.rs index a72b685..78196e8 100644 --- a/crates/wac-parser/src/resolution/encoding.rs +++ b/crates/wac-parser/src/resolution/encoding.rs @@ -620,7 +620,17 @@ impl<'a> TypeEncoder<'a> { // Otherwise, export all exports for (name, kind) in &interface.exports { match kind { - ItemKind::Type(_) | ItemKind::Resource(_) => { + ItemKind::Type(ty) => { + let index = self.export(state, name, *kind)?; + + // Map the encoded type index to any remapped types. + if let Some(prev) = interface.remapped_types.get(ty) { + for ty in prev { + state.current.type_indexes.insert(*ty, index); + } + } + } + ItemKind::Resource(_) => { self.export(state, name, *kind)?; } _ => { diff --git a/crates/wac-parser/src/resolution/package.rs b/crates/wac-parser/src/resolution/package.rs index 9d8ff93..8c42441 100644 --- a/crates/wac-parser/src/resolution/package.rs +++ b/crates/wac-parser/src/resolution/package.rs @@ -364,6 +364,7 @@ impl<'a> TypeConverter<'a> { let instance_ty = &types[id]; let id = self.definitions.interfaces.alloc(Interface { id: name.and_then(|n| n.contains(':').then(|| n.to_owned())), + remapped_types: Default::default(), uses: Default::default(), exports: IndexMap::with_capacity(instance_ty.exports.len()), }); diff --git a/crates/wac-parser/src/resolution/types.rs b/crates/wac-parser/src/resolution/types.rs index bcfbb52..84bb4c4 100644 --- a/crates/wac-parser/src/resolution/types.rs +++ b/crates/wac-parser/src/resolution/types.rs @@ -387,6 +387,15 @@ pub struct Interface { /// /// This may be `None` for inline interfaces. pub id: Option, + /// Represents a remapping of types that may occur when an interface is merged. + /// + /// The map is from the type present in this interface to a set of types + /// originating from the merged interfaces. + /// + /// Encoding uses this map to populate the encoded type index map for the + /// original types. + #[serde(skip_serializing_if = "IndexMap::is_empty")] + pub remapped_types: IndexMap>, /// A map from used interface to set of used type export indexes. #[serde(serialize_with = "serialize_id_key_map")] pub uses: IndexMap>, diff --git a/crates/wac-parser/tests/encoding/merged-functions.wac b/crates/wac-parser/tests/encoding/merged-functions.wac new file mode 100644 index 0000000..ec81753 --- /dev/null +++ b/crates/wac-parser/tests/encoding/merged-functions.wac @@ -0,0 +1,9 @@ +package test:comp; + +let a = new foo:bar { + ... +}; + +let b = new foo:baz { + ... +}; diff --git a/crates/wac-parser/tests/encoding/merged-functions.wat.result b/crates/wac-parser/tests/encoding/merged-functions.wat.result new file mode 100644 index 0000000..c2ed88e --- /dev/null +++ b/crates/wac-parser/tests/encoding/merged-functions.wat.result @@ -0,0 +1,52 @@ +(component + (type (;0;) + (instance + (type (;0;) (record (field "x" u32))) + (export (;1;) "x" (type (eq 0))) + (type (;2;) (func (param "x" 1))) + (export (;0;) "y" (func (type 2))) + (type (;3;) (func (param "x" 1))) + (export (;1;) "z" (func (type 3))) + ) + ) + (import "foo:bar/baz" (instance (;0;) (type 0))) + (type (;1;) + (component + (type (;0;) + (instance + (type (;0;) (record (field "x" u32))) + (export (;1;) "x" (type (eq 0))) + (type (;2;) (func (param "x" 1))) + (export (;0;) "y" (func (type 2))) + ) + ) + (import "foo:bar/baz" (instance (;0;) (type 0))) + ) + ) + (import "unlocked-dep=" (component (;0;) (type 1))) + (instance (;1;) (instantiate 0 + (with "foo:bar/baz" (instance 0)) + ) + ) + (type (;2;) + (component + (type (;0;) + (instance + (type (;0;) (record (field "x" u32))) + (export (;1;) "x" (type (eq 0))) + (type (;2;) (func (param "x" 1))) + (export (;0;) "z" (func (type 2))) + ) + ) + (import "foo:bar/baz" (instance (;0;) (type 0))) + ) + ) + (import "unlocked-dep=" (component (;1;) (type 2))) + (instance (;2;) (instantiate 1 + (with "foo:bar/baz" (instance 0)) + ) + ) + (@producers + (processed-by "wac-parser" "0.1.0") + ) +) \ No newline at end of file diff --git a/crates/wac-parser/tests/encoding/merged-functions/foo/bar.wat b/crates/wac-parser/tests/encoding/merged-functions/foo/bar.wat new file mode 100644 index 0000000..9e6ec4e --- /dev/null +++ b/crates/wac-parser/tests/encoding/merged-functions/foo/bar.wat @@ -0,0 +1,7 @@ +(component + (import "foo:bar/baz" (instance + (type (record (field "x" u32))) + (export "x" (type (eq 0))) + (export "y" (func (param "x" 1))) + )) +) diff --git a/crates/wac-parser/tests/encoding/merged-functions/foo/baz.wat b/crates/wac-parser/tests/encoding/merged-functions/foo/baz.wat new file mode 100644 index 0000000..5165631 --- /dev/null +++ b/crates/wac-parser/tests/encoding/merged-functions/foo/baz.wat @@ -0,0 +1,7 @@ +(component + (import "foo:bar/baz" (instance + (type (record (field "x" u32))) + (export "x" (type (eq 0))) + (export "z" (func (param "x" 1))) + )) +) From 0df7a511fc8d17c9a07861cbfc0dd9899a7e892d Mon Sep 17 00:00:00 2001 From: Peter Huene Date: Wed, 29 Nov 2023 17:16:11 -0800 Subject: [PATCH 2/2] Skip serialization of remapped types. JSON only supports string keys in maps and the remapped types isn't needed in the `resolve` command output. --- crates/wac-parser/src/resolution/types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wac-parser/src/resolution/types.rs b/crates/wac-parser/src/resolution/types.rs index 84bb4c4..24fe519 100644 --- a/crates/wac-parser/src/resolution/types.rs +++ b/crates/wac-parser/src/resolution/types.rs @@ -394,7 +394,7 @@ pub struct Interface { /// /// Encoding uses this map to populate the encoded type index map for the /// original types. - #[serde(skip_serializing_if = "IndexMap::is_empty")] + #[serde(skip)] pub remapped_types: IndexMap>, /// A map from used interface to set of used type export indexes. #[serde(serialize_with = "serialize_id_key_map")]