[VM/GC] Ensure that all the GC verification flags do not produce trace

output unless there is an assertion or Fatal error.

(This is needed to ensure that we are able to run the flutter tests with
all these verifications turned on and not disturbing the output from the
tests with these extraneous trace messages)

TEST=ci

Change-Id: Ifb7211e340f1cf29f6d2bff412eb4107f9cb64a2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/299440
Commit-Queue: Siva Annamalai <asiva@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
This commit is contained in:
asiva 2023-04-28 23:25:43 +00:00 committed by Commit Queue
parent 50e0e616c0
commit c4cb74cb9d
14 changed files with 97 additions and 83 deletions

View file

@ -314,6 +314,13 @@ void Expect::Null(const T p) {
if (!(cond)) dart::Assert(__FILE__, __LINE__).Fail("expected: %s", #cond); \
} while (false)
#define RELEASE_ASSERT_WITH_MSG(cond, msg) \
do { \
if (!(cond)) { \
dart::Assert(__FILE__, __LINE__).Fail("%s: expected: %s", msg, #cond); \
} \
} while (false)
#define COMPILE_ASSERT(expr) static_assert(expr, "")
#if defined(TESTING)

View file

@ -8808,7 +8808,7 @@ void Deserializer::Deserialize(DeserializationRoots* roots) {
#if defined(DEBUG)
isolate_group->ValidateClassTable();
if (isolate_group != Dart::vm_isolate()->group()) {
isolate_group->heap()->Verify();
isolate_group->heap()->Verify("Deserializer::Deserialize");
}
#endif

View file

@ -309,7 +309,7 @@ void ClassFinalizer::VerifyBootstrapClasses() {
if (FLAG_trace_class_finalization) {
OS::PrintErr("VerifyBootstrapClasses END.\n");
}
IsolateGroup::Current()->heap()->Verify();
IsolateGroup::Current()->heap()->Verify("VerifyBootstrapClasses END");
}
#endif // defined(DART_PRECOMPILED_RUNTIME)
@ -1051,7 +1051,7 @@ void ClassFinalizer::RemapClassIds(intptr_t* old_to_new_cid) {
}
#if defined(DEBUG)
IG->heap()->Verify();
IG->heap()->Verify("RemapClassIds");
#endif
}

View file

@ -474,7 +474,7 @@ char* Dart::DartInit(const Dart_InitializeParams* params) {
Object::FinalizeVMIsolate(vm_isolate_->group());
}
#if defined(DEBUG)
vm_isolate_group()->heap()->Verify(kRequireMarked);
vm_isolate_group()->heap()->Verify("Dart::DartInit", kRequireMarked);
#endif
}
// Allocate the "persistent" scoped handles for the predefined API
@ -988,7 +988,7 @@ ErrorPtr Dart::InitializeIsolate(const uint8_t* snapshot_data,
Object::VerifyBuiltinVtables();
if (T->isolate()->origin_id() == 0) {
DEBUG_ONLY(IG->heap()->Verify(kForbidMarked));
DEBUG_ONLY(IG->heap()->Verify("InitializeIsolate", kForbidMarked));
}
#if defined(DART_PRECOMPILED_RUNTIME)

View file

@ -759,21 +759,22 @@ ObjectSet* Heap::CreateAllocatedObjectSet(Zone* zone,
return allocated_set;
}
bool Heap::Verify(MarkExpectation mark_expectation) {
bool Heap::Verify(const char* msg, MarkExpectation mark_expectation) {
if (FLAG_disable_heap_verification) {
return true;
}
HeapIterationScope heap_iteration_scope(Thread::Current());
return VerifyGC(mark_expectation);
return VerifyGC(msg, mark_expectation);
}
bool Heap::VerifyGC(MarkExpectation mark_expectation) {
bool Heap::VerifyGC(const char* msg, MarkExpectation mark_expectation) {
ASSERT(msg != nullptr);
auto thread = Thread::Current();
StackZone stack_zone(thread);
ObjectSet* allocated_set =
CreateAllocatedObjectSet(stack_zone.GetZone(), mark_expectation);
VerifyPointersVisitor visitor(isolate_group(), allocated_set);
VerifyPointersVisitor visitor(isolate_group(), allocated_set, msg);
VisitObjectPointers(&visitor);
// Only returning a value so that Heap::Validate can be called from an ASSERT.

View file

@ -156,7 +156,8 @@ class Heap {
intptr_t max_old_gen_words);
// Verify that all pointers in the heap point to the heap.
bool Verify(MarkExpectation mark_expectation = kForbidMarked);
bool Verify(const char* msg,
MarkExpectation mark_expectation = kForbidMarked);
// Print heap sizes.
void PrintSizes() const;
@ -360,7 +361,8 @@ class Heap {
// Like Verify, but does not wait for concurrent sweeper, so caller must
// ensure thread-safety.
bool VerifyGC(MarkExpectation mark_expectation = kForbidMarked);
bool VerifyGC(const char* msg,
MarkExpectation mark_expectation = kForbidMarked);
// Helper functions for garbage collection.
void CollectNewSpaceGarbage(Thread* thread, GCType type, GCReason reason);

View file

@ -1036,7 +1036,7 @@ class VerifyAfterMarkingVisitor : public ObjectVisitor,
ObjectPtr obj = *ptr;
if (obj->IsHeapObject() && obj->IsOldObject() &&
!obj->untag()->IsMarked()) {
FATAL("Not marked: *0x%" Px " = 0x%" Px "\n",
FATAL("Verifying after marking: Not marked: *0x%" Px " = 0x%" Px "\n",
reinterpret_cast<uword>(ptr), static_cast<uword>(obj));
}
}
@ -1050,7 +1050,7 @@ class VerifyAfterMarkingVisitor : public ObjectVisitor,
ObjectPtr obj = ptr->Decompress(heap_base);
if (obj->IsHeapObject() && obj->IsOldObject() &&
!obj->untag()->IsMarked()) {
FATAL("Not marked: *0x%" Px " = 0x%" Px "\n",
FATAL("Verifying after marking: Not marked: *0x%" Px " = 0x%" Px "\n",
reinterpret_cast<uword>(ptr), static_cast<uword>(obj));
}
}
@ -1148,10 +1148,8 @@ void GCMarker::MarkObjects(PageSpace* page_space) {
// Separate from verify_after_gc because that verification interferes with
// concurrent marking.
if (FLAG_verify_after_marking) {
OS::PrintErr("Verifying after marking...");
VerifyAfterMarkingVisitor visitor;
heap_->VisitObjects(&visitor);
OS::PrintErr(" done.\n");
}
Epilogue();

View file

@ -993,9 +993,8 @@ void PageSpace::CollectGarbageHelper(Thread* thread,
}
if (FLAG_verify_before_gc) {
OS::PrintErr("Verifying before marking...");
heap_->VerifyGC(phase() == kDone ? kForbidMarked : kAllowMarked);
OS::PrintErr(" done.\n");
heap_->VerifyGC("Verifying before marking",
phase() == kDone ? kForbidMarked : kAllowMarked);
}
// Make code pages writable.
@ -1033,9 +1032,7 @@ void PageSpace::CollectGarbageHelper(Thread* thread,
}
if (FLAG_verify_before_gc) {
OS::PrintErr("Verifying before sweeping...");
heap_->VerifyGC(kAllowMarked);
OS::PrintErr(" done.\n");
heap_->VerifyGC("Verifying before sweeping", kAllowMarked);
}
{
@ -1092,9 +1089,7 @@ void PageSpace::CollectGarbageHelper(Thread* thread,
}
if (FLAG_verify_after_gc && can_verify) {
OS::PrintErr("Verifying after sweeping...");
heap_->VerifyGC(kForbidMarked);
OS::PrintErr(" done.\n");
heap_->VerifyGC("Verifying after sweeping", kForbidMarked);
}
TryReserveForOOM();
@ -1205,9 +1200,7 @@ void PageSpace::Compact(Thread* thread) {
compactor.Compact(pages_, &freelists_[kDataFreelist], &pages_lock_);
if (FLAG_verify_after_gc) {
OS::PrintErr("Verifying after compacting...");
heap_->VerifyGC(kForbidMarked);
OS::PrintErr(" done.\n");
heap_->VerifyGC("Verifying after compacting", kForbidMarked);
}
}

View file

@ -817,21 +817,23 @@ intptr_t Scavenger::NewSizeInWords(intptr_t old_size_in_words,
class CollectStoreBufferVisitor : public ObjectPointerVisitor {
public:
explicit CollectStoreBufferVisitor(ObjectSet* in_store_buffer)
CollectStoreBufferVisitor(ObjectSet* in_store_buffer, const char* msg)
: ObjectPointerVisitor(IsolateGroup::Current()),
in_store_buffer_(in_store_buffer) {}
in_store_buffer_(in_store_buffer),
msg_(msg) {}
void VisitPointers(ObjectPtr* from, ObjectPtr* to) override {
for (ObjectPtr* ptr = from; ptr <= to; ptr++) {
ObjectPtr raw_obj = *ptr;
RELEASE_ASSERT(raw_obj->untag()->IsRemembered());
RELEASE_ASSERT(raw_obj->IsOldObject());
RELEASE_ASSERT_WITH_MSG(raw_obj->untag()->IsRemembered(), msg_);
RELEASE_ASSERT_WITH_MSG(raw_obj->IsOldObject(), msg_);
RELEASE_ASSERT(!raw_obj->untag()->IsCardRemembered());
RELEASE_ASSERT_WITH_MSG(!raw_obj->untag()->IsCardRemembered(), msg_);
if (raw_obj.GetClassId() == kArrayCid) {
const uword length =
Smi::Value(static_cast<UntaggedArray*>(raw_obj.untag())->length());
RELEASE_ASSERT(!Array::UseCardMarkingForAllocation(length));
RELEASE_ASSERT_WITH_MSG(!Array::UseCardMarkingForAllocation(length),
msg_);
}
in_store_buffer_->Add(raw_obj);
}
@ -847,29 +849,34 @@ class CollectStoreBufferVisitor : public ObjectPointerVisitor {
private:
ObjectSet* const in_store_buffer_;
const char* msg_;
};
class CheckStoreBufferVisitor : public ObjectVisitor,
public ObjectPointerVisitor {
public:
CheckStoreBufferVisitor(ObjectSet* in_store_buffer, const SemiSpace* to)
CheckStoreBufferVisitor(ObjectSet* in_store_buffer,
const SemiSpace* to,
const char* msg)
: ObjectVisitor(),
ObjectPointerVisitor(IsolateGroup::Current()),
in_store_buffer_(in_store_buffer),
to_(to) {}
to_(to),
msg_(msg) {}
void VisitObject(ObjectPtr raw_obj) override {
if (raw_obj->IsPseudoObject()) return;
RELEASE_ASSERT(raw_obj->IsOldObject());
RELEASE_ASSERT_WITH_MSG(raw_obj->IsOldObject(), msg_);
RELEASE_ASSERT(raw_obj->untag()->IsRemembered() ==
in_store_buffer_->Contains(raw_obj));
RELEASE_ASSERT_WITH_MSG(
raw_obj->untag()->IsRemembered() == in_store_buffer_->Contains(raw_obj),
msg_);
visiting_ = raw_obj;
is_remembered_ = raw_obj->untag()->IsRemembered();
is_card_remembered_ = raw_obj->untag()->IsCardRemembered();
if (is_card_remembered_) {
RELEASE_ASSERT(!is_remembered_);
RELEASE_ASSERT_WITH_MSG(!is_remembered_, msg_);
}
raw_obj->untag()->VisitPointers(this);
}
@ -881,24 +888,25 @@ class CheckStoreBufferVisitor : public ObjectVisitor,
if (is_card_remembered_) {
if (!Page::Of(visiting_)->IsCardRemembered(ptr)) {
FATAL(
"Old object %#" Px " references new object %#" Px
"%s: Old object %#" Px " references new object %#" Px
", but the "
"slot's card is not remembered. Consider using rr to watch the "
"slot %p and reverse-continue to find the store with a missing "
"barrier.\n",
static_cast<uword>(visiting_), static_cast<uword>(raw_obj),
ptr);
msg_, static_cast<uword>(visiting_),
static_cast<uword>(raw_obj), ptr);
}
} else if (!is_remembered_) {
FATAL("Old object %#" Px " references new object %#" Px
FATAL("%s: Old object %#" Px " references new object %#" Px
", but it is "
"not in any store buffer. Consider using rr to watch the "
"slot %p and reverse-continue to find the store with a missing "
"barrier.\n",
static_cast<uword>(visiting_), static_cast<uword>(raw_obj),
ptr);
msg_, static_cast<uword>(visiting_),
static_cast<uword>(raw_obj), ptr);
}
RELEASE_ASSERT(to_->Contains(UntaggedObject::ToAddr(raw_obj)));
RELEASE_ASSERT_WITH_MSG(to_->Contains(UntaggedObject::ToAddr(raw_obj)),
msg_);
}
}
}
@ -913,24 +921,25 @@ class CheckStoreBufferVisitor : public ObjectVisitor,
if (is_card_remembered_) {
if (!Page::Of(visiting_)->IsCardRemembered(ptr)) {
FATAL(
"Old object %#" Px " references new object %#" Px
"%s: Old object %#" Px " references new object %#" Px
", but the "
"slot's card is not remembered. Consider using rr to watch the "
"slot %p and reverse-continue to find the store with a missing "
"barrier.\n",
static_cast<uword>(visiting_), static_cast<uword>(raw_obj),
ptr);
msg_, static_cast<uword>(visiting_),
static_cast<uword>(raw_obj), ptr);
}
} else if (!is_remembered_) {
FATAL("Old object %#" Px " references new object %#" Px
FATAL("%s: Old object %#" Px " references new object %#" Px
", but it is "
"not in any store buffer. Consider using rr to watch the "
"slot %p and reverse-continue to find the store with a missing "
"barrier.\n",
static_cast<uword>(visiting_), static_cast<uword>(raw_obj),
ptr);
msg_, static_cast<uword>(visiting_),
static_cast<uword>(raw_obj), ptr);
}
RELEASE_ASSERT(to_->Contains(UntaggedObject::ToAddr(raw_obj)));
RELEASE_ASSERT_WITH_MSG(to_->Contains(UntaggedObject::ToAddr(raw_obj)),
msg_);
}
}
}
@ -942,9 +951,11 @@ class CheckStoreBufferVisitor : public ObjectVisitor,
ObjectPtr visiting_;
bool is_remembered_;
bool is_card_remembered_;
const char* msg_;
};
void Scavenger::VerifyStoreBuffers() {
void Scavenger::VerifyStoreBuffers(const char* msg) {
ASSERT(msg != nullptr);
Thread* thread = Thread::Current();
StackZone stack_zone(thread);
Zone* zone = stack_zone.GetZone();
@ -953,12 +964,12 @@ void Scavenger::VerifyStoreBuffers() {
heap_->AddRegionsToObjectSet(in_store_buffer);
{
CollectStoreBufferVisitor visitor(in_store_buffer);
CollectStoreBufferVisitor visitor(in_store_buffer, msg);
heap_->isolate_group()->store_buffer()->VisitObjectPointers(&visitor);
}
{
CheckStoreBufferVisitor visitor(in_store_buffer, to_);
CheckStoreBufferVisitor visitor(in_store_buffer, to_, msg);
heap_->old_space()->VisitObjects(&visitor);
}
}
@ -969,10 +980,8 @@ SemiSpace* Scavenger::Prologue(GCReason reason) {
heap_->isolate_group()->ReleaseStoreBuffers();
if (FLAG_verify_store_buffer) {
OS::PrintErr("Verifying remembered set before Scavenge...");
heap_->WaitForSweeperTasksAtSafepoint(Thread::Current());
VerifyStoreBuffers();
OS::PrintErr(" done.\n");
VerifyStoreBuffers("Verifying remembered set before Scavenge");
}
// Need to stash the old remembered set before any worker begins adding to the
@ -1066,10 +1075,8 @@ void Scavenger::Epilogue(SemiSpace* from) {
// are very rare.
heap_->isolate_group()->ReleaseStoreBuffers();
OS::PrintErr("Verifying remembered set after Scavenge...");
heap_->WaitForSweeperTasksAtSafepoint(Thread::Current());
VerifyStoreBuffers();
OS::PrintErr(" done.\n");
VerifyStoreBuffers("Verifying remembered set after Scavenge");
}
delete from;
@ -1706,10 +1713,9 @@ void Scavenger::Scavenge(Thread* thread, GCType type, GCReason reason) {
}
if (FLAG_verify_before_gc) {
OS::PrintErr("Verifying before Scavenge...");
heap_->WaitForSweeperTasksAtSafepoint(thread);
heap_->VerifyGC(thread->is_marking() ? kAllowMarked : kForbidMarked);
OS::PrintErr(" done.\n");
heap_->VerifyGC("Verifying before Scavenge",
thread->is_marking() ? kAllowMarked : kForbidMarked);
}
// Prepare for a scavenge.
@ -1772,10 +1778,9 @@ void Scavenger::Scavenge(Thread* thread, GCType type, GCReason reason) {
Epilogue(from);
if (FLAG_verify_after_gc) {
OS::PrintErr("Verifying after Scavenge...");
heap_->WaitForSweeperTasksAtSafepoint(thread);
heap_->VerifyGC(thread->is_marking() ? kAllowMarked : kForbidMarked);
OS::PrintErr(" done.\n");
heap_->VerifyGC("Verifying after Scavenge...",
thread->is_marking() ? kAllowMarked : kForbidMarked);
}
// Done scavenging. Reset the marker.

View file

@ -276,7 +276,7 @@ class Scavenger {
void MournWeakHandles();
void Epilogue(SemiSpace* from);
void VerifyStoreBuffers();
void VerifyStoreBuffers(const char* msg);
void UpdateMaxHeapCapacity();
void UpdateMaxHeapUsage();

View file

@ -52,7 +52,7 @@ void VerifyPointersVisitor::VisitPointers(ObjectPtr* from, ObjectPtr* to) {
allocated_set_->Contains(Page::ToWritable(obj))) {
continue;
}
FATAL("Invalid pointer: *0x%" Px " = 0x%" Px "\n",
FATAL("%s: Invalid pointer: *0x%" Px " = 0x%" Px "\n", msg_,
reinterpret_cast<uword>(ptr), static_cast<uword>(obj));
}
}
@ -71,7 +71,7 @@ void VerifyPointersVisitor::VisitCompressedPointers(uword heap_base,
allocated_set_->Contains(Page::ToWritable(obj))) {
continue;
}
FATAL("Invalid pointer: *0x%" Px " = 0x%" Px "\n",
FATAL("%s: Invalid pointer: *0x%" Px " = 0x%" Px "\n", msg_,
reinterpret_cast<uword>(ptr), static_cast<uword>(obj));
}
}
@ -86,7 +86,8 @@ void VerifyWeakPointersVisitor::VisitHandle(uword addr) {
visitor_->VisitPointer(&raw_obj);
}
void VerifyPointersVisitor::VerifyPointers(MarkExpectation mark_expectation) {
void VerifyPointersVisitor::VerifyPointers(const char* msg,
MarkExpectation mark_expectation) {
Thread* thread = Thread::Current();
auto isolate_group = thread->isolate_group();
HeapIterationScope iteration(thread);
@ -94,7 +95,7 @@ void VerifyPointersVisitor::VerifyPointers(MarkExpectation mark_expectation) {
ObjectSet* allocated_set = isolate_group->heap()->CreateAllocatedObjectSet(
stack_zone.GetZone(), mark_expectation);
VerifyPointersVisitor visitor(isolate_group, allocated_set);
VerifyPointersVisitor visitor(isolate_group, allocated_set, msg);
// Visit all strongly reachable objects.
iteration.IterateObjectPointers(&visitor, ValidationPolicy::kValidateFrames);
VerifyWeakPointersVisitor weak_visitor(&visitor);

View file

@ -43,9 +43,14 @@ class VerifyObjectVisitor : public ObjectVisitor {
// the pointers visited are contained in the isolate heap.
class VerifyPointersVisitor : public ObjectPointerVisitor {
public:
explicit VerifyPointersVisitor(IsolateGroup* isolate_group,
ObjectSet* allocated_set)
: ObjectPointerVisitor(isolate_group), allocated_set_(allocated_set) {}
VerifyPointersVisitor(IsolateGroup* isolate_group,
ObjectSet* allocated_set,
const char* msg)
: ObjectPointerVisitor(isolate_group),
allocated_set_(allocated_set),
msg_(msg) {
ASSERT(msg_ != nullptr);
}
void VisitPointers(ObjectPtr* first, ObjectPtr* last) override;
#if defined(DART_COMPRESSED_POINTERS)
@ -54,10 +59,12 @@ class VerifyPointersVisitor : public ObjectPointerVisitor {
CompressedObjectPtr* last) override;
#endif
static void VerifyPointers(MarkExpectation mark_expectation = kForbidMarked);
static void VerifyPointers(const char* msg,
MarkExpectation mark_expectation = kForbidMarked);
private:
ObjectSet* allocated_set_;
const char* msg_;
DISALLOW_COPY_AND_ASSIGN(VerifyPointersVisitor);
};

View file

@ -108,8 +108,8 @@ DECLARE_FLAG(bool, reload_every_back_off);
void VerifyOnTransition() {
Thread* thread = Thread::Current();
TransitionGeneratedToVM transition(thread);
VerifyPointersVisitor::VerifyPointers();
thread->isolate_group()->heap()->Verify();
VerifyPointersVisitor::VerifyPointers("VerifyOnTransition");
thread->isolate_group()->heap()->Verify("VerifyOnTransition");
}
#endif

View file

@ -23,7 +23,7 @@ ISOLATE_UNIT_TEST_CASE(EmptyStackFrameIteration) {
StackFrameIterator::kNoCrossThreadIteration);
EXPECT(!iterator.HasNextFrame());
EXPECT(iterator.NextFrame() == nullptr);
VerifyPointersVisitor::VerifyPointers();
VerifyPointersVisitor::VerifyPointers("EmptyStackFrameIterationTest");
}
// Unit test for empty dart stack frame iteration.
@ -31,7 +31,7 @@ ISOLATE_UNIT_TEST_CASE(EmptyDartStackFrameIteration) {
DartFrameIterator iterator(Thread::Current(),
StackFrameIterator::kNoCrossThreadIteration);
EXPECT(iterator.NextFrame() == nullptr);
VerifyPointersVisitor::VerifyPointers();
VerifyPointersVisitor::VerifyPointers("EmptyDartStackFrameIterationTest");
}
#define FUNCTION_NAME(name) StackFrame_##name
@ -62,7 +62,7 @@ void FUNCTION_NAME(StackFrame_frameCount)(Dart_NativeArguments args) {
while (frames.NextFrame() != nullptr) {
count += 1; // Count the frame.
}
VerifyPointersVisitor::VerifyPointers();
VerifyPointersVisitor::VerifyPointers("StackFrame_frameCount_Test");
arguments->SetReturn(Object::Handle(Smi::New(count)));
}
@ -74,7 +74,7 @@ void FUNCTION_NAME(StackFrame_dartFrameCount)(Dart_NativeArguments args) {
while (frames.NextFrame() != nullptr) {
count += 1; // Count the dart frame.
}
VerifyPointersVisitor::VerifyPointers();
VerifyPointersVisitor::VerifyPointers("StackFrame_dartFrameCount_Test");
NativeArguments* arguments = reinterpret_cast<NativeArguments*>(args);
arguments->SetReturn(Object::Handle(Smi::New(count)));
}