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

Reland "[heap] Skip read-only space in Heap::Contains"

Browse Source 
This is a reland of 2b24cd035a

Original change's description:
> [heap] Skip read-only space in Heap::Contains
> 
> Bug: v8:7464
> Change-Id: I27e82cdf0f8cc56ff68dcfaecab9644fe74916c7
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1559861
> Commit-Queue: Maciej Goszczycki <goszczycki@google.com>
> Reviewed-by: Dan Elphick <delphick@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#61350}

Bug: v8:7464
Change-Id: Ic5a9221f62537c1711c70b48fc0069288bfda80f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1601509
Reviewed-by: Dan Elphick <delphick@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Maciej Goszczycki <goszczycki@google.com>
Cr-Commit-Position: refs/heads/master@{#61489}
This commit is contained in:
Maciej Goszczycki 2019-05-13 17:13:44 +01:00 committed by Commit Bot
parent d2ea316f2a
commit 8dc7f24913
14 changed files with 38 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/arm/simulator-arm.cc
View File

@ -15,6 +15,7 @@
#include "src/base/bits.h"
#include "src/base/lazy-instance.h"
#include "src/disasm.h"
#include "src/heap/combined-heap.h"
#include "src/macro-assembler.h"
#include "src/objects-inl.h"
#include "src/ostreams.h"
@ -332,7 +333,8 @@ void ArmDebugger::Debug() {
reinterpret_cast<intptr_t>(cur), *cur, *cur);
Object obj(*cur);
Heap* current_heap = sim_->isolate_->heap();
if (obj.IsSmi() || current_heap->Contains(HeapObject::cast(obj))) {
if (obj.IsSmi() ||
IsValidHeapObject(current_heap, HeapObject::cast(obj))) {
PrintF(" (");
if (obj.IsSmi()) {
PrintF("smi %d", Smi::ToInt(obj));

4
src/arm64/simulator-arm64.cc
View File

@ -15,6 +15,7 @@
#include "src/assembler-inl.h"
#include "src/base/lazy-instance.h"
#include "src/disasm.h"
#include "src/heap/combined-heap.h"
#include "src/macro-assembler.h"
#include "src/objects-inl.h"
#include "src/ostreams.h"
@ -3289,7 +3290,8 @@ void Simulator::Debug() {
reinterpret_cast<uint64_t>(cur), *cur, *cur);
Object obj(*cur);
Heap* current_heap = isolate_->heap();
if (obj.IsSmi() || current_heap->Contains(HeapObject::cast(obj))) {
if (obj.IsSmi() ||
IsValidHeapObject(current_heap, HeapObject::cast(obj))) {
PrintF(" (");
if (obj.IsSmi()) {
PrintF("smi %" PRId32, Smi::ToInt(obj));

5
src/heap/combined-heap.h
View File

@ -30,6 +30,11 @@ class V8_EXPORT_PRIVATE CombinedHeapIterator final {
ReadOnlyHeapIterator ro_heap_iterator_;
};
V8_WARN_UNUSED_RESULT inline bool IsValidHeapObject(Heap* heap,
HeapObject object) {
return ReadOnlyHeap::Contains(object) || heap->Contains(object);
}
} // namespace internal
} // namespace v8

12
src/heap/heap.cc
View File

@ -25,6 +25,7 @@
#include "src/heap/array-buffer-tracker-inl.h"
#include "src/heap/barrier.h"
#include "src/heap/code-stats.h"
#include "src/heap/combined-heap.h"
#include "src/heap/concurrent-marking.h"
#include "src/heap/embedder-tracing.h"
#include "src/heap/gc-idle-time-handler.h"
@ -3595,13 +3596,6 @@ const char* Heap::GarbageCollectionReasonToString(
}
bool Heap::Contains(HeapObject value) {
// Check RO_SPACE first because IsOutsideAllocatedSpace cannot account for a
// shared RO_SPACE.
// TODO(v8:7464): Exclude read-only space. Use ReadOnlyHeap::Contains where
// appropriate.
if (read_only_space_ != nullptr && read_only_space_->Contains(value)) {
return true;
}
if (memory_allocator()->IsOutsideAllocatedSpace(value->address())) {
return false;
}
@ -4842,7 +4836,7 @@ void Heap::RegisterExternallyReferencedObject(Address* location) {
Object object(*location);
if (!object->IsHeapObject()) return;
HeapObject heap_object = HeapObject::cast(object);
DCHECK(Contains(heap_object));
DCHECK(IsValidHeapObject(this, heap_object));
if (FLAG_incremental_marking_wrappers && incremental_marking()->IsMarking()) {
incremental_marking()->WhiteToGreyAndPush(heap_object);
} else {
@ -5663,7 +5657,7 @@ void VerifyPointersVisitor::VisitRootPointers(Root root,
}
void VerifyPointersVisitor::VerifyHeapObjectImpl(HeapObject heap_object) {
CHECK(heap_->Contains(heap_object));
CHECK(IsValidHeapObject(heap_, heap_object));
CHECK(heap_object->map()->IsMap());
}

3
src/heap/spaces.cc
View File

@ -12,6 +12,7 @@
#include "src/base/template-utils.h"
#include "src/counters.h"
#include "src/heap/array-buffer-tracker.h"
#include "src/heap/combined-heap.h"
#include "src/heap/concurrent-marking.h"
#include "src/heap/gc-tracer.h"
#include "src/heap/heap-controller.h"
@ -3760,7 +3761,7 @@ void LargeObjectSpace::Verify(Isolate* isolate) {
Object element = array->get(j);
if (element->IsHeapObject()) {
HeapObject element_object = HeapObject::cast(element);
CHECK(heap()->Contains(element_object));
CHECK(IsValidHeapObject(heap(), element_object));
CHECK(element_object->map()->IsMap());
}
}

4
src/mips/simulator-mips.cc
View File

@ -16,6 +16,7 @@
#include "src/base/bits.h"
#include "src/base/lazy-instance.h"
#include "src/disasm.h"
#include "src/heap/combined-heap.h"
#include "src/macro-assembler.h"
#include "src/mips/constants-mips.h"
#include "src/ostreams.h"
@ -536,7 +537,8 @@ void MipsDebugger::Debug() {
reinterpret_cast<intptr_t>(cur), *cur, *cur);
Object obj(*cur);
Heap* current_heap = sim_->isolate_->heap();
if (obj.IsSmi() || current_heap->Contains(HeapObject::cast(obj))) {
if (obj.IsSmi() ||
IsValidHeapObject(current_heap, HeapObject::cast(obj))) {
PrintF(" (");
if (obj.IsSmi()) {
PrintF("smi %d", Smi::ToInt(obj));

4
src/mips64/simulator-mips64.cc
View File

@ -15,6 +15,7 @@
#include "src/assembler-inl.h"
#include "src/base/bits.h"
#include "src/disasm.h"
#include "src/heap/combined-heap.h"
#include "src/macro-assembler.h"
#include "src/mips64/constants-mips64.h"
#include "src/ostreams.h"
@ -466,7 +467,8 @@ void MipsDebugger::Debug() {
reinterpret_cast<intptr_t>(cur), *cur, *cur);
Object obj(*cur);
Heap* current_heap = sim_->isolate_->heap();
if (obj.IsSmi() || current_heap->Contains(HeapObject::cast(obj))) {
if (obj.IsSmi() ||
IsValidHeapObject(current_heap, HeapObject::cast(obj))) {
PrintF(" (");
if (obj.IsSmi()) {
PrintF("smi %d", Smi::ToInt(obj));

4
src/objects-debug.cc
View File

@ -12,6 +12,7 @@
#include "src/disassembler.h"
#include "src/elements.h"
#include "src/field-type.h"
#include "src/heap/combined-heap.h"
#include "src/heap/heap-write-barrier-inl.h"
#include "src/ic/handler-configuration-inl.h"
#include "src/layout-descriptor.h"
@ -488,8 +489,7 @@ void HeapObject::HeapObjectVerify(Isolate* isolate) {
// static
void HeapObject::VerifyHeapPointer(Isolate* isolate, Object p) {
CHECK(p->IsHeapObject());
HeapObject ho = HeapObject::cast(p);
CHECK(isolate->heap()->Contains(ho));
CHECK(IsValidHeapObject(isolate->heap(), HeapObject::cast(p)));
}
void Symbol::SymbolVerify(Isolate* isolate) {

3
src/objects/string.cc
View File

@ -8,6 +8,7 @@
#include "src/conversions.h"
#include "src/handles-inl.h"
#include "src/heap/heap-inl.h" // For LooksValid implementation.
#include "src/heap/read-only-heap.h"
#include "src/objects/map.h"
#include "src/objects/oddball.h"
#include "src/objects/string-comparator.h"
@ -396,7 +397,7 @@ bool String::LooksValid() {
// basically the same logic as the way we access the heap in the first place.
MemoryChunk* chunk = MemoryChunk::FromHeapObject(*this);
// RO_SPACE objects should always be valid.
if (chunk->owner()->identity() == RO_SPACE) return true;
if (ReadOnlyHeap::Contains(*this)) return true;
if (chunk->heap() == nullptr) return false;
return chunk->heap()->Contains(*this);
}

4
src/ppc/simulator-ppc.cc
View File

@ -14,6 +14,7 @@
#include "src/base/bits.h"
#include "src/base/lazy-instance.h"
#include "src/disasm.h"
#include "src/heap/combined-heap.h"
#include "src/macro-assembler.h"
#include "src/objects-inl.h"
#include "src/ostreams.h"
@ -401,7 +402,8 @@ void PPCDebugger::Debug() {
reinterpret_cast<intptr_t>(cur), *cur, *cur);
Object obj(*cur);
Heap* current_heap = sim_->isolate_->heap();
if (obj.IsSmi() || current_heap->Contains(HeapObject::cast(obj))) {
if (obj.IsSmi() ||
IsValidHeapObject(current_heap, HeapObject::cast(obj))) {
PrintF(" (");
if (obj.IsSmi()) {
PrintF("smi %d", Smi::ToInt(obj));

3
src/s390/simulator-s390.cc
View File

@ -15,6 +15,7 @@
#include "src/base/bits.h"
#include "src/base/once.h"
#include "src/disasm.h"
#include "src/heap/combined-heap.h"
#include "src/macro-assembler.h"
#include "src/objects-inl.h"
#include "src/ostreams.h"
@ -424,7 +425,7 @@ void S390Debugger::Debug() {
Heap* current_heap = sim_->isolate_->heap();
if (obj.IsSmi()) {
PrintF(" (smi %d)", Smi::ToInt(obj));
} else if (current_heap->Contains(HeapObject::cast(obj))) {
} else if (IsValidHeapObject(current_heap, HeapObject::cast(obj))) {
PrintF(" (");
obj->ShortPrint();
PrintF(")");

3
src/snapshot/partial-serializer.cc
View File

@ -6,6 +6,7 @@
#include "src/snapshot/startup-serializer.h"
#include "src/api-inl.h"
#include "src/heap/combined-heap.h"
#include "src/math-random.h"
#include "src/microtask-queue.h"
#include "src/objects-inl.h"
@ -171,7 +172,7 @@ bool PartialSerializer::SerializeJSObjectWithEmbedderFields(Object obj) {
original_embedder_values.emplace_back(embedder_data_slot.load_raw(no_gc));
Object object = embedder_data_slot.load_tagged();
if (object->IsHeapObject()) {
DCHECK(isolate()->heap()->Contains(HeapObject::cast(object)));
DCHECK(IsValidHeapObject(isolate()->heap(), HeapObject::cast(object)));
serialized_data.push_back({nullptr, 0});
} else {
// If no serializer is provided and the field was empty, we serialize it

5
test/cctest/heap/test-heap.cc
View File

@ -38,6 +38,7 @@
#include "src/field-type.h"
#include "src/global-handles.h"
#include "src/hash-seed-inl.h"
#include "src/heap/combined-heap.h"
#include "src/heap/factory.h"
#include "src/heap/gc-tracer.h"
#include "src/heap/heap-inl.h"
@ -74,9 +75,7 @@ static const int kPretenureCreationCount =
static void CheckMap(Map map, int type, int instance_size) {
CHECK(map->IsHeapObject());
#ifdef DEBUG
CHECK(CcTest::heap()->Contains(map));
#endif
DCHECK(IsValidHeapObject(CcTest::heap(), map));
CHECK_EQ(ReadOnlyRoots(CcTest::heap()).meta_map(), map->map());
CHECK_EQ(type, map->instance_type());
CHECK_EQ(instance_size, map->instance_size());

5
test/cctest/heap/test-iterators.cc
View File

@ -51,7 +51,6 @@ Object CreateWritableObject() {
}
} // namespace
// TODO(v8:7464): Add more CHECKs once Contains doesn't include read-only space.
TEST(ReadOnlyHeapIterator) {
CcTest::InitializeVM();
HandleScope handle_scope(CcTest::i_isolate());
@ -61,6 +60,7 @@ TEST(ReadOnlyHeapIterator) {
for (HeapObject obj = iterator.next(); !obj.is_null();
obj = iterator.next()) {
CHECK(ReadOnlyHeap::Contains(obj));
CHECK(!CcTest::heap()->Contains(obj));
CHECK_NE(sample_object, obj);
}
}
@ -75,6 +75,7 @@ TEST(HeapIterator) {
for (HeapObject obj = iterator.next(); !obj.is_null();
obj = iterator.next()) {
CHECK(!ReadOnlyHeap::Contains(obj));
CHECK(CcTest::heap()->Contains(obj));
if (sample_object == obj) seen_sample_object = true;
}
CHECK(seen_sample_object);
@ -89,7 +90,7 @@ TEST(CombinedHeapIterator) {
for (HeapObject obj = iterator.next(); !obj.is_null();
obj = iterator.next()) {
CHECK(CcTest::heap()->Contains(obj));
CHECK(IsValidHeapObject(CcTest::heap(), obj));
if (sample_object == obj) seen_sample_object = true;
}
CHECK(seen_sample_object);