WebAssembly / component-model

Repository for design and specification of the Component Model
Other
971 stars 81 forks source link

Consider changing validation of resources-are-exported slightly #179

Closed alexcrichton closed 1 year ago

alexcrichton commented 1 year ago

Currently the explainer has this example component:

(component
  (import "R1" (type $R1 (sub resource)))
  (type $R2 (resource (rep i32)))
  (export $R2' "R2" (type $R2))
  (func $f1 (result (own $R1)) (canon lift ...))
  (func $f2 (result (own $R2)) (canon lift ...))
  (func $f2' (result (own $R2')) (canon lift ...))
  (export "f1" (func $f1))
  ;; (export "f2" (func $f2)) -- invalid
  (export "f2" (func $f2) (func (result (own $R2'))))
  (export "f2" (func $f2'))
)

where specifically (export "f2" (func $f2)) is considered invalid because $f2 refers to $R2 which is not an exported resource (sort of). In the wasm-tools initial implementation at https://github.com/bytecodealliance/wasm-tools/pull/966, however, type indexes are sort of "boiled away" to the point where the validator does indeed think that $R2 is exported under the "R2" name. This ends up coming about because $R2 is a type index which points to a unique resource identifier. When $R2 is exported that flags the unique resource identifier as exported, not the type index, and then generates a new type index $R2' which points to the same unique resource identifier.

Given this sort of implementation the question for "is $f2 exported valid" would be "yes" because the unique resource identifiers that it refers to are all exported. This doesn't agree with the explainer, though, so I wanted to open an issue about this.

It's worth pointing out, though, that wasmparser doesn't yet implement validation that structural types are all named when used. For example you could have unnamed record types used in function parameters, which isn't supposed to be valid. Partly why this hasn't been implemented though is I don't know how best to do it. Everything else in validation deals with an "elaborated" type of sorts where the export restrictions here are applied moreso to type indices rather than the elaborated form of types. This means that there's no natural way to implement this sort of validation in wasmparser, at least for now.

Although I also feel that while this issue with resources is similar to the problem about not exporting anonymous record types it also feels distinct. In the formalism that @lukewagner shared for what could be the foundations of resource types I believe the property about whether or not a resource was exported wasn't tied to its index but rather the entire shape of the component after it finished being translated. This I think would allow, for example, exporting $f2 above as well.

Overall I don't know best what to do about all this myself. I don't see a particularly obviously-best-route by which to implement this all.

lukewagner commented 1 year ago

This is certainly worth considering. One question the alternative approach raises is: if it's valid to export $f2 (in the above example) and $R2 is exported twice (with different names), when we render this wat as wit, which name do we use? The goal of having exports introduce a new type index is that it's always unambiguous which name to use.

lukewagner commented 1 year ago

I think currently we've resolved to keep things as-is so as to answer the above question, but we can reopen and re-discuss if there's a new reason to change it.