From 49c507dc99b5825ccdf551059c34481f32c14650 Mon Sep 17 00:00:00 2001 From: Leszek Swirski Date: Tue, 5 Apr 2022 10:16:57 +0200 Subject: [PATCH] [test] Make cctest run one test, with maybe custom platform Remove cctest's ability to run multiple tests (which has long been deprecated and mostly broken). We can then make platform & V8 initialisation be part of running the test's Run method. In particular, this allows us to inject custom logic into the platform initialisation, like setting up a platform wrapper. Add a TEST_WITH_PLATFORM which exercises this by registering a platform factory on the test, and wrapping the default platform using this factory. This allows these tests to guarantee that the lifetime of the platform is longer than the lifetime of the isolate. As a result of this, we can also remove the complexity around draining platform state in the TestPlatform (since it will now have a longer lifetime than the Isolate using it), and as a drive-by clean up the TestPlaform to use a CcTest-global "default platform" instead of trying to scope over the "current" platform. As another drive-by, change the linked-list of CcTests and the linear search through it into an std::map of tests. Change-Id: I610f6312fe042f29f45cc4dfba311e4184bc7759 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3569223 Reviewed-by: Michael Lippautz Commit-Queue: Leszek Swirski Cr-Commit-Position: refs/heads/main@{#79772} --- test/cctest/cctest.cc | 266 +++++++----------- test/cctest/cctest.h | 98 ++++--- test/cctest/heap/test-incremental-marking.cc | 13 +- test/cctest/heap/test-memory-measurement.cc | 14 +- test/cctest/heap/test-unmapper.cc | 4 +- test/cctest/test-allocation.cc | 27 +- test/cctest/test-api.cc | 9 +- test/cctest/test-profile-generator.cc | 17 +- test/cctest/test-trace-event.cc | 42 +-- .../cctest/wasm/test-streaming-compilation.cc | 28 +- test/cctest/wasm/test-wasm-metrics.cc | 34 +-- 11 files changed, 213 insertions(+), 339 deletions(-) diff --git a/test/cctest/cctest.cc b/test/cctest/cctest.cc index e8b740b4e8..73307cd266 100644 --- a/test/cctest/cctest.cc +++ b/test/cctest/cctest.cc @@ -35,6 +35,8 @@ #include "include/v8-isolate.h" #include "include/v8-local-handle.h" #include "include/v8-locker.h" +#include "src/base/lazy-instance.h" +#include "src/base/logging.h" #include "src/base/platform/condition-variable.h" #include "src/base/platform/mutex.h" #include "src/base/platform/semaphore.h" @@ -65,19 +67,23 @@ enum InitializationState { kUnset, kUninitialized, kInitialized }; static InitializationState initialization_state_ = kUnset; -CcTest* CcTest::last_ = nullptr; +static v8::base::LazyInstance::type g_cctests = + LAZY_INSTANCE_INITIALIZER; + +std::unordered_map* tests_ = + new std::unordered_map(); bool CcTest::initialize_called_ = false; v8::base::Atomic32 CcTest::isolate_used_ = 0; v8::ArrayBuffer::Allocator* CcTest::allocator_ = nullptr; v8::Isolate* CcTest::isolate_ = nullptr; +v8::Platform* CcTest::default_platform_ = nullptr; CcTest::CcTest(TestFunction* callback, const char* file, const char* name, - bool enabled, bool initialize) + bool enabled, bool initialize, + TestPlatformFactory* test_platform_factory) : callback_(callback), - name_(name), - enabled_(enabled), initialize_(initialize), - prev_(last_) { + test_platform_factory_(test_platform_factory) { // Find the base name of this test (const_cast required on Windows). char *basename = strrchr(const_cast(file), '/'); if (!basename) { @@ -92,25 +98,57 @@ CcTest::CcTest(TestFunction* callback, const char* file, const char* name, char *extension = strrchr(basename, '.'); if (extension) *extension = 0; // Install this test in the list of tests - file_ = basename; - prev_ = last_; - last_ = this; + + if (enabled) { + auto it = + g_cctests.Pointer()->emplace(std::string(basename) + "/" + name, this); + CHECK_WITH_MSG(it.second, "Test with same name already exists"); + } + v8::internal::DeleteArray(basename); } +void CcTest::Run(const char* snapshot_directory) { + v8::V8::InitializeICUDefaultLocation(snapshot_directory); + std::unique_ptr underlying_default_platform( + v8::platform::NewDefaultPlatform()); + default_platform_ = underlying_default_platform.get(); + std::unique_ptr platform; + if (test_platform_factory_) { + platform = test_platform_factory_(); + } else { + platform = std::move(underlying_default_platform); + } + v8::V8::InitializePlatform(platform.get()); +#ifdef V8_SANDBOX + CHECK(v8::V8::InitializeSandbox()); +#endif + cppgc::InitializeProcess(platform->GetPageAllocator()); + v8::V8::Initialize(); + v8::V8::InitializeExternalStartupData(snapshot_directory); + +#if V8_ENABLE_WEBASSEMBLY && V8_TRAP_HANDLER_SUPPORTED + constexpr bool kUseDefaultTrapHandler = true; + CHECK(v8::V8::EnableWebAssemblyTrapHandler(kUseDefaultTrapHandler)); +#endif // V8_ENABLE_WEBASSEMBLY && V8_TRAP_HANDLER_SUPPORTED + + CcTest::set_array_buffer_allocator( + v8::ArrayBuffer::Allocator::NewDefaultAllocator()); + + v8::RegisterExtension(std::make_unique()); + v8::RegisterExtension(std::make_unique()); + v8::RegisterExtension(std::make_unique()); -void CcTest::Run() { if (!initialize_) { CHECK_NE(initialization_state_, kInitialized); initialization_state_ = kUninitialized; - CHECK_NULL(CcTest::isolate_); + CHECK_NULL(isolate_); } else { CHECK_NE(initialization_state_, kUninitialized); initialization_state_ = kInitialized; - if (isolate_ == nullptr) { - v8::Isolate::CreateParams create_params; - create_params.array_buffer_allocator = allocator_; - isolate_ = v8::Isolate::New(create_params); - } + CHECK_NULL(isolate_); + v8::Isolate::CreateParams create_params; + create_params.array_buffer_allocator = allocator_; + isolate_ = v8::Isolate::New(create_params); isolate_->Enter(); } #ifdef DEBUG @@ -132,7 +170,15 @@ void CcTest::Run() { EmptyMessageQueues(isolate_); } isolate_->Exit(); + isolate_->Dispose(); + isolate_ = nullptr; + } else { + CHECK_NULL(isolate_); } + + v8::V8::Dispose(); + cppgc::ShutdownProcess(); + v8::V8::DisposePlatform(); } i::Heap* CcTest::heap() { return i_isolate()->heap(); } @@ -200,10 +246,6 @@ void CcTest::InitializeVM() { v8::Context::New(CcTest::isolate())->Enter(); } -void CcTest::TearDown() { - if (isolate_ != nullptr) isolate_->Dispose(); -} - v8::Local CcTest::NewContext(CcTestExtensionFlags extension_flags, v8::Isolate* isolate) { const char* extension_names[kMaxExtensions]; @@ -293,17 +335,10 @@ i::Handle Optimize( return function; } -static void PrintTestList(CcTest* current) { - if (current == nullptr) return; - PrintTestList(current->prev()); - printf("%s/%s\n", current->file(), current->name()); -} - - -static void SuggestTestHarness(int tests) { - if (tests == 0) return; - printf("Running multiple tests in sequence is deprecated and may cause " - "bogus failure. Consider using tools/run-tests.py instead.\n"); +static void PrintTestList() { + for (auto [name, test] : g_cctests.Get()) { + printf("%s\n", name.c_str()); + } } int main(int argc, char* argv[]) { @@ -336,82 +371,44 @@ int main(int argc, char* argv[]) { perfetto::Tracing::Initialize(init_args); #endif // V8_USE_PERFETTO - v8::V8::InitializeICUDefaultLocation(argv[0]); - std::unique_ptr platform(v8::platform::NewDefaultPlatform()); - v8::V8::InitializePlatform(platform.get()); -#ifdef V8_SANDBOX - CHECK(v8::V8::InitializeSandbox()); -#endif - cppgc::InitializeProcess(platform->GetPageAllocator()); using HelpOptions = v8::internal::FlagList::HelpOptions; v8::internal::FlagList::SetFlagsFromCommandLine( &argc, argv, true, HelpOptions(HelpOptions::kExit, usage.c_str())); - v8::V8::Initialize(); - v8::V8::InitializeExternalStartupData(argv[0]); -#if V8_ENABLE_WEBASSEMBLY && V8_TRAP_HANDLER_SUPPORTED - constexpr bool kUseDefaultTrapHandler = true; - CHECK(v8::V8::EnableWebAssemblyTrapHandler(kUseDefaultTrapHandler)); -#endif // V8_ENABLE_WEBASSEMBLY && V8_TRAP_HANDLER_SUPPORTED - - CcTest::set_array_buffer_allocator( - v8::ArrayBuffer::Allocator::NewDefaultAllocator()); - - v8::RegisterExtension(std::make_unique()); - v8::RegisterExtension(std::make_unique()); - v8::RegisterExtension(std::make_unique()); - - int tests_run = 0; - bool print_run_count = true; - for (int i = 1; i < argc; i++) { - char* arg = argv[i]; + const char* test_arg = nullptr; + for (int i = 1; i < argc; ++i) { + const char* arg = argv[i]; if (strcmp(arg, "--list") == 0) { - PrintTestList(CcTest::last()); - print_run_count = false; - - } else { - char* arg_copy = v8::internal::StrDup(arg); - char* testname = strchr(arg_copy, '/'); - if (testname) { - // Split the string in two by nulling the slash and then run - // exact matches. - *testname = 0; - char* file = arg_copy; - char* name = testname + 1; - CcTest* test = CcTest::last(); - while (test != nullptr) { - if (test->enabled() - && strcmp(test->file(), file) == 0 - && strcmp(test->name(), name) == 0) { - SuggestTestHarness(tests_run++); - test->Run(); - } - test = test->prev(); - } - - } else { - // Run all tests with the specified file or test name. - char* file_or_name = arg_copy; - CcTest* test = CcTest::last(); - while (test != nullptr) { - if (test->enabled() - && (strcmp(test->file(), file_or_name) == 0 - || strcmp(test->name(), file_or_name) == 0)) { - SuggestTestHarness(tests_run++); - test->Run(); - } - test = test->prev(); - } - } - v8::internal::DeleteArray(arg_copy); + PrintTestList(); + return 0; } + if (*arg == '-') { + // Ignore flags that weren't removed by SetFlagsFromCommandLine + continue; + } + if (test_arg != nullptr) { + fprintf(stderr, + "Running multiple tests in sequence is not allowed. Use " + "tools/run-tests.py instead.\n"); + return 1; + } + test_arg = arg; } - if (print_run_count && tests_run != 1) { - printf("Ran %i tests.\n", tests_run); + + if (test_arg == nullptr) { + printf("Ran 0 tests.\n"); + return 0; } - CcTest::TearDown(); - v8::V8::Dispose(); - v8::V8::DisposePlatform(); + + auto it = g_cctests.Get().find(test_arg); + if (it == g_cctests.Get().end()) { + fprintf(stderr, "ERROR: Did not find test %s.\n", test_arg); + return 1; + } + + CcTest* test = it->second; + test->Run(argv[0]); + return 0; } @@ -462,63 +459,56 @@ ManualGCScope::~ManualGCScope() { flag_detect_ineffective_gcs_near_heap_limit_; } -TestPlatform::TestPlatform() : old_platform_(i::V8::GetCurrentPlatform()) {} - -void TestPlatform::NotifyPlatformReady() { - i::V8::SetPlatformForTesting(this); - CHECK(!active_); - active_ = true; -} - v8::PageAllocator* TestPlatform::GetPageAllocator() { - return old_platform()->GetPageAllocator(); + return CcTest::default_platform()->GetPageAllocator(); } void TestPlatform::OnCriticalMemoryPressure() { - old_platform()->OnCriticalMemoryPressure(); + CcTest::default_platform()->OnCriticalMemoryPressure(); } bool TestPlatform::OnCriticalMemoryPressure(size_t length) { - return old_platform()->OnCriticalMemoryPressure(length); + return CcTest::default_platform()->OnCriticalMemoryPressure(length); } int TestPlatform::NumberOfWorkerThreads() { - return old_platform()->NumberOfWorkerThreads(); + return CcTest::default_platform()->NumberOfWorkerThreads(); } std::shared_ptr TestPlatform::GetForegroundTaskRunner( v8::Isolate* isolate) { - return old_platform()->GetForegroundTaskRunner(isolate); + return CcTest::default_platform()->GetForegroundTaskRunner(isolate); } void TestPlatform::CallOnWorkerThread(std::unique_ptr task) { - old_platform()->CallOnWorkerThread(std::move(task)); + CcTest::default_platform()->CallOnWorkerThread(std::move(task)); } void TestPlatform::CallDelayedOnWorkerThread(std::unique_ptr task, double delay_in_seconds) { - old_platform()->CallDelayedOnWorkerThread(std::move(task), delay_in_seconds); + CcTest::default_platform()->CallDelayedOnWorkerThread(std::move(task), + delay_in_seconds); } std::unique_ptr TestPlatform::PostJob( v8::TaskPriority priority, std::unique_ptr job_task) { - return old_platform()->PostJob(priority, std::move(job_task)); + return CcTest::default_platform()->PostJob(priority, std::move(job_task)); } double TestPlatform::MonotonicallyIncreasingTime() { - return old_platform()->MonotonicallyIncreasingTime(); + return CcTest::default_platform()->MonotonicallyIncreasingTime(); } double TestPlatform::CurrentClockTimeMillis() { - return old_platform()->CurrentClockTimeMillis(); + return CcTest::default_platform()->CurrentClockTimeMillis(); } bool TestPlatform::IdleTasksEnabled(v8::Isolate* isolate) { - return old_platform()->IdleTasksEnabled(isolate); + return CcTest::default_platform()->IdleTasksEnabled(isolate); } v8::TracingController* TestPlatform::GetTracingController() { - return old_platform()->GetTracingController(); + return CcTest::default_platform()->GetTracingController(); } namespace { @@ -555,43 +545,3 @@ class ShutdownTask final : public v8::Task { }; } // namespace - -void TestPlatform::RemovePlatform() { - DCHECK_EQ(i::V8::GetCurrentPlatform(), this); - // Destruction helpers. - // Barrier to wait until all shutdown tasks actually run (and subsequently - // block). - v8::base::Semaphore destruction_barrier{0}; - // Primitives for blocking until `can_destruct` is true. - v8::base::Mutex destruction_mutex; - v8::base::ConditionVariable destruction_condition; - bool can_destruct = false; - - for (int i = 0; i < NumberOfWorkerThreads(); i++) { - old_platform()->CallOnWorkerThread( - std::make_unique(&destruction_barrier, &destruction_mutex, - &destruction_condition, &can_destruct)); - } - // Wait till all worker threads reach the barrier. - for (int i = 0; i < NumberOfWorkerThreads(); i++) { - destruction_barrier.Wait(); - } - // At this point all worker threads are blocked, so the platform can be - // swapped back. - i::V8::SetPlatformForTesting(old_platform_); - CHECK(active_); - active_ = false; - // Release all worker threads again. - { - v8::base::MutexGuard guard(&destruction_mutex); - can_destruct = true; - destruction_condition.NotifyAll(); - } - // Wait till all worker threads resume. This is necessary as the threads would - // otherwise try to unlock `destruction_mutex` which may already be gone. - for (int i = 0; i < NumberOfWorkerThreads(); i++) { - destruction_barrier.Wait(); - } -} - -TestPlatform::~TestPlatform() { CHECK(!active_); } diff --git a/test/cctest/cctest.h b/test/cctest/cctest.h index 7297aeef4e..3e4cd3cf12 100644 --- a/test/cctest/cctest.h +++ b/test/cctest/cctest.h @@ -69,23 +69,40 @@ class JSHeapBroker; } // namespace v8 #ifndef TEST -#define TEST(Name) \ - static void Test##Name(); \ - CcTest register_test_##Name(Test##Name, __FILE__, #Name, true, true); \ +#define TEST(Name) \ + static void Test##Name(); \ + CcTest register_test_##Name(Test##Name, __FILE__, #Name, true, true, \ + nullptr); \ static void Test##Name() #endif #ifndef UNINITIALIZED_TEST -#define UNINITIALIZED_TEST(Name) \ - static void Test##Name(); \ - CcTest register_test_##Name(Test##Name, __FILE__, #Name, true, false); \ +#define UNINITIALIZED_TEST(Name) \ + static void Test##Name(); \ + CcTest register_test_##Name(Test##Name, __FILE__, #Name, true, false, \ + nullptr); \ static void Test##Name() #endif +#ifndef TEST_WITH_PLATFORM +#define TEST_WITH_PLATFORM(Name, PlatformClass) \ + static void Test##Name(PlatformClass& platform); \ + static void TestWithoutPlatform##Name() { \ + Test##Name(*static_cast(i::V8::GetCurrentPlatform())); \ + } \ + CcTest register_test_##Name(TestWithoutPlatform##Name, __FILE__, #Name, \ + true, true, \ + []() -> std::unique_ptr { \ + return std::make_unique(); \ + }); \ + static void Test##Name(PlatformClass& platform) +#endif + #ifndef DISABLED_TEST -#define DISABLED_TEST(Name) \ - static void Test##Name(); \ - CcTest register_test_##Name(Test##Name, __FILE__, #Name, false, true); \ +#define DISABLED_TEST(Name) \ + static void Test##Name(); \ + CcTest register_test_##Name(Test##Name, __FILE__, #Name, false, true, \ + nullptr); \ static void Test##Name() #endif @@ -97,9 +114,9 @@ class JSHeapBroker; // to correctly associate the tests with the test suite using them. // 2. To actually execute the tests, create an instance of the class // containing the MEMBER_TESTs. -#define MEMBER_TEST(Name) \ - CcTest register_test_##Name = \ - CcTest(Test##Name, kTestFileName, #Name, true, true); \ +#define MEMBER_TEST(Name) \ + CcTest register_test_##Name = \ + CcTest(Test##Name, kTestFileName, #Name, true, true, nullptr); \ static void Test##Name() #define EXTENSION_LIST(V) \ @@ -119,18 +136,19 @@ static constexpr const char* kExtensionName[kMaxExtensions] = { EXTENSION_LIST(DEFINE_EXTENSION_NAME)}; #undef DEFINE_EXTENSION_NAME +class CcTest; +class TestPlatform; + +using CcTestMapType = std::map; + class CcTest { public: using TestFunction = void(); + using TestPlatformFactory = std::unique_ptr(); CcTest(TestFunction* callback, const char* file, const char* name, - bool enabled, bool initialize); - ~CcTest() { i::DeleteArray(file_); } - void Run(); - static CcTest* last() { return last_; } - CcTest* prev() { return prev_; } - const char* file() { return file_; } - const char* name() { return name_; } - bool enabled() { return enabled_; } + bool enabled, bool initialize, + TestPlatformFactory* platform_factory = nullptr); + void Run(const char* argv0); static v8::Isolate* isolate() { CHECK_NOT_NULL(isolate_); @@ -150,6 +168,8 @@ class CcTest { static i::Heap* heap(); static i::ReadOnlyHeap* read_only_heap(); + static v8::Platform* default_platform() { return default_platform_; } + static void AddGlobalFunction(v8::Local env, const char* name, v8::FunctionCallback callback); static void CollectGarbage(i::AllocationSpace space, @@ -178,9 +198,6 @@ class CcTest { // This must be called first in a test. static void InitializeVM(); - // Only for UNINITIALIZED_TESTs - static void DisableAutomaticDispose(); - // Helper function to configure a context. // Must be in a HandleScope. static v8::Local NewContext( @@ -196,21 +213,17 @@ class CcTest { return NewContext(CcTestExtensionFlags{extensions}, isolate); } - static void TearDown(); - private: - static CcTest* last_; + static std::unordered_map* tests_; static v8::ArrayBuffer::Allocator* allocator_; static v8::Isolate* isolate_; + static v8::Platform* default_platform_; static bool initialize_called_; static v8::base::Atomic32 isolate_used_; TestFunction* callback_; - const char* file_; - const char* name_; - bool enabled_; bool initialize_; - CcTest* prev_; + TestPlatformFactory* test_platform_factory_; friend int main(int argc, char** argv); friend class ManualGCScope; @@ -632,8 +645,7 @@ static inline void DisableDebugger(v8::Isolate* isolate) { static inline void EmptyMessageQueues(v8::Isolate* isolate) { - while (v8::platform::PumpMessageLoop(v8::internal::V8::GetCurrentPlatform(), - isolate)) { + while (v8::platform::PumpMessageLoop(CcTest::default_platform(), isolate)) { } } @@ -701,19 +713,10 @@ class V8_NODISCARD ManualGCScope { }; // This is a base class that can be overridden to implement a test platform. It -// delegates all operations to a given platform at the time of construction. -// Breaks if tasks cache the platform themselves. +// delegates all operations to the default platform. class TestPlatform : public v8::Platform { public: - // Users inheriting from `TestPlatform` need to invoke `NotifyPlatformReady()` - // at the end of their constructor. - void NotifyPlatformReady(); - - // Eagerly removes the platform from being used by V8. - void RemovePlatform(); - - TestPlatform(const TestPlatform&) = delete; - TestPlatform& operator=(const TestPlatform&) = delete; + ~TestPlatform() override = default; // v8::Platform implementation. v8::PageAllocator* GetPageAllocator() override; @@ -734,14 +737,7 @@ class TestPlatform : public v8::Platform { v8::TracingController* GetTracingController() override; protected: - TestPlatform(); - ~TestPlatform() override; - - v8::Platform* old_platform() const { return old_platform_; } - - private: - std::atomic old_platform_; - bool active_ = false; + TestPlatform() = default; }; #if defined(USE_SIMULATOR) diff --git a/test/cctest/heap/test-incremental-marking.cc b/test/cctest/heap/test-incremental-marking.cc index cfc15ebafd..7bda4c808a 100644 --- a/test/cctest/heap/test-incremental-marking.cc +++ b/test/cctest/heap/test-incremental-marking.cc @@ -35,11 +35,10 @@ namespace heap { class MockPlatform : public TestPlatform { public: - MockPlatform() : taskrunner_(new MockTaskRunner()) { NotifyPlatformReady(); } + MockPlatform() : taskrunner_(new MockTaskRunner()) {} ~MockPlatform() override { - RemovePlatform(); for (auto& task : worker_tasks_) { - old_platform()->CallOnWorkerThread(std::move(task)); + CcTest::default_platform()->CallOnWorkerThread(std::move(task)); } worker_tasks_.clear(); } @@ -103,14 +102,11 @@ class MockPlatform : public TestPlatform { std::vector> worker_tasks_; }; -UNINITIALIZED_TEST(IncrementalMarkingUsingTasks) { +TEST_WITH_PLATFORM(IncrementalMarkingUsingTasks, MockPlatform) { if (!i::FLAG_incremental_marking) return; FLAG_stress_concurrent_allocation = false; // For SimulateFullSpace. FLAG_stress_incremental_marking = false; - MockPlatform platform; - v8::Isolate::CreateParams create_params; - create_params.array_buffer_allocator = CcTest::array_buffer_allocator(); - v8::Isolate* isolate = v8::Isolate::New(create_params); + v8::Isolate* isolate = CcTest::isolate(); { v8::HandleScope handle_scope(isolate); v8::Local context = CcTest::NewContext(isolate); @@ -134,7 +130,6 @@ UNINITIALIZED_TEST(IncrementalMarkingUsingTasks) { } CHECK(marking->IsStopped()); } - isolate->Dispose(); } } // namespace heap diff --git a/test/cctest/heap/test-memory-measurement.cc b/test/cctest/heap/test-memory-measurement.cc index d7b4dc5522..8ad4247514 100644 --- a/test/cctest/heap/test-memory-measurement.cc +++ b/test/cctest/heap/test-memory-measurement.cc @@ -132,10 +132,7 @@ namespace { class MockPlatform : public TestPlatform { public: - MockPlatform() : mock_task_runner_(new MockTaskRunner()) { - NotifyPlatformReady(); - } - ~MockPlatform() override { RemovePlatform(); } + MockPlatform() : mock_task_runner_(new MockTaskRunner()) {} std::shared_ptr GetForegroundTaskRunner( v8::Isolate*) override { @@ -199,14 +196,10 @@ class MockMeasureMemoryDelegate : public v8::MeasureMemoryDelegate { } // namespace -TEST(RandomizedTimeout) { - MockPlatform platform; +TEST_WITH_PLATFORM(RandomizedTimeout, MockPlatform) { v8::Isolate::CreateParams create_params; create_params.array_buffer_allocator = CcTest::array_buffer_allocator(); - // We have to create the isolate manually here. Using CcTest::isolate() would - // lead to the situation when the isolate outlives MockPlatform which may lead - // to UAF on the background thread. - v8::Isolate* isolate = v8::Isolate::New(create_params); + v8::Isolate* isolate = CcTest::isolate(); std::vector delays; for (int i = 0; i < 10; i++) { isolate->MeasureMemory(std::make_unique()); @@ -214,7 +207,6 @@ TEST(RandomizedTimeout) { platform.PerformTask(); } std::sort(delays.begin(), delays.end()); - isolate->Dispose(); CHECK_LT(delays[0], delays.back()); } diff --git a/test/cctest/heap/test-unmapper.cc b/test/cctest/heap/test-unmapper.cc index 02694f3e69..2297409f2d 100644 --- a/test/cctest/heap/test-unmapper.cc +++ b/test/cctest/heap/test-unmapper.cc @@ -20,11 +20,9 @@ namespace heap { class MockPlatformForUnmapper : public TestPlatform { public: - MockPlatformForUnmapper() { NotifyPlatformReady(); } ~MockPlatformForUnmapper() override { - RemovePlatform(); for (auto& task : worker_tasks_) { - old_platform()->CallOnWorkerThread(std::move(task)); + CcTest::default_platform()->CallOnWorkerThread(std::move(task)); } worker_tasks_.clear(); } diff --git a/test/cctest/test-allocation.cc b/test/cctest/test-allocation.cc index e868a70c3b..0081e8e29c 100644 --- a/test/cctest/test-allocation.cc +++ b/test/cctest/test-allocation.cc @@ -32,11 +32,7 @@ namespace { // Implementation of v8::Platform that can register OOM callbacks. class AllocationPlatform : public TestPlatform { public: - AllocationPlatform() { - current_platform = this; - NotifyPlatformReady(); - } - ~AllocationPlatform() override { RemovePlatform(); } + AllocationPlatform() { current_platform = this; } void OnCriticalMemoryPressure() override { oom_callback_called = true; } @@ -94,8 +90,7 @@ void OnAlignedAllocOOM(const char* location, const char* message) { } // namespace -TEST(AccountingAllocatorOOM) { - AllocationPlatform platform; +TEST_WITH_PLATFORM(AccountingAllocatorOOM, AllocationPlatform) { v8::internal::AccountingAllocator allocator; CHECK(!platform.oom_callback_called); const bool support_compression = false; @@ -105,8 +100,7 @@ TEST(AccountingAllocatorOOM) { CHECK_EQ(result == nullptr, platform.oom_callback_called); } -TEST(AccountingAllocatorCurrentAndMax) { - AllocationPlatform platform; +TEST_WITH_PLATFORM(AccountingAllocatorCurrentAndMax, AllocationPlatform) { v8::internal::AccountingAllocator allocator; static constexpr size_t kAllocationSizes[] = {51, 231, 27}; std::vector segments; @@ -134,8 +128,7 @@ TEST(AccountingAllocatorCurrentAndMax) { CHECK(!platform.oom_callback_called); } -TEST(MallocedOperatorNewOOM) { - AllocationPlatform platform; +TEST_WITH_PLATFORM(MallocedOperatorNewOOM, AllocationPlatform) { CHECK(!platform.oom_callback_called); CcTest::isolate()->SetFatalErrorHandler(OnMallocedOperatorNewOOM); // On failure, this won't return, since a Malloced::New failure is fatal. @@ -145,8 +138,7 @@ TEST(MallocedOperatorNewOOM) { CHECK_EQ(result == nullptr, platform.oom_callback_called); } -TEST(NewArrayOOM) { - AllocationPlatform platform; +TEST_WITH_PLATFORM(NewArrayOOM, AllocationPlatform) { CHECK(!platform.oom_callback_called); CcTest::isolate()->SetFatalErrorHandler(OnNewArrayOOM); // On failure, this won't return, since a NewArray failure is fatal. @@ -156,8 +148,7 @@ TEST(NewArrayOOM) { CHECK_EQ(result == nullptr, platform.oom_callback_called); } -TEST(AlignedAllocOOM) { - AllocationPlatform platform; +TEST_WITH_PLATFORM(AlignedAllocOOM, AllocationPlatform) { CHECK(!platform.oom_callback_called); CcTest::isolate()->SetFatalErrorHandler(OnAlignedAllocOOM); // On failure, this won't return, since an AlignedAlloc failure is fatal. @@ -168,8 +159,7 @@ TEST(AlignedAllocOOM) { CHECK_EQ(result == nullptr, platform.oom_callback_called); } -TEST(AllocVirtualMemoryOOM) { - AllocationPlatform platform; +TEST_WITH_PLATFORM(AllocVirtualMemoryOOM, AllocationPlatform) { CHECK(!platform.oom_callback_called); v8::internal::VirtualMemory result(v8::internal::GetPlatformPageAllocator(), GetHugeMemoryAmount(), nullptr); @@ -177,8 +167,7 @@ TEST(AllocVirtualMemoryOOM) { CHECK_IMPLIES(!result.IsReserved(), platform.oom_callback_called); } -TEST(AlignedAllocVirtualMemoryOOM) { - AllocationPlatform platform; +TEST_WITH_PLATFORM(AlignedAllocVirtualMemoryOOM, AllocationPlatform) { CHECK(!platform.oom_callback_called); v8::internal::VirtualMemory result(v8::internal::GetPlatformPageAllocator(), GetHugeMemoryAmount(), nullptr, diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index e91045d8b6..a828b45062 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -33,6 +33,8 @@ #include #include +#include "test/cctest/cctest.h" + #if V8_OS_POSIX #include #endif @@ -23104,9 +23106,6 @@ namespace { class MockPlatform final : public TestPlatform { public: - MockPlatform() { NotifyPlatformReady(); } - ~MockPlatform() final { RemovePlatform(); } - bool dump_without_crashing_called() const { return dump_without_crashing_called_; } @@ -23119,9 +23118,7 @@ class MockPlatform final : public TestPlatform { } // namespace -TEST(DumpOnJavascriptExecution) { - MockPlatform platform; - +TEST_WITH_PLATFORM(DumpOnJavascriptExecution, MockPlatform) { LocalContext context; v8::Isolate* isolate = context->GetIsolate(); v8::HandleScope scope(isolate); diff --git a/test/cctest/test-profile-generator.cc b/test/cctest/test-profile-generator.cc index 216cd280db..83369a469a 100644 --- a/test/cctest/test-profile-generator.cc +++ b/test/cctest/test-profile-generator.cc @@ -530,10 +530,7 @@ class DiscardedSamplesDelegateImpl : public v8::DiscardedSamplesDelegate { class MockPlatform final : public TestPlatform { public: - MockPlatform() : mock_task_runner_(new MockTaskRunner()) { - NotifyPlatformReady(); - } - ~MockPlatform() override { RemovePlatform(); } + MockPlatform() : mock_task_runner_(new MockTaskRunner()) {} std::shared_ptr GetForegroundTaskRunner( v8::Isolate*) override { @@ -561,6 +558,8 @@ class MockPlatform final : public TestPlatform { } bool IdleTasksEnabled() override { return false; } + bool NonNestableTasksEnabled() const override { return true; } + bool NonNestableDelayedTasksEnabled() const override { return true; } int posted_count() { return posted_count_; } @@ -574,12 +573,11 @@ class MockPlatform final : public TestPlatform { }; } // namespace -TEST(MaxSamplesCallback) { +TEST_WITH_PLATFORM(MaxSamplesCallback, MockPlatform) { i::Isolate* isolate = CcTest::i_isolate(); CpuProfilesCollection profiles(isolate); CpuProfiler profiler(isolate); profiles.set_cpu_profiler(&profiler); - MockPlatform* mock_platform = new MockPlatform(); std::unique_ptr impl = std::make_unique( DiscardedSamplesDelegateImpl()); @@ -600,7 +598,7 @@ TEST(MaxSamplesCallback) { profiles.AddPathToCurrentProfiles( sample1.timestamp, symbolized.stack_trace, symbolized.src_line, true, base::TimeDelta(), StateTag::JS, EmbedderStateTag::EMPTY); - CHECK_EQ(0, mock_platform->posted_count()); + CHECK_EQ(0, platform.posted_count()); TickSample sample2; sample2.timestamp = v8::base::TimeTicks::Now(); sample2.pc = ToPointer(0x1925); @@ -610,7 +608,7 @@ TEST(MaxSamplesCallback) { profiles.AddPathToCurrentProfiles( sample2.timestamp, symbolized.stack_trace, symbolized.src_line, true, base::TimeDelta(), StateTag::JS, EmbedderStateTag::EMPTY); - CHECK_EQ(1, mock_platform->posted_count()); + CHECK_EQ(1, platform.posted_count()); TickSample sample3; sample3.timestamp = v8::base::TimeTicks::Now(); sample3.pc = ToPointer(0x1510); @@ -619,11 +617,10 @@ TEST(MaxSamplesCallback) { profiles.AddPathToCurrentProfiles( sample3.timestamp, symbolized.stack_trace, symbolized.src_line, true, base::TimeDelta(), StateTag::JS, EmbedderStateTag::EMPTY); - CHECK_EQ(1, mock_platform->posted_count()); + CHECK_EQ(1, platform.posted_count()); // Teardown profiles.StopProfiling(""); - delete mock_platform; } TEST(NoSamples) { diff --git a/test/cctest/test-trace-event.cc b/test/cctest/test-trace-event.cc index cd345565a2..1f25a9a212 100644 --- a/test/cctest/test-trace-event.cc +++ b/test/cctest/test-trace-event.cc @@ -6,6 +6,7 @@ #include #include "include/v8-function.h" +#include "include/v8-platform.h" #include "src/init/v8.h" #include "src/tracing/trace-event.h" #include "test/cctest/cctest.h" @@ -86,9 +87,6 @@ class MockTracingController : public v8::TracingController { class MockTracingPlatform : public TestPlatform { public: - MockTracingPlatform() { NotifyPlatformReady(); } - ~MockTracingPlatform() override { RemovePlatform(); } - v8::TracingController* GetTracingController() override { return &tracing_controller_; } @@ -107,18 +105,14 @@ class MockTracingPlatform : public TestPlatform { } // namespace -TEST(TraceEventDisabledCategory) { - MockTracingPlatform platform; - +TEST_WITH_PLATFORM(TraceEventDisabledCategory, MockTracingPlatform) { // Disabled category, will not add events. TRACE_EVENT_BEGIN0("cat", "e1"); TRACE_EVENT_END0("cat", "e1"); CHECK_EQ(0, platform.NumberOfTraceObjects()); } -TEST(TraceEventNoArgs) { - MockTracingPlatform platform; - +TEST_WITH_PLATFORM(TraceEventNoArgs, MockTracingPlatform) { // Enabled category will add 2 events. TRACE_EVENT_BEGIN0("v8-cat", "e1"); TRACE_EVENT_END0("v8-cat", "e1"); @@ -133,9 +127,7 @@ TEST(TraceEventNoArgs) { CHECK_EQ(0, platform.GetTraceObject(1)->num_args); } -TEST(TraceEventWithOneArg) { - MockTracingPlatform platform; - +TEST_WITH_PLATFORM(TraceEventWithOneArg, MockTracingPlatform) { TRACE_EVENT_BEGIN1("v8-cat", "e1", "arg1", 42); TRACE_EVENT_END1("v8-cat", "e1", "arg1", 42); TRACE_EVENT_BEGIN1("v8-cat", "e2", "arg1", "abc"); @@ -149,9 +141,7 @@ TEST(TraceEventWithOneArg) { CHECK_EQ(1, platform.GetTraceObject(3)->num_args); } -TEST(TraceEventWithTwoArgs) { - MockTracingPlatform platform; - +TEST_WITH_PLATFORM(TraceEventWithTwoArgs, MockTracingPlatform) { TRACE_EVENT_BEGIN2("v8-cat", "e1", "arg1", 42, "arg2", "abc"); TRACE_EVENT_END2("v8-cat", "e1", "arg1", 42, "arg2", "abc"); TRACE_EVENT_BEGIN2("v8-cat", "e2", "arg1", "abc", "arg2", 43); @@ -165,9 +155,7 @@ TEST(TraceEventWithTwoArgs) { CHECK_EQ(2, platform.GetTraceObject(3)->num_args); } -TEST(ScopedTraceEvent) { - MockTracingPlatform platform; - +TEST_WITH_PLATFORM(ScopedTraceEvent, MockTracingPlatform) { { TRACE_EVENT0("v8-cat", "e"); } CHECK_EQ(1, platform.NumberOfTraceObjects()); @@ -184,9 +172,7 @@ TEST(ScopedTraceEvent) { CHECK_EQ(2, platform.GetTraceObject(2)->num_args); } -TEST(TestEventWithFlow) { - MockTracingPlatform platform; - +TEST_WITH_PLATFORM(TestEventWithFlow, MockTracingPlatform) { static uint64_t bind_id = 21; { TRACE_EVENT_WITH_FLOW0("v8-cat", "f1", bind_id, TRACE_EVENT_FLAG_FLOW_OUT); @@ -208,9 +194,7 @@ TEST(TestEventWithFlow) { CHECK_EQ(TRACE_EVENT_FLAG_FLOW_IN, platform.GetTraceObject(2)->flags); } -TEST(TestEventWithId) { - MockTracingPlatform platform; - +TEST_WITH_PLATFORM(TestEventWithId, MockTracingPlatform) { static uint64_t event_id = 21; TRACE_EVENT_ASYNC_BEGIN0("v8-cat", "a1", event_id); TRACE_EVENT_ASYNC_END0("v8-cat", "a1", event_id); @@ -222,9 +206,7 @@ TEST(TestEventWithId) { CHECK_EQ(event_id, platform.GetTraceObject(1)->id); } -TEST(TestEventWithTimestamp) { - MockTracingPlatform platform; - +TEST_WITH_PLATFORM(TestEventWithTimestamp, MockTracingPlatform) { TRACE_EVENT_INSTANT_WITH_TIMESTAMP0("v8-cat", "0arg", TRACE_EVENT_SCOPE_GLOBAL, 1729); TRACE_EVENT_INSTANT_WITH_TIMESTAMP1("v8-cat", "1arg", @@ -251,9 +233,8 @@ TEST(TestEventWithTimestamp) { CHECK_EQ(32832, platform.GetTraceObject(4)->timestamp); } -TEST(BuiltinsIsTraceCategoryEnabled) { +TEST_WITH_PLATFORM(BuiltinsIsTraceCategoryEnabled, MockTracingPlatform) { CcTest::InitializeVM(); - MockTracingPlatform platform; v8::Isolate* isolate = CcTest::isolate(); v8::HandleScope handle_scope(isolate); @@ -299,9 +280,8 @@ TEST(BuiltinsIsTraceCategoryEnabled) { } } -TEST(BuiltinsTrace) { +TEST_WITH_PLATFORM(BuiltinsTrace, MockTracingPlatform) { CcTest::InitializeVM(); - MockTracingPlatform platform; v8::Isolate* isolate = CcTest::isolate(); v8::HandleScope handle_scope(isolate); diff --git a/test/cctest/wasm/test-streaming-compilation.cc b/test/cctest/wasm/test-streaming-compilation.cc index 9e29059665..b30d8983f4 100644 --- a/test/cctest/wasm/test-streaming-compilation.cc +++ b/test/cctest/wasm/test-streaming-compilation.cc @@ -29,12 +29,9 @@ namespace wasm { class MockPlatform final : public TestPlatform { public: - MockPlatform() : task_runner_(std::make_shared()) { - NotifyPlatformReady(); - } + MockPlatform() : task_runner_(std::make_shared()) {} ~MockPlatform() { - RemovePlatform(); for (auto* job_handle : job_handles_) job_handle->ResetPlatform(); } @@ -239,25 +236,18 @@ class StreamTester { }; } // namespace -#define RUN_STREAM(name) \ - MockPlatform mock_platform; \ - CHECK_EQ(V8::GetCurrentPlatform(), &mock_platform); \ - v8::Isolate::CreateParams create_params; \ - create_params.array_buffer_allocator = CcTest::array_buffer_allocator(); \ - v8::Isolate* isolate = v8::Isolate::New(create_params); \ - { \ - v8::HandleScope handle_scope(isolate); \ - v8::Local context = v8::Context::New(isolate); \ - v8::Context::Scope context_scope(context); \ - RunStream_##name(&mock_platform, isolate); \ - } \ - isolate->Dispose(); +#define RUN_STREAM(name) \ + v8::Isolate* isolate = CcTest::isolate(); \ + v8::HandleScope handle_scope(isolate); \ + v8::Local context = v8::Context::New(isolate); \ + v8::Context::Scope context_scope(context); \ + RunStream_##name(&platform, isolate); #define STREAM_TEST(name) \ void RunStream_##name(MockPlatform*, v8::Isolate*); \ - UNINITIALIZED_TEST(Async##name) { RUN_STREAM(name); } \ + TEST_WITH_PLATFORM(Async##name, MockPlatform) { RUN_STREAM(name); } \ \ - UNINITIALIZED_TEST(SingleThreaded##name) { \ + TEST_WITH_PLATFORM(SingleThreaded##name, MockPlatform) { \ i::FlagScope single_threaded_scope(&i::FLAG_single_threaded, true); \ RUN_STREAM(name); \ } \ diff --git a/test/cctest/wasm/test-wasm-metrics.cc b/test/cctest/wasm/test-wasm-metrics.cc index e575136764..6e54c0535f 100644 --- a/test/cctest/wasm/test-wasm-metrics.cc +++ b/test/cctest/wasm/test-wasm-metrics.cc @@ -6,6 +6,7 @@ #include "include/libplatform/libplatform.h" #include "include/v8-metrics.h" +#include "include/v8-platform.h" #include "src/api/api-inl.h" #include "src/base/platform/time.h" #include "src/wasm/wasm-engine.h" @@ -24,12 +25,9 @@ namespace { class MockPlatform final : public TestPlatform { public: - MockPlatform() : task_runner_(std::make_shared()) { - NotifyPlatformReady(); - } + MockPlatform() : task_runner_(std::make_shared()) {} ~MockPlatform() override { - RemovePlatform(); for (auto* job_handle : job_handles_) job_handle->ResetPlatform(); } @@ -208,32 +206,24 @@ class TestCompileResolver : public CompilationResultResolver { } // namespace -#define RUN_COMPILE(name) \ - MockPlatform mock_platform; \ - CHECK_EQ(V8::GetCurrentPlatform(), &mock_platform); \ - v8::Isolate::CreateParams create_params; \ - create_params.array_buffer_allocator = CcTest::array_buffer_allocator(); \ - v8::Isolate* isolate = v8::Isolate::New(create_params); \ - { \ - v8::HandleScope handle_scope(isolate); \ - v8::Local context = v8::Context::New(isolate); \ - v8::Context::Scope context_scope(context); \ - Isolate* i_isolate = reinterpret_cast(isolate); \ - testing::SetupIsolateForWasmModule(i_isolate); \ - RunCompile_##name(&mock_platform, i_isolate); \ - } \ - isolate->Dispose(); +#define RUN_COMPILE(name) \ + v8::HandleScope handle_scope(CcTest::isolate()); \ + v8::Local context = v8::Context::New(CcTest::isolate()); \ + v8::Context::Scope context_scope(context); \ + Isolate* i_isolate = CcTest::i_isolate(); \ + testing::SetupIsolateForWasmModule(i_isolate); \ + RunCompile_##name(&platform, i_isolate); #define COMPILE_TEST(name) \ void RunCompile_##name(MockPlatform*, i::Isolate*); \ - UNINITIALIZED_TEST(Sync##name) { \ + TEST_WITH_PLATFORM(Sync##name, MockPlatform) { \ i::FlagScope sync_scope(&i::FLAG_wasm_async_compilation, false); \ RUN_COMPILE(name); \ } \ \ - UNINITIALIZED_TEST(Async##name) { RUN_COMPILE(name); } \ + TEST_WITH_PLATFORM(Async##name, MockPlatform) { RUN_COMPILE(name); } \ \ - UNINITIALIZED_TEST(Streaming##name) { \ + TEST_WITH_PLATFORM(Streaming##name, MockPlatform) { \ i::FlagScope streaming_scope(&i::FLAG_wasm_test_streaming, true); \ RUN_COMPILE(name); \ } \