bytecodealliance / wac

WebAssembly Composition (WAC) tooling
Apache License 2.0
99 stars 13 forks source link

Bug: panic when adding an implicit import with a `use` #55

Open peterhuene opened 7 months ago

peterhuene commented 7 months ago

This issue was discovered by @salmans.

Let's say we have the following WIT:

package foo:bar;

interface a {
    type x = u32;
}

interface b {
    use a.{x};
    f: func() -> x;
}

world repro {
    import b;
    export run: func();
}

and we have a component named repro:comp1 that targets the foo:bar/repro world.

We can instantiate this component in WAC using implicit imports:

package repro:wac;

let i = new repro:comp1 { ... };

This encodes just fine because the repro:comp1 imports foo:bar/a before foo:bar/b, so an implicit import for foo:bar/a is added before the implicit import for foo:bar/b; when the implicit import foo:bar/b is added, foo:bar/a is already imported so it can properly remap some internal data structures for the dependency from foo:bar/b to foo:bar/a.

However, let's say we have a repro:comp2 component:

(component (;0;)
  (type (;0;) u32)
  (export (;1;) "x" (type 0))
)

And we now instantiate repro:comp1 like this:

package repro:wac;

let i = new repro:comp1 {
  a: new repro:comp2 { ... },
  ...
};

This results in a panic:

thread 'main' panicked at crates/wac-parser/src/resolution/ast.rs:2039:40:
IndexMap: key not found

This is because foo:bar/a is no longer being implicitly imported, so the expectation that foo:bar/b can find it as an implicit import is violated.

A naive fix would be to recurse on the dependencies of an implicit import to ensure they are also implicitly imported so that the data structures can be remapped, but that's not exactly what we want here.

What we want is for the instance of repro:comp2 to also satisfy the dependency that foo:bar/b has on foo:bar/a, so that the exported type is equivilant between the import of foo:bar/a and foo:bar/b for repro:comp1; this is more relevant when the type in question is a resource and not a u32, for example.

peterhuene commented 7 months ago

I think this is what the expected WAT would look like for the implicit import of foo:bar/b:

(alias export 0 "x" (type (;0;))) ;; assume instance 0 is that of `repro:comp2`
(type (;1;)
  (instance
    (alias outer 1 0 (type (;0;)))
    (export (;1;) "x" (type (eq 0)))
    (type (;2;) (own 1))
    (type (;3;) (func (result 2)))
    (export (;0;) "f" (func (type 3)))
  )
)
(import "component:testing/b" (instance (;1;) (type 1)))

My question would then be: assuming we were actually using a resource here and not a u32, could this import be satisfied in the future given that instance 0 is internal to the composition? I'm still not 100% confident in my understanding of how realized resource type equivalence works.

lukewagner commented 7 months ago

Great question, this is a really interesting case. It's useful to imagine that x is a resource type since it makes the constraints more clear-cut. So, in your WAC:

let i = new repro:comp1 {
  a: new repro:comp2 { ... },
  ...
};

comp2 may be defining and implementing the resource type x and that resource-implementation of x is specific to that particular instance of comp2 (b/c the resources may be implemented in that particular instance's linear memory). Thus, if we were to attempt to implicitly propagate the import of b.f to the parent component, we'd have no way to describe the required type in the type of the imports of the parent component (intuitively, b/c the instance of comp2 is only created after the imports of the parent have been satisfied, and so the type doesn't exist soon enough).

What I'd suggest is that ... gives an error pointing to the un-satisfiable import of foo:bar/b (and, if you're feeling generous, which used resource type could not be transitively implicitly imported b/c it was already satisfied).