浏览代码

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.
Jon Meow 4 年之前
父节点
当前提交
8dee661adb
共有 2 个文件被更改,包括 17 次插入3 次删除
  1. 15 3
      common/check_internal.h
  2. 2 0
      common/check_test.cpp

+ 15 - 3
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:

+ 2 - 0
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");
 }