diff --git a/BUILD.gn b/BUILD.gn index 4177f2dacf..7da6e68730 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -3644,6 +3644,7 @@ v8_header_set("v8_internal_headers") { if (v8_enable_conservative_stack_scanning) { sources += [ "src/heap/conservative-stack-visitor.h", + "src/heap/object-start-bitmap-inl.h", "src/heap/object-start-bitmap.h", ] } diff --git a/src/heap/heap-allocator-inl.h b/src/heap/heap-allocator-inl.h index 7d8d06ba6f..4bc6d2506d 100644 --- a/src/heap/heap-allocator-inl.h +++ b/src/heap/heap-allocator-inl.h @@ -15,6 +15,10 @@ #include "src/heap/read-only-spaces.h" #include "src/heap/third-party/heap-api.h" +#ifdef V8_ENABLE_CONSERVATIVE_STACK_SCANNING +#include "src/heap/object-start-bitmap-inl.h" +#endif + namespace v8 { namespace internal { diff --git a/src/heap/object-start-bitmap-inl.h b/src/heap/object-start-bitmap-inl.h new file mode 100644 index 0000000000..8336fdeac0 --- /dev/null +++ b/src/heap/object-start-bitmap-inl.h @@ -0,0 +1,146 @@ +// 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. + +#ifndef V8_HEAP_OBJECT_START_BITMAP_INL_H_ +#define V8_HEAP_OBJECT_START_BITMAP_INL_H_ + +#include +#include + +#include + +#include "include/v8-internal.h" +#include "src/base/bits.h" +#include "src/base/macros.h" +#include "src/common/globals.h" +#include "src/heap/object-start-bitmap.h" +#include "src/heap/paged-spaces-inl.h" +#include "src/heap/paged-spaces.h" + +namespace v8 { +namespace internal { + +ObjectStartBitmap::ObjectStartBitmap(size_t offset) : offset_(offset) { + Clear(); +} + +Address ObjectStartBitmap::FindBasePtrImpl(Address maybe_inner_ptr) const { + DCHECK_LE(offset(), maybe_inner_ptr); + size_t object_offset = maybe_inner_ptr - offset(); + size_t object_start_number = object_offset / kAllocationGranularity; + size_t cell_index = object_start_number / kBitsPerCell; + DCHECK_GT(object_start_bit_map_.size(), cell_index); + const size_t bit = object_start_number & kCellMask; + // check if maybe_inner_ptr is the base pointer + uint32_t byte = + load(cell_index) & static_cast((1 << (bit + 1)) - 1); + while (byte == 0 && cell_index > 0) { + byte = load(--cell_index); + } + if (byte == 0) { + DCHECK_EQ(0, cell_index); + return kNullAddress; + } + const int leading_zeroes = v8::base::bits::CountLeadingZeros(byte); + DCHECK_GT(kBitsPerCell, leading_zeroes); + object_start_number = + (cell_index * kBitsPerCell) + (kBitsPerCell - 1) - leading_zeroes; + return StartIndexToAddress(object_start_number); +} + +Address ObjectStartBitmap::FindBasePtr(Address maybe_inner_ptr) { + Address baseptr = FindBasePtrImpl(maybe_inner_ptr); + if (baseptr == maybe_inner_ptr) { + DCHECK(CheckBit(baseptr)); + return baseptr; + } + // TODO(v8:12851): If the ObjectStartBitmap implementation stays, this part of + // code involving Page and the PagedSpaceObjectIterator is its only connection + // with V8 internals. It should be moved to some different abstraction. + Page* page = Page::FromAddress(offset_); + DCHECK_EQ(page->area_start(), offset_); + if (baseptr == kNullAddress) baseptr = offset_; + DCHECK_LE(baseptr, maybe_inner_ptr); + PagedSpaceObjectIterator it( + page->heap(), static_cast(page->owner()), page, baseptr); + for (HeapObject obj = it.Next(); !obj.is_null(); obj = it.Next()) { + Address start = obj.address(); + SetBit(start); + if (maybe_inner_ptr < start) break; + if (maybe_inner_ptr < start + obj.Size()) return start; + } + return kNullAddress; +} + +void ObjectStartBitmap::SetBit(Address base_ptr) { + size_t cell_index, object_bit; + ObjectStartIndexAndBit(base_ptr, &cell_index, &object_bit); + store(cell_index, load(cell_index) | static_cast(1 << object_bit)); +} + +void ObjectStartBitmap::ClearBit(Address base_ptr) { + size_t cell_index, object_bit; + ObjectStartIndexAndBit(base_ptr, &cell_index, &object_bit); + store(cell_index, + load(cell_index) & static_cast(~(1 << object_bit))); +} + +bool ObjectStartBitmap::CheckBit(Address base_ptr) const { + size_t cell_index, object_bit; + ObjectStartIndexAndBit(base_ptr, &cell_index, &object_bit); + return (load(cell_index) & static_cast(1 << object_bit)) != 0; +} + +void ObjectStartBitmap::store(size_t cell_index, uint32_t value) { + object_start_bit_map_[cell_index] = value; +} + +uint32_t ObjectStartBitmap::load(size_t cell_index) const { + return object_start_bit_map_[cell_index]; +} + +Address ObjectStartBitmap::offset() const { return offset_; } + +void ObjectStartBitmap::ObjectStartIndexAndBit(Address base_ptr, + size_t* cell_index, + size_t* bit) const { + const size_t object_offset = base_ptr - offset(); + DCHECK(!(object_offset & kAllocationMask)); + const size_t object_start_number = object_offset / kAllocationGranularity; + *cell_index = object_start_number / kBitsPerCell; + DCHECK_GT(kBitmapSize, *cell_index); + *bit = object_start_number & kCellMask; +} + +Address ObjectStartBitmap::StartIndexToAddress( + size_t object_start_index) const { + return offset() + (kAllocationGranularity * object_start_index); +} + +template +inline void ObjectStartBitmap::Iterate(Callback callback) const { + for (size_t cell_index = 0; cell_index < kReservedForBitmap; cell_index++) { + uint32_t value = load(cell_index); + while (value != 0) { + const int trailing_zeroes = + v8::base::bits::CountTrailingZerosNonZero(value); + DCHECK_GT(kBitsPerCell, trailing_zeroes); + const size_t object_start_number = + (cell_index * kBitsPerCell) + trailing_zeroes; + const Address object_address = StartIndexToAddress(object_start_number); + callback(object_address); + // Clear current object bit in temporary value to advance iteration. + value &= value - 1; + } + } +} + +void ObjectStartBitmap::Clear() { + std::fill(object_start_bit_map_.begin(), object_start_bit_map_.end(), 0); +} + +} // namespace internal +} // namespace v8 + +#endif // V8_HEAP_OBJECT_START_BITMAP_INL_H_ diff --git a/src/heap/object-start-bitmap.h b/src/heap/object-start-bitmap.h index b15e1c58c4..05c9229caa 100644 --- a/src/heap/object-start-bitmap.h +++ b/src/heap/object-start-bitmap.h @@ -5,15 +5,10 @@ #ifndef V8_HEAP_OBJECT_START_BITMAP_H_ #define V8_HEAP_OBJECT_START_BITMAP_H_ -#include -#include - #include -#include "include/v8-internal.h" -#include "src/base/bits.h" -#include "src/base/macros.h" #include "src/common/globals.h" +#include "testing/gtest/include/gtest/gtest_prod.h" // nogncheck namespace v8 { namespace internal { @@ -43,11 +38,11 @@ class V8_EXPORT_PRIVATE ObjectStartBitmap { explicit inline ObjectStartBitmap(size_t offset = 0); - // Finds an object header based on a maybe_inner_ptr. Will search for an - // object start in decreasing address order. - // - // This must only be used when there exists at least one entry in the bitmap. - inline Address FindBasePtr(Address maybe_inner_ptr) const; + // Finds an object header based on a maybe_inner_ptr. If the object start + // bitmap is not fully populated, this iterates through the objects of the + // page to recalculate the part of the bitmap that is required, for this + // method to return the correct result. + inline Address FindBasePtr(Address maybe_inner_ptr); inline void SetBit(Address); inline void ClearBit(Address); @@ -82,106 +77,23 @@ class V8_EXPORT_PRIVATE ObjectStartBitmap { inline Address StartIndexToAddress(size_t object_start_index) const; + // Finds an object header based on a maybe_inner_ptr. Will search for an + // object start in decreasing address order. If the object start bitmap is + // not fully populated, this may incorrectly return |kNullPointer| or the base + // pointer of a previous object on the page. + inline Address FindBasePtrImpl(Address maybe_inner_ptr) const; + size_t offset_; std::array object_start_bit_map_; + + FRIEND_TEST(V8ObjectStartBitmapTest, FindBasePtrExact); + FRIEND_TEST(V8ObjectStartBitmapTest, FindBasePtrApproximate); + FRIEND_TEST(V8ObjectStartBitmapTest, FindBasePtrIteratingWholeBitmap); + FRIEND_TEST(V8ObjectStartBitmapTest, FindBasePtrNextCell); + FRIEND_TEST(V8ObjectStartBitmapTest, FindBasePtrSameCell); }; -ObjectStartBitmap::ObjectStartBitmap(size_t offset) : offset_(offset) { - Clear(); -} - -Address ObjectStartBitmap::FindBasePtr(Address maybe_inner_ptr) const { - DCHECK_LE(offset(), maybe_inner_ptr); - size_t object_offset = maybe_inner_ptr - offset(); - size_t object_start_number = object_offset / kAllocationGranularity; - size_t cell_index = object_start_number / kBitsPerCell; - DCHECK_GT(object_start_bit_map_.size(), cell_index); - const size_t bit = object_start_number & kCellMask; - // check if maybe_inner_ptr is the base pointer - uint32_t byte = - load(cell_index) & static_cast((1 << (bit + 1)) - 1); - while (byte == 0 && cell_index > 0) { - byte = load(--cell_index); - } - if (byte == 0) { - DCHECK_EQ(0, cell_index); - return kNullAddress; - } - const int leading_zeroes = v8::base::bits::CountLeadingZeros(byte); - DCHECK_GT(kBitsPerCell, leading_zeroes); - object_start_number = - (cell_index * kBitsPerCell) + (kBitsPerCell - 1) - leading_zeroes; - return StartIndexToAddress(object_start_number); -} - -void ObjectStartBitmap::SetBit(Address base_ptr) { - size_t cell_index, object_bit; - ObjectStartIndexAndBit(base_ptr, &cell_index, &object_bit); - store(cell_index, load(cell_index) | static_cast(1 << object_bit)); -} - -void ObjectStartBitmap::ClearBit(Address base_ptr) { - size_t cell_index, object_bit; - ObjectStartIndexAndBit(base_ptr, &cell_index, &object_bit); - store(cell_index, - load(cell_index) & static_cast(~(1 << object_bit))); -} - -bool ObjectStartBitmap::CheckBit(Address base_ptr) const { - size_t cell_index, object_bit; - ObjectStartIndexAndBit(base_ptr, &cell_index, &object_bit); - return (load(cell_index) & static_cast(1 << object_bit)) != 0; -} - -void ObjectStartBitmap::store(size_t cell_index, uint32_t value) { - object_start_bit_map_[cell_index] = value; -} - -uint32_t ObjectStartBitmap::load(size_t cell_index) const { - return object_start_bit_map_[cell_index]; -} - -Address ObjectStartBitmap::offset() const { return offset_; } - -void ObjectStartBitmap::ObjectStartIndexAndBit(Address base_ptr, - size_t* cell_index, - size_t* bit) const { - const size_t object_offset = base_ptr - offset(); - DCHECK(!(object_offset & kAllocationMask)); - const size_t object_start_number = object_offset / kAllocationGranularity; - *cell_index = object_start_number / kBitsPerCell; - DCHECK_GT(kBitmapSize, *cell_index); - *bit = object_start_number & kCellMask; -} - -Address ObjectStartBitmap::StartIndexToAddress( - size_t object_start_index) const { - return offset() + (kAllocationGranularity * object_start_index); -} - -template -inline void ObjectStartBitmap::Iterate(Callback callback) const { - for (size_t cell_index = 0; cell_index < kReservedForBitmap; cell_index++) { - uint32_t value = load(cell_index); - while (value != 0) { - const int trailing_zeroes = - v8::base::bits::CountTrailingZerosNonZero(value); - DCHECK_GT(kBitsPerCell, trailing_zeroes); - const size_t object_start_number = - (cell_index * kBitsPerCell) + trailing_zeroes; - const Address object_address = StartIndexToAddress(object_start_number); - callback(object_address); - // Clear current object bit in temporary value to advance iteration. - value &= value - 1; - } - } -} - -void ObjectStartBitmap::Clear() { - std::fill(object_start_bit_map_.begin(), object_start_bit_map_.end(), 0); -} - } // namespace internal } // namespace v8 diff --git a/src/heap/paged-spaces.cc b/src/heap/paged-spaces.cc index 8640d001d4..f823dafded 100644 --- a/src/heap/paged-spaces.cc +++ b/src/heap/paged-spaces.cc @@ -59,10 +59,25 @@ PagedSpaceObjectIterator::PagedSpaceObjectIterator(Heap* heap, #endif // V8_COMPRESS_POINTERS { heap->MakeHeapIterable(); -#ifdef DEBUG - AllocationSpace owner = page->owner_identity(); - DCHECK(owner == OLD_SPACE || owner == MAP_SPACE || owner == CODE_SPACE); -#endif // DEBUG +} + +PagedSpaceObjectIterator::PagedSpaceObjectIterator(Heap* heap, + const PagedSpace* space, + const Page* page, + Address start_address) + : cur_addr_(start_address), + cur_end_(page->area_end()), + space_(space), + page_range_(page, page), + current_page_(page_range_.begin()) +#if V8_COMPRESS_POINTERS + , + cage_base_(heap->isolate()) +#endif // V8_COMPRESS_POINTERS +{ + heap->MakeHeapIterable(); + DCHECK(page->Contains(start_address)); + DCHECK(page->SweepingDone()); } // We have hit the end of the page and should advance to the next block of diff --git a/src/heap/paged-spaces.h b/src/heap/paged-spaces.h index 16d0b668d1..21ab80a599 100644 --- a/src/heap/paged-spaces.h +++ b/src/heap/paged-spaces.h @@ -44,6 +44,8 @@ class V8_EXPORT_PRIVATE PagedSpaceObjectIterator : public ObjectIterator { PagedSpaceObjectIterator(Heap* heap, const PagedSpace* space); PagedSpaceObjectIterator(Heap* heap, const PagedSpace* space, const Page* page); + PagedSpaceObjectIterator(Heap* heap, const PagedSpace* space, + const Page* page, Address start_address); // Advance to the next object, skipping free spaces and other fillers and // skipping the special garbage section of which there is one per space. diff --git a/test/cctest/heap/test-mark-compact.cc b/test/cctest/heap/test-mark-compact.cc index d08727d0e4..cc68c2c05d 100644 --- a/test/cctest/heap/test-mark-compact.cc +++ b/test/cctest/heap/test-mark-compact.cc @@ -237,10 +237,8 @@ HEAP_TEST(ObjectStartBitmap) { auto* factory = isolate->factory(); - // TODO(v8:12851): Using handles because conservative stack scanning currently - // does not work. - Handle h1 = factory->NewStringFromStaticChars("hello"); - Handle h2 = factory->NewStringFromStaticChars("world"); + Handle h1 = factory->NewStringFromStaticChars("hello"); + Handle h2 = factory->NewStringFromStaticChars("world"); HeapObject obj1 = *h1; HeapObject obj2 = *h2; @@ -250,53 +248,61 @@ HEAP_TEST(ObjectStartBitmap) { CHECK(page1->object_start_bitmap()->CheckBit(obj1.address())); CHECK(page2->object_start_bitmap()->CheckBit(obj2.address())); - for (int k = 0; k < obj1.Size(); ++k) { - Address obj1_inner_ptr = obj1.address() + k; - CHECK_EQ(obj1.address(), - page1->object_start_bitmap()->FindBasePtr(obj1_inner_ptr)); - } - for (int k = 0; k < obj2.Size(); ++k) { - Address obj2_inner_ptr = obj2.address() + k; - CHECK_EQ(obj2.address(), - page2->object_start_bitmap()->FindBasePtr(obj2_inner_ptr)); + { + // We need a safepoint for calling FindBasePtr. + SafepointScope scope(heap); + + for (int k = 0; k < obj1.Size(); ++k) { + Address obj1_inner_ptr = obj1.address() + k; + CHECK_EQ(obj1.address(), + page1->object_start_bitmap()->FindBasePtr(obj1_inner_ptr)); + } + for (int k = 0; k < obj2.Size(); ++k) { + Address obj2_inner_ptr = obj2.address() + k; + CHECK_EQ(obj2.address(), + page2->object_start_bitmap()->FindBasePtr(obj2_inner_ptr)); + } } + // TODO(v8:12851): Patch the location of handle h2 with an inner pointer. + // For now, garbage collection doesn't work with inner pointers in handles, + // so we're sticking to a zero offset. + const size_t offset = 0; + h2.PatchValue(String::FromAddress(h2->address() + offset)); + CcTest::CollectAllGarbage(); obj1 = *h1; - obj2 = *h2; + obj2 = HeapObject::FromAddress(h2->address() - offset); page1 = Page::FromAddress(obj1.ptr()); page2 = Page::FromAddress(obj2.ptr()); CHECK(obj1.IsString()); CHECK(obj2.IsString()); - // TODO(v8:12851): Bits set in the object_start_bitmap are currently not - // preserved when objects are evacuated. For this reason, in the following - // checks, FindBasePtr is expected to return null after garbage collection. - for (int k = 0; k < obj1.Size(); ++k) { - Address obj1_inner_ptr = obj1.address() + k; - CHECK_EQ(kNullAddress, - page1->object_start_bitmap()->FindBasePtr(obj1_inner_ptr)); - } - for (int k = 0; k < obj2.Size(); ++k) { - Address obj2_inner_ptr = obj2.address() + k; - CHECK_EQ(kNullAddress, - page2->object_start_bitmap()->FindBasePtr(obj2_inner_ptr)); - } - - obj1 = *h1; - obj2 = *h2; - page1 = Page::FromAddress(obj1.ptr()); - page2 = Page::FromAddress(obj2.ptr()); - - CHECK(obj1.IsString()); - CHECK(obj2.IsString()); - - // TODO(v8:12851): Bits set in the object_start_bitmap are currently not - // preserved when objects are evacuated. + // Bits set in the object_start_bitmap are not preserved when objects are + // evacuated. CHECK(!page1->object_start_bitmap()->CheckBit(obj1.address())); CHECK(!page2->object_start_bitmap()->CheckBit(obj2.address())); + + { + // We need a safepoint for calling FindBasePtr. + SafepointScope scope(heap); + + // After FindBasePtr, the bits should be properly set again. + for (int k = 0; k < obj1.Size(); ++k) { + Address obj1_inner_ptr = obj1.address() + k; + CHECK_EQ(obj1.address(), + page1->object_start_bitmap()->FindBasePtr(obj1_inner_ptr)); + } + CHECK(page1->object_start_bitmap()->CheckBit(obj1.address())); + for (int k = obj2.Size() - 1; k >= 0; --k) { + Address obj2_inner_ptr = obj2.address() + k; + CHECK_EQ(obj2.address(), + page2->object_start_bitmap()->FindBasePtr(obj2_inner_ptr)); + } + CHECK(page2->object_start_bitmap()->CheckBit(obj2.address())); + } #endif } diff --git a/test/unittests/heap/object-start-bitmap-unittest.cc b/test/unittests/heap/object-start-bitmap-unittest.cc index 8e57d86d95..7177fa8410 100644 --- a/test/unittests/heap/object-start-bitmap-unittest.cc +++ b/test/unittests/heap/object-start-bitmap-unittest.cc @@ -5,13 +5,12 @@ #include "src/heap/object-start-bitmap.h" #include "src/base/macros.h" +#include "src/heap/object-start-bitmap-inl.h" #include "testing/gtest/include/gtest/gtest.h" namespace v8 { namespace internal { -class ObjectStartBitmap; - namespace { bool IsEmpty(const ObjectStartBitmap& bitmap) { @@ -128,47 +127,51 @@ TEST(V8ObjectStartBitmapTest, FindBasePtrExact) { ObjectStartBitmap bitmap(TestObject::kBaseOffset); TestObject object(654); bitmap.SetBit(object); - EXPECT_EQ(object.base_ptr(), bitmap.FindBasePtr(object.base_ptr())); + EXPECT_EQ(object.base_ptr(), bitmap.FindBasePtrImpl(object.base_ptr())); } TEST(V8ObjectStartBitmapTest, FindBasePtrApproximate) { - static const size_t kInternalDelta = 37; + const size_t kInternalDelta = 37; ObjectStartBitmap bitmap(TestObject::kBaseOffset); TestObject object(654); bitmap.SetBit(object); EXPECT_EQ(object.base_ptr(), - bitmap.FindBasePtr(object.base_ptr() + kInternalDelta)); + bitmap.FindBasePtrImpl(object.base_ptr() + kInternalDelta)); } TEST(V8ObjectStartBitmapTest, FindBasePtrIteratingWholeBitmap) { + const size_t kLastWordDelta = ObjectStartBitmap::MaxEntries() - 1; ObjectStartBitmap bitmap(TestObject::kBaseOffset); - TestObject object_to_find(TestObject(0)); - Address hint_index = TestObject(ObjectStartBitmap::MaxEntries() - 1); + TestObject object_to_find(0); bitmap.SetBit(object_to_find); - EXPECT_EQ(object_to_find.base_ptr(), bitmap.FindBasePtr(hint_index)); + Address hint_index = TestObject(kLastWordDelta); + EXPECT_EQ(object_to_find.base_ptr(), bitmap.FindBasePtrImpl(hint_index)); } TEST(V8ObjectStartBitmapTest, FindBasePtrNextCell) { // This white box test makes use of the fact that cells are of type uint32_t. const size_t kCellSize = sizeof(uint32_t); ObjectStartBitmap bitmap(TestObject::kBaseOffset); - TestObject object_to_find(TestObject(kCellSize - 1)); + TestObject object_to_find(kCellSize - 1); Address hint = TestObject(kCellSize); bitmap.SetBit(TestObject(0)); bitmap.SetBit(object_to_find); - EXPECT_EQ(object_to_find.base_ptr(), bitmap.FindBasePtr(hint)); + EXPECT_EQ(object_to_find.base_ptr(), bitmap.FindBasePtrImpl(hint)); } TEST(V8ObjectStartBitmapTest, FindBasePtrSameCell) { // This white box test makes use of the fact that cells are of type uint32_t. const size_t kCellSize = sizeof(uint32_t); ObjectStartBitmap bitmap(TestObject::kBaseOffset); - TestObject object_to_find(TestObject(kCellSize - 1)); + TestObject object_to_find(kCellSize - 1); + Address hint = object_to_find; bitmap.SetBit(TestObject(0)); bitmap.SetBit(object_to_find); - EXPECT_EQ(object_to_find.base_ptr(), - bitmap.FindBasePtr(object_to_find.base_ptr())); + EXPECT_EQ(object_to_find.base_ptr(), bitmap.FindBasePtrImpl(hint)); } +// TODO(v8:12851): If the ObjectStartBitmap implementation stays, unit tests +// should be added to test the functionality of method FindBasePtr. + } // namespace internal } // namespace v8