1249480 - Ensure that the order of signature definitions is preserved in WASM text files.

Status: RESOLVED WONTFIX

(Core :: JavaScript: WebAssembly, defect, P5)

Description

[Michael Bebenita [:mbx]]

Currently, the list of function signatures is shared between declared and defined signatures. This is messing up signature indexing, for example the following example should not work:

(module (func (param i32)) (func (type 0))

"(func (param i32))" should not define an indexable signature, only (type ...) expressions should do that.

Comment 1

[Michael Bebenita [:mbx]]

Attachment: sig.patch

Comment 2

[Michael Bebenita [:mbx]]

Looking for feedback, WasmDeclaredSigBaseIndex stuff feels a bit messy.

Comment 3

[Michael Bebenita [:mbx]]

This patch keeps declared signatures in a separate list from the defined signatures and encodes references to them as |WasmDeclaredSigBaseIndex + index|. During resolution, any WasmRef that points to a declared signature (by testing if (index >= WasmDeclaredSigBaseIndex|), adds the declared signature to the defined list of signatures, or uses an existing index, thus maintaining the invariant that no two signatures in the defined signature list are alike.

Comment 4

[Luke Wagner [:luke]]

Oops, I forgot about this patch, sorry!  I'm not sure I understand with comment 0 for why
  (module (func (param i32)) (func (type 0)))
shouldn't work.  In ml-proto, functions implicitly append and use a new signature and, since all signatures have indices, they would be accessible via (type 0).

Comment 5

[Michael Bebenita [:mbx]]

Perhaps I'm not reading the spec right. I assumed that only explicitly defined types can be indexed explicitly. 

(module 
 (func (param i32)) 
 (func (type 0)) <-- I assumed this would be an error.
)

Comment 6

[Luke Wagner [:luke]]

Well, since the s-expr language isn't really part of wasm proper (unless it later became the official text format), the "spec" is mostly "what ml-proto/host/parser.mly" does.  What it does is to append every signature to the 1 array of types.  The difference is that (type (func ..)) is an explicit declaration that always adds a new entry to the end of the list and (func (param ...) (result ...)) is an implicit declaration that attempts to reuse a previous entry if possible but, if not, it does add an entry to the list.  Only (type ...) allows you to name an entry but the names are purely optional labels for indices.

Comment 7

[Luke Wagner [:luke]]

Comment on attachment 8721077 [details] [diff] [review]
sig.patch

Canceling based on my current understanding; feel free to re-request if I'm wrong.

Comment 8

[Benjamin Bouvier [:bbouvier] (inactive)]

Michael, what do you think the way forward is, here?

Comment 9

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.