From b4375b77ecea6e575e927dfe6c6e61809b644196 Mon Sep 17 00:00:00 2001 From: "jkummerow@chromium.org" Date: Fri, 12 Sep 2014 08:44:14 +0000 Subject: [PATCH] Fix Smi vs. HeapObject confusion in HConstants. Representation and HType should agree with each other. BUG=chromium:412215 LOG=y R=bmeurer@chromium.org Review URL: https://codereview.chromium.org/556563005 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@23901 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/conversions.h | 6 +++++ src/hydrogen-instructions.cc | 7 ++++++ src/hydrogen-types.cc | 5 ++++- test/mjsunit/regress/regress-crbug-412215.js | 33 ++++++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 test/mjsunit/regress/regress-crbug-412215.js diff --git a/src/conversions.h b/src/conversions.h index c33de77..1b76ac5 100644 --- a/src/conversions.h +++ b/src/conversions.h @@ -153,6 +153,12 @@ static inline bool IsMinusZero(double value) { } +static inline bool IsSmiDouble(double value) { + return !IsMinusZero(value) && value >= Smi::kMinValue && + value <= Smi::kMaxValue && value == FastI2D(FastD2I(value)); +} + + // Integer32 is an integer that can be represented as a signed 32-bit // integer. It has to be in the range [-2^31, 2^31 - 1]. // We also have to check for negative 0 as it is not an Integer32. diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index 8539da2..a057217 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -2815,6 +2815,13 @@ void HConstant::Initialize(Representation r) { r = Representation::Tagged(); } } + if (r.IsSmi()) { + // If we have an existing handle, zap it, because it might be a heap + // number which we must not re-use when copying this HConstant to + // Tagged representation later, because having Smi representation now + // could cause heap object checks not to get emitted. + object_ = Unique(Handle::null()); + } set_representation(r); SetFlag(kUseGVN); } diff --git a/src/hydrogen-types.cc b/src/hydrogen-types.cc index c83ff3c..87047a2 100644 --- a/src/hydrogen-types.cc +++ b/src/hydrogen-types.cc @@ -42,7 +42,10 @@ HType HType::FromType(Handle type); HType HType::FromValue(Handle value) { if (value->IsSmi()) return HType::Smi(); if (value->IsNull()) return HType::Null(); - if (value->IsHeapNumber()) return HType::HeapNumber(); + if (value->IsHeapNumber()) { + double n = Handle::cast(value)->value(); + return IsSmiDouble(n) ? HType::Smi() : HType::HeapNumber(); + } if (value->IsString()) return HType::String(); if (value->IsBoolean()) return HType::Boolean(); if (value->IsUndefined()) return HType::Undefined(); diff --git a/test/mjsunit/regress/regress-crbug-412215.js b/test/mjsunit/regress/regress-crbug-412215.js new file mode 100644 index 0000000..ad926fc --- /dev/null +++ b/test/mjsunit/regress/regress-crbug-412215.js @@ -0,0 +1,33 @@ +// Copyright 2014 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 + +var dummy = {foo: "true"}; + +var a = {y:0.5}; +a.y = 357; +var b = a.y; + +var d; +function f( ) { + d = 357; + return {foo: b}; +} +f(); +f(); +%OptimizeFunctionOnNextCall(f); +var x = f(); + +// With the bug, x is now an invalid object; the code below +// triggers a crash. + +function g(obj) { + return obj.foo.length; +} + +g(dummy); +g(dummy); +%OptimizeFunctionOnNextCall(g); +g(x); -- 2.7.4