Commit 509ac942 authored by Michael Lippautz's avatar Michael Lippautz Committed by Commit Bot

heap,test: Fix test for TracedGlobal destructors

The tests were assuming that the destructor leaves behind memory in a
defined state when the object was allocated with placement new. Turns
out gcc with no component builds optimizes away the resetting of the
memory.

There's a simpler way to test the functionality by inspecting global
handle counts.

Bug: v8:9639, chromium:995684
Change-Id: I253d84910414c62ca314507b20d2c819f925ea6e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1762512
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63303}
parent 104e9b86
...@@ -569,14 +569,6 @@ TEST(TracePrologueCallingIntoV8WriteBarrier) { ...@@ -569,14 +569,6 @@ TEST(TracePrologueCallingIntoV8WriteBarrier) {
SimulateIncrementalMarking(CcTest::i_isolate()->heap()); SimulateIncrementalMarking(CcTest::i_isolate()->heap());
} }
namespace {
bool all_zeroes(char* begin, size_t len) {
return std::all_of(begin, begin + len, [](char byte) { return byte == 0; });
}
} // namespace
TEST(TracedGlobalWithDestructor) { TEST(TracedGlobalWithDestructor) {
ManualGCScope manual_gc; ManualGCScope manual_gc;
CcTest::InitializeVM(); CcTest::InitializeVM();
...@@ -584,27 +576,27 @@ TEST(TracedGlobalWithDestructor) { ...@@ -584,27 +576,27 @@ TEST(TracedGlobalWithDestructor) {
v8::HandleScope scope(isolate); v8::HandleScope scope(isolate);
TestEmbedderHeapTracer tracer; TestEmbedderHeapTracer tracer;
heap::TemporaryEmbedderHeapTracerScope tracer_scope(isolate, &tracer); heap::TemporaryEmbedderHeapTracerScope tracer_scope(isolate, &tracer);
i::GlobalHandles* global_handles = CcTest::i_isolate()->global_handles();
void* first_field = reinterpret_cast<void*>(0x2); const size_t initial_count = global_handles->handles_count();
auto* traced = new v8::TracedGlobal<v8::Object>();
char* memory = new char[sizeof(v8::TracedGlobal<v8::Object>)];
auto* traced = new (memory) v8::TracedGlobal<v8::Object>();
{ {
v8::HandleScope scope(isolate); v8::HandleScope scope(isolate);
v8::Local<v8::Object> object(ConstructTraceableJSApiObject( v8::Local<v8::Object> object(ConstructTraceableJSApiObject(
isolate->GetCurrentContext(), first_field, nullptr)); isolate->GetCurrentContext(), nullptr, nullptr));
CHECK(traced->IsEmpty()); CHECK(traced->IsEmpty());
*traced = v8::TracedGlobal<v8::Object>(isolate, object); *traced = v8::TracedGlobal<v8::Object>(isolate, object);
CHECK(!traced->IsEmpty()); CHECK(!traced->IsEmpty());
CHECK_EQ(initial_count + 1, global_handles->handles_count());
} }
static_assert(TracedGlobalTrait<
traced->~TracedGlobal<v8::Object>(); v8::TracedGlobal<v8::Object>>::kRequiresExplicitDestruction,
// Note: The following checks refer to the implementation detail that a "destructor expected");
// cleared handle is zeroed out. delete traced;
CHECK(all_zeroes(memory, sizeof(v8::TracedGlobal<v8::Object>))); CHECK_EQ(initial_count, global_handles->handles_count());
// GC should not need to clear the handle.
heap::InvokeMarkSweep(); heap::InvokeMarkSweep();
CHECK(all_zeroes(memory, sizeof(v8::TracedGlobal<v8::Object>))); CHECK_EQ(initial_count, global_handles->handles_count());
delete[] memory;
} }
TEST(TracedGlobalNoDestructor) { TEST(TracedGlobalNoDestructor) {
...@@ -614,26 +606,28 @@ TEST(TracedGlobalNoDestructor) { ...@@ -614,26 +606,28 @@ TEST(TracedGlobalNoDestructor) {
v8::HandleScope scope(isolate); v8::HandleScope scope(isolate);
TestEmbedderHeapTracer tracer; TestEmbedderHeapTracer tracer;
heap::TemporaryEmbedderHeapTracerScope tracer_scope(isolate, &tracer); heap::TemporaryEmbedderHeapTracerScope tracer_scope(isolate, &tracer);
i::GlobalHandles* global_handles = CcTest::i_isolate()->global_handles();
void* first_field = reinterpret_cast<void*>(0x2); const size_t initial_count = global_handles->handles_count();
char* memory = new char[sizeof(v8::TracedGlobal<v8::Value>)]; char* memory = new char[sizeof(v8::TracedGlobal<v8::Value>)];
auto* traced = new (memory) v8::TracedGlobal<v8::Value>(); auto* traced = new (memory) v8::TracedGlobal<v8::Value>();
{ {
v8::HandleScope scope(isolate); v8::HandleScope scope(isolate);
v8::Local<v8::Value> object(ConstructTraceableJSApiObject( v8::Local<v8::Value> object(ConstructTraceableJSApiObject(
isolate->GetCurrentContext(), first_field, nullptr)); isolate->GetCurrentContext(), nullptr, nullptr));
CHECK(traced->IsEmpty()); CHECK(traced->IsEmpty());
*traced = v8::TracedGlobal<v8::Value>(isolate, object); *traced = v8::TracedGlobal<v8::Value>(isolate, object);
CHECK(!traced->IsEmpty()); CHECK(!traced->IsEmpty());
CHECK_EQ(initial_count + 1, global_handles->handles_count());
} }
static_assert(!TracedGlobalTrait<
v8::TracedGlobal<v8::Value>>::kRequiresExplicitDestruction,
"no destructor expected");
traced->~TracedGlobal<v8::Value>(); traced->~TracedGlobal<v8::Value>();
// Note: The following checks refer to the implementation detail that a CHECK_EQ(initial_count + 1, global_handles->handles_count());
// cleared handle is zeroed out. // GC should clear the handle.
CHECK(!all_zeroes(memory, sizeof(v8::TracedGlobal<v8::Value>)));
heap::InvokeMarkSweep(); heap::InvokeMarkSweep();
CHECK(all_zeroes(memory, sizeof(v8::TracedGlobal<v8::Value>))); CHECK_EQ(initial_count, global_handles->handles_count());
delete[] memory; delete[] memory;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment