feat(interface-types) Greatly improve errors #1285
Conversation
Hywan
added
🎉 enhancement
|
bors try |
tryBuild succeeded |
The new `errors` module contains structure to represent errors, instead of using basic strings. The first usage is in the interpreter itself.
Hywan
force-pushed
the
feat-interface-types-better-error
branch
from
04bf6d2 to
4ffb158
Compare
| "`{}` failed because there is not enough data on the stack (needs 2).", | ||
| instruction_name, | ||
| )) | ||
| if memory_view.len() < pointer + length { |
There was a problem hiding this comment.
I think this may be affected by the same bug I recently fixed in WasmPtr,
Given a String, "hi" at address 0xFFFE the memory looks like
0xFFFE: 'h'
0xFFFF: 'i'
ptr (0xFFFE) + len (2) = 0x10000, failing this bounds check. The fix, in general, is annoying when you handle length = 0 and zero sized types.
See #1272 for my fix.
Better to have false positives than false negatives in bounds-checking code, but leaving a comment here as a heads up. It might make sense to do a full pass for these types of bugs and fix them in a separate PR if you think it might clutter this one.
There was a problem hiding this comment.
To clarify more, we want to make sure out of bounds pointers are never derefed, so ptr = 0x10000 and length = 0 will not be caught by the naive solution of changing < to <= here.
There was a problem hiding this comment.
I'm not sure I understand. I've pushed a new commit to use inclusive an range, and consequently a special condition to handle empty string, but I'm not sure there is a bug right now. The test_memory_to_string already reads at bounds.
|
bors r+ |
Build succeeded |
This PR is build on top of #1284. It must be merged first. View the exact diff.Errors in
wasmer-interface-typeswere just&'static str, which isn't quite great 😉. This PR introduces theerrorsmodule, with structures that represent errors. ADisplayimplementation maps the errors to strings.