search

WebAssembly / reference-types

Proposal for adding basic reference types (anyref)
https://webassembly.github.io/reference-types/
Other
162 stars 40 forks source link

`table.fill` needs to bounds check before executing `table.set`? #105

Closed fitzgen closed 4 years ago

fitzgen commented 4 years ago

I want to

  1. confirm my understanding of table.fill's intended semantics, and
  2. double check my reading of table.fill's spec text.

It is my understanding that the intention is that if any part of the table region to be filled is out of bounds, then a trap must be raised and the in-bounds subset of the table's elements left unmodified. This understanding is based on this assertion that checks that we see the old table element value after a partially out-of-bounds table.fill, and the spec interpreter also does an eager bounds check. If my understanding of table.fill's intended semantics is not correct, then that assertion and bounds check in the spec interpreter need to change.

If my understanding of table.fill's intended semantics is correct, then I want to double check my reading of table.fill's spec text. The bounds checking that raises the trap only happens after n reaches 0 (step 12), which means that for a partially-in-bounds table.fill, the in-bounds subset of elements will be mutated via the nested table.sets before we get to the n = 0 base case and the trap is raised (or more likely: we do an nested table.set that is out of bounds and raises a trap). This contradicts my understanding of the intended semantics, the assertion linked above, and the spec interpreter.

memory.fill, on the other hand, does the bounds check before checking for its n = 0 base case (steps 12 and 13), and does not suffer these issues with partially-in-bounds fills. I believe that table.fill should be changed to do this bounds check before checking for its n = 0 base case as well.

lars-t-hansen commented 4 years ago

I believe your understanding of table.fill's semantics is correct: https://github.com/WebAssembly/bulk-memory-operations/pull/123

binji commented 4 years ago

Yes, it looks like the memory.fill instruction was updated in November 2019, but table.fill never was (that part of the spec seems to be from April 2019).

fitzgen commented 4 years ago

Fix at https://github.com/WebAssembly/reference-types/pull/106