Make ToInteger always truncate -0

The spec was changed in February TC39 to make ToInteger always normalize
-0 to +0. This only observably affects Atomics.store.

Bug: v8:10271
Change-Id: I0e8f6c35cef982eae242cf6619f6f24fa75b1759
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2076509Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66543}

src/builtins/array-lastindexof.tq

@@ -61,10 +61,8 @@ namespace array {
   GetFromIndex(context: Context, length: Number, arguments: Arguments): Number {
     // 4. If fromIndex is present, let n be ? ToInteger(fromIndex);
     //    else let n be len - 1.
-    const n: Number = arguments.length < 2 ?
-        length - 1 :
-        ToInteger_Inline(
-            arguments[1], ToIntegerTruncationMode::kTruncateMinusZero);
+    const n: Number =
+        arguments.length < 2 ? length - 1 : ToInteger_Inline(arguments[1]);
 
     // 5. If n >= 0, then.
     let k: Number = SmiConstant(0);

src/builtins/base.tq

@@ -345,12 +345,6 @@ const kSloppyArgumentsContextIndex: constexpr int31
 const kSloppyArgumentsParameterMapStart: constexpr int31
     generates 'SloppyArgumentsElements::kParameterMapStart';
 
-extern enum ToIntegerTruncationMode extends int32
-constexpr 'compiler::CodeAssembler::ToIntegerTruncationMode' {
-  kNoTruncation,
-  kTruncateMinusZero
-}
-
 extern enum PrimitiveType { kString, kBoolean, kSymbol, kNumber }
 
 const kNameDictionaryInitialCapacity:
@@ -437,8 +431,8 @@ extern macro Print(Object);
 extern macro DebugBreak();
 
 // ES6 7.1.4 ToInteger ( argument )
-transitioning macro ToIntegerImpl(implicit context: Context)(
-    input: Object, mode: constexpr ToIntegerTruncationMode): Number {
+transitioning macro ToIntegerImpl(implicit context: Context)(input: Object):
+    Number {
   let input = input;
 
   while (true) {
@@ -450,13 +444,10 @@ transitioning macro ToIntegerImpl(implicit context: Context)(
         let value = Convert<float64>(hn);
         if (Float64IsNaN(value)) return SmiConstant(0);
         value = math::Float64Trunc(value);
-        if constexpr (mode == ToIntegerTruncationMode::kTruncateMinusZero) {
-          if (value == 0.0) return SmiConstant(0);
-        }
+        // ToInteger normalizes -0 to +0.
+        if (value == 0.0) return SmiConstant(0);
         const result = ChangeFloat64ToTagged(value);
-        if constexpr (mode == ToIntegerTruncationMode::kTruncateMinusZero) {
-          assert(IsNumberNormalized(result));
-        }
+        assert(IsNumberNormalized(result));
         return result;
       }
       case (ho: HeapObject): {
@@ -469,33 +460,18 @@ transitioning macro ToIntegerImpl(implicit context: Context)(
 
 transitioning builtin ToInteger(implicit context: Context)(input: Object):
     Number {
-  return ToIntegerImpl(input, ToIntegerTruncationMode::kNoTruncation);
-}
-
-transitioning builtin ToInteger_TruncateMinusZero(implicit context: Context)(
-    input: Object): Number {
-  return ToIntegerImpl(input, ToIntegerTruncationMode::kTruncateMinusZero);
+  return ToIntegerImpl(input);
 }
 
 @export
 transitioning macro ToInteger_Inline(implicit context: Context)(input: Object):
     Number {
-  return ToInteger_Inline(input, ToIntegerTruncationMode::kNoTruncation);
-}
-
-@export
-transitioning macro ToInteger_Inline(implicit context: Context)(
-    input: Object, mode: constexpr ToIntegerTruncationMode): Number {
   typeswitch (input) {
     case (s: Smi): {
       return s;
     }
     case (ho: HeapObject): {
-      if constexpr (mode == ToIntegerTruncationMode::kTruncateMinusZero) {
-        return ToInteger_TruncateMinusZero(ho);
-      } else {
-        return ToInteger(ho);
-      }
+      return ToIntegerImpl(ho);
     }
   }
 }
@@ -1276,8 +1252,7 @@ macro ChangeSafeIntegerNumberToUintPtr(value: Number):
 transitioning macro ToUintPtr(implicit context: Context)(value: JSAny):
     uintptr labels IfLessThanZero, IfUIntPtrOverflow, IfSafeIntegerOverflow {
   if (value == Undefined) return 0;
-  const indexNumber =
-      ToInteger_Inline(value, ToIntegerTruncationMode::kTruncateMinusZero);
+  const indexNumber = ToInteger_Inline(value);
   return TryNumberToUintPtr(indexNumber, kModeValueIsAnyNumber)
       otherwise IfLessThanZero, IfUIntPtrOverflow, IfSafeIntegerOverflow;
 }
@@ -1291,8 +1266,7 @@ transitioning macro ToUintPtr(implicit context: Context)(value: JSAny):
 transitioning macro ToIndex(implicit context: Context)(value: JSAny):
     uintptr labels IfRangeError {
   if (value == Undefined) return 0;
-  const indexNumber =
-      ToInteger_Inline(value, ToIntegerTruncationMode::kTruncateMinusZero);
+  const indexNumber = ToInteger_Inline(value);
   // Less than 0 case, uintptr range overflow and safe integer range overflow
   // imply IfRangeError.
   return TryNumberToUintPtr(indexNumber, kModeValueIsAnyNumber)
@@ -1365,8 +1339,7 @@ extern macro IsOneByteStringInstanceType(InstanceType): bool;
 @export
 transitioning macro ConvertToRelativeIndex(implicit context: Context)(
     index: JSAny, length: uintptr): uintptr {
-  const indexNumber: Number =
-      ToInteger_Inline(index, ToIntegerTruncationMode::kTruncateMinusZero);
+  const indexNumber: Number = ToInteger_Inline(index);
   return ConvertToRelativeIndex(indexNumber, length);
 }
 
@@ -1413,8 +1386,7 @@ macro ConvertToRelativeIndex(indexNumber: Number, length: uintptr): uintptr {
 @export
 transitioning macro ClampToIndexRange(implicit context: Context)(
     index: JSAny, limit: uintptr): uintptr {
-  const indexNumber: Number =
-      ToInteger_Inline(index, ToIntegerTruncationMode::kTruncateMinusZero);
+  const indexNumber: Number = ToInteger_Inline(index);
   return ClampToIndexRange(indexNumber, limit);
 }
 

src/builtins/builtins-string.tq

@@ -68,8 +68,7 @@ namespace string {
     const string: String = ToThisString(receiver, methodName);
 
     // 3. Let position be ? ToInteger(pos).
-    const indexNumber: Number =
-        ToInteger_Inline(position, ToIntegerTruncationMode::kTruncateMinusZero);
+    const indexNumber: Number = ToInteger_Inline(position);
 
     // Convert the {position} to a uintptr and check that it's in bounds of
     // the {string}.

src/builtins/number.tq

@@ -33,9 +33,8 @@ namespace number {
     // 3. Else if radix is undefined, let radixNumber be 10.
     // 4. Else, let radixNumber be ? ToInteger(radix).
     const radix: JSAny = arguments[0];
-    const radixNumber: Number = radix == Undefined ?
-        10 :
-        ToInteger_Inline(radix, ToIntegerTruncationMode::kTruncateMinusZero);
+    const radixNumber: Number =
+        radix == Undefined ? 10 : ToInteger_Inline(radix);
 
     // 5. If radixNumber < 2 or radixNumber > 36, throw a RangeError exception.
     if (radixNumber < 2 || radixNumber > 36) {

src/builtins/string-repeat.tq

@@ -36,8 +36,7 @@ namespace string {
 
     try {
       // 3. Let n be ? ToInteger(count).
-      typeswitch (ToInteger_Inline(
-          count, ToIntegerTruncationMode::kTruncateMinusZero)) {
+      typeswitch (ToInteger_Inline(count)) {
         case (n: Smi): {
           // 4. If n < 0, throw a RangeError exception.
           if (n < 0) goto InvalidCount;

src/compiler/code-assembler.h

@@ -371,11 +371,6 @@ TNode<Float64T> Float64Add(TNode<Float64T> a, TNode<Float64T> b);
 // a CodeAssemblerState instance.
 class V8_EXPORT_PRIVATE CodeAssembler {
  public:
-  enum ToIntegerTruncationMode {
-    kNoTruncation,
-    kTruncateMinusZero,
-  };
-
   explicit CodeAssembler(CodeAssemblerState* state) : state_(state) {}
   ~CodeAssembler();
 

src/compiler/typer.cc

@@ -530,13 +530,12 @@ Type Typer::Visitor::ToBoolean(Type type, Typer* t) {
 Type Typer::Visitor::ToInteger(Type type, Typer* t) {
   // ES6 section 7.1.4 ToInteger ( argument )
   type = ToNumber(type, t);
-  if (type.Is(t->cache_->kIntegerOrMinusZero)) return type;
+  if (type.Is(t->cache_->kInteger)) return type;
   if (type.Is(t->cache_->kIntegerOrMinusZeroOrNaN)) {
-    return Type::Union(
-        Type::Intersect(type, t->cache_->kIntegerOrMinusZero, t->zone()),
-        t->cache_->kSingletonZero, t->zone());
+    return Type::Union(Type::Intersect(type, t->cache_->kInteger, t->zone()),
+                       t->cache_->kSingletonZero, t->zone());
   }
-  return t->cache_->kIntegerOrMinusZero;
+  return t->cache_->kInteger;
 }
 
 

src/date/date.cc

@@ -66,7 +66,7 @@ void DateCache::ResetDateCache(
 // ECMA 262 - ES#sec-timeclip TimeClip (time)
 double DateCache::TimeClip(double time) {
   if (-kMaxTimeInMs <= time && time <= kMaxTimeInMs) {
-    return DoubleToInteger(time) + 0.0;
+    return DoubleToInteger(time);
   }
   return std::numeric_limits<double>::quiet_NaN();
 }

src/debug/debug-evaluate.cc

@@ -993,7 +993,6 @@ static bool TransitivelyCalledBuiltinHasNoSideEffect(Builtins::Name caller,
     case Builtins::kStringIndexOf:
     case Builtins::kStringRepeat:
     case Builtins::kToInteger:
-    case Builtins::kToInteger_TruncateMinusZero:
     case Builtins::kToLength:
     case Builtins::kToName:
     case Builtins::kToObject:

src/numbers/conversions-inl.h

@@ -81,7 +81,9 @@ inline float DoubleToFloat32(double x) {
 
 inline double DoubleToInteger(double x) {
   if (std::isnan(x)) return 0;
-  if (!std::isfinite(x) || x == 0) return x;
+  if (!std::isfinite(x)) return x;
+  // ToInteger normalizes -0 to +0.
+  if (x == 0.0) return 0;
   return (x >= 0) ? std::floor(x) : std::ceil(x);
 }
 

src/objects/objects.cc

@@ -583,7 +583,7 @@ MaybeHandle<Object> Object::ConvertToIndex(Isolate* isolate,
   if (input->IsUndefined(isolate)) return handle(Smi::zero(), isolate);
   ASSIGN_RETURN_ON_EXCEPTION(isolate, input, ToNumber(isolate, input), Object);
   if (input->IsSmi() && Smi::ToInt(*input) >= 0) return input;
-  double len = DoubleToInteger(input->Number()) + 0.0;
+  double len = DoubleToInteger(input->Number());
   auto js_len = isolate->factory()->NewNumber(len);
   if (len < 0.0 || len > kMaxSafeInteger) {
     THROW_NEW_ERROR(isolate, NewRangeError(error_index, js_len), Object);

src/objects/objects.h

@@ -702,16 +702,17 @@ class Object : public TaggedImpl<HeapObjectReferenceType::STRONG, Address> {
   ConvertToString(Isolate* isolate, Handle<Object> input);
   V8_WARN_UNUSED_RESULT static MaybeHandle<Object> ConvertToNumberOrNumeric(
       Isolate* isolate, Handle<Object> input, Conversion mode);
-  V8_WARN_UNUSED_RESULT static MaybeHandle<Object> ConvertToInteger(
-      Isolate* isolate, Handle<Object> input);
+  V8_EXPORT_PRIVATE V8_WARN_UNUSED_RESULT static MaybeHandle<Object>
+  ConvertToInteger(Isolate* isolate, Handle<Object> input);
   V8_WARN_UNUSED_RESULT static MaybeHandle<Object> ConvertToInt32(
       Isolate* isolate, Handle<Object> input);
   V8_WARN_UNUSED_RESULT static MaybeHandle<Object> ConvertToUint32(
       Isolate* isolate, Handle<Object> input);
-  V8_WARN_UNUSED_RESULT static MaybeHandle<Object> ConvertToLength(
-      Isolate* isolate, Handle<Object> input);
-  V8_WARN_UNUSED_RESULT static MaybeHandle<Object> ConvertToIndex(
-      Isolate* isolate, Handle<Object> input, MessageTemplate error_index);
+  V8_EXPORT_PRIVATE V8_WARN_UNUSED_RESULT static MaybeHandle<Object>
+  ConvertToLength(Isolate* isolate, Handle<Object> input);
+  V8_EXPORT_PRIVATE V8_WARN_UNUSED_RESULT static MaybeHandle<Object>
+  ConvertToIndex(Isolate* isolate, Handle<Object> input,
+                 MessageTemplate error_index);
 };
 
 V8_EXPORT_PRIVATE std::ostream& operator<<(std::ostream& os, const Object& obj);

test/cctest/test-object.cc

@@ -255,6 +255,29 @@ TEST(EnumCache) {
   }
 }
 
+TEST(ObjectMethodsThatTruncateMinusZero) {
+  LocalContext env;
+  Isolate* isolate = CcTest::i_isolate();
+  Factory* factory = isolate->factory();
+  v8::HandleScope scope(env->GetIsolate());
+
+  Handle<Object> minus_zero = factory->NewNumber(-1.0 * 0.0);
+  CHECK(minus_zero->IsMinusZero());
+
+  Handle<Object> result =
+      Object::ToInteger(isolate, minus_zero).ToHandleChecked();
+  CHECK(result->IsZero());
+
+  result = Object::ToLength(isolate, minus_zero).ToHandleChecked();
+  CHECK(result->IsZero());
+
+  // Choose an error message template, doesn't matter which.
+  result = Object::ToIndex(isolate, minus_zero,
+                           MessageTemplate::kInvalidAtomicAccessIndex)
+               .ToHandleChecked();
+  CHECK(result->IsZero());
+}
+
 #define TEST_FUNCTION_KIND(Name)                                \
   TEST(Name) {                                                  \
     for (int i = 0; i < FunctionKind::kLastFunctionKind; i++) { \

test/test262/test262.status

@@ -553,9 +553,6 @@
   'intl402/NumberFormat/prototype/formatToParts/signDisplay-ko-KR': [FAIL],
   'intl402/NumberFormat/prototype/formatToParts/signDisplay-zh-TW': [FAIL],
 
-  # https://bugs.chromium.org/p/v8/issues/detail?id=10271
-  'built-ins/Atomics/store/expected-return-value-negative-zero': [FAIL],
-
   # https://bugs.chromium.org/p/v8/issues/detail?id=10272
   'intl402/DateTimeFormat/invalid-numbering-system-calendar-options': [FAIL],
   'intl402/NumberFormat/invalid-numbering-system-options': [FAIL],