binary-reader.cc: detect more malformed modules #2251
Conversation
| (;; STDERR ;;; | ||
| out/test/regress/regress-28/regress-28.wasm:000001c: error: type mismatch at end of function, expected [] but got [any] | ||
| 000001c: error: EndFunctionBody callback failed | ||
| 0000019: error: function body shorter than given size |
There was a problem hiding this comment.
Why did we loose the filename here?
There was a problem hiding this comment.
Hmm, nice catch. :-/ It turns out that right now every error that's caught inside the BinaryReader itself is getting saved without a filename, because the BinaryReader doesn't know anything about filenames and when it creates an Error, it sets the error's Location where only the offset is set (the filename field is empty).
We can change this if you want -- e.g.
- we could make
BinaryReadertake an optional filename in its constructor that's just used for error logging, or - we could make BinaryReaderIR/BinaryReaderInterp::OnError method modify the error's Location before saving it, i.e. from their own
GetLocation()function (which produces a richer Location with the filename filled in)
I just tried this and it ended up changing a ton of the test diagnostic output, but it's definitely doable either way.
Hope you're okay if we merge this now and can revisit the filenames in error logging in a future PR.
There was a problem hiding this comment.
yeah I'm not too bothered about that.
This PR lets the BinaryReader detect malformed modules in the binary format that:
Previously we were only catching some of these situations later in validation. This is a prelude for a future PR that will make the spectest interpreter require that
assert_malformedfail to parse the module (currentlyassert_malformedis treated the same asassert_invalid, i.e. the command passes as long as validation fails).