Skip to content

Commit

Permalink
Merged: [turboshaft] Prevent Load offsets underflow when kind has tag…
Browse files Browse the repository at this point in the history
…ged_base

Bug: v8:12783, chromium:1520362
(cherry picked from commit 7c0f4cc)

Change-Id: Ie76b54d1596ff15e1abe91e1ea0f2d9ef6283bfb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5237382
Commit-Queue: Jakob Kummerow <[email protected]>
Reviewed-by: Jakob Kummerow <[email protected]>
Auto-Submit: Darius Mercadier <[email protected]>
Cr-Commit-Position: refs/branch-heads/12.2@{v8#14}
Cr-Branched-From: 6eb5a96-refs/heads/12.2.281@{#1}
Cr-Branched-From: 44cf56d-refs/heads/main@{#91934}
  • Loading branch information
DadaIsCrazy authored and V8 LUCI CQ committed Jan 26, 2024
1 parent da023d0 commit 3883ca5
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 10 deletions.
22 changes: 12 additions & 10 deletions src/compiler/turboshaft/machine-optimization-reducer.h
Original file line number Diff line number Diff line change
Expand Up @@ -1711,8 +1711,8 @@ class MachineOptimizationReducer : public Next {
if (stored_rep.SizeInBytes() <= 4) {
value = TryRemoveWord32ToWord64Conversion(value);
}
index =
ReduceMemoryIndex(index.value_or_invalid(), &offset, &element_scale);
index = ReduceMemoryIndex(index.value_or_invalid(), &offset, &element_scale,
kind.tagged_base);
switch (stored_rep) {
case MemoryRepresentation::Uint8():
case MemoryRepresentation::Int8():
Expand Down Expand Up @@ -1775,13 +1775,14 @@ class MachineOptimizationReducer : public Next {
#endif

while (true) {
index =
ReduceMemoryIndex(index.value_or_invalid(), &offset, &element_scale);
index = ReduceMemoryIndex(index.value_or_invalid(), &offset,
&element_scale, kind.tagged_base);
if (!kind.tagged_base && !index.valid()) {
if (OpIndex left, right;
matcher.MatchWordAdd(base_idx, &left, &right,
WordRepresentation::PointerSized()) &&
TryAdjustOffset(&offset, matcher.Get(right), element_scale)) {
TryAdjustOffset(&offset, matcher.Get(right), element_scale,
kind.tagged_base)) {
base_idx = left;
continue;
}
Expand Down Expand Up @@ -1954,7 +1955,7 @@ class MachineOptimizationReducer : public Next {
// Try to match a constant and add it to `offset`. Return `true` if
// successful.
bool TryAdjustOffset(int32_t* offset, const Operation& maybe_constant,
uint8_t element_scale) {
uint8_t element_scale, bool tagged_base) {
if (!maybe_constant.Is<ConstantOp>()) return false;
const ConstantOp& constant = maybe_constant.Cast<ConstantOp>();
if (constant.rep != WordRepresentation::PointerSized() ||
Expand All @@ -1971,7 +1972,8 @@ class MachineOptimizationReducer : public Next {
!base::bits::SignedAddOverflow32(
*offset,
static_cast<int32_t>(base::bits::Unsigned(diff) << element_scale),
&new_offset)) {
&new_offset) &&
LoadOp::OffsetIsValid(new_offset, tagged_base)) {
*offset = new_offset;
return true;
}
Expand Down Expand Up @@ -2019,10 +2021,10 @@ class MachineOptimizationReducer : public Next {
// `element_scale` and returning the updated `index`.
// Return `OpIndex::Invalid()` if the resulting index is zero.
OpIndex ReduceMemoryIndex(OpIndex index, int32_t* offset,
uint8_t* element_scale) {
uint8_t* element_scale, bool tagged_base) {
while (index.valid()) {
const Operation& index_op = matcher.Get(index);
if (TryAdjustOffset(offset, index_op, *element_scale)) {
if (TryAdjustOffset(offset, index_op, *element_scale, tagged_base)) {
index = OpIndex::Invalid();
*element_scale = 0;
} else if (TryAdjustIndex(*offset, &index, index_op, *element_scale)) {
Expand All @@ -2041,7 +2043,7 @@ class MachineOptimizationReducer : public Next {
index_op.TryCast<WordBinopOp>()) {
if (binary_op->kind == WordBinopOp::Kind::kAdd &&
TryAdjustOffset(offset, matcher.Get(binary_op->right()),
*element_scale)) {
*element_scale, tagged_base)) {
index = binary_op->left();
continue;
}
Expand Down
13 changes: 13 additions & 0 deletions src/compiler/turboshaft/operations.h
Original file line number Diff line number Diff line change
Expand Up @@ -2501,6 +2501,16 @@ struct LoadOp : OperationT<LoadOp> {
return input_count == 2 ? input(1) : OpIndex::Invalid();
}

static constexpr bool OffsetIsValid(int32_t offset, bool tagged_base) {
if (tagged_base) {
// When a Load has the tagged_base Kind, it means that {offset} will
// eventually need a "-kHeapObjectTag" eventually. If the {offset} is
// min_int, then subtracting kHeapObjectTag will underflow.
return offset >= std::numeric_limits<int32_t>::min() + kHeapObjectTag;
}
return true;
}

LoadOp(OpIndex base, OptionalOpIndex index, Kind kind,
MemoryRepresentation loaded_rep, RegisterRepresentation result_rep,
int32_t offset, uint8_t element_size_log2)
Expand All @@ -2524,6 +2534,7 @@ struct LoadOp : OperationT<LoadOp> {
DCHECK_IMPLIES(element_size_log2 > 0, index().valid());
DCHECK_IMPLIES(kind.maybe_unaligned,
!SupportedOperations::IsUnalignedLoadSupported(loaded_rep));
DCHECK(OffsetIsValid(offset, kind.tagged_base));
}
static LoadOp& New(Graph* graph, OpIndex base, OptionalOpIndex index,
Kind kind, MemoryRepresentation loaded_rep,
Expand Down Expand Up @@ -2909,8 +2920,10 @@ struct StoreOp : OperationT<StoreOp> {
}

void Validate(const Graph& graph) const {
DCHECK_IMPLIES(element_size_log2 > 0, index().valid());
DCHECK_IMPLIES(kind.maybe_unaligned,
!SupportedOperations::IsUnalignedLoadSupported(stored_rep));
DCHECK(LoadOp::OffsetIsValid(offset, kind.tagged_base));
}
static StoreOp& New(
Graph* graph, OpIndex base, OptionalOpIndex index, OpIndex value,
Expand Down
20 changes: 20 additions & 0 deletions test/mjsunit/regress/wasm/regress-1520362.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2024 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');

const builder = new WasmModuleBuilder();
let array = builder.addArray(kWasmI32, true);

builder.addFunction("main", kSig_v_v).exportFunc().addBody([
kExprRefNull, array,
...wasmI32Const(0xbffffffa),
kExprI32Const, 1,
kExprI32ShrS,
kExprI32Const, 42,
kGCPrefix, kExprArraySet, array,
]);

const instance = builder.instantiate();
assertThrows(() => instance.exports.main(), WebAssembly.RuntimeError);

0 comments on commit 3883ca5

Please sign in to comment.