Skip to content

V
V8
Commit efd8c2d9 authored by Paolo Severini's avatar Paolo Severini Committed by Commit Bot
Browse files
Options

Remove --win64-unwinding-info flag and always generate unwind info on Win/x64 

The generation of unwind info to enable stack walking on Windows/x64
(https://chromium-review.googlesource.com/c/v8/v8/+/1469329) was implemented
behind a temporary flag, in order to coordinate these changes with the
corresponding changes in Chromium.

The required changes to Chromium
(https://chromium-review.googlesource.com/c/chromium/src/+/1474703) have also
been merged, so we can now remove the flag and enable the generation of stack
unwinding info by default on Windows/x64.

Bug: v8:3598
Change-Id: I88814aaeabecc007f5262227aa0681a1d16156d5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1573138Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Commit-Queue: Paolo Severini <paolosev@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#61020}
parent be913785
Hide whitespace changes
Inline Side-by-side
Showing with 47 additions and 13 deletions
BUILD.gn
View file @ efd8c2d9
......@@ -89,7 +89,7 @@ declare_args() {
v8_enable_embedded_builtins = true
# Enable the registration of unwinding info for Windows/x64.
v8_win64_unwinding_info = false
v8_win64_unwinding_info = true
# Enable code comments for builtins in the snapshot (impacts performance).
v8_enable_snapshot_code_comments = false
......@@ -1170,10 +1170,6 @@ template("run_mksnapshot") {
args += invoker.args
if (v8_win64_unwinding_info) {
args += [ "--win64-unwinding-info" ]
}
if (v8_enable_embedded_builtins) {
outputs += [ "$target_gen_dir/embedded${suffix}.S" ]
args += [
......
src/flag-definitions.h
View file @ efd8c2d9
......@@ -1418,8 +1418,7 @@ DEFINE_STRING(redirect_code_traces_to, nullptr,
DEFINE_BOOL(print_opt_source, false,
"print source code of optimized and inlined functions")
DEFINE_BOOL(win64_unwinding_info, false,
"Enable unwinding info for Windows/x64 (experimental).")
DEFINE_BOOL(win64_unwinding_info, true, "Enable unwinding info for Windows/x64")
#ifdef V8_TARGET_ARCH_ARM
// Unsupported on arm. See https://crbug.com/v8/8713.
......
src/wasm/wasm-code-manager.cc
View file @ efd8c2d9
......@@ -447,8 +447,8 @@ NativeModule::NativeModule(WasmEngine* engine, const WasmFeatures& enabled,
// See src/heap/spaces.cc, MemoryAllocator::InitializeCodePageAllocator() and
// https://cs.chromium.org/chromium/src/components/crash/content/app/crashpad_win.cc?rcl=fd680447881449fba2edcf0589320e7253719212&l=204
// for details.
if (win64_unwindinfo::CanRegisterUnwindInfoForNonABICompliantCodeRange() &&
FLAG_win64_unwinding_info) {
if (engine_->code_manager()
->CanRegisterUnwindInfoForNonABICompliantCodeRange()) {
AllocateForCode(Heap::GetCodeRangeReservedAreaSize());
}
#endif
......@@ -1096,11 +1096,22 @@ WasmCodeManager::WasmCodeManager(WasmMemoryTracker* memory_tracker,
size_t max_committed)
: memory_tracker_(memory_tracker),
max_committed_code_space_(max_committed),
#if defined(V8_OS_WIN_X64)
is_win64_unwind_info_disabled_for_testing_(false),
#endif
total_committed_code_space_(0),
critical_committed_code_space_(max_committed / 2) {
DCHECK_LE(max_committed, kMaxWasmCodeMemory);
}
#if defined(V8_OS_WIN_X64)
bool WasmCodeManager::CanRegisterUnwindInfoForNonABICompliantCodeRange() const {
return win64_unwindinfo::CanRegisterUnwindInfoForNonABICompliantCodeRange() &&
FLAG_win64_unwinding_info &&
!is_win64_unwind_info_disabled_for_testing_;
}
#endif
bool WasmCodeManager::Commit(Address start, size_t size) {
// TODO(v8:8462) Remove eager commit once perf supports remapping.
if (FLAG_perf_prof) return true;
......@@ -1254,8 +1265,7 @@ std::shared_ptr<NativeModule> WasmCodeManager::NewNativeModule(
size);
#if defined(V8_OS_WIN_X64)
if (win64_unwindinfo::CanRegisterUnwindInfoForNonABICompliantCodeRange() &&
FLAG_win64_unwinding_info) {
if (CanRegisterUnwindInfoForNonABICompliantCodeRange()) {
win64_unwindinfo::RegisterNonABICompliantCodeRange(
reinterpret_cast<void*>(start), size);
}
......@@ -1414,8 +1424,7 @@ void WasmCodeManager::FreeNativeModule(NativeModule* native_module) {
code_space.address(), code_space.end(), code_space.size());
#if defined(V8_OS_WIN_X64)
if (win64_unwindinfo::CanRegisterUnwindInfoForNonABICompliantCodeRange() &&
FLAG_win64_unwinding_info) {
if (CanRegisterUnwindInfoForNonABICompliantCodeRange()) {
win64_unwindinfo::UnregisterNonABICompliantCodeRange(
reinterpret_cast<void*>(code_space.address()));
}
......
src/wasm/wasm-code-manager.h
View file @ efd8c2d9
......@@ -546,6 +546,10 @@ class V8_EXPORT_PRIVATE WasmCodeManager final {
}
#endif
#if defined(V8_OS_WIN_X64)
bool CanRegisterUnwindInfoForNonABICompliantCodeRange() const;
#endif
NativeModule* LookupNativeModule(Address pc) const;
WasmCode* LookupCode(Address pc) const;
size_t committed_code_space() const {
......@@ -554,6 +558,12 @@ class V8_EXPORT_PRIVATE WasmCodeManager final {
void SetMaxCommittedMemoryForTesting(size_t limit);
#if defined(V8_OS_WIN_X64)
void DisableWin64UnwindInfoForTesting() {
is_win64_unwind_info_disabled_for_testing_ = true;
}
#endif
static size_t EstimateNativeModuleCodeSize(const WasmModule* module);
static size_t EstimateNativeModuleNonCodeSize(const WasmModule* module);
......@@ -581,6 +591,10 @@ class V8_EXPORT_PRIVATE WasmCodeManager final {
size_t max_committed_code_space_;
#if defined(V8_OS_WIN_X64)
bool is_win64_unwind_info_disabled_for_testing_;
#endif
std::atomic<size_t> total_committed_code_space_;
// 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
......
test/unittests/wasm/wasm-code-manager-unittest.cc
View file @ efd8c2d9
......@@ -193,6 +193,12 @@ class WasmCodeManagerTest : public TestWithContext,
void SetMaxCommittedMemory(size_t limit) {
manager()->SetMaxCommittedMemoryForTesting(limit);
}
void DisableWin64UnwindInfoForTesting() {
#if defined(V8_OS_WIN_X64)
manager()->DisableWin64UnwindInfoForTesting();
#endif
}
};
// static
......@@ -212,6 +218,8 @@ TEST_P(WasmCodeManagerTest, EmptyCase) {
TEST_P(WasmCodeManagerTest, AllocateAndGoOverLimit) {
SetMaxCommittedMemory(page_size);
DisableWin64UnwindInfoForTesting();
CHECK_EQ(0, manager()->committed_code_space());
NativeModulePtr native_module = AllocModule(page_size, GetParam());
CHECK(native_module);
......@@ -241,6 +249,8 @@ TEST_P(WasmCodeManagerTest, AllocateAndGoOverLimit) {
TEST_P(WasmCodeManagerTest, TotalLimitIrrespectiveOfModuleCount) {
SetMaxCommittedMemory(3 * page_size);
DisableWin64UnwindInfoForTesting();
NativeModulePtr nm1 = AllocModule(2 * page_size, GetParam());
NativeModulePtr nm2 = AllocModule(2 * page_size, GetParam());
CHECK(nm1);
......@@ -255,6 +265,8 @@ TEST_P(WasmCodeManagerTest, TotalLimitIrrespectiveOfModuleCount) {
TEST_P(WasmCodeManagerTest, GrowingVsFixedModule) {
SetMaxCommittedMemory(3 * page_size);
DisableWin64UnwindInfoForTesting();
NativeModulePtr nm = AllocModule(page_size, GetParam());
size_t module_size = GetParam() == Fixed ? kMaxWasmCodeMemory : page_size;
size_t remaining_space_in_module = module_size - kJumpTableSize;
......@@ -275,6 +287,8 @@ TEST_P(WasmCodeManagerTest, GrowingVsFixedModule) {
TEST_P(WasmCodeManagerTest, CommitIncrements) {
SetMaxCommittedMemory(10 * page_size);
DisableWin64UnwindInfoForTesting();
NativeModulePtr nm = AllocModule(3 * page_size, GetParam());
WasmCodeRefScope code_ref_scope;
WasmCode* code = AddCode(nm.get(), 0, kCodeAlignment);
......@@ -290,6 +304,7 @@ TEST_P(WasmCodeManagerTest, CommitIncrements) {
TEST_P(WasmCodeManagerTest, Lookup) {
SetMaxCommittedMemory(2 * page_size);
DisableWin64UnwindInfoForTesting();
NativeModulePtr nm1 = AllocModule(page_size, GetParam());
NativeModulePtr nm2 = AllocModule(page_size, GetParam());
......@@ -335,6 +350,7 @@ TEST_P(WasmCodeManagerTest, Lookup) {
TEST_P(WasmCodeManagerTest, LookupWorksAfterRewrite) {
SetMaxCommittedMemory(2 * page_size);
DisableWin64UnwindInfoForTesting();
NativeModulePtr nm1 = AllocModule(page_size, GetParam());
......
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