[interp] Pop memory.copy/table.copy length using the minimum index type#2800
Conversation
sbc100
left a comment
There was a problem hiding this comment.
Maybe another case where the upstream spec tests are missing some coverage?
|
Looks like it. table_copy_mixed.wast defines test_64to32 but never invokes it, so only the validator sees the mixed case. Same on the memory side, the memory64 memory_copy.wast never executes a mixed-width copy. I only hit the runtime path by running the exports directly with --run-all-exports. An assert_return on test_64to32 upstream would have caught this. Thanks for merging. |
|
Yes, looks that way. table_copy_mixed.wast defines test_64to32 and test_32to64 but never invokes them, so the mixed path only ever gets validated, never executed. I only tripped it because --run-all-exports calls everything. For memory.copy the proposal suite has no mixed execution test at all, just assert_invalid cases. Worth raising upstream. Thanks for merging this. |
A valid module doing a mixed-width
table.copyaborts wasm-interp in a debug build:The module copies from an i64-indexed table into an i32-indexed one (
table.copy $t32 $t64). Found it running the memory64 tests under--run-all-exports.DoTableCopy/DoMemoryCopypop the length operand with the source's index type (PopPtr(table_src)/PopPtr(mem_src)). But the length's type is the minimum of the two index types, not the source's, so for a 64-bit source into a 32-bit destination the length is i32 while the interpreter reads it as i64.TypeChecker::OnMemoryCopy/OnTableCopyalready spell out the min rule; the runtime side just didn't follow it.Debug builds trip the type assertion. In release the read only gives the right answer because the
Valueunion happens to be zero-initialised.Pop the length as u64 only when both memories/tables are 64-bit. Same-width copies are byte-identical. Regression test added under test/interp.