AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

heap: Recalculate the object start bitmap if needed

Browse Source 
This CL adds to the existing experimental implementation of the
object start bitmap, that is evaluated as a mechanism for resolving
inner pointers (behind the flag v8_enable_conservative_stack_scanning).

It fixes method ObjectStartBitmap::FindBasePtr to ensure that the
correct base pointer is returned, even if the bitmap is not fully
populated (e.g., with object evacuation or inline object allocation).
This method now recalculates the part of the bitmap that is
required for returning the correct result, by iterating through
objects of the page. A special constructor has been introduced to the
PagedSpaceObjectIterator for this purpose.

It also moves the existing inline methods of ObjectStartBitmap to a
new -inl.h header file, to avoid circular dependencies.

Bug: v8:12851
Change-Id: Iabd0df020bee3bb63ef9d4888591b25d24d79dd9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3641179
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80538}
This commit is contained in:
Nikolaos Papaspyrou 2022-05-13 21:32:15 +02:00 committed by V8 LUCI CQ
parent f6c8cd8dac
commit 36610bbdd7
8 changed files with 250 additions and 161 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
BUILD.gn
View File

@ -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",
]
}

4
src/heap/heap-allocator-inl.h
View File

@ -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 {

146
src/heap/object-start-bitmap-inl.h Normal file
View File

@ -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 <limits.h>
#include <stdint.h>
#include <array>
#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<uint32_t>((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<PagedSpace*>(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<uint32_t>(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<uint32_t>(~(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<uint32_t>(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 <typename Callback>
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_

124
src/heap/object-start-bitmap.h
View File

@ -5,15 +5,10 @@
#ifndef V8_HEAP_OBJECT_START_BITMAP_H_
#define V8_HEAP_OBJECT_START_BITMAP_H_
#include <limits.h>
#include <stdint.h>
#include <array>
#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<uint32_t, kReservedForBitmap> 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<uint32_t>((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<uint32_t>(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<uint32_t>(~(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<uint32_t>(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 <typename Callback>
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

23
src/heap/paged-spaces.cc
View File

@ -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

2
src/heap/paged-spaces.h
View File

@ -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.

82
test/cctest/heap/test-mark-compact.cc
View File

@ -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<String> h1 = factory->NewStringFromStaticChars("hello");
Handle<String> h2 = factory->NewStringFromStaticChars("world");
Handle<HeapObject> h1 = factory->NewStringFromStaticChars("hello");
Handle<HeapObject> 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
}

29
test/unittests/heap/object-start-bitmap-unittest.cc
View File

@ -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