From c75db59a0a85f0c4798b8d5e20f9bdb9f714baf2 Mon Sep 17 00:00:00 2001 From: Clemens Hammacher Date: Thu, 1 Aug 2019 11:08:07 +0200 Subject: [PATCH] [wasm] Add test mode without implicit allocations The unittest for {WasmCodeManager} currently disables implicit allocations for win64 unwind info, but still deals with the implicitly allocated jump table. With the addition of a far jump table, this logic would get even more complex. Thus this CL introduces a testing flag on the {WasmCodeManager} to disable all implicit allocations, and uses that instead in the {WasmCodeManagerTest}. R=mstarzinger@chromium.org Bug: v8:9477 Change-Id: I45e4bc6b9fec6d7286bf6b45f778681ae0dba746 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1725622 Commit-Queue: Clemens Hammacher Reviewed-by: Michael Starzinger Cr-Commit-Position: refs/heads/master@{#63025} --- src/wasm/wasm-code-manager.cc | 34 ++++++++++-------- src/wasm/wasm-code-manager.h | 16 ++++----- .../wasm/wasm-code-manager-unittest.cc | 35 ++++++------------- 3 files changed, 38 insertions(+), 47 deletions(-) diff --git a/src/wasm/wasm-code-manager.cc b/src/wasm/wasm-code-manager.cc index 9016b9fa2f..f1a1044d00 100644 --- a/src/wasm/wasm-code-manager.cc +++ b/src/wasm/wasm-code-manager.cc @@ -643,6 +643,8 @@ NativeModule::NativeModule(WasmEngine* engine, const WasmFeatures& enabled, CompilationState::New(*shared_this, std::move(async_counters)); DCHECK_NOT_NULL(module_); + const bool implicit_alloc_disabled = + engine->code_manager()->IsImplicitAllocationsDisabledForTesting(); #if defined(V8_OS_WIN64) // On some platforms, specifically Win64, we need to reserve some pages at // the beginning of an executable space. @@ -650,7 +652,8 @@ NativeModule::NativeModule(WasmEngine* engine, const WasmFeatures& enabled, // https://cs.chromium.org/chromium/src/components/crash/content/app/crashpad_win.cc?rcl=fd680447881449fba2edcf0589320e7253719212&l=204 // for details. if (engine_->code_manager() - ->CanRegisterUnwindInfoForNonABICompliantCodeRange()) { + ->CanRegisterUnwindInfoForNonABICompliantCodeRange() && + !implicit_alloc_disabled) { code_allocator_.AllocateForCode(this, Heap::GetCodeRangeReservedAreaSize()); } #endif // V8_OS_WIN64 @@ -659,9 +662,11 @@ NativeModule::NativeModule(WasmEngine* engine, const WasmFeatures& enabled, if (num_wasm_functions > 0) { code_table_.reset(new WasmCode* [num_wasm_functions] {}); - WasmCodeRefScope code_ref_scope; - jump_table_ = CreateEmptyJumpTable( - JumpTableAssembler::SizeForNumberOfSlots(num_wasm_functions)); + if (!implicit_alloc_disabled) { + WasmCodeRefScope code_ref_scope; + jump_table_ = CreateEmptyJumpTable( + JumpTableAssembler::SizeForNumberOfSlots(num_wasm_functions)); + } } } @@ -1001,8 +1006,12 @@ WasmCode* NativeModule::PublishCodeLocked(std::unique_ptr code) { // Populate optimized code to the jump table unless there is an active // redirection to the interpreter that should be preserved. - bool update_jump_table = - update_code_table && !has_interpreter_redirection(code->index()); + DCHECK_IMPLIES( + jump_table_ == nullptr, + engine_->code_manager()->IsImplicitAllocationsDisabledForTesting()); + bool update_jump_table = update_code_table && + !has_interpreter_redirection(code->index()) && + jump_table_; // Ensure that interpreter entries always populate to the jump table. if (code->kind_ == WasmCode::Kind::kInterpreterEntry) { @@ -1187,10 +1196,6 @@ WasmCodeManager::WasmCodeManager(WasmMemoryTracker* memory_tracker, size_t max_committed) : memory_tracker_(memory_tracker), max_committed_code_space_(max_committed), -#if defined(V8_OS_WIN64) - is_win64_unwind_info_disabled_for_testing_(false), -#endif // V8_OS_WIN64 - total_committed_code_space_(0), critical_committed_code_space_(max_committed / 2) { DCHECK_LE(max_committed, kMaxWasmCodeMemory); } @@ -1198,8 +1203,7 @@ WasmCodeManager::WasmCodeManager(WasmMemoryTracker* memory_tracker, #if defined(V8_OS_WIN64) bool WasmCodeManager::CanRegisterUnwindInfoForNonABICompliantCodeRange() const { return win64_unwindinfo::CanRegisterUnwindInfoForNonABICompliantCodeRange() && - FLAG_win64_unwinding_info && - !is_win64_unwind_info_disabled_for_testing_; + FLAG_win64_unwinding_info; } #endif // V8_OS_WIN64 @@ -1370,7 +1374,8 @@ std::shared_ptr WasmCodeManager::NewNativeModule( size); #if defined(V8_OS_WIN64) - if (CanRegisterUnwindInfoForNonABICompliantCodeRange()) { + if (CanRegisterUnwindInfoForNonABICompliantCodeRange() && + !implicit_allocations_disabled_for_testing_) { win64_unwindinfo::RegisterNonABICompliantCodeRange( reinterpret_cast(start), size); } @@ -1488,7 +1493,8 @@ void WasmCodeManager::FreeNativeModule(Vector owned_code_space, code_space.address(), code_space.end(), code_space.size()); #if defined(V8_OS_WIN64) - if (CanRegisterUnwindInfoForNonABICompliantCodeRange()) { + if (CanRegisterUnwindInfoForNonABICompliantCodeRange() && + !implicit_allocations_disabled_for_testing_) { win64_unwindinfo::UnregisterNonABICompliantCodeRange( reinterpret_cast(code_space.address())); } diff --git a/src/wasm/wasm-code-manager.h b/src/wasm/wasm-code-manager.h index cdc818872d..5583435392 100644 --- a/src/wasm/wasm-code-manager.h +++ b/src/wasm/wasm-code-manager.h @@ -622,11 +622,13 @@ class V8_EXPORT_PRIVATE WasmCodeManager final { void SetMaxCommittedMemoryForTesting(size_t limit); -#if defined(V8_OS_WIN64) - void DisableWin64UnwindInfoForTesting() { - is_win64_unwind_info_disabled_for_testing_ = true; + void DisableImplicitAllocationsForTesting() { + implicit_allocations_disabled_for_testing_ = true; + } + + bool IsImplicitAllocationsDisabledForTesting() const { + return implicit_allocations_disabled_for_testing_; } -#endif // V8_OS_WIN64 static size_t EstimateNativeModuleCodeSize(const WasmModule* module); static size_t EstimateNativeModuleNonCodeSize(const WasmModule* module); @@ -654,11 +656,9 @@ class V8_EXPORT_PRIVATE WasmCodeManager final { size_t max_committed_code_space_; -#if defined(V8_OS_WIN64) - bool is_win64_unwind_info_disabled_for_testing_; -#endif // V8_OS_WIN64 + bool implicit_allocations_disabled_for_testing_ = false; - std::atomic total_committed_code_space_; + std::atomic total_committed_code_space_{0}; // If the committed code space exceeds {critical_committed_code_space_}, then // we trigger a GC before creating the next module. This value is set to the // currently committed space plus 50% of the available code space on creation diff --git a/test/unittests/wasm/wasm-code-manager-unittest.cc b/test/unittests/wasm/wasm-code-manager-unittest.cc index eea1f8208d..a6b29ffc6c 100644 --- a/test/unittests/wasm/wasm-code-manager-unittest.cc +++ b/test/unittests/wasm/wasm-code-manager-unittest.cc @@ -156,8 +156,6 @@ class WasmCodeManagerTest : public TestWithContext, public ::testing::WithParamInterface { public: static constexpr uint32_t kNumFunctions = 10; - static constexpr uint32_t kJumpTableSize = RoundUp( - JumpTableAssembler::SizeForNumberOfSlots(kNumFunctions)); static size_t allocate_page_size; static size_t commit_page_size; @@ -169,6 +167,7 @@ class WasmCodeManagerTest : public TestWithContext, } CHECK_NE(0, allocate_page_size); CHECK_NE(0, commit_page_size); + manager()->DisableImplicitAllocationsForTesting(); } using NativeModulePtr = std::shared_ptr; @@ -199,12 +198,6 @@ class WasmCodeManagerTest : public TestWithContext, void SetMaxCommittedMemory(size_t limit) { manager()->SetMaxCommittedMemoryForTesting(limit); } - - void DisableWin64UnwindInfoForTesting() { -#if defined(V8_OS_WIN_X64) - manager()->DisableWin64UnwindInfoForTesting(); -#endif - } }; // static @@ -219,18 +212,18 @@ TEST_P(WasmCodeManagerTest, EmptyCase) { SetMaxCommittedMemory(0); CHECK_EQ(0, manager()->committed_code_space()); - ASSERT_DEATH_IF_SUPPORTED(AllocModule(allocate_page_size, GetParam()), + NativeModulePtr native_module = AllocModule(allocate_page_size, GetParam()); + ASSERT_DEATH_IF_SUPPORTED(AddCode(native_module.get(), 0, kCodeAlignment), "OOM in wasm code commit"); } TEST_P(WasmCodeManagerTest, AllocateAndGoOverLimit) { SetMaxCommittedMemory(allocate_page_size); - DisableWin64UnwindInfoForTesting(); CHECK_EQ(0, manager()->committed_code_space()); NativeModulePtr native_module = AllocModule(allocate_page_size, GetParam()); CHECK(native_module); - CHECK_EQ(commit_page_size, manager()->committed_code_space()); + CHECK_EQ(0, manager()->committed_code_space()); WasmCodeRefScope code_ref_scope; uint32_t index = 0; WasmCode* code = AddCode(native_module.get(), index++, 1 * kCodeAlignment); @@ -242,7 +235,7 @@ TEST_P(WasmCodeManagerTest, AllocateAndGoOverLimit) { CHECK_EQ(commit_page_size, manager()->committed_code_space()); code = AddCode(native_module.get(), index++, - allocate_page_size - 4 * kCodeAlignment - kJumpTableSize); + allocate_page_size - 4 * kCodeAlignment); CHECK_NOT_NULL(code); CHECK_EQ(allocate_page_size, manager()->committed_code_space()); @@ -256,29 +249,25 @@ TEST_P(WasmCodeManagerTest, AllocateAndGoOverLimit) { TEST_P(WasmCodeManagerTest, TotalLimitIrrespectiveOfModuleCount) { SetMaxCommittedMemory(3 * allocate_page_size); - DisableWin64UnwindInfoForTesting(); NativeModulePtr nm1 = AllocModule(2 * allocate_page_size, GetParam()); NativeModulePtr nm2 = AllocModule(2 * allocate_page_size, GetParam()); CHECK(nm1); CHECK(nm2); WasmCodeRefScope code_ref_scope; - WasmCode* code = - AddCode(nm1.get(), 0, 2 * allocate_page_size - kJumpTableSize); + WasmCode* code = AddCode(nm1.get(), 0, 2 * allocate_page_size); CHECK_NOT_NULL(code); - ASSERT_DEATH_IF_SUPPORTED( - AddCode(nm2.get(), 0, 2 * allocate_page_size - kJumpTableSize), - "OOM in wasm code commit"); + ASSERT_DEATH_IF_SUPPORTED(AddCode(nm2.get(), 0, 2 * allocate_page_size), + "OOM in wasm code commit"); } TEST_P(WasmCodeManagerTest, GrowingVsFixedModule) { SetMaxCommittedMemory(3 * allocate_page_size); - DisableWin64UnwindInfoForTesting(); NativeModulePtr nm = AllocModule(allocate_page_size, GetParam()); size_t module_size = GetParam() == Fixed ? kMaxWasmCodeMemory : allocate_page_size; - size_t remaining_space_in_module = module_size - kJumpTableSize; + size_t remaining_space_in_module = module_size; if (GetParam() == Fixed) { // Requesting more than the remaining space fails because the module cannot // grow. @@ -297,7 +286,6 @@ TEST_P(WasmCodeManagerTest, GrowingVsFixedModule) { TEST_P(WasmCodeManagerTest, CommitIncrements) { SetMaxCommittedMemory(10 * allocate_page_size); - DisableWin64UnwindInfoForTesting(); NativeModulePtr nm = AllocModule(3 * allocate_page_size, GetParam()); WasmCodeRefScope code_ref_scope; @@ -308,15 +296,13 @@ TEST_P(WasmCodeManagerTest, CommitIncrements) { CHECK_NOT_NULL(code); CHECK_EQ(commit_page_size + 2 * allocate_page_size, manager()->committed_code_space()); - code = AddCode(nm.get(), 2, - allocate_page_size - kCodeAlignment - kJumpTableSize); + code = AddCode(nm.get(), 2, allocate_page_size - kCodeAlignment); CHECK_NOT_NULL(code); CHECK_EQ(3 * allocate_page_size, manager()->committed_code_space()); } TEST_P(WasmCodeManagerTest, Lookup) { SetMaxCommittedMemory(2 * allocate_page_size); - DisableWin64UnwindInfoForTesting(); NativeModulePtr nm1 = AllocModule(allocate_page_size, GetParam()); NativeModulePtr nm2 = AllocModule(allocate_page_size, GetParam()); @@ -362,7 +348,6 @@ TEST_P(WasmCodeManagerTest, Lookup) { TEST_P(WasmCodeManagerTest, LookupWorksAfterRewrite) { SetMaxCommittedMemory(2 * allocate_page_size); - DisableWin64UnwindInfoForTesting(); NativeModulePtr nm1 = AllocModule(allocate_page_size, GetParam());