[wasm] Fix more 32/64 bit issues

For simplicity, we currently use the approach to do all computations
and bounds checks on 32 bit values, and only convert to pointer size
right before using the value as memory offset.
Unfortunately, there are still cases left where we use 32-bit values
for 64-bit operations, which can lead to subtle bugs.
This CL hopefully fixes the last of these bugs.

R=titzer@chromium.org

Bug: v8:7257

Change-Id: I8d340f83ad17925c0d18d4e788350ef6101786ea
Reviewed-on: https://chromium-review.googlesource.com/852299
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50409}
This commit is contained in:
Clemens Hammacher 2018-01-05 17:54:12 +01:00 committed by Commit Bot
parent a4de840cd3
commit 9180b2ca46

View File

@ -3565,23 +3565,17 @@ Node* WasmGraphBuilder::BoundsCheckMem(uint8_t access_size, Node* index,
// The end offset is larger than the smallest memory. // The end offset is larger than the smallest memory.
// Dynamically check the end offset against the actual memory size, which // Dynamically check the end offset against the actual memory size, which
// is not known at compile time. // is not known at compile time.
Node* cond; Node* cond =
if (jsgraph()->machine()->Is32()) { graph()->NewNode(jsgraph()->machine()->Uint32LessThanOrEqual(),
cond = graph()->NewNode(jsgraph()->machine()->Uint32LessThanOrEqual(), jsgraph()->Int32Constant(end_offset), mem_size);
jsgraph()->Int32Constant(end_offset), mem_size);
} else {
cond = graph()->NewNode(
jsgraph()->machine()->Uint64LessThanOrEqual(),
jsgraph()->Int64Constant(static_cast<int64_t>(end_offset)), mem_size);
}
TrapIfFalse(wasm::kTrapMemOutOfBounds, cond, position); TrapIfFalse(wasm::kTrapMemOutOfBounds, cond, position);
} else { } else {
// The end offset is within the bounds of the smallest memory, so only // The end offset is within the bounds of the smallest memory, so only
// one check is required. Check to see if the index is also a constant. // one check is required. Check to see if the index is also a constant.
UintPtrMatcher match(index); Uint32Matcher match(index);
if (match.HasValue()) { if (match.HasValue()) {
uint64_t index_val = match.Value(); uint32_t index_val = match.Value();
if ((index_val + offset + access_size) <= min_size) { if (index_val <= min_size - end_offset) {
// The input index is a constant and everything is statically within // The input index is a constant and everything is statically within
// bounds of the smallest possible memory. // bounds of the smallest possible memory.
return Uint32ToUintptr(index); return Uint32ToUintptr(index);
@ -3592,16 +3586,10 @@ Node* WasmGraphBuilder::BoundsCheckMem(uint8_t access_size, Node* index,
// Compute the effective size of the memory, which is the size of the memory // Compute the effective size of the memory, which is the size of the memory
// minus the statically known offset, minus the byte size of the access minus // minus the statically known offset, minus the byte size of the access minus
// one. // one.
Node* effective_size; // This produces a positive number since {end_offset <= min_size <= mem_size}.
if (jsgraph()->machine()->Is32()) { Node* effective_size =
effective_size = graph()->NewNode(jsgraph()->machine()->Int32Sub(), mem_size,
graph()->NewNode(jsgraph()->machine()->Int32Sub(), mem_size, jsgraph()->Int32Constant(end_offset - 1));
jsgraph()->Int32Constant(end_offset - 1));
} else {
effective_size = graph()->NewNode(
jsgraph()->machine()->Int64Sub(), mem_size,
jsgraph()->Int64Constant(static_cast<int64_t>(end_offset - 1)));
}
// Introduce the actual bounds check. // Introduce the actual bounds check.
Node* cond = graph()->NewNode(m->Uint32LessThan(), index, effective_size); Node* cond = graph()->NewNode(m->Uint32LessThan(), index, effective_size);