Fix memory.grow bounds and overflow checks for mem64 #7112
Conversation
Previously the interpreter only executed overflow and bounds checks for memory.grow on 32-bit memories. Run the checks on 64-bit memories as well.
src/wasm-interpreter.h
Outdated
| @@ -3836,10 +3836,14 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> { | |||
| auto fail = Literal::makeFromInt64(-1, memory->addressType); | |||
| Flow ret = Literal::makeFromInt64(memorySize, addressType); | |||
| uint64_t delta = flow.getSingleValue().getUnsigned(); | |||
| if (delta > uint32_t(-1) / Memory::kPageSize && addressType == Type::i32) { | |||
| uint64_t maxAddr = | |||
| addressType == Type::i32 ? uint64_t(uint32_t(-1)) : uint64_t(-1); | |||
There was a problem hiding this comment.
Use UINT64_MAX and UINT32_MAX here?
There was a problem hiding this comment.
We use std::numeric_limits<int32_t>::min() etc. in other places.
There was a problem hiding this comment.
Yuck, that is so much harder to read than UINT32_MAX, no?
There was a problem hiding this comment.
Heh, I don't feel strongly. I guess it is more C++ey and can support all types and all bounds. But I wouldn't object to switching.
There was a problem hiding this comment.
I get that binaryen is fully in on all the C++ things :) So lgtm either way, I just wish C++ wasn't so verbose sometimes.
| @@ -3836,10 +3836,14 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> { | |||
| auto fail = Literal::makeFromInt64(-1, memory->addressType); | |||
| Flow ret = Literal::makeFromInt64(memorySize, addressType); | |||
| uint64_t delta = flow.getSingleValue().getUnsigned(); | |||
| if (delta > uint32_t(-1) / Memory::kPageSize && addressType == Type::i32) { | |||
There was a problem hiding this comment.
Strange there wan't event a TODO here!
src/wasm-interpreter.h
Outdated
| @@ -3836,10 +3836,14 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> { | |||
| auto fail = Literal::makeFromInt64(-1, memory->addressType); | |||
| Flow ret = Literal::makeFromInt64(memorySize, addressType); | |||
| uint64_t delta = flow.getSingleValue().getUnsigned(); | |||
| if (delta > uint32_t(-1) / Memory::kPageSize && addressType == Type::i32) { | |||
| uint64_t maxAddr = | |||
| addressType == Type::i32 ? uint64_t(uint32_t(-1)) : uint64_t(-1); | |||
There was a problem hiding this comment.
We use std::numeric_limits<int32_t>::min() etc. in other places.
Previously the interpreter only executed overflow and bounds checks for
memory.grow on 32-bit memories. Run the checks on 64-bit memories as well.