From ce9cc71c4b99bc4d96d14a14b3645a95c395be05 Mon Sep 17 00:00:00 2001 From: Michael Achenbach Date: Fri, 11 Jun 2021 18:05:23 +0000 Subject: [PATCH] Revert "[no-wasm] Exclude trap-handler implementation" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 5d84b6cb9a95de61ff8aab7379cacaf92dff82bf. Reason for revert: Breaks mac-arm64: https://ci.chromium.org/p/v8/builders/ci/V8%20Mac%20-%20arm64%20-%20release/4636 https://chromium-swarm.appspot.com/task?id=5414a227cc3d6b10 Original change's description: > [no-wasm] Exclude trap-handler implementation > > The trap handler is only needed for WebAssembly, hence it can be > excluded in no-wasm builds (v8_enable_webassembly = false). > This makes it easier to port WebAssembly to platforms that do not need > to support WebAssembly. > > R=​ahaas@chromium.org, jkummerow@chromium.org > CC=​johnx@google.com > > Bug: v8:11877 > Change-Id: I25c34c2c4f1122227047e13add532ee2b9f73d2f > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2953285 > Reviewed-by: Andreas Haas > Reviewed-by: Jakob Kummerow > Commit-Queue: Clemens Backes > Cr-Commit-Position: refs/heads/master@{#75101} Bug: v8:11877 Change-Id: I7a98341f6c03667c6400dced2bc69746011dd3d4 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2956868 Auto-Submit: Michael Achenbach Commit-Queue: Rubber Stamper Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/heads/master@{#75106} --- BUILD.gn | 66 ++++++++++++++----------------- src/api/api.cc | 16 +++----- src/execution/isolate.cc | 10 ++--- src/execution/thread-local-top.cc | 2 - src/objects/backing-store.cc | 7 +--- 5 files changed, 38 insertions(+), 63 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index e48d83e3ee..9d5e64c854 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -3103,6 +3103,8 @@ v8_header_set("v8_internal_headers") { "src/tracing/trace-event.h", "src/tracing/traced-value.h", "src/tracing/tracing-category-observer.h", + "src/trap-handler/trap-handler-internal.h", + "src/trap-handler/trap-handler.h", "src/utils/address-map.h", "src/utils/allocation.h", "src/utils/bit-vector.h", @@ -3150,8 +3152,6 @@ v8_header_set("v8_internal_headers") { "src/compiler/wasm-compiler.h", "src/debug/debug-wasm-objects-inl.h", "src/debug/debug-wasm-objects.h", - "src/trap-handler/trap-handler-internal.h", - "src/trap-handler/trap-handler.h", "src/wasm/baseline/liftoff-assembler-defs.h", "src/wasm/baseline/liftoff-assembler.h", "src/wasm/baseline/liftoff-compiler.h", @@ -3294,19 +3294,16 @@ v8_header_set("v8_internal_headers") { "src/wasm/baseline/x64/liftoff-assembler-x64.h", ] - if (is_win) { - sources += [ "src/diagnostics/unwinding-info-win64.h" ] + # iOS Xcode simulator builds run on an x64 target. iOS and macOS are both + # based on Darwin and thus POSIX-compliant to a similar degree. + if (is_linux || is_chromeos || is_mac || is_ios || target_os == "freebsd") { + sources += [ "src/trap-handler/handler-inside-posix.h" ] } - - if (v8_enable_webassembly) { - # iOS Xcode simulator builds run on an x64 target. iOS and macOS are both - # based on Darwin and thus POSIX-compliant to a similar degree. - if (is_linux || is_chromeos || is_mac || is_ios || - target_os == "freebsd") { - sources += [ "src/trap-handler/handler-inside-posix.h" ] - } else if (is_win) { - sources += [ "src/trap-handler/handler-inside-win.h" ] - } + if (is_win) { + sources += [ + "src/diagnostics/unwinding-info-win64.h", + "src/trap-handler/handler-inside-win.h", + ] } } else if (v8_current_cpu == "arm") { sources += [ ### gcmole(arch:arm) ### @@ -3351,7 +3348,7 @@ v8_header_set("v8_internal_headers") { if (v8_control_flow_integrity) { sources += [ "src/execution/arm64/pointer-authentication-arm64.h" ] } - if (v8_enable_webassembly && current_cpu == "arm64" && is_mac) { + if (current_cpu == "arm64" && is_mac) { sources += [ "src/trap-handler/handler-inside-posix.h" ] } if (is_win) { @@ -4071,6 +4068,9 @@ v8_source_set("v8_base_without_compiler") { "src/tracing/trace-event.cc", "src/tracing/traced-value.cc", "src/tracing/tracing-category-observer.cc", + "src/trap-handler/handler-inside.cc", + "src/trap-handler/handler-outside.cc", + "src/trap-handler/handler-shared.cc", "src/utils/address-map.cc", "src/utils/allocation.cc", "src/utils/bit-vector.cc", @@ -4097,9 +4097,6 @@ v8_source_set("v8_base_without_compiler") { "src/debug/debug-wasm-objects.cc", "src/runtime/runtime-test-wasm.cc", "src/runtime/runtime-wasm.cc", - "src/trap-handler/handler-inside.cc", - "src/trap-handler/handler-outside.cc", - "src/trap-handler/handler-shared.cc", "src/wasm/baseline/liftoff-assembler.cc", "src/wasm/baseline/liftoff-compiler.cc", "src/wasm/function-body-decoder.cc", @@ -4196,25 +4193,20 @@ v8_source_set("v8_base_without_compiler") { "src/regexp/x64/regexp-macro-assembler-x64.cc", ] - if (is_win) { - sources += [ "src/diagnostics/unwinding-info-win64.cc" ] + # iOS Xcode simulator builds run on an x64 target. iOS and macOS are both + # based on Darwin and thus POSIX-compliant to a similar degree. + if (is_linux || is_chromeos || is_mac || is_ios || target_os == "freebsd") { + sources += [ + "src/trap-handler/handler-inside-posix.cc", + "src/trap-handler/handler-outside-posix.cc", + ] } - - if (v8_enable_webassembly) { - # iOS Xcode simulator builds run on an x64 target. iOS and macOS are both - # based on Darwin and thus POSIX-compliant to a similar degree. - if (is_linux || is_chromeos || is_mac || is_ios || - target_os == "freebsd") { - sources += [ - "src/trap-handler/handler-inside-posix.cc", - "src/trap-handler/handler-outside-posix.cc", - ] - } else if (is_win) { - sources += [ - "src/trap-handler/handler-inside-win.cc", - "src/trap-handler/handler-outside-win.cc", - ] - } + if (is_win) { + sources += [ + "src/diagnostics/unwinding-info-win64.cc", + "src/trap-handler/handler-inside-win.cc", + "src/trap-handler/handler-outside-win.cc", + ] } } else if (v8_current_cpu == "arm") { sources += [ ### gcmole(arch:arm) ### @@ -4258,7 +4250,7 @@ v8_source_set("v8_base_without_compiler") { "src/execution/arm64/simulator-logic-arm64.cc", "src/regexp/arm64/regexp-macro-assembler-arm64.cc", ] - if (v8_enable_webassembly && current_cpu == "arm64" && is_mac) { + if (current_cpu == "arm64" && is_mac) { sources += [ "src/trap-handler/handler-inside-posix.cc", "src/trap-handler/handler-outside-posix.cc", diff --git a/src/api/api.cc b/src/api/api.cc index 5df3f64e25..c7f9959cfd 100644 --- a/src/api/api.cc +++ b/src/api/api.cc @@ -109,11 +109,11 @@ #include "src/strings/string-hasher.h" #include "src/strings/unicode-inl.h" #include "src/tracing/trace-event.h" +#include "src/trap-handler/trap-handler.h" #include "src/utils/detachable-vector.h" #include "src/utils/version.h" #if V8_ENABLE_WEBASSEMBLY -#include "src/trap-handler/trap-handler.h" #include "src/wasm/streaming-decoder.h" #include "src/wasm/value-type.h" #include "src/wasm/wasm-engine.h" @@ -5854,9 +5854,8 @@ bool TryHandleWebAssemblyTrapPosix(int sig_code, siginfo_t* info, // code rather than the wasm code, so the trap handler cannot find the landing // pad and lets the process crash. Therefore, only enable trap handlers if // the host and target arch are the same. -#if V8_ENABLE_WEBASSEMBLY && \ - ((V8_TARGET_ARCH_X64 && !V8_OS_ANDROID) || \ - (V8_HOST_ARCH_ARM64 && V8_TARGET_ARCH_ARM64 && V8_OS_MACOSX)) +#if (V8_TARGET_ARCH_X64 && !V8_OS_ANDROID) || \ + (V8_HOST_ARCH_ARM64 && V8_TARGET_ARCH_ARM64 && V8_OS_MACOSX) return i::trap_handler::TryHandleSignal(sig_code, info, context); #else return false; @@ -5871,20 +5870,15 @@ bool V8::TryHandleSignal(int signum, void* info, void* context) { #if V8_OS_WIN bool TryHandleWebAssemblyTrapWindows(EXCEPTION_POINTERS* exception) { -#if V8_ENABLE_WEBASSEMBLY && V8_TARGET_ARCH_X64 +#if V8_TARGET_ARCH_X64 return i::trap_handler::TryHandleWasmTrap(exception); -#else - return false; #endif + return false; } #endif bool V8::EnableWebAssemblyTrapHandler(bool use_v8_signal_handler) { -#if V8_ENABLE_WEBASSEMBLY && V8_TARGET_ARCH_X64 return v8::internal::trap_handler::EnableTrapHandler(use_v8_signal_handler); -#else - return false; -#endif } #if defined(V8_OS_WIN) diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc index 9e93b644f8..85c93a9d31 100644 --- a/src/execution/isolate.cc +++ b/src/execution/isolate.cc @@ -92,6 +92,7 @@ #include "src/strings/string-stream.h" #include "src/tasks/cancelable-task.h" #include "src/tracing/tracing-category-observer.h" +#include "src/trap-handler/trap-handler.h" #include "src/utils/address-map.h" #include "src/utils/ostreams.h" #include "src/utils/version.h" @@ -102,7 +103,6 @@ #endif // V8_INTL_SUPPORT #if V8_ENABLE_WEBASSEMBLY -#include "src/trap-handler/trap-handler.h" #include "src/wasm/wasm-code-manager.h" #include "src/wasm/wasm-engine.h" #include "src/wasm/wasm-module.h" @@ -1582,8 +1582,8 @@ Handle Isolate::CreateMessageOrAbort( Object Isolate::ThrowInternal(Object raw_exception, MessageLocation* location) { DCHECK(!has_pending_exception()); - IF_WASM(DCHECK_IMPLIES, trap_handler::IsTrapHandlerEnabled(), - !trap_handler::IsThreadInWasm()); + DCHECK_IMPLIES(trap_handler::IsTrapHandlerEnabled(), + !trap_handler::IsThreadInWasm()); HandleScope scope(this); Handle exception(raw_exception, this); @@ -1678,7 +1678,6 @@ Object Isolate::ReThrow(Object exception) { } namespace { -#if V8_ENABLE_WEBASSEMBLY // This scope will set the thread-in-wasm flag after the execution of all // destructors. The thread-in-wasm flag is only set when the scope gets enabled. class SetThreadInWasmFlagScope { @@ -1697,11 +1696,9 @@ class SetThreadInWasmFlagScope { private: bool enabled_ = false; }; -#endif // V8_ENABLE_WEBASSEMBLY } // namespace Object Isolate::UnwindAndFindHandler() { -#if V8_ENABLE_WEBASSEMBLY // Create the {SetThreadInWasmFlagScope} first in this function so that its // destructor gets called after all the other destructors. It is important // that the destructor sets the thread-in-wasm flag after all other @@ -1709,7 +1706,6 @@ Object Isolate::UnwindAndFindHandler() { // Windows, which would invalidate the thread-in-wasm flag when the wasm trap // handler handles such non-wasm exceptions. SetThreadInWasmFlagScope set_thread_in_wasm_flag_scope; -#endif // V8_ENABLE_WEBASSEMBLY Object exception = pending_exception(); auto FoundHandler = [&](Context context, Address instruction_start, diff --git a/src/execution/thread-local-top.cc b/src/execution/thread-local-top.cc index 64962c8a84..4af5dc6407 100644 --- a/src/execution/thread-local-top.cc +++ b/src/execution/thread-local-top.cc @@ -42,10 +42,8 @@ void ThreadLocalTop::Initialize(Isolate* isolate) { Clear(); isolate_ = isolate; thread_id_ = ThreadId::Current(); -#if V8_ENABLE_WEBASSEMBLY thread_in_wasm_flag_address_ = reinterpret_cast
( trap_handler::GetThreadInWasmThreadLocalAddress()); -#endif // V8_ENABLE_WEBASSEMBLY #ifdef USE_SIMULATOR simulator_ = Simulator::current(isolate); #endif diff --git a/src/objects/backing-store.cc b/src/objects/backing-store.cc index 89bb17b03a..a1826a3022 100644 --- a/src/objects/backing-store.cc +++ b/src/objects/backing-store.cc @@ -10,9 +10,9 @@ #include "src/execution/isolate.h" #include "src/handles/global-handles.h" #include "src/logging/counters.h" +#include "src/trap-handler/trap-handler.h" #if V8_ENABLE_WEBASSEMBLY -#include "src/trap-handler/trap-handler.h" #include "src/wasm/wasm-constants.h" #include "src/wasm/wasm-engine.h" #include "src/wasm/wasm-limits.h" @@ -349,12 +349,7 @@ std::unique_ptr BackingStore::TryAllocateAndPartiallyCommitMemory( TRACE_BS("BSw:try %zu pages, %zu max\n", initial_pages, maximum_pages); -#if V8_ENABLE_WEBASSEMBLY bool guards = is_wasm_memory && trap_handler::IsTrapHandlerEnabled(); -#else - CHECK(!is_wasm_memory); - bool guards = false; -#endif // V8_ENABLE_WEBASSEMBLY // For accounting purposes, whether a GC was necessary. bool did_retry = false;