[wasm][memory64] Fix types used for bounds checks

The memory offset is read as a u64 in the memory64 proposal, independent
of the actual type of the memory. The actual memory size of a module (at
runtime) can only be within intptr_t/uintptr_t range though. This
assumption was already used when constructing the TurboFan graph, but
the C++ types did not reflect it yet.

This CL fixes that:

1) Use uint64_t type for bounds checks (only within the method for now,
   callers still pass a uint32_t).
2) Use uintptr_t for storing the minimum and maximum possible memory
   size at runtime (in CompilationEnv); clamp memory sizes to values
   that can actually happen at runtime.

R=manoskouk@chromium.org

Bug: v8:10949
Change-Id: I6559f9a3abc2aa338eba4618479456f6efb5e772
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2426405
Reviewed-by: Manos Koukoutos <manoskouk@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70121}
This commit is contained in:
Clemens Backes 2020-09-24 14:54:00 +02:00 committed by Commit Bot
parent 620c13b58a
commit 618a28883c
6 changed files with 42 additions and 22 deletions

View File

@ -3670,7 +3670,7 @@ Node* WasmGraphBuilder::CheckBoundsAndAlignment(
// bounds-checked index, which is guaranteed to have (the equivalent of)
// {uintptr_t} representation.
Node* WasmGraphBuilder::BoundsCheckMem(uint8_t access_size, Node* index,
uint32_t offset,
uint64_t offset,
wasm::WasmCodePosition position,
EnforceBoundsCheck enforce_check) {
DCHECK_LE(1, access_size);
@ -3681,13 +3681,17 @@ Node* WasmGraphBuilder::BoundsCheckMem(uint8_t access_size, Node* index,
return index;
}
if (!base::IsInBounds<uint64_t>(offset, access_size, env_->max_memory_size)) {
// If the offset does not fit in a uintptr_t, this can never succeed on this
// machine.
if (offset > std::numeric_limits<uintptr_t>::max() ||
!base::IsInBounds<uintptr_t>(offset, access_size,
env_->max_memory_size)) {
// The access will be out of bounds, even for the largest memory.
TrapIfEq32(wasm::kTrapMemOutOfBounds, Int32Constant(0), 0, position);
return mcgraph()->IntPtrConstant(0);
return mcgraph()->UintPtrConstant(0);
}
uint64_t end_offset = uint64_t{offset} + access_size - 1u;
Node* end_offset_node = IntPtrConstant(end_offset);
uintptr_t end_offset = offset + access_size - 1u;
Node* end_offset_node = mcgraph_->UintPtrConstant(end_offset);
// The accessed memory is [index + offset, index + end_offset].
// Check that the last read byte (at {index + end_offset}) is in bounds.

View File

@ -454,7 +454,7 @@ class WasmGraphBuilder {
Node* MemBuffer(uint32_t offset);
// BoundsCheckMem receives a uint32 {index} node and returns a ptrsize index.
Node* BoundsCheckMem(uint8_t access_size, Node* index, uint32_t offset,
Node* BoundsCheckMem(uint8_t access_size, Node* index, uint64_t offset,
wasm::WasmCodePosition, EnforceBoundsCheck);
// Check that the range [start, start + size) is in the range [0, max).
// Also updates *size with the valid range. Returns true if the range is

View File

@ -2095,10 +2095,14 @@ class LiftoffCompiler {
// Returns true if the memory access is statically known to be out of bounds
// (a jump to the trap was generated then); return false otherwise.
bool BoundsCheckMem(FullDecoder* decoder, uint32_t access_size,
uint32_t offset, Register index, LiftoffRegList pinned,
uint64_t offset, Register index, LiftoffRegList pinned,
ForceCheck force_check) {
// If the offset does not fit in a uintptr_t, this can never succeed on this
// machine.
const bool statically_oob =
!base::IsInBounds<uint64_t>(offset, access_size, env_->max_memory_size);
offset > std::numeric_limits<uintptr_t>::max() ||
!base::IsInBounds<uintptr_t>(offset, access_size,
env_->max_memory_size);
if (!force_check && !statically_oob &&
(!FLAG_wasm_bounds_checks || env_->use_trap_handler)) {
@ -2118,7 +2122,7 @@ class LiftoffCompiler {
return true;
}
uint64_t end_offset = uint64_t{offset} + access_size - 1u;
uintptr_t end_offset = offset + access_size - 1u;
// If the end offset is larger than the smallest memory, dynamically check
// the end offset against the actual memory size, which is not known at
@ -2128,12 +2132,7 @@ class LiftoffCompiler {
Register mem_size = __ GetUnusedRegister(kGpReg, pinned).gp();
LOAD_INSTANCE_FIELD(mem_size, MemorySize, kSystemPointerSize);
if (kSystemPointerSize == 8) {
__ LoadConstant(end_offset_reg, WasmValue(end_offset));
} else {
__ LoadConstant(end_offset_reg,
WasmValue(static_cast<uint32_t>(end_offset)));
}
__ LoadConstant(end_offset_reg, WasmValue::ForUintPtr(end_offset));
if (end_offset >= env_->min_memory_size) {
__ emit_cond_jump(kUnsignedGreaterEqual, trap_label,

View File

@ -52,17 +52,21 @@ struct CompilationEnv {
// The smallest size of any memory that could be used with this module, in
// bytes.
const uint64_t min_memory_size;
const uintptr_t min_memory_size;
// The largest size of any memory that could be used with this module, in
// bytes.
const uint64_t max_memory_size;
const uintptr_t max_memory_size;
// Features enabled for this compilation.
const WasmFeatures enabled_features;
const LowerSimd lower_simd;
static constexpr uint32_t kMaxMemoryPagesAtRuntime =
std::min(kV8MaxWasmMemoryPages,
std::numeric_limits<uintptr_t>::max() / kWasmPageSize);
constexpr CompilationEnv(const WasmModule* module,
UseTrapHandler use_trap_handler,
RuntimeExceptionSupport runtime_exception_support,
@ -71,12 +75,16 @@ struct CompilationEnv {
: module(module),
use_trap_handler(use_trap_handler),
runtime_exception_support(runtime_exception_support),
min_memory_size(module ? module->initial_pages * uint64_t{kWasmPageSize}
: 0),
max_memory_size((module && module->has_maximum_pages
? module->maximum_pages
: max_mem_pages()) *
// During execution, the memory can never be bigger than what fits in a
// uintptr_t.
min_memory_size(std::min(kMaxMemoryPagesAtRuntime,
module ? module->initial_pages : 0) *
uint64_t{kWasmPageSize}),
max_memory_size(static_cast<uintptr_t>(
std::min(kMaxMemoryPagesAtRuntime,
module && module->has_maximum_pages ? module->maximum_pages
: max_mem_pages()) *
uint64_t{kWasmPageSize})),
enabled_features(enabled_features),
lower_simd(lower_simd) {}
};

View File

@ -811,6 +811,9 @@ void UpdateFeatureUseCounts(Isolate* isolate, const WasmFeatures& detected) {
} // namespace
// static
constexpr uint32_t CompilationEnv::kMaxMemoryPagesAtRuntime;
//////////////////////////////////////////////////////
// PIMPL implementation of {CompilationState}.

View File

@ -106,6 +106,12 @@ class WasmValue {
template <typename T>
inline T to_unchecked() const;
static WasmValue ForUintPtr(uintptr_t value) {
using type =
std::conditional<kSystemPointerSize == 8, uint64_t, uint32_t>::type;
return WasmValue{type{value}};
}
private:
ValueType type_;
uint8_t bit_pattern_[16];