Skip to content

Commit

Permalink
Give V8 a chance to handle faults in stack dump crash handler
Browse files Browse the repository at this point in the history
V8 has the ability to use guard regions and signal handlers to do WebAssembly
bounds checks on x86_64 Linux. This requires any in-process signal handlers to
let V8 try to handle the signal first. This CL adds support for this to the
stack dump signal handler.

Bug: chromium:722585, v8:5277
Change-Id: Iccb72b66118c743f78b3ddbaf7887728b213ddb2
Reviewed-on: https://chromium-review.googlesource.com/541956
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Eric Holk <eholk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487153}
  • Loading branch information
eholk authored and Commit Bot committed Jul 17, 2017
1 parent a31c717 commit dc499db
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 1 deletion.
6 changes: 6 additions & 0 deletions base/debug/stack_trace.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ namespace debug {
// done in official builds because it has security implications).
BASE_EXPORT bool EnableInProcessStackDumping();

#if defined(OS_POSIX)
BASE_EXPORT void SetStackDumpFirstChanceCallback(bool (*handler)(int,
void*,
void*));
#endif

// Returns end of the stack, or 0 if we couldn't get it.
#if BUILDFLAG(CAN_UNWIND_WITH_FRAME_POINTERS)
BASE_EXPORT uintptr_t GetStackEnd();
Expand Down
28 changes: 28 additions & 0 deletions base/debug/stack_trace_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ namespace {

volatile sig_atomic_t in_signal_handler = 0;

bool (*try_handle_signal)(int, void*, void*) = nullptr;

#if !defined(USE_SYMBOLIZE)
// The prefix used for mangled symbols, per the Itanium C++ ABI:
// http://www.codesourcery.com/cxx-abi/abi.html#mangling
Expand Down Expand Up @@ -216,6 +218,27 @@ void StackDumpSignalHandler(int signal, siginfo_t* info, void* void_context) {
// NOTE: This code MUST be async-signal safe.
// NO malloc or stdio is allowed here.

// Give a registered callback a chance to recover from this signal
//
// V8 uses guard regions to guarantee memory safety in WebAssembly. This means
// some signals might be expected if they originate from Wasm code while
// accessing the guard region. We give V8 the chance to handle and recover
// from these signals first.
if (try_handle_signal != nullptr &&
try_handle_signal(signal, info, void_context)) {
// The first chance handler took care of this. The SA_RESETHAND flag
// replaced this signal handler upon entry, but we want to stay
// installed. Thus, we reinstall ourselves before returning.
struct sigaction action;
memset(&action, 0, sizeof(action));
action.sa_flags = SA_RESETHAND | SA_SIGINFO;
action.sa_sigaction = &StackDumpSignalHandler;
sigemptyset(&action.sa_mask);

sigaction(signal, &action, NULL);
return;
}

// Record the fact that we are in the signal handler now, so that the rest
// of StackTrace can behave in an async-signal-safe manner.
in_signal_handler = 1;
Expand Down Expand Up @@ -717,6 +740,11 @@ bool EnableInProcessStackDumping() {
return success;
}

void SetStackDumpFirstChanceCallback(bool (*handler)(int, void*, void*)) {
DCHECK(try_handle_signal == nullptr || handler == nullptr);
try_handle_signal = handler;
}

StackTrace::StackTrace(size_t count) {
// NOTE: This code MUST be async-signal safe (it's used by in-process
// stack dumping signal handler). NO malloc or stdio is allowed here.
Expand Down
9 changes: 8 additions & 1 deletion content/renderer/render_process_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "base/command_line.h"
#include "base/compiler_specific.h"
#include "base/debug/crash_logging.h"
#include "base/debug/stack_trace.h"
#include "base/feature_list.h"
#include "base/memory/ptr_util.h"
#include "base/sys_info.h"
Expand Down Expand Up @@ -144,10 +145,16 @@ RenderProcessImpl::RenderProcessImpl(
SetV8FlagIfFeature(features::kAsmJsToWebAssembly, "--validate-asm");
SetV8FlagIfNotFeature(features::kWebAssembly,
"--wasm-disable-structured-cloning");
SetV8FlagIfFeature(features::kWebAssemblyTrapHandler, "--wasm-trap-handler");
SetV8FlagIfFeature(features::kSharedArrayBuffer,
"--harmony-sharedarraybuffer");

SetV8FlagIfFeature(features::kWebAssemblyTrapHandler, "--wasm-trap-handler");
#if defined(OS_LINUX) && defined(ARCH_CPU_X86_64) && !defined(OS_ANDROID)
if (base::FeatureList::IsEnabled(features::kWebAssemblyTrapHandler)) {
base::debug::SetStackDumpFirstChanceCallback(v8::V8::TryHandleSignal);
}
#endif

const base::CommandLine& command_line =
*base::CommandLine::ForCurrentProcess();

Expand Down

0 comments on commit dc499db

Please sign in to comment.