[base] Use IMMEDIATE_CRASH on official build FATAL errors.

Release-official builds strip error messages from CHECK messages.
This can make it difficult to distinguish a CHECK crash location in
crash reports. As such, instead of using V8_FatalNoContext, import the
IMMEDIATE_CRASH macro from chromium and use that instead, which should
cause a crash directly in the instruction stream so that the top
stackframe on the crash report directly identifies the CHECK location
that failed.

More details here:
https://docs.google.com/document/d/1tyMwzxUNH8BctM_urSQIYdcbwmzP4kTnwEjnFamBpKY

Change-Id: I5b8175f19571834f790060d641db08d0b9c2c17b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2756223
Reviewed-by: Mythri Alle <mythria@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73430}
This commit is contained in:
Ross McIlroy 2021-03-12 13:34:23 +00:00 committed by Commit Bot
parent de51603597
commit 26d85acee2
8 changed files with 177 additions and 31 deletions

View File

@ -4497,6 +4497,7 @@ v8_component("v8_libbase") {
"src/base/hashmap.h", "src/base/hashmap.h",
"src/base/ieee754.cc", "src/base/ieee754.cc",
"src/base/ieee754.h", "src/base/ieee754.h",
"src/base/immediate-crash.h",
"src/base/iterator.h", "src/base/iterator.h",
"src/base/lazy-instance.h", "src/base/lazy-instance.h",
"src/base/logging.cc", "src/base/logging.cc",

162
src/base/immediate-crash.h Normal file
View File

@ -0,0 +1,162 @@
// 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_BASE_IMMEDIATE_CRASH_H_
#define V8_BASE_IMMEDIATE_CRASH_H_
#include "include/v8config.h"
#include "src/base/build_config.h"
// Crashes in the fastest possible way with no attempt at logging.
// There are several constraints; see http://crbug.com/664209 for more context.
//
// - TRAP_SEQUENCE_() must be fatal. It should not be possible to ignore the
// resulting exception or simply hit 'continue' to skip over it in a debugger.
// - Different instances of TRAP_SEQUENCE_() must not be folded together, to
// ensure crash reports are debuggable. Unlike __builtin_trap(), asm volatile
// blocks will not be folded together.
// Note: TRAP_SEQUENCE_() previously required an instruction with a unique
// nonce since unlike clang, GCC folds together identical asm volatile
// blocks.
// - TRAP_SEQUENCE_() must produce a signal that is distinct from an invalid
// memory access.
// - TRAP_SEQUENCE_() must be treated as a set of noreturn instructions.
// __builtin_unreachable() is used to provide that hint here. clang also uses
// this as a heuristic to pack the instructions in the function epilogue to
// improve code density.
//
// Additional properties that are nice to have:
// - TRAP_SEQUENCE_() should be as compact as possible.
// - The first instruction of TRAP_SEQUENCE_() should not change, to avoid
// shifting crash reporting clusters. As a consequence of this, explicit
// assembly is preferred over intrinsics.
// Note: this last bullet point may no longer be true, and may be removed in
// the future.
// Note: TRAP_SEQUENCE Is currently split into two macro helpers due to the fact
// that clang emits an actual instruction for __builtin_unreachable() on certain
// platforms (see https://crbug.com/958675). In addition, the int3/bkpt/brk will
// be removed in followups, so splitting it up like this now makes it easy to
// land the followups.
#if V8_CC_GNU
#if V8_TARGET_ARCH_X64 || V8_TARGET_ARCH_IA32
// TODO(https://crbug.com/958675): In theory, it should be possible to use just
// int3. However, there are a number of crashes with SIGILL as the exception
// code, so it seems likely that there's a signal handler that allows execution
// to continue after SIGTRAP.
#define TRAP_SEQUENCE1_() asm volatile("int3")
#if V8_OS_MACOSX
// Intentionally empty: __builtin_unreachable() is always part of the sequence
// (see IMMEDIATE_CRASH below) and already emits a ud2 on Mac.
#define TRAP_SEQUENCE2_() asm volatile("")
#else
#define TRAP_SEQUENCE2_() asm volatile("ud2")
#endif // V8_OS_MACOSX
#elif V8_HOST_ARCH_ARM
// bkpt will generate a SIGBUS when running on armv7 and a SIGTRAP when running
// as a 32 bit userspace app on arm64. There doesn't seem to be any way to
// cause a SIGTRAP from userspace without using a syscall (which would be a
// problem for sandboxing).
// TODO(https://crbug.com/958675): Remove bkpt from this sequence.
#define TRAP_SEQUENCE1_() asm volatile("bkpt #0")
#define TRAP_SEQUENCE2_() asm volatile("udf #0")
#elif V8_HOST_ARCH_ARM64
// This will always generate a SIGTRAP on arm64.
// TODO(https://crbug.com/958675): Remove brk from this sequence.
#define TRAP_SEQUENCE1_() asm volatile("brk #0")
#define TRAP_SEQUENCE2_() asm volatile("hlt #0")
#else
// Crash report accuracy will not be guaranteed on other architectures, but at
// least this will crash as expected.
#define TRAP_SEQUENCE1_() __builtin_trap()
#define TRAP_SEQUENCE2_() asm volatile("")
#endif // V8_HOST_ARCH_*
#elif V8_CC_MSVC
#if !defined(__clang__)
// MSVC x64 doesn't support inline asm, so use the MSVC intrinsic.
#define TRAP_SEQUENCE1_() __debugbreak()
#define TRAP_SEQUENCE2_()
#elif V8_HOST_ARCH_ARM64
// Windows ARM64 uses "BRK #F000" as its breakpoint instruction, and
// __debugbreak() generates that in both VC++ and clang.
#define TRAP_SEQUENCE1_() __debugbreak()
// Intentionally empty: __builtin_unreachable() is always part of the sequence
// (see IMMEDIATE_CRASH below) and already emits a ud2 on Win64,
// https://crbug.com/958373
#define TRAP_SEQUENCE2_() __asm volatile("")
#else
#define TRAP_SEQUENCE1_() asm volatile("int3")
#define TRAP_SEQUENCE2_() asm volatile("ud2")
#endif // __clang__
#else
#error No supported trap sequence!
#endif // V8_CC_GNU
#define TRAP_SEQUENCE_() \
do { \
TRAP_SEQUENCE1_(); \
TRAP_SEQUENCE2_(); \
} while (false)
// CHECK() and the trap sequence can be invoked from a constexpr function.
// This could make compilation fail on GCC, as it forbids directly using inline
// asm inside a constexpr function. However, it allows calling a lambda
// expression including the same asm.
// The side effect is that the top of the stacktrace will not point to the
// calling function, but to this anonymous lambda. This is still useful as the
// full name of the lambda will typically include the name of the function that
// calls CHECK() and the debugger will still break at the right line of code.
#if !V8_CC_GNU
#define WRAPPED_TRAP_SEQUENCE_() TRAP_SEQUENCE_()
#else
#define WRAPPED_TRAP_SEQUENCE_() \
do { \
[] { TRAP_SEQUENCE_(); }(); \
} while (false)
#endif // !V8_CC_GCC
#if defined(__clang__) || V8_CC_GCC
// __builtin_unreachable() hints to the compiler that this is noreturn and can
// be packed in the function epilogue.
#define IMMEDIATE_CRASH() \
({ \
WRAPPED_TRAP_SEQUENCE_(); \
__builtin_unreachable(); \
})
#else
// This is supporting build with MSVC where there is no __builtin_unreachable().
#define IMMEDIATE_CRASH() WRAPPED_TRAP_SEQUENCE_()
#endif // defined(__clang__) || defined(COMPILER_GCC)
#endif // V8_BASE_IMMEDIATE_CRASH_H_

View File

@ -167,15 +167,6 @@ void V8_Fatal(const char* format, ...) {
v8::base::OS::Abort(); v8::base::OS::Abort();
} }
#if !defined(DEBUG) && defined(OFFICIAL_BUILD)
void V8_FatalNoContext() {
v8::base::OS::PrintError("V8 CHECK or FATAL\n");
if (v8::base::g_print_stack_trace) v8::base::g_print_stack_trace();
fflush(stderr);
v8::base::OS::Abort();
}
#endif
void V8_Dcheck(const char* file, int line, const char* message) { void V8_Dcheck(const char* file, int line, const char* message) {
v8::base::g_dcheck_function(file, line, message); v8::base::g_dcheck_function(file, line, message);
} }

View File

@ -12,6 +12,7 @@
#include "src/base/base-export.h" #include "src/base/base-export.h"
#include "src/base/build_config.h" #include "src/base/build_config.h"
#include "src/base/compiler-specific.h" #include "src/base/compiler-specific.h"
#include "src/base/immediate-crash.h"
#include "src/base/template-utils.h" #include "src/base/template-utils.h"
V8_BASE_EXPORT V8_NOINLINE void V8_Dcheck(const char* file, int line, V8_BASE_EXPORT V8_NOINLINE void V8_Dcheck(const char* file, int line,
@ -24,28 +25,25 @@ V8_BASE_EXPORT V8_NOINLINE void V8_Dcheck(const char* file, int line,
void V8_Fatal(const char* file, int line, const char* format, ...); void V8_Fatal(const char* file, int line, const char* format, ...);
#define FATAL(...) V8_Fatal(__FILE__, __LINE__, __VA_ARGS__) #define FATAL(...) V8_Fatal(__FILE__, __LINE__, __VA_ARGS__)
#elif !defined(OFFICIAL_BUILD) #else
[[noreturn]] PRINTF_FORMAT(1, 2) V8_BASE_EXPORT V8_NOINLINE
void V8_Fatal(const char* format, ...);
#if !defined(OFFICIAL_BUILD)
// In non-official release, include full error message, but drop file & line // In non-official release, include full error message, but drop file & line
// numbers. It saves binary size to drop the |file| & |line| as opposed to just // numbers. It saves binary size to drop the |file| & |line| as opposed to just
// passing in "", 0 for them. // passing in "", 0 for them.
[[noreturn]] PRINTF_FORMAT(1, 2) V8_BASE_EXPORT V8_NOINLINE
void V8_Fatal(const char* format, ...);
#define FATAL(...) V8_Fatal(__VA_ARGS__) #define FATAL(...) V8_Fatal(__VA_ARGS__)
#else #else
// In official builds, include only messages that contain parameters because // FATAL(msg) -> IMMEDIATE_CRASH()
// single-message errors can always be derived from stack traces. // FATAL(msg, ...) -> V8_Fatal(msg, ...)
[[noreturn]] V8_BASE_EXPORT V8_NOINLINE void V8_FatalNoContext();
[[noreturn]] PRINTF_FORMAT(1, 2) V8_BASE_EXPORT V8_NOINLINE
void V8_Fatal(const char* format, ...);
// FATAL(msg) -> V8_FatalNoContext()
// FATAL(msg, ...) -> V8_Fatal()
#define FATAL_HELPER(_7, _6, _5, _4, _3, _2, _1, _0, ...) _0 #define FATAL_HELPER(_7, _6, _5, _4, _3, _2, _1, _0, ...) _0
#define FATAL_DISCARD_ARG(arg) V8_FatalNoContext() #define FATAL_DISCARD_ARG(arg) IMMEDIATE_CRASH()
#define FATAL(...) \ #define FATAL(...) \
FATAL_HELPER(__VA_ARGS__, V8_Fatal, V8_Fatal, V8_Fatal, V8_Fatal, V8_Fatal, \ FATAL_HELPER(__VA_ARGS__, V8_Fatal, V8_Fatal, V8_Fatal, V8_Fatal, V8_Fatal, \
V8_Fatal, V8_Fatal, FATAL_DISCARD_ARG) \ V8_Fatal, FATAL_DISCARD_ARG) \
(__VA_ARGS__) (__VA_ARGS__)
#endif #endif // !defined(OFFICIAL_BUILD)
#endif // DEBUG
#define UNIMPLEMENTED() FATAL("unimplemented code") #define UNIMPLEMENTED() FATAL("unimplemented code")
#define UNREACHABLE() FATAL("unreachable code") #define UNREACHABLE() FATAL("unreachable code")

View File

@ -183,12 +183,6 @@ V8_INLINE Dest bit_cast(Source const& source) {
#define DISABLE_CFI_ICALL V8_CLANG_NO_SANITIZE("cfi-icall") #define DISABLE_CFI_ICALL V8_CLANG_NO_SANITIZE("cfi-icall")
#endif #endif
#if V8_CC_GNU
#define V8_IMMEDIATE_CRASH() __builtin_trap()
#else
#define V8_IMMEDIATE_CRASH() ((void(*)())0)()
#endif
// A convenience wrapper around static_assert without a string message argument. // A convenience wrapper around static_assert without a string message argument.
// Once C++17 becomes the default, this macro can be removed in favor of the // Once C++17 becomes the default, this macro can be removed in favor of the
// new static_assert(condition) overload. // new static_assert(condition) overload.

View File

@ -499,7 +499,7 @@ void OS::Sleep(TimeDelta interval) {
void OS::Abort() { void OS::Abort() {
if (g_hard_abort) { if (g_hard_abort) {
V8_IMMEDIATE_CRASH(); IMMEDIATE_CRASH();
} }
// Redirect to std abort to signal abnormal program termination. // Redirect to std abort to signal abnormal program termination.
abort(); abort();

View File

@ -929,7 +929,7 @@ void OS::Abort() {
fflush(stderr); fflush(stderr);
if (g_hard_abort) { if (g_hard_abort) {
V8_IMMEDIATE_CRASH(); IMMEDIATE_CRASH();
} }
// Make the MSVCRT do a silent abort. // Make the MSVCRT do a silent abort.
raise(SIGABRT); raise(SIGABRT);

View File

@ -2291,7 +2291,7 @@ void Shell::Fuzzilli(const v8::FunctionCallbackInfo<v8::Value>& args) {
.FromMaybe(0); .FromMaybe(0);
switch (arg) { switch (arg) {
case 0: case 0:
V8_IMMEDIATE_CRASH(); IMMEDIATE_CRASH();
break; break;
case 1: case 1:
CHECK(0); CHECK(0);