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>
 #include <cstring>
 #include <sstream>
 #include <string>
 #include <type_traits>
 #include <utility>
 
 #include "base/base_export.h"
+#include "base/compiler_specific.h"
 #include "base/debug/debugger.h"
 #include "base/macros.h"
 #include "base/template_util.h"
 #include "build/build_config.h"
 
 //
 // Optional message capabilities
 // -----------------------------
 // Assertion failed messages and fatal errors are displayed in a dialog box
 // before the application exits. However, running this UI creates a message
@@ skipping 423 matching lines @@
 //
 // We make sure CHECK et al. always evaluates their arguments, as
 // doing CHECK(FunctionWithSideEffect()) is a common idiom.
 
 #if defined(OFFICIAL_BUILD) && defined(NDEBUG)
 
 // Make all CHECK functions discard their log strings to reduce code
 // bloat, and improve performance, for official release builds.
 
 #if defined(COMPILER_GCC) || __clang__
-#define LOGGING_CRASH() __builtin_trap()
-#else
-#define LOGGING_CRASH() ((void)(*(volatile char*)0 = 0))
+
+// 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:
+// - The asm volatile statement is a cross-architecture version of
+//   mov %eax, LINE. It guarantees that the assembly is unique for each line
+//   and the multiple crash instructions generated by CHECK() don't get folded.
+// - __builtin_trap() is the instruction that actually causes the crash,
+//   either a SIGTRAP or a SIGABRT depending on the platform.
+// - __builtin_unreachable() enables no-return optimizations.
+#define LOGGING_CRASH(LINE)                    \
+  ({                                           \
+    asm volatile("" : : "r"(LINE) : "memory"); \
+    __builtin_trap();                          \
+    __builtin_unreachable();                   \
+  })
+
+#else  // !(defined(COMPILER_GCC) || __clang__)
+#define LOGGING_CRASH(LINE) ((void)(*(volatile char*)0 = 0))

[Nico, 2016/12/02 02:04:03]

This doesn't help on Windows, does it. Would

  ((void)(*(volatile char*)0 = LINE))

be something that works everywhere? x86 has up to 32bit immediates and supports
immediate->mem moves, but I guess on arm it'd still clobber a reg (or several).

How does Torne's suggestion help? Aren't these ud2s mergable too? Or do
compilers not try to reason about inline asm? (LLVM probably doesn't.) 

(https://gcc.gnu.org/onlinedocs/gcc-4.6.4/gcc/Other-Builtins.html claims that
__builtin_unreachable would generate a return, but at least clang seems to not
do this.)

 #endif
 
 // This is not calling BreakDebugger since this is called frequently, and
 // calling an out-of-line function instead of a noreturn inline macro prevents
 // compiler optimizations.
-#define CHECK(condition)                                                \
-  !(condition) ? LOGGING_CRASH() : EAT_STREAM_PARAMETERS
+#define CHECK(condition) \
+  !(condition) ? LOGGING_CRASH(__LINE__) : EAT_STREAM_PARAMETERS
 
 #define PCHECK(condition) CHECK(condition)
 
 #define CHECK_OP(name, op, val1, val2) CHECK((val1) op (val2))
 
 #else  // !(OFFICIAL_BUILD && NDEBUG)
 
 #if defined(_PREFAST_) && defined(OS_WIN)
 // Use __analysis_assume to tell the VC++ static analysis engine that
 // assert conditions are true, to suppress warnings.  The LAZY_STREAM
@@ skipping 507 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_