Restrict range for int64_t to immediate conversions

The included test case illustrates the problem. It subtracts (16 << 27)
from another number. The Machine Operator Reducer would replace the
shift computation with 0x0000000080000000, and then change the subtract
to an add of -(0x0000000080000000), which is 0xffffffff80000000.
The instruction selector would determine that this value could be an
immediate, because it fits in 32 bits, so it would select the lea
instruction. Finally, the code generator would detect that the
immediate was less than 0, flip the sign and replace the add with a
subtract of 0x80000000. Because the x64 subtract instruction's
immediate field is 32 bits, the processor would interpret this as
0xffffffff80000000 instead of an unsigned value.

This change fixes the issue by making the CanBeImmediate check
explicitly compare against INT_MIN and INT_MAX. We disallow INT_MIN
as an immediate precisely because we cannot tell 0x0000000080000000
from 0xffffffff80000000 when truncated to 32 bits.

Bug: chromium:711203
Change-Id: Ie371b8ea290684a6bb723bae9c693a866f961850
Reviewed-on: https://chromium-review.googlesource.com/482448
Commit-Queue: Eric Holk <eholk@chromium.org>
Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44758}
This commit is contained in:
Eric Holk 2017-04-20 13:11:04 -07:00 committed by Commit Bot
parent de9daff0f7
commit ec772a4fd8
3 changed files with 36 additions and 1 deletions

View File

@ -146,6 +146,10 @@ void GraphReducer::ReduceTop() {
// Check if the reduction is an in-place update of the {node}.
Node* const replacement = reduction.replacement();
if (replacement == node) {
if (FLAG_trace_turbo_reduction) {
OFStream os(stdout);
os << "- In-place update of " << *replacement << std::endl;
}
// In-place update of {node}, may need to recurse on an input.
Node::Inputs node_inputs = node->inputs();
for (int i = 0; i < node_inputs.count(); ++i) {

View File

@ -26,7 +26,8 @@ class X64OperandGenerator final : public OperandGenerator {
return true;
case IrOpcode::kInt64Constant: {
const int64_t value = OpParameter<int64_t>(node);
return value == static_cast<int64_t>(static_cast<int32_t>(value));
return std::numeric_limits<int32_t>::min() < value &&
value <= std::numeric_limits<int32_t>::max();
}
case IrOpcode::kNumberConstant: {
const double value = OpParameter<double>(node);

View File

@ -0,0 +1,30 @@
// Copyright 2017 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.
load("test/mjsunit/wasm/wasm-constants.js");
load("test/mjsunit/wasm/wasm-module-builder.js");
(function() {
var builder = new WasmModuleBuilder();
builder.addMemory(16, 32, false);
builder.addFunction("test", kSig_i_iii)
.addBodyWithEnd([
// body:
kExprI64Const, 0,
kExprI64Const, 0x1,
kExprI64Clz,
kExprI64Sub,
kExprI64Const, 0x10,
kExprI64Const, 0x1b,
kExprI64Shl,
kExprI64Sub,
kExprI64Popcnt,
kExprI32ConvertI64,
kExprEnd, // @207
])
.exportFunc();
var module = builder.instantiate();
const result = module.exports.test(1, 2, 3);
assertEquals(58, result);
})();