From 8dee661adbd965faa388b3d70398b46d4f2a2725 Mon Sep 17 00:00:00 2001
From: Jon Meow <jperkins@google.com>
Date: Wed, 27 Apr 2022 09:17:23 -0700
Subject: [PATCH] Move CHECK stack traces to be before the message (#1216)

Before, if the llvm hook to print a stack trace on CHECK was bound, it would trace after the message rather than before the message. I'm thinking swapping the order will generally be easier to debug.
---
 common/check_internal.h | 18 +++++++++++++++---
 common/check_test.cpp   |  2 ++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/common/check_internal.h b/common/check_internal.h
index e723beedc32e..1b52ba48e285 100644
--- a/common/check_internal.h
+++ b/common/check_internal.h
@@ -5,6 +5,8 @@
 #ifndef COMMON_CHECK_INTERNAL_H_
 #define COMMON_CHECK_INTERNAL_H_
 
+#include <unistd.h>
+
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
@@ -23,6 +25,12 @@ class ExitingStream {
   // Internal type used in macros to dispatch to the `operator|` overload.
   struct Helper {};
 
+  ExitingStream() {
+    // Start all messages with a stack trace.
+    llvm::errs() << "Stack trace:\n";
+    llvm::sys::PrintStackTrace(llvm::errs());
+  }
+
   [[noreturn]] ~ExitingStream() {
     llvm_unreachable(
         "Exiting streams should only be constructed by check.h macros that "
@@ -45,7 +53,7 @@ class ExitingStream {
     return *this;
   }
 
-  auto operator<<(AddSeparator /*discarded*/) -> ExitingStream& {
+  auto operator<<(AddSeparator /*add_separator*/) -> ExitingStream& {
     separator_ = true;
     return *this;
   }
@@ -53,10 +61,14 @@ class ExitingStream {
   // Low-precedence binary operator overload used in check.h macros to flush the
   // output and exit the program. We do this in a binary operator rather than
   // the destructor to ensure good debug info and backtraces for errors.
-  [[noreturn]] friend auto operator|(Helper /*discarded*/, ExitingStream& rhs) {
+  [[noreturn]] friend auto operator|(Helper /*helper*/,
+                                     ExitingStream& /*rhs*/) {
     // Finish with a newline.
     llvm::errs() << "\n";
-    std::abort();
+    // We assume LLVM's exit handling is installed, which will stack trace on
+    // std::abort(). We print a stack trace on construction, so this avoids that
+    // stack trace on exit.
+    _exit(1);
   }
 
  private:
diff --git a/common/check_test.cpp b/common/check_test.cpp
index 810860aee0bf..4b88baa54402 100644
--- a/common/check_test.cpp
+++ b/common/check_test.cpp
@@ -14,6 +14,8 @@ TEST(CheckTest, CheckTrue) { CHECK(true); }
 TEST(CheckTest, CheckFalse) {
   // TODO: figure out why we can't use \\d+ instead of .+ in these patterns.
   ASSERT_DEATH({ CHECK(false); },
+               "Stack trace:\n"
+               ".+\n"
                "CHECK failure at common/check_test.cpp:.+: false\n");
 }