From 0858134396226ab8078b18bcf56331e0368a44f4 Mon Sep 17 00:00:00 2001 From: Clemens Backes Date: Tue, 20 Jul 2021 06:01:50 +0000 Subject: [PATCH] Revert "[traphandler] Add simulator support" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 431fff66f5db7cdd9a9b25f1d1a5548c188d4e1a. Reason for revert: Causes link error in chrome: https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Builder/24667/overview Original change's description: > [traphandler] Add simulator support > > This prepares the trap handler to support being used from simulators. > Modifications to the arm64 simulator will be done in a follow-up CL. For > now, the trap handler will be registered but not used in Wasm (we emit > explicit bounds checks instead, as before). > > The implementation uses inline assembly, so it is only available on x64 > POSIX systems for now. This is the main platform we use for testing and > for fuzzing, so it should give us the test coverage we need. If needed, > inline assembly for other platforms can be added later. > The new code will be executed by the existing arm64 simulator bots, e.g. > "V8 Linux - arm64 - sim". > > R=​ahaas@chromium.org, mseaborn@chromium.org > > Bug: v8:11955 > Change-Id: Idc50291c704d9dea902ae0098e5309f19055816c > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3011160 > Commit-Queue: Clemens Backes > Reviewed-by: Andreas Haas > Cr-Commit-Position: refs/heads/master@{#75780} Bug: v8:11955 Change-Id: I74d2e41864fc515bd9727898f12ec1498b97ee62 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3040839 Auto-Submit: Clemens Backes Commit-Queue: Rubber Stamper Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/heads/master@{#75798} --- BUILD.gn | 28 +--- src/runtime/runtime-test-wasm.cc | 6 - src/trap-handler/handler-inside-posix.cc | 56 ++------ src/trap-handler/handler-outside-simulator.cc | 30 ----- src/trap-handler/trap-handler-simulator.h | 37 ----- src/trap-handler/trap-handler.h | 17 +-- src/wasm/wasm-code-manager.cc | 3 - test/unittests/BUILD.gn | 6 - .../wasm/trap-handler-simulator-unittest.cc | 126 ------------------ tools/generate-header-include-checks.py | 2 - 10 files changed, 26 insertions(+), 285 deletions(-) delete mode 100644 src/trap-handler/handler-outside-simulator.cc delete mode 100644 src/trap-handler/trap-handler-simulator.h delete mode 100644 test/unittests/wasm/trap-handler-simulator-unittest.cc diff --git a/BUILD.gn b/BUILD.gn index b0eb0ba2db..d4cf36efa3 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -3377,15 +3377,8 @@ v8_header_set("v8_internal_headers") { if (v8_control_flow_integrity) { sources += [ "src/execution/arm64/pointer-authentication-arm64.h" ] } - if (v8_enable_webassembly) { - # Trap handling is enabled on arm64 Mac and in simulators on x64 on Linux. - if ((current_cpu == "arm64" && is_mac) || - (current_cpu == "x64" && is_linux)) { - sources += [ "src/trap-handler/handler-inside-posix.h" ] - } - if (current_cpu == "x64" && is_linux) { - sources += [ "src/trap-handler/trap-handler-simulator.h" ] - } + if (v8_enable_webassembly && current_cpu == "arm64" && is_mac) { + sources += [ "src/trap-handler/handler-inside-posix.h" ] } if (is_win) { sources += [ "src/diagnostics/unwinding-info-win64.h" ] @@ -4285,18 +4278,11 @@ 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) { - # Trap handling is enabled on arm64 Mac and in simulators on x64 on Linux. - if ((current_cpu == "arm64" && is_mac) || - (current_cpu == "x64" && is_linux)) { - sources += [ - "src/trap-handler/handler-inside-posix.cc", - "src/trap-handler/handler-outside-posix.cc", - ] - } - if (current_cpu == "x64" && is_linux) { - sources += [ "src/trap-handler/handler-outside-simulator.cc" ] - } + if (v8_enable_webassembly && current_cpu == "arm64" && is_mac) { + sources += [ + "src/trap-handler/handler-inside-posix.cc", + "src/trap-handler/handler-outside-posix.cc", + ] } if (is_win) { sources += [ "src/diagnostics/unwinding-info-win64.cc" ] diff --git a/src/runtime/runtime-test-wasm.cc b/src/runtime/runtime-test-wasm.cc index c010b8ba03..72598c7345 100644 --- a/src/runtime/runtime-test-wasm.cc +++ b/src/runtime/runtime-test-wasm.cc @@ -277,13 +277,7 @@ RUNTIME_FUNCTION(Runtime_IsWasmCode) { RUNTIME_FUNCTION(Runtime_IsWasmTrapHandlerEnabled) { DisallowGarbageCollection no_gc; DCHECK_EQ(0, args.length()); - // We currently lie to tests about enabled trap handling in simulator builds. - // TODO(clemensb): Remove this once the arm64 simulator support trap handling. -#ifdef USE_SIMULATOR - return isolate->heap()->ToBoolean(false); -#else return isolate->heap()->ToBoolean(trap_handler::IsTrapHandlerEnabled()); -#endif } RUNTIME_FUNCTION(Runtime_IsThreadInWasm) { diff --git a/src/trap-handler/handler-inside-posix.cc b/src/trap-handler/handler-inside-posix.cc index 6f521929b2..a75b5143fa 100644 --- a/src/trap-handler/handler-inside-posix.cc +++ b/src/trap-handler/handler-inside-posix.cc @@ -43,16 +43,6 @@ namespace v8 { namespace internal { namespace trap_handler { -#if V8_OS_LINUX -#define CONTEXT_REG(reg, REG) &uc->uc_mcontext.gregs[REG_##REG] -#elif V8_OS_MACOSX -#define CONTEXT_REG(reg, REG) &uc->uc_mcontext->__ss.__##reg -#elif V8_OS_FREEBSD -#define CONTEXT_REG(reg, REG) &uc->uc_mcontext.mc_##reg -#else -#error "Unsupported platform." -#endif - bool IsKernelGeneratedSignal(siginfo_t* info) { // On macOS, only `info->si_code > 0` is relevant, because macOS leaves // si_code at its default of 0 for signals that don’t originate in hardware. @@ -82,18 +72,11 @@ class UnmaskOobSignalScope { sigset_t old_mask_; }; -#ifdef V8_TRAP_HANDLER_VIA_SIMULATOR -// These are addresses inside the "ProbeMemory" function, defined in -// "handler-outside-simulators.cc". -extern "C" char v8_probe_memory_address[]; -extern "C" char v8_probe_memory_continuation[]; -#endif // V8_TRAP_HANDLER_VIA_SIMULATOR - bool TryHandleSignal(int signum, siginfo_t* info, void* context) { // Ensure the faulting thread was actually running Wasm code. This should be - // the first check in the trap handler to guarantee that the - // g_thread_in_wasm_code flag is only set in wasm code. Otherwise a later - // signal handler is executed with the flag set. + // the first check in the trap handler to guarantee that the IsThreadInWasm + // flag is only set in wasm code. Otherwise a later signal handler is executed + // with the flag set. if (!g_thread_in_wasm_code) return false; // Clear g_thread_in_wasm_code, primarily to protect against nested faults. @@ -119,38 +102,23 @@ bool TryHandleSignal(int signum, siginfo_t* info, void* context) { UnmaskOobSignalScope unmask_oob_signal; ucontext_t* uc = reinterpret_cast(context); -#if V8_HOST_ARCH_X64 - auto* context_ip = CONTEXT_REG(rip, RIP); -#elif V8_HOST_ARCH_ARM64 - auto* context_ip = CONTEXT_REG(pc, PC); +#if V8_OS_LINUX && V8_TARGET_ARCH_X64 + auto* context_ip = &uc->uc_mcontext.gregs[REG_RIP]; +#elif V8_OS_MACOSX && V8_TARGET_ARCH_ARM64 + auto* context_ip = &uc->uc_mcontext->__ss.__pc; +#elif V8_OS_MACOSX && V8_TARGET_ARCH_X64 + auto* context_ip = &uc->uc_mcontext->__ss.__rip; +#elif V8_OS_FREEBSD && V8_TARGET_ARCH_X64 + auto* context_ip = &uc->uc_mcontext.mc_rip; #else -#error "Unsupported architecture." +#error Unsupported platform #endif - uintptr_t fault_addr = *context_ip; uintptr_t landing_pad = 0; - -#ifdef V8_TRAP_HANDLER_VIA_SIMULATOR - // Only handle signals triggered by the load in {ProbeMemory}. - if (fault_addr != reinterpret_cast(&v8_probe_memory_address)) { - return false; - } - - // The simulated ip will be in the second parameter register (%rsi). - auto* simulated_ip_reg = CONTEXT_REG(rsi, RSI); - if (!TryFindLandingPad(*simulated_ip_reg, &landing_pad)) return false; - TH_DCHECK(landing_pad != 0); - - auto* return_reg = CONTEXT_REG(rax, RAX); - *return_reg = landing_pad; - // Continue at the memory probing continuation. - *context_ip = reinterpret_cast(&v8_probe_memory_continuation); -#else if (!TryFindLandingPad(fault_addr, &landing_pad)) return false; // Tell the caller to return to the landing pad. *context_ip = landing_pad; -#endif } // We will return to wasm code, so restore the g_thread_in_wasm_code flag. // This should only be done once the signal is blocked again (outside the diff --git a/src/trap-handler/handler-outside-simulator.cc b/src/trap-handler/handler-outside-simulator.cc deleted file mode 100644 index d28ba0b053..0000000000 --- a/src/trap-handler/handler-outside-simulator.cc +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright 2021 the V8 project authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "include/v8config.h" -#include "src/trap-handler/trap-handler-simulator.h" - -#if !V8_OS_LINUX -#error "The inline assembly only works on Linux so far." -#endif - -asm( - // Define the ProbeMemory function declared in trap-handler-simulators.h. - ".pushsection .text \n" - ".globl ProbeMemory \n" - ".type ProbeMemory, %function \n" - ".globl v8_probe_memory_address \n" - ".globl v8_probe_memory_continuation \n" - "ProbeMemory: \n" - // First parameter (address) passed in %rdi. - // The second parameter (pc) is unused here. It is read by the trap handler - // instead. - "v8_probe_memory_address: \n" - " movb (%rdi), %al \n" - // Return 0 on success. - " xorl %eax, %eax \n" - "v8_probe_memory_continuation: \n" - // If the trap handler continues here, it wrote the landing pad in %rax. - " ret \n" - ".popsection \n"); diff --git a/src/trap-handler/trap-handler-simulator.h b/src/trap-handler/trap-handler-simulator.h deleted file mode 100644 index bfceb49697..0000000000 --- a/src/trap-handler/trap-handler-simulator.h +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2021 the V8 project authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef V8_TRAP_HANDLER_TRAP_HANDLER_SIMULATOR_H_ -#define V8_TRAP_HANDLER_TRAP_HANDLER_SIMULATOR_H_ - -#include - -// This header defines the ProbeMemory function to be used by simulators to -// trigger a signal at a defined location, before doing an actual memory access. - -// This implementation is only usable on an x64 host with non-x64 target (i.e. a -// simulator build on x64). -#if (!defined(_M_X64) && !defined(__x86_64__)) || defined(V8_TARGET_ARCH_X64) -#error "Do only include this file on simulator builds on x64." -#endif - -namespace v8 { -namespace internal { -namespace trap_handler { - -// Probe a memory address by doing a 1-byte read from the given address. If the -// address is not readable, this will cause a trap as usual, but the trap -// handler will recognise the address of the instruction doing the access and -// treat it specially. It will use the given {pc} to look up the respective -// landing pad and return to this function to return that landing pad. If {pc} -// is not registered as a protected instruction, the signal will be propagated -// as usual. -// If the read at {address} succeeds, this function returns {0} instead. -extern "C" uintptr_t ProbeMemory(uintptr_t address, uintptr_t pc); - -} // namespace trap_handler -} // namespace internal -} // namespace v8 - -#endif // V8_TRAP_HANDLER_TRAP_HANDLER_SIMULATOR_H_ diff --git a/src/trap-handler/trap-handler.h b/src/trap-handler/trap-handler.h index d173e13e32..a27ea236e7 100644 --- a/src/trap-handler/trap-handler.h +++ b/src/trap-handler/trap-handler.h @@ -17,19 +17,16 @@ namespace v8 { namespace internal { namespace trap_handler { -// X64 on Linux, Windows, MacOS, FreeBSD. -#if V8_HOST_ARCH_X64 && V8_TARGET_ARCH_X64 && \ - ((V8_OS_LINUX && !V8_OS_ANDROID) || V8_OS_WIN || V8_OS_MACOSX || \ - V8_OS_FREEBSD) +#if V8_TARGET_ARCH_X64 && V8_OS_LINUX && !V8_OS_ANDROID #define V8_TRAP_HANDLER_SUPPORTED true -// Arm64 (non-simulator) on Mac. -#elif V8_TARGET_ARCH_ARM64 && V8_HOST_ARCH_ARM64 && V8_OS_MACOSX +#elif V8_TARGET_ARCH_X64 && V8_OS_WIN #define V8_TRAP_HANDLER_SUPPORTED true -// Arm64 simulator on x64 on Linux. -#elif V8_TARGET_ARCH_ARM64 && V8_HOST_ARCH_X64 && V8_OS_LINUX -#define V8_TRAP_HANDLER_VIA_SIMULATOR +#elif V8_TARGET_ARCH_X64 && V8_OS_MACOSX +#define V8_TRAP_HANDLER_SUPPORTED true +#elif V8_TARGET_ARCH_X64 && V8_OS_FREEBSD +#define V8_TRAP_HANDLER_SUPPORTED true +#elif V8_HOST_ARCH_ARM64 && V8_TARGET_ARCH_ARM64 && V8_OS_MACOSX #define V8_TRAP_HANDLER_SUPPORTED true -// Everything else is unsupported. #else #define V8_TRAP_HANDLER_SUPPORTED false #endif diff --git a/src/wasm/wasm-code-manager.cc b/src/wasm/wasm-code-manager.cc index b77a3a7d0d..ae257c06d5 100644 --- a/src/wasm/wasm-code-manager.cc +++ b/src/wasm/wasm-code-manager.cc @@ -823,10 +823,7 @@ BoundsCheckStrategy GetBoundsChecks(const WasmModule* module) { if (FLAG_wasm_enforce_bounds_checks) return kExplicitBoundsChecks; // We do not have trap handler support for memory64 yet. if (module->is_memory64) return kExplicitBoundsChecks; - // TODO(clemensb): Enable trap handling in the arm64 simulator. -#ifndef USE_SIMULATOR if (trap_handler::IsTrapHandlerEnabled()) return kTrapHandler; -#endif // USE_SIMULATOR return kExplicitBoundsChecks; } } // namespace diff --git a/test/unittests/BUILD.gn b/test/unittests/BUILD.gn index 8134f639ed..c115daa047 100644 --- a/test/unittests/BUILD.gn +++ b/test/unittests/BUILD.gn @@ -488,12 +488,6 @@ v8_source_set("unittests_sources") { sources += [ "wasm/trap-handler-win-unittest.cc" ] } - # Include this test only on arm64 simulator builds on x64 on Linux. - if (current_cpu == "x64" && v8_current_cpu == "arm64" && is_linux && - v8_enable_webassembly) { - sources += [ "wasm/trap-handler-simulator-unittest.cc" ] - } - configs = [ "../..:cppgc_base_config", "../..:external_config", diff --git a/test/unittests/wasm/trap-handler-simulator-unittest.cc b/test/unittests/wasm/trap-handler-simulator-unittest.cc deleted file mode 100644 index 280206b5ea..0000000000 --- a/test/unittests/wasm/trap-handler-simulator-unittest.cc +++ /dev/null @@ -1,126 +0,0 @@ -// Copyright 2021 the V8 project authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "src/trap-handler/trap-handler-simulator.h" - -#include "include/v8.h" -#include "src/codegen/macro-assembler-inl.h" -#include "src/execution/simulator.h" -#include "src/trap-handler/trap-handler.h" -#include "test/common/assembler-tester.h" -#include "test/unittests/test-utils.h" - -#if !V8_HOST_ARCH_X64 || !V8_TARGET_ARCH_ARM64 -#error "Only include this file on arm64 simulator builds on x64." -#endif - -namespace v8 { -namespace internal { -namespace trap_handler { - -constexpr uintptr_t kFakePc = 11; - -class SimulatorTrapHandlerTest : public TestWithIsolate { - public: - static void SetThreadInWasm() { - EXPECT_EQ(0, g_thread_in_wasm_code); - g_thread_in_wasm_code = 1; - } - - static void ResetThreadInWasm() { - EXPECT_EQ(1, g_thread_in_wasm_code); - g_thread_in_wasm_code = 0; - } -}; - -TEST_F(SimulatorTrapHandlerTest, ProbeMemorySuccess) { - int x = 47; - EXPECT_EQ(0u, ProbeMemory(reinterpret_cast(&x), kFakePc)); -} - -TEST_F(SimulatorTrapHandlerTest, ProbeMemoryFail) { - constexpr uintptr_t kNullAddress = 0; - EXPECT_DEATH_IF_SUPPORTED(ProbeMemory(kNullAddress, kFakePc), ""); -} - -TEST_F(SimulatorTrapHandlerTest, ProbeMemoryFailWhileInWasm) { - // Test that we still crash if the trap handler is set up and the "thread in - // wasm" flag is set, but the PC is not registered as a protected instruction. - constexpr bool kUseDefaultHandler = true; - CHECK(v8::V8::EnableWebAssemblyTrapHandler(kUseDefaultHandler)); - - constexpr uintptr_t kNullAddress = 0; - SetThreadInWasm(); - EXPECT_DEATH_IF_SUPPORTED(ProbeMemory(kNullAddress, kFakePc), ""); -} - -TEST_F(SimulatorTrapHandlerTest, ProbeMemoryWithTrapHandled) { - constexpr uintptr_t kNullAddress = 0; - constexpr uintptr_t kFakeLandingPad = 19; - - constexpr bool kUseDefaultHandler = true; - CHECK(v8::V8::EnableWebAssemblyTrapHandler(kUseDefaultHandler)); - - ProtectedInstructionData fake_protected_instruction{kFakePc, kFakeLandingPad}; - int handler_data_index = - RegisterHandlerData(0, 128, 1, &fake_protected_instruction); - - SetThreadInWasm(); - EXPECT_EQ(kFakeLandingPad, ProbeMemory(kNullAddress, kFakePc)); - - // Reset everything. - ResetThreadInWasm(); - ReleaseHandlerData(handler_data_index); - RemoveTrapHandler(); -} - -TEST_F(SimulatorTrapHandlerTest, ProbeMemoryWithLandingPad) { - EXPECT_EQ(0u, GetRecoveredTrapCount()); - - // Test that the trap handler can recover a memory access violation in - // wasm code (we fake the wasm code and the access violation). - std::unique_ptr buffer = AllocateAssemblerBuffer(); - constexpr Register scratch = x0; - MacroAssembler masm(nullptr, AssemblerOptions{}, CodeObjectRequired::kNo, - buffer->CreateView()); - // Generate an illegal memory access. - masm.Mov(scratch, 0); - uint32_t crash_offset = masm.pc_offset(); - masm.Str(scratch, MemOperand(scratch, 0)); // nullptr access - uint32_t recovery_offset = masm.pc_offset(); - // Return. - masm.Ret(); - - CodeDesc desc; - masm.GetCode(nullptr, &desc); - - constexpr bool kUseDefaultHandler = true; - CHECK(v8::V8::EnableWebAssemblyTrapHandler(kUseDefaultHandler)); - - ProtectedInstructionData protected_instruction{crash_offset, recovery_offset}; - int handler_data_index = - RegisterHandlerData(reinterpret_cast
(desc.buffer), - desc.instr_size, 1, &protected_instruction); - - // Now execute the code. - buffer->MakeExecutable(); - GeneratedCode code = GeneratedCode::FromAddress( - i_isolate(), reinterpret_cast
(desc.buffer)); - - SetThreadInWasm(); - // TODO(clemensb): This should pass after adding memory probing in the - // simulator. - // code.Call(); - EXPECT_DEATH_IF_SUPPORTED(code.Call(), ""); - ResetThreadInWasm(); - - ReleaseHandlerData(handler_data_index); - RemoveTrapHandler(); - - // EXPECT_EQ(1u, GetRecoveredTrapCount()); -} - -} // namespace trap_handler -} // namespace internal -} // namespace v8 diff --git a/tools/generate-header-include-checks.py b/tools/generate-header-include-checks.py index 250b741068..2171ee8a0d 100755 --- a/tools/generate-header-include-checks.py +++ b/tools/generate-header-include-checks.py @@ -33,8 +33,6 @@ AUTO_EXCLUDE = [ 'src/flags/flag-definitions.h', # recorder.h should only be included conditionally. 'src/libplatform/tracing/recorder.h', - # trap-handler-simulator.h can only be included in simulator builds. - 'src/trap-handler/trap-handler-simulator.h', ] AUTO_EXCLUDE_PATTERNS = [ 'src/base/atomicops_internals_.*',