base: make CHECK macros trap at distinct addresses in official builds

base/logging.h

 // Copyright (c) 2012 The Chromium 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 BASE_LOGGING_H_
 #define BASE_LOGGING_H_
 
 #include <stddef.h>
 
 #include <cassert>
@@ skipping 444 matching lines @@
   operator bool() const { return !message_; }
   // Returns the message.
   std::string* message() { return message_; }
 
  private:
   std::string* message_;
 };
 
 // Crashes in the fastest, simplest possible way with no attempt at logging.
 #if defined(COMPILER_GCC) || defined(__clang__)
-#define IMMEDIATE_CRASH() __builtin_trap()
+
+// This is to make sure that the crash report can pinpoint to the right source
+// line even in presence of multiple CHECK()s in the same function. See
+// http://crbug.com/664209 for more context.
+// There are three things we need to guarantee here:
+// - The trap instructions, and hence the PC value at crash time, are distinct
+//   and don't get collapsed into the same op due to compiler optimizations.
+// - The debug line info for the trap instructions get attributed to the caller
+//   of CHECK(), to make sure that crash reports show actionable data. This in
+//   practice rules out the ability of using a inline function, at least as
+//   long as clang doesn't support attribute(artificial).
+// - The compiler should treat the CHECK as no-return instructions, so that the
+//   trap code can be efficiently packed in the prologue of the function and
+//   doesn't interfere with the main execution flow.
+// Code-wise:
+// - asm volatile seems to be enough to make the compiler emit distinct
+//   instructions for each CHECK() and not fold them together.
+// - the actual instructions (e.g., "ud2") are borrowed from implementation of
+//   __builtin_trap() in the various toolchain.
+//   either a SIGTRAP or a SIGABRT depending on the platform.

[Mark Mentovai, 2017/02/13 16:22:09]

On x86 and x86_64, ud2 will be SIGILL. That’s good, but it’s not distinct in
that true bad codegen will also be SIGILL. SIGABRT or SIGTRAP, as this comment
says, would be much better.

On the Windows equivalent (https://codereview.chromium.org/2676483002/), we’re
discussing int3 with a ud2 backstop. int3 will be SIGTRAP which is somewhat
distinct. The ud2 backstop is there to provide a non-continuable barrier to
further execution, otherwise it’s not really noreturn because it’s trivial to
continue past int3.

The reserved illegal opcode backstop could also work for ARM. I don’t know that
it’ll solve multiple CHECK()s jumping to a single target that you’re seeing, but
it’s a further refinement.

+// - __builtin_unreachable() enables no-return optimizations.
+
+#if defined(ARCH_CPU_X86_FAMILY)
+#define TRAP_INSTRUCTION() asm volatile("ud2" ::: "memory")
+#elif defined(ARCH_CPU_ARMEL)
+#define TRAP_INSTRUCTION() asm volatile("udf $0xff" ::: "memory")
+#elif defined(ARCH_CPU_ARM64)
+#define TRAP_INSTRUCTION() asm volatile("brk #0x3e8" ::: "memory")
 #else
+// Crash report accuracy will not be guaranteed on other architectures, but at
+// least this will crash as expected.
+#define TRAP_INSTRUCTION() __builtin_trap()
+#endif
+
+#define IMMEDIATE_CRASH()    \
+  ({                         \
+    TRAP_INSTRUCTION();      \
+    __builtin_unreachable(); \
+  })
+
+#else  // defined(COMPILER_GCC) || defined(__clang__)
 #define IMMEDIATE_CRASH() ((void)(*(volatile char*)0 = 0))
 #endif
 
 // CHECK dies with a fatal error if condition is not true.  It is *not*
 // controlled by NDEBUG, so the check will be executed regardless of
 // compilation mode.
 //
 // We make sure CHECK et al. always evaluates their arguments, as
 // doing CHECK(FunctionWithSideEffect()) is a common idiom.
 
@@ skipping 594 matching lines @@
 #elif NOTIMPLEMENTED_POLICY == 5
 #define NOTIMPLEMENTED() do {\
   static bool logged_once = false;\
   LOG_IF(ERROR, !logged_once) << NOTIMPLEMENTED_MSG;\
   logged_once = true;\
 } while(0);\
 EAT_STREAM_PARAMETERS
 #endif
 
 #endif  // BASE_LOGGING_H_