[bigint] Encapsulate internals in MutableBigInt

This CL creates the invariant that the BigInt class treats
BigInt objects as immutable. Writing to new BigInt objects
as part of their construction is done by the MutableBigInt
helper class, which in turn is hidden as an implementation
detail in bigint.cc.
As a side effect, this refactoring enforces right-trimming
checks for all newly created BigInts, and ensures that all
BigInt allocations possibly exceeding kMaxLength check for
this case and throw a RangeError instead of crashing.

Bug: v8:6791
Tbr: mlippautz@chromium.org
Change-Id: Id239746108e6b076b47a03ba37462001eb501507
Reviewed-on: https://chromium-review.googlesource.com/742329
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49462}

BUILD.gn

@@ -1774,7 +1774,6 @@ v8_source_set("v8_base") {
     "src/objects.h",
     "src/objects/arguments-inl.h",
     "src/objects/arguments.h",
-    "src/objects/bigint-inl.h",
     "src/objects/bigint.cc",
     "src/objects/bigint.h",
     "src/objects/code-inl.h",

src/builtins/builtins-bigint.cc

@@ -44,8 +44,7 @@ BUILTIN(BigIntParseInt) {
   // Convert {string} to a String and flatten it.
   // Fast path: avoid back-and-forth conversion for Smi inputs.
   if (string->IsSmi() && radix->IsUndefined(isolate)) {
-    int num = Smi::ToInt(*string);
-    return *isolate->factory()->NewBigIntFromInt(num);
+    RETURN_RESULT_OR_FAILURE(isolate, BigInt::FromNumber(isolate, string));
   }
   Handle<String> subject;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, subject,

src/conversions.cc

@@ -903,15 +903,13 @@ class BigIntParseIntHelper : public StringToIntHelper {
           return MaybeHandle<BigInt>();
         }
       case kZero:
-        return isolate()->factory()->NewBigIntFromInt(0);
+        return BigInt::Zero(isolate());
       case kError:
         DCHECK_EQ(should_throw() == kThrowOnError,
                   isolate()->has_pending_exception());
         return MaybeHandle<BigInt>();
       case kDone:
-        result_->set_sign(negative());
-        result_->RightTrim();
-        return result_;
+        return BigInt::Finalize(result_, negative());
       case kEmpty:
       case kRunning:
         break;
@@ -927,7 +925,7 @@ class BigIntParseIntHelper : public StringToIntHelper {
     // junk before allocating the result?
     int charcount = length() - cursor();
     // TODO(adamk): Pretenure if this is for a literal.
-    MaybeHandle<BigInt> maybe =
+    MaybeHandle<FreshlyAllocatedBigInt> maybe =
         BigInt::AllocateFor(isolate(), radix(), charcount, should_throw());
     if (!maybe.ToHandle(&result_)) {
       set_state(kError);
@@ -935,8 +933,8 @@ class BigIntParseIntHelper : public StringToIntHelper {
   }
 
   virtual void ResultMultiplyAdd(uint32_t multiplier, uint32_t part) {
-    result_->InplaceMultiplyAdd(static_cast<uintptr_t>(multiplier),
-                                static_cast<uintptr_t>(part));
+    BigInt::InplaceMultiplyAdd(result_, static_cast<uintptr_t>(multiplier),
+                               static_cast<uintptr_t>(part));
   }
 
  private:
@@ -944,7 +942,7 @@ class BigIntParseIntHelper : public StringToIntHelper {
     return behavior_ == Behavior::kParseInt ? kThrowOnError : kDontThrow;
   }
 
-  Handle<BigInt> result_;
+  Handle<FreshlyAllocatedBigInt> result_;
   Behavior behavior_;
 };
 

src/factory.cc

@@ -14,7 +14,7 @@
 #include "src/conversions.h"
 #include "src/isolate-inl.h"
 #include "src/macro-assembler.h"
-#include "src/objects/bigint-inl.h"
+#include "src/objects/bigint.h"
 #include "src/objects/debug-objects-inl.h"
 #include "src/objects/frame-array-inl.h"
 #include "src/objects/module.h"
@@ -1413,55 +1413,9 @@ Handle<HeapNumber> Factory::NewHeapNumber(MutableMode mode,
                      HeapNumber);
 }
 
-Handle<BigInt> Factory::NewBigInt(int length, PretenureFlag pretenure) {
-  CALL_HEAP_FUNCTION(isolate(),
-                     isolate()->heap()->AllocateBigInt(length, true, pretenure),
-                     BigInt);
-}
-
-Handle<BigInt> Factory::NewBigIntRaw(int length, PretenureFlag pretenure) {
-  CALL_HEAP_FUNCTION(
-      isolate(), isolate()->heap()->AllocateBigInt(length, false, pretenure),
-      BigInt);
-}
-
-Handle<BigInt> Factory::NewBigIntFromSafeInteger(double value,
-                                                 PretenureFlag pretenure) {
-  if (value == 0) return NewBigInt(0);
-
-  uint64_t absolute = std::abs(value);
-
-#if V8_TARGET_ARCH_64_BIT
-  static_assert(sizeof(BigInt::digit_t) == sizeof(uint64_t),
-                "unexpected BigInt digit size");
-  Handle<BigInt> result = NewBigIntRaw(1);
-  result->set_digit(0, absolute);
-#else
-  static_assert(sizeof(BigInt::digit_t) == sizeof(uint32_t),
-                "unexpected BigInt digit size");
-  Handle<BigInt> result = NewBigIntRaw(2);
-  result->set_digit(0, absolute);
-  result->set_digit(1, absolute >> 32);
-#endif
-
-  result->set_sign(value < 0);  // Treats -0 like 0.
-  return result;
-}
-
-Handle<BigInt> Factory::NewBigIntFromInt(int value, PretenureFlag pretenure) {
-  if (value == 0) return NewBigInt(0);
-  Handle<BigInt> result = NewBigIntRaw(1);
-  if (value > 0) {
-    result->set_digit(0, value);
-  } else if (value == kMinInt) {
-    STATIC_ASSERT(kMinInt == -kMaxInt - 1);
-    result->set_digit(0, static_cast<BigInt::digit_t>(kMaxInt) + 1);
-    result->set_sign(true);
-  } else {
-    result->set_digit(0, -value);
-    result->set_sign(true);
-  }
-  return result;
+Handle<FreshlyAllocatedBigInt> Factory::NewBigInt(int length) {
+  CALL_HEAP_FUNCTION(isolate(), isolate()->heap()->AllocateBigInt(length),
+                     FreshlyAllocatedBigInt);
 }
 
 Handle<Object> Factory::NewError(Handle<JSFunction> constructor,

src/factory.h

@@ -22,15 +22,15 @@ namespace internal {
 
 // Forward declarations.
 class AliasedArgumentsEntry;
-class BigInt;
 class BreakPointInfo;
 class BreakPoint;
 class BoilerplateDescription;
 class ConstantElementsPair;
 class CoverageInfo;
 class DebugInfo;
-class NewFunctionArgs;
+class FreshlyAllocatedBigInt;
 class JSModuleNamespace;
+class NewFunctionArgs;
 struct SourceRange;
 class PreParsedScopeData;
 class TemplateObjectDescription;
@@ -485,15 +485,9 @@ class V8_EXPORT_PRIVATE Factory final {
   Handle<HeapNumber> NewHeapNumber(MutableMode mode,
                                    PretenureFlag pretenure = NOT_TENURED);
 
-  // Allocates a new BigInt with {length} digits and zero-initializes them.
-  Handle<BigInt> NewBigInt(int length, PretenureFlag pretenure = NOT_TENURED);
-  // Initializes length and sign fields, but leaves digits uninitialized.
-  Handle<BigInt> NewBigIntRaw(int length,
-                              PretenureFlag pretenure = NOT_TENURED);
-  Handle<BigInt> NewBigIntFromInt(int value,
-                                  PretenureFlag pretenure = NOT_TENURED);
-  Handle<BigInt> NewBigIntFromSafeInteger(
-      double value, PretenureFlag pretenure = NOT_TENURED);
+  // Allocates a new BigInt with {length} digits. Only to be used by
+  // MutableBigInt::New*.
+  Handle<FreshlyAllocatedBigInt> NewBigInt(int length);
 
   Handle<JSObject> NewArgumentsObject(Handle<JSFunction> callee, int length);
 

src/heap/heap.cc

@@ -2496,20 +2496,18 @@ AllocationResult Heap::AllocateHeapNumber(MutableMode mode,
   return result;
 }
 
-AllocationResult Heap::AllocateBigInt(int length, bool zero_initialize,
-                                      PretenureFlag pretenure) {
+AllocationResult Heap::AllocateBigInt(int length) {
   if (length < 0 || length > BigInt::kMaxLength) {
     v8::internal::Heap::FatalProcessOutOfMemory("invalid BigInt length", true);
   }
   int size = BigInt::SizeFor(length);
-  AllocationSpace space = SelectSpace(pretenure);
+  AllocationSpace space = SelectSpace(NOT_TENURED);
   HeapObject* result = nullptr;
   {
     AllocationResult allocation = AllocateRaw(size, space);
     if (!allocation.To(&result)) return allocation;
   }
   result->set_map_after_allocation(bigint_map(), SKIP_WRITE_BARRIER);
-  BigInt::cast(result)->Initialize(length, zero_initialize);
   return result;
 }
 

src/heap/heap.h

@@ -2030,9 +2030,7 @@ class Heap {
   MUST_USE_RESULT AllocationResult AllocateHeapNumber(
       MutableMode mode = IMMUTABLE, PretenureFlag pretenure = NOT_TENURED);
 
-  MUST_USE_RESULT AllocationResult AllocateBigInt(int length,
-                                                  bool zero_initialize,
-                                                  PretenureFlag pretenure);
+  MUST_USE_RESULT AllocationResult AllocateBigInt(int length);
 
   // Allocates a byte array of the specified length
   MUST_USE_RESULT AllocationResult

src/objects-debug.cc

@@ -13,7 +13,7 @@
 #include "src/layout-descriptor.h"
 #include "src/macro-assembler.h"
 #include "src/objects-inl.h"
-#include "src/objects/bigint-inl.h"
+#include "src/objects/bigint.h"
 #include "src/objects/debug-objects-inl.h"
 #include "src/objects/literal-objects.h"
 #include "src/objects/module.h"

src/objects-inl.h

@@ -33,7 +33,7 @@
 #include "src/lookup.h"
 #include "src/objects.h"
 #include "src/objects/arguments-inl.h"
-#include "src/objects/bigint-inl.h"
+#include "src/objects/bigint.h"
 #include "src/objects/hash-table-inl.h"
 #include "src/objects/hash-table.h"
 #include "src/objects/js-array-inl.h"
@@ -74,6 +74,7 @@ int PropertyDetails::field_width_in_words() const {
   return representation().IsDouble() ? kDoubleSize / kPointerSize : 1;
 }
 
+TYPE_CHECKER(BigInt, BIGINT_TYPE)
 TYPE_CHECKER(BreakPoint, TUPLE2_TYPE)
 TYPE_CHECKER(BreakPointInfo, TUPLE2_TYPE)
 TYPE_CHECKER(ByteArray, BYTE_ARRAY_TYPE)

src/objects/bigint-inl.h

-// 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.
-
-#ifndef V8_OBJECTS_BIGINT_INL_H_
-#define V8_OBJECTS_BIGINT_INL_H_
-
-#include "src/objects/bigint.h"
-
-#include "src/objects.h"
-
-// Has to be the last include (doesn't have include guards):
-#include "src/objects/object-macros.h"
-
-namespace v8 {
-namespace internal {
-
-int BigInt::length() const {
-  intptr_t bitfield = READ_INTPTR_FIELD(this, kBitfieldOffset);
-  return LengthBits::decode(static_cast<uint32_t>(bitfield));
-}
-void BigInt::set_length(int new_length) {
-  intptr_t bitfield = READ_INTPTR_FIELD(this, kBitfieldOffset);
-  bitfield = LengthBits::update(static_cast<uint32_t>(bitfield), new_length);
-  WRITE_INTPTR_FIELD(this, kBitfieldOffset, bitfield);
-}
-
-bool BigInt::sign() const {
-  intptr_t bitfield = READ_INTPTR_FIELD(this, kBitfieldOffset);
-  return SignBits::decode(static_cast<uint32_t>(bitfield));
-}
-void BigInt::set_sign(bool new_sign) {
-  intptr_t bitfield = READ_INTPTR_FIELD(this, kBitfieldOffset);
-  bitfield = SignBits::update(static_cast<uint32_t>(bitfield), new_sign);
-  WRITE_INTPTR_FIELD(this, kBitfieldOffset, bitfield);
-}
-
-BigInt::digit_t BigInt::digit(int n) const {
-  SLOW_DCHECK(0 <= n && n < length());
-  const byte* address = FIELD_ADDR_CONST(this, kDigitsOffset + n * kDigitSize);
-  return *reinterpret_cast<digit_t*>(reinterpret_cast<intptr_t>(address));
-}
-void BigInt::set_digit(int n, digit_t value) {
-  SLOW_DCHECK(0 <= n && n < length());
-  byte* address = FIELD_ADDR(this, kDigitsOffset + n * kDigitSize);
-  (*reinterpret_cast<digit_t*>(reinterpret_cast<intptr_t>(address))) = value;
-}
-
-TYPE_CHECKER(BigInt, BIGINT_TYPE)
-
-}  // namespace internal
-}  // namespace v8
-
-#include "src/objects/object-macros-undef.h"
-
-#endif  // V8_OBJECTS_BIGINT_INL_H_

src/v8.gyp

@@ -1149,7 +1149,6 @@
         'objects.h',
         'objects/arguments-inl.h',
         'objects/arguments.h',
-        'objects/bigint-inl.h',
         'objects/bigint.cc',
         'objects/bigint.h',
         'objects/code-inl.h',

test/cctest/test-object.cc

@@ -57,7 +57,9 @@ TEST(NoSideEffectsToString) {
   CheckBoolean(isolate, true, "true");
   CheckBoolean(isolate, false, "false");
   CheckBoolean(isolate, false, "false");
-  CheckObject(isolate, factory->NewBigIntFromInt(42), "42");
+  Handle<Object> smi_42 = handle(Smi::FromInt(42), isolate);
+  CheckObject(isolate, BigInt::FromNumber(isolate, smi_42).ToHandleChecked(),
+              "42");
   CheckObject(isolate, factory->undefined_value(), "undefined");
   CheckObject(isolate, factory->null_value(), "null");
 

test/unittests/bigint-unittest.cc

@@ -8,7 +8,7 @@
 #include "src/factory.h"
 #include "src/isolate.h"
 #include "src/objects-inl.h"
-#include "src/objects/bigint-inl.h"
+#include "src/objects/bigint.h"
 #include "test/unittests/test-utils.h"
 
 #include "testing/gtest/include/gtest/gtest.h"
@@ -22,11 +22,15 @@ void Compare(Handle<BigInt> x, double value, ComparisonResult expected) {
   CHECK_EQ(expected, BigInt::CompareToDouble(x, value));
 }
 
+Handle<BigInt> NewFromInt(Isolate* isolate, int value) {
+  Handle<Smi> smi_value = handle(Smi::FromInt(value), isolate);
+  return BigInt::FromNumber(isolate, smi_value).ToHandleChecked();
+}
+
 TEST_F(BigIntWithIsolate, CompareToDouble) {
-  Factory* factory = isolate()->factory();
-  Handle<BigInt> zero = factory->NewBigIntFromInt(0);
-  Handle<BigInt> one = factory->NewBigIntFromInt(1);
-  Handle<BigInt> minus_one = factory->NewBigIntFromInt(-1);
+  Handle<BigInt> zero = NewFromInt(isolate(), 0);
+  Handle<BigInt> one = NewFromInt(isolate(), 1);
+  Handle<BigInt> minus_one = NewFromInt(isolate(), -1);
 
   // Non-finite doubles.
   Compare(zero, std::nan(""), ComparisonResult::kUndefined);
@@ -53,8 +57,8 @@ TEST_F(BigIntWithIsolate, CompareToDouble) {
   Compare(minus_one, -0.5, ComparisonResult::kLessThan);
 
   // Different bit lengths.
-  Handle<BigInt> four = factory->NewBigIntFromInt(4);
-  Handle<BigInt> minus_five = factory->NewBigIntFromInt(-5);
+  Handle<BigInt> four = NewFromInt(isolate(), 4);
+  Handle<BigInt> minus_five = NewFromInt(isolate(), -5);
   Compare(four, 3.9, ComparisonResult::kGreaterThan);
   Compare(four, 1.5, ComparisonResult::kGreaterThan);
   Compare(four, 8, ComparisonResult::kLessThan);
@@ -90,7 +94,7 @@ TEST_F(BigIntWithIsolate, CompareToDouble) {
   // Same bit length, difference in fractional part.
   Compare(one, 1.5, ComparisonResult::kLessThan);
   Compare(minus_one, -1.25, ComparisonResult::kGreaterThan);
-  big = factory->NewBigIntFromInt(0xF00D00);
+  big = NewFromInt(isolate(), 0xF00D00);
   Compare(big, 15731968.125, ComparisonResult::kLessThan);
   Compare(big, 15731967.875, ComparisonResult::kGreaterThan);
   big = BigIntLiteral(isolate(), "0x123456789ab").ToHandleChecked();