[compiler] Fix a bug in SimplifiedLowering
SL's VisitSpeculativeIntegerAdditiveOp was setting Signed32 as restriction type even when relying on a Word32 truncation in order to skip the overflow check. This is not sound. Bug: chromium:1150649 Change-Id: I3113a2102c62d6ecef342c98d25daf31431c01ed Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2557498 Reviewed-by: Tobias Tebbi <tebbi@chromium.org> Reviewed-by: Nico Hartmann <nicohartmann@chromium.org> Commit-Queue: Georg Neis <neis@chromium.org> Cr-Commit-Position: refs/heads/master@{#71364}
This commit is contained in:
parent
1a37d561b2
commit
ba1b2cc09a
@ -1409,7 +1409,6 @@ class RepresentationSelector {
|
||||
IsSomePositiveOrderedNumber(input1_type)
|
||||
? CheckForMinusZeroMode::kDontCheckForMinusZero
|
||||
: CheckForMinusZeroMode::kCheckForMinusZero;
|
||||
|
||||
NodeProperties::ChangeOp(node, simplified()->CheckedInt32Mul(mz_mode));
|
||||
}
|
||||
|
||||
@ -1453,6 +1452,13 @@ class RepresentationSelector {
|
||||
|
||||
Type left_feedback_type = TypeOf(node->InputAt(0));
|
||||
Type right_feedback_type = TypeOf(node->InputAt(1));
|
||||
|
||||
// Using Signed32 as restriction type amounts to promising there won't be
|
||||
// signed overflow. This is incompatible with relying on a Word32
|
||||
// truncation in order to skip the overflow check.
|
||||
Type const restriction =
|
||||
truncation.IsUsedAsWord32() ? Type::Any() : Type::Signed32();
|
||||
|
||||
// Handle the case when no int32 checks on inputs are necessary (but
|
||||
// an overflow check is needed on the output). Note that we do not
|
||||
// have to do any check if at most one side can be minus zero. For
|
||||
@ -1466,7 +1472,7 @@ class RepresentationSelector {
|
||||
right_upper.Is(Type::Signed32OrMinusZero()) &&
|
||||
(left_upper.Is(Type::Signed32()) || right_upper.Is(Type::Signed32()))) {
|
||||
VisitBinop<T>(node, UseInfo::TruncatingWord32(),
|
||||
MachineRepresentation::kWord32, Type::Signed32());
|
||||
MachineRepresentation::kWord32, restriction);
|
||||
} else {
|
||||
// If the output's truncation is identify-zeros, we can pass it
|
||||
// along. Moreover, if the operation is addition and we know the
|
||||
@ -1486,8 +1492,9 @@ class RepresentationSelector {
|
||||
UseInfo right_use = CheckedUseInfoAsWord32FromHint(hint, FeedbackSource(),
|
||||
kIdentifyZeros);
|
||||
VisitBinop<T>(node, left_use, right_use, MachineRepresentation::kWord32,
|
||||
Type::Signed32());
|
||||
restriction);
|
||||
}
|
||||
|
||||
if (lower<T>()) {
|
||||
if (truncation.IsUsedAsWord32() ||
|
||||
!CanOverflowSigned32(node->op(), left_feedback_type,
|
||||
|
24
test/mjsunit/compiler/regress-1150649.js
Normal file
24
test/mjsunit/compiler/regress-1150649.js
Normal file
@ -0,0 +1,24 @@
|
||||
// Copyright 2020 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.
|
||||
|
||||
// Flags: --allow-natives-syntax
|
||||
|
||||
function foo(a) {
|
||||
var y = 0x7fffffff; // 2^31 - 1
|
||||
|
||||
// Widen the static type of y (this condition never holds).
|
||||
if (a == NaN) y = NaN;
|
||||
|
||||
// The next condition holds only in the warmup run. It leads to Smi
|
||||
// (SignedSmall) feedback being collected for the addition below.
|
||||
if (a) y = -1;
|
||||
|
||||
const z = (y + 1)|0;
|
||||
return z < 0;
|
||||
}
|
||||
|
||||
%PrepareFunctionForOptimization(foo);
|
||||
assertFalse(foo(true));
|
||||
%OptimizeFunctionOnNextCall(foo);
|
||||
assertTrue(foo(false));
|
Loading…
Reference in New Issue
Block a user