Skip to content

[test] Reclassify some tests between "invalid" and "malformed" #2130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alexcrichton merged 2 commits into from
Apr 9, 2025
Merged

[test] Reclassify some tests between "invalid" and "malformed" #2130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc5a974
[test] Reclassify some tests between "invalid" and "malformed"
keithw Mar 21, 2025
fb6642f
PR review: add `module binary` versions of some `module quote` tests
keithw Apr 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions tests/cli/branch-hinting/branch-hinting-simple.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@
)
)

(assert_invalid
(assert_malformed
(module quote "(@metadata.code.branch_hint)")
"expected valid module field")
(assert_invalid
(assert_malformed
(module quote "(func (@metadata.code.branch_hint))")
"expected a string")
(assert_invalid
(assert_malformed
(module quote "(func (@metadata.code.branch_hint \"a\"))")
"invalid value for branch hint")

Expand Down
84 changes: 63 additions & 21 deletions tests/cli/code-after-end.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,78 @@
;; RUN: wast --assert default --snapshot tests/snapshots %

(assert_invalid
(module
(func end))
(assert_malformed
(module quote
"(func end)")
"operators remaining after end of function")

(assert_invalid
(module
(func end block))
;; binary version of previous test
(assert_malformed
(module binary
"\00asm\01\00\00\00\01\04\01\60\00\00\03\02\01\00\0a\05\01\03\00\0b\0b")
"operators remaining after end of function")

(assert_invalid
(module
(func end i32.add))
(assert_malformed
(module quote
"(func end block)")
"operators remaining after end of function")

(assert_invalid
(module
(func end unreachable))
;; binary version of previous test
(assert_malformed
(module binary
"\00asm\01\00\00\00\01\04\01\60\00\00\03\02\01\00\0a\07\01\05\00\0b\02\40\0b")
"operators remaining after end of function")

(assert_invalid
(module
(func end br 0))
(assert_malformed
(module quote
"(func end i32.add)")
"operators remaining after end of function")

(assert_invalid
(module
(func end return))
;; binary version of previous test
(assert_malformed
(module binary
"\00asm\01\00\00\00\01\04\01\60\00\00\03\02\01\00\0a\06\01\04\00\0b\6a\0b")
"operators remaining after end of function")

(assert_invalid
(module
(func end return_call 0))
(assert_malformed
(module quote
"(func end unreachable)")
"operators remaining after end of function")

;; binary version of previous test
(assert_malformed
(module binary
"\00asm\01\00\00\00\01\04\01\60\00\00\03\02\01\00\0a\06\01\04\00\0b\00\0b")
"operators remaining after end of function")

(assert_malformed
(module quote
"(func end br 0)")
"operators remaining after end of function")

;; binary version of previous test
(assert_malformed
(module binary
"\00asm\01\00\00\00\01\04\01\60\00\00\03\02\01\00\0a\07\01\05\00\0b\0c\00\0b")
"operators remaining after end of function")

(assert_malformed
(module quote
"(func end return)")
"operators remaining after end of function")

;; binary version of previous test
(assert_malformed
(module binary
"\00asm\01\00\00\00\01\04\01\60\00\00\03\02\01\00\0a\06\01\04\00\0b\0f\0b")
"operators remaining after end of function")

(assert_malformed
(module quote
"(func end return_call 0)")
Copy link
Member

@alexcrichton alexcrichton Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC we've had bugs in both the text and binary format in the past about handling operators-after-end, notably panics. Would you be up for adding module binary versions of these tests in addition to module quote so we can be guaranteed at least one of them hits the binary validator as opposed to having them all fail in the text parser?

Copy link
Contributor Author

@keithw keithw Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to do it, but fwiw I'm basically not anything to the text parser and the text parser won't catch this by itself -- it's only the binary parser that will keep track of the blocks and make sure the ends are well-matched. But I don't mind extra safety -- will add. :-)

(Per above: the method I'm proposing for the assert_malformed tests is basically not to change the text parsers at all, and instead run each test by first (1) using the wast crate to convert WAT to bytes, and then (2) running wasmparser on those bytes to see if they parse as binary Wasm. That way all the safety checks are done in the binary reader/parser and it doesn't matter where the module originally came from (text or binary).)

Copy link
Contributor Author

@keithw keithw Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

"operators remaining after end of function")

;; binary version of previous test
(assert_malformed
(module binary
"\00asm\01\00\00\00\01\04\01\60\00\00\03\02\01\00\0a\07\01\05\00\0b\12\00\0b")
"operators remaining after end of function")
4 changes: 2 additions & 2 deletions tests/cli/component-model/adapt.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@
)
)

(assert_invalid
(assert_malformed
(component quote
"(core module $m (func (export \"\")))"
"(core instance $i (instantiate $m))"
Expand Down Expand Up @@ -279,7 +279,7 @@
)
"not a function type")

(assert_invalid
(assert_malformed
(component quote
"(import \"a\" (func $f))"
"(func (export \"foo\") (canon lift (core func $f)))"
Expand Down
8 changes: 4 additions & 4 deletions tests/cli/component-model/definedtypes.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,21 @@
(type $A22d (result $A9 (error $A10a)))
)

(assert_invalid
(assert_malformed
(component quote
"(type $t (variant (case $x \"x\" string (refines $x))))"
)
"variant case cannot refine itself"
)

(assert_invalid
(assert_malformed
(component quote
"(type $t (variant (case \"x\" (refines $y)) (case $y \"y\" string)))"
)
"unknown variant case"
)

(assert_invalid
(assert_malformed
(component quote
"(type $t string)"
"(type $v (variant (case \"x\" $t (refines $z))))"
Expand All @@ -83,7 +83,7 @@
"variant case can only refine a previously defined case"
)

(assert_invalid
(assert_malformed
(component quote
"(type $t string)"
"(type $v (variant (case $x \"x\" $t) (case $x \"y\" $t)))"
Expand Down
2 changes: 1 addition & 1 deletion tests/cli/component-model/export-introduces-alias.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
))
)

(assert_invalid
(assert_malformed
(component quote
"(type (instance"
"(type $t u8)"
Expand Down
4 changes: 2 additions & 2 deletions tests/cli/component-model/func.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@
"canonical option `realloc` is required"
)

(assert_invalid
(assert_malformed
(component binary
"\00asm" "\0d\00\01\00" ;; component header
"\07\05" ;; component type section, 5 bytes
Expand All @@ -134,7 +134,7 @@
)
"invalid leading byte (0x1) for number of results")

(assert_invalid
(assert_malformed
(component binary
"\00asm" "\0d\00\01\00" ;; component header
"\07\05" ;; component type section, 5 bytes
Expand Down
20 changes: 10 additions & 10 deletions tests/cli/component-model/import.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,19 @@
)
"duplicate import name `:a`")

(assert_malformed
(component quote
"(import \"a\" (func))"
"(import \"a\" (func))"
(assert_invalid
(component
(import "a" (func))
(import "a" (func))
)
"import name `a` conflicts with previous name `a`")

(assert_malformed
(component quote
"(type (component"
"(import \"a\" (func))"
"(import \"a\" (func))"
"))"
(assert_invalid
(component
(type (component
(import "a" (func))
(import "a" (func))
))
)
"import name `a` conflicts with previous name `a`")

Expand Down
2 changes: 1 addition & 1 deletion tests/cli/component-model/instance-type.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@
)))
"core type index 0 is not a module type")

(assert_invalid
(assert_malformed
(component quote
"(type $t (func))"
"(type (instance (export \"a\" (core module (type $t)))))"
Expand Down
16 changes: 8 additions & 8 deletions tests/cli/component-model/start.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@
(export "b" (value $a))
)

(assert_invalid
(component
(import "a" (func $f))
(start $f)
(start $f)
(assert_malformed
(component quote
"(import \"a\" (func $f)) "
"(start $f) "
"(start $f) "
)
"cannot have more than one start")
"multiple start sections found")

(assert_invalid
(assert_malformed
(component binary
"\00asm" "\0d\00\01\00" ;; component header

Expand All @@ -76,7 +76,7 @@
)
"start function results size is out of bounds")

(assert_invalid
(assert_malformed
(component binary
"\00asm" "\0d\00\01\00" ;; component header

Expand Down
6 changes: 3 additions & 3 deletions tests/cli/component-model/types.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@
)
"import name `a` conflicts with previous name `A`")

(assert_invalid
(assert_malformed
(component quote
"(component $c (core type $t (module (alias outer $c $t (type)))))"
)
Expand All @@ -141,7 +141,7 @@
))
)

(assert_invalid
(assert_malformed
(component quote
"(component $c (type $t (component (alias outer $c $t (type)))))"
)
Expand Down Expand Up @@ -193,7 +193,7 @@
)
"export name `FOO-bar-BAZ` conflicts with previous name `foo-BAR-baz`")

(assert_invalid
(assert_malformed
(component quote
"(component $c (type $t (instance (alias outer $c $t (type)))))"
)
Expand Down
2 changes: 1 addition & 1 deletion tests/cli/component-model/wrong-order.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
;; RUN: wast --assert default --snapshot tests/snapshots %

(assert_invalid
(assert_malformed
(module binary
"\00asm\01\00\00\00"

Expand Down
2 changes: 1 addition & 1 deletion tests/cli/data-count-big.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
;; RUN: wast --assert default --snapshot tests/snapshots %

(assert_invalid
(assert_malformed
(module binary
"\00asm\01\00\00\00"

Expand Down
6 changes: 3 additions & 3 deletions tests/cli/folding/branch-hinting-simple.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@
)
)

(assert_invalid
(assert_malformed
(module quote "(@metadata.code.branch_hint)")
"expected valid module field")
(assert_invalid
(assert_malformed
(module quote "(func (@metadata.code.branch_hint))")
"expected a string")
(assert_invalid
(assert_malformed
(module quote "(func (@metadata.code.branch_hint \"a\"))")
"invalid value for branch hint")

Expand Down
6 changes: 3 additions & 3 deletions tests/cli/gc/gc-subtypes-invalid.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,9 @@
)
"type index out of bounds"
)
(assert_invalid
(module
(type (struct (field $vt (mut i32)) (field $vt (mut i64))))
(assert_malformed
(module quote
"(type (struct (field $vt (mut i32)) (field $vt (mut i64))))"
)
"duplicate identifier"
)
Expand Down
2 changes: 1 addition & 1 deletion tests/cli/gc/invalid.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@
)
"array.new_data can only create arrays with numeric and vector elements")

(assert_invalid
(assert_malformed
(module binary
"\00asm" "\01\00\00\00" ;; module header

Expand Down
10 changes: 5 additions & 5 deletions tests/cli/if-else-parsing.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,29 +15,29 @@
(if (i32.const 1) (i32.eqz) (then nop))
)
)
(assert_invalid
(assert_malformed
(module quote
"(func (if))"
)
"no `then`")

(assert_invalid
(assert_malformed
(module quote
"(func (if (else)))"
)
"no `then`")

(assert_invalid
(assert_malformed
(module quote
"(func (if nop (else)))"
)
"expected `(`")
(assert_invalid
(assert_malformed
(module quote
"(func (if (nop) (else)))"
)
"no `then`")
(assert_invalid
(assert_malformed
(module quote
"(func (if (nop) nop (then)))"
)
Expand Down
2 changes: 1 addition & 1 deletion tests/cli/invalid/bad_br_table.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
;; RUN: wast --assert default --snapshot tests/snapshots %

(assert_invalid
(assert_malformed
(module binary
"\00\61\73\6d" ;; header
"\01\00\00\00" ;; version
Expand Down
Loading