[bigint] Make competing read/write operations on bitfield atomic.

GC needs to be able to read a bigint's length while the main thread may
change the length and the sign (bigints are intentionally mutable as
long as they haven't escaped to user code). Since both values are stored
in the same bitfield, we need to make these accesses atomic.

Also change right-trimming to not insert a filler when the object is
in large object space (it makes no sense there).

Bug: v8:8440
Change-Id: I72a1b6f1eda54566d3cfad554dda1a98ddd61975
Reviewed-on: https://chromium-review.googlesource.com/c/1337737
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57576}

src/objects-body-descriptors-inl.h

@@ -352,7 +352,7 @@ class BigInt::BodyDescriptor final : public BodyDescriptorBase {
                                  ObjectVisitor* v) {}
 
   static inline int SizeOf(Map map, HeapObject* obj) {
-    return BigInt::SizeFor(BigInt::cast(obj)->length());
+    return BigInt::SizeFor(BigInt::cast(obj)->synchronized_length());
   }
 };
 

src/objects/bigint.cc

@@ -183,14 +183,14 @@ class MutableBigInt : public FreshlyAllocatedBigInt,
 // Internal field setters. Non-mutable BigInts don't have these.
 #include "src/objects/object-macros.h"
   inline void set_sign(bool new_sign) {
-    intptr_t bitfield = READ_INTPTR_FIELD(this, kBitfieldOffset);
+    intptr_t bitfield = RELAXED_READ_INTPTR_FIELD(this, kBitfieldOffset);
     bitfield = SignBits::update(static_cast<uint32_t>(bitfield), new_sign);
-    WRITE_INTPTR_FIELD(this, kBitfieldOffset, bitfield);
+    RELAXED_WRITE_INTPTR_FIELD(this, kBitfieldOffset, bitfield);
   }
-  inline void set_length(int new_length) {
-    intptr_t bitfield = READ_INTPTR_FIELD(this, kBitfieldOffset);
+  inline void synchronized_set_length(int new_length) {
+    intptr_t bitfield = RELAXED_READ_INTPTR_FIELD(this, kBitfieldOffset);
     bitfield = LengthBits::update(static_cast<uint32_t>(bitfield), new_length);
-    WRITE_INTPTR_FIELD(this, kBitfieldOffset, bitfield);
+    RELEASE_WRITE_INTPTR_FIELD(this, kBitfieldOffset, bitfield);
   }
   inline void initialize_bitfield(bool sign, int length) {
     intptr_t bitfield = LengthBits::encode(length) | SignBits::encode(sign);
@@ -340,8 +340,13 @@ Handle<BigInt> MutableBigInt::MakeImmutable(Handle<MutableBigInt> result) {
     int size_delta = to_trim * kDigitSize;
     Address new_end = result->address() + BigInt::SizeFor(new_length);
     Heap* heap = result->GetHeap();
-    heap->CreateFillerObjectAt(new_end, size_delta, ClearRecordedSlots::kNo);
-    result->set_length(new_length);
+    if (!heap->lo_space()->Contains(*result)) {
+      // We do not create a filler for objects in large object space.
+      // TODO(hpayer): We should shrink the large object page if the size
+      // of the object changed significantly.
+      heap->CreateFillerObjectAt(new_end, size_delta, ClearRecordedSlots::kNo);
+    }
+    result->synchronized_set_length(new_length);
 
     // Canonicalize -0n.
     if (new_length == 0) {

src/objects/bigint.h

@@ -24,7 +24,13 @@ class ValueSerializer;
 class BigIntBase : public HeapObject {
  public:
   inline int length() const {
-    intptr_t bitfield = READ_INTPTR_FIELD(this, kBitfieldOffset);
+    intptr_t bitfield = RELAXED_READ_INTPTR_FIELD(this, kBitfieldOffset);
+    return LengthBits::decode(static_cast<uint32_t>(bitfield));
+  }
+
+  // For use by the GC.
+  inline int synchronized_length() const {
+    intptr_t bitfield = ACQUIRE_READ_INTPTR_FIELD(this, kBitfieldOffset);
     return LengthBits::decode(static_cast<uint32_t>(bitfield));
   }
 
@@ -32,6 +38,8 @@ class BigIntBase : public HeapObject {
   static const int kMaxLengthBits = kMaxInt - kPointerSize * kBitsPerByte - 1;
   static const int kMaxLength = kMaxLengthBits / (kPointerSize * kBitsPerByte);
 
+  // Sign and length are stored in the same bitfield.  Since the GC needs to be
+  // able to read the length concurrently, the getters and setters are atomic.
   static const int kLengthFieldBits = 30;
   STATIC_ASSERT(kMaxLength <= ((1 << kLengthFieldBits) - 1));
   class SignBits : public BitField<bool, 0, 1> {};
@@ -57,7 +65,7 @@ class BigIntBase : public HeapObject {
 
   // sign() == true means negative.
   inline bool sign() const {
-    intptr_t bitfield = READ_INTPTR_FIELD(this, kBitfieldOffset);
+    intptr_t bitfield = RELAXED_READ_INTPTR_FIELD(this, kBitfieldOffset);
     return SignBits::decode(static_cast<uint32_t>(bitfield));
   }
 

src/objects/object-macros.h

@@ -354,6 +354,10 @@
 #define WRITE_INT_FIELD(p, offset, value) \
   (*reinterpret_cast<int*>(FIELD_ADDR(p, offset)) = value)
 
+#define ACQUIRE_READ_INTPTR_FIELD(p, offset) \
+  static_cast<intptr_t>(base::Acquire_Load(  \
+      reinterpret_cast<const base::AtomicWord*>(FIELD_ADDR(p, offset))))
+
 #define RELAXED_READ_INTPTR_FIELD(p, offset) \
   static_cast<intptr_t>(base::Relaxed_Load(  \
       reinterpret_cast<const base::AtomicWord*>(FIELD_ADDR(p, offset))))
@@ -361,6 +365,11 @@
 #define READ_INTPTR_FIELD(p, offset) \
   (*reinterpret_cast<const intptr_t*>(FIELD_ADDR(p, offset)))
 
+#define RELEASE_WRITE_INTPTR_FIELD(p, offset, value)              \
+  base::Release_Store(                                            \
+      reinterpret_cast<base::AtomicWord*>(FIELD_ADDR(p, offset)), \
+      static_cast<base::AtomicWord>(value));
+
 #define RELAXED_WRITE_INTPTR_FIELD(p, offset, value)              \
   base::Relaxed_Store(                                            \
       reinterpret_cast<base::AtomicWord*>(FIELD_ADDR(p, offset)), \

test/mjsunit/harmony/bigint/misc.js

+// Copyright 2018 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.
+
+// Create BigInt in large object space for which MakeImmutable reduces the
+// length.
+const x = 2n ** (2n ** 22n);
+assertEquals(1n, x - (x - 1n));