Skip to content
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.

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

wast2json miscompiles "if.wast" from the specification tests #2417

Open
gcoakes opened this issue May 12, 2024 · 4 comments
Open

wast2json miscompiles "if.wast" from the specification tests #2417

gcoakes opened this issue May 12, 2024 · 4 comments

Comments

@gcoakes
Copy link

gcoakes commented May 12, 2024

The following WAST test expression from if.wast miscompiles, omitting the empty else block:

(assert_invalid
  (module (func $type-else-value-empty-vs-num (result i32)
    (if (result i32) (i32.const 1) (then (i32.const 0)) (else))
  ))
  "type mismatch"
)
$ wast2json if.wast
$ wasm-objdump -x -d if.0.wasm

if.0.wasm:	file format wasm 0x1

Section Details:

Type[1]:
 - type[0] () -> i32
Function[1]:
 - func[0] sig=0
Code[1]:
 - func[0] size=9

Code Disassembly:

000017 func[0]:
 000018: 41 01                      | i32.const 1
 00001a: 04 7f                      | if i32
 00001c: 41 00                      |   i32.const 0
 00001e: 0b                         | end
 00001f: 0b                         | end

WABT version: 0df6c26

@sbc100
Copy link
Member

sbc100 commented May 13, 2024

Do you know how the test is currently passing give that this looks like it is actually a valid module?

@sbc100
Copy link
Member

sbc100 commented May 13, 2024

Oh I see, its still failing, since its not valid to have and empty else block:

  2 out/test/spec/if.wast:4: assert_invalid passed:                                  
  3   out/test/spec/if/if.0.wasm:000001f: error: type mismatch in `if false` branch, expected [i32] but got []
  4   000001f: error: OnEndExpr callback failed    

Is the expectation that wabt should write out the empty else block as part of the binary? Either way produces an invalid binary, so I'm not sure it really matters does it?

@sbc100
Copy link
Member

sbc100 commented May 13, 2024

@keithw WDYT?

@gcoakes
Copy link
Author

gcoakes commented May 14, 2024

Is the expectation that wabt should write out the empty else block as part of the binary?

That is what I expected.

Either way produces an invalid binary, so I'm not sure it really matters does it?

That's probably true for this case. I would still expect the WAST to literally translate to the equivalent binary.

Perhaps the specification tests should add the following as an additional test case.

(assert_invalid
  (module (func $type-else-value-empty-vs-num (result i32)
    (if (result i32) (i32.const 1) (then (i32.const 0)))
  ))
  "type mismatch"
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants