[Reland] 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 478 matching lines @@
   CheckOpResult(std::string* message) : message_(message) {}
   // Returns true if the check succeeded.
   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.
+// Crashes in the fastest possible way with no attempt at logging.
+// There are different constraints to satisfy here, see http://crbug.com/664209
+// for more context:
+// - The trap instructions, and hence the PC value at crash time, have to be
+//   distinct and not get folded into the same opcode by the compiler.
+//   On Linux/Android this is tricky because GCC still folds identical
+//   asm volatile blocks. The workaround is generating distinct opcodes for
+//   each CHECK using the __COUNTER__ macro.
+// - The debug info for the trap instruction has to be attributed to the source
+//   line that has the CHECK(), to make crash reports actionable. This rules
+//   out the ability of using a inline function, at least as long as clang
+//   doesn't support attribute(artificial).
+// - Failed CHECKs should produce a signal that is distinguishable from an
+//   invalid memory access, to improve the actionability of crash reports.
+// - 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.
+// - When debugging, developers shouldn't be able to accidentally step over a
+//   CHECK. This is achieved by putting opcodes that will cause a non
+//   continuable exception after the actual trap instruction.
+// - Don't cause too much binary bloat.
 #if defined(COMPILER_GCC)
-#define IMMEDIATE_CRASH() __builtin_trap()
+
+#if defined(ARCH_CPU_X86_FAMILY) && !defined(OS_NACL)
+// int 3 will generate a SIGTRAP.
+#define TRAP_SEQUENCE() \
+  asm volatile(         \
+      "int3; ud2; push %0;" ::"i"(static_cast<unsigned char>(__COUNTER__)))
+
+#elif defined(ARCH_CPU_ARMEL) && !defined(OS_NACL)
+// 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).
+#define TRAP_SEQUENCE() \
+  asm volatile("bkpt #0; udf %0;" ::"i"(__COUNTER__ % 256))
+
+#elif defined(ARCH_CPU_ARM64) && !defined(OS_NACL)
+// This will always generate a SIGTRAP on arm64.
+#define TRAP_SEQUENCE() \
+  asm volatile("brk #0; hlt %0;" ::"i"(__COUNTER__ % 65536))
+
+#else
+// Crash report accuracy will not be guaranteed on other architectures, but at
+// least this will crash as expected.
+#define TRAP_SEQUENCE() __builtin_trap()
+#endif  // ARCH_CPU_*
+
+#define IMMEDIATE_CRASH()    \
+  ({                         \
+    TRAP_SEQUENCE();         \
+    __builtin_unreachable(); \
+  })
+
 #elif defined(COMPILER_MSVC)
 
 // Clang is cleverer about coalescing int3s, so we need to add a unique-ish
 // instruction following the __debugbreak() to have it emit distinct locations
 // for CHECKs rather than collapsing them all together. It would be nice to use
 // a short intrinsic to do this (and perhaps have only one implementation for
 // both clang and MSVC), however clang-cl currently does not support intrinsics
 // here. Adding the nullptr store to the MSVC path adds unnecessary bloat.
 // TODO(scottmg): Reinvestigate a short sequence that will work on both
 // compilers once clang supports more intrinsics. See https://crbug.com/693713.
 #if defined(__clang__)
 #define IMMEDIATE_CRASH()                                                 \
   (__debugbreak(), (void)(*reinterpret_cast<volatile unsigned char*>(0) = \

[Mark Mentovai, 2017/02/21 14:21:50]

If Lexan supports GCC-style inline asm, we could use the above for this.

[Primiano Tucci (use gerrit), 2017/02/21 14:27:50]

Yup, realized the same too late. I just left a comment on Scott's CL in
https://codereview.chromium.org/2701253005/diff/1/base/logging.h#newcode515

                               static_cast<unsigned char>(__COUNTER__)))
 #else
 #define IMMEDIATE_CRASH() __debugbreak()
 #endif  // __clang__
 
 #else
 #error Port
 #endif
 
 // CHECK dies with a fatal error if condition is not true.  It is *not*
@@ skipping 548 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_