Make LOG(FATAL) [[noreturn]]

This also changes NOTREACHED() to use LOG(FATAL) which both enables log streaming and printing something useful when hit. Bug: chromium:40254046 Change-Id: I187da25fe42f67dbf06f1e5a8c507e6b8cc46d6e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/mini_chromium/+/5904318 Reviewed-by: Mark Mentovai <[email protected]>
chromium · Oct 3, 2024 · 4027559 · 4027559
1 parent e04c707
commit 4027559
Show file tree

Hide file tree

Showing 7 changed files with 120 additions and 11 deletions.
diff --git a/base/apple/mach_logging.cc b/base/apple/mach_logging.cc
@@ -7,6 +7,7 @@
 #include <iomanip>
 #include <string>
 
+#include "base/immediate_crash.h"
 #include "base/strings/stringprintf.h"
 
 #if !BUILDFLAG(IS_IOS)
@@ -37,10 +38,20 @@ MachLogMessage::MachLogMessage(const char* function,
     : LogMessage(function, file_path, line, severity), mach_err_(mach_err) {}
 
 MachLogMessage::~MachLogMessage() {
+  AppendError();
+}
+
+void MachLogMessage::AppendError() {
   stream() << ": " << mach_error_string(mach_err_)
            << FormatMachErrorNumber(mach_err_);
 }
 
+MachLogMessageFatal::~MachLogMessageFatal() {
+  AppendError();
+  Flush();
+  base::ImmediateCrash();
+}
+
 #if !BUILDFLAG(IS_IOS)
 
 BootstrapLogMessage::BootstrapLogMessage(const char* function,
@@ -52,6 +63,10 @@ BootstrapLogMessage::BootstrapLogMessage(const char* function,
       bootstrap_err_(bootstrap_err) {}
 
 BootstrapLogMessage::~BootstrapLogMessage() {
+  AppendError();
+}
+
+void BootstrapLogMessage::AppendError() {
   stream() << ": " << bootstrap_strerror(bootstrap_err_);
 
   switch (bootstrap_err_) {
@@ -79,6 +94,12 @@ BootstrapLogMessage::~BootstrapLogMessage() {
   }
 }
 
+BootstrapLogMessageFatal::~BootstrapLogMessageFatal() {
+  AppendError();
+  Flush();
+  base::ImmediateCrash();
+}
+
 #endif  // !BUILDFLAG(IS_IOS)
 
 }  // namespace logging
diff --git a/base/apple/mach_logging.h b/base/apple/mach_logging.h
@@ -44,10 +44,19 @@ class MachLogMessage : public logging::LogMessage {
 
   ~MachLogMessage();
 
+ protected:
+  void AppendError();
+
  private:
   mach_error_t mach_err_;
 };
 
+class MachLogMessageFatal final : public MachLogMessage {
+ public:
+  using MachLogMessage::MachLogMessage;
+  [[noreturn]] ~MachLogMessageFatal() override;
+};
+
 }  // namespace logging
 
 #define MACH_LOG_STREAM(severity, mach_err) \
@@ -108,10 +117,19 @@ class BootstrapLogMessage : public logging::LogMessage {
 
   ~BootstrapLogMessage();
 
+ protected:
+  void AppendError();
+
  private:
   kern_return_t bootstrap_err_;
 };
 
+class BootstrapLogMessageFatal final : public BootstrapLogMessage {
+ public:
+  using BootstrapLogMessage::BootstrapLogMessage;
+  [[noreturn]] ~BootstrapLogMessageFatal() override;
+};
+
 }  // namespace logging
 
 #define BOOTSTRAP_LOG_STREAM(severity, bootstrap_err) \

diff --git a/base/fuchsia/fuchsia_logging.cc b/base/fuchsia/fuchsia_logging.cc
@@ -18,10 +18,20 @@ ZxLogMessage::ZxLogMessage(const char* function,
     : LogMessage(function, file_path, line, severity), zx_err_(zx_err) {}
 
 ZxLogMessage::~ZxLogMessage() {
+  AppendError();
+}
+
+void ZxLogMessage::AppendError() {
   // zx_status_t error values are negative, so log the numeric version as
   // decimal rather than hex. This is also useful to match zircon/errors.h for
   // grepping.
   stream() << ": " << zx_status_get_string(zx_err_) << " (" << zx_err_ << ")";
 }
 
+ZxLogMessageFatal::~ZxLogMessageFatal() {
+  AppendError();
+  Flush();
+  base::ImmediateCrash();
+}
+
 }  // namespace logging
diff --git a/base/fuchsia/fuchsia_logging.h b/base/fuchsia/fuchsia_logging.h
@@ -28,10 +28,19 @@ class ZxLogMessage : public logging::LogMessage {
 
   ~ZxLogMessage();
 
+ protected:
+  void AppendError();
+
  private:
   zx_status_t zx_err_;
 };
 
+class ZxLogMessageFatal final : public ZxLogMessage {
+ public:
+  using ZxLogMessage::ZxLogMessage;
+  [[noreturn]] ~ZxLogMessageFatal() override;
+};
+
 }  // namespace logging
 
 #define ZX_LOG_STREAM(severity, zx_err) \

diff --git a/base/logging.cc b/base/logging.cc
@@ -476,6 +476,11 @@ void LogMessage::Init(const char* function) {
   message_start_ = stream_.str().size();
 }
 
+LogMessageFatal::~LogMessageFatal() {
+  Flush();
+  base::ImmediateCrash();
+}
+
 #if BUILDFLAG(IS_WIN)
 
 unsigned long GetLastSystemErrorCode() {
@@ -491,9 +496,19 @@ Win32ErrorLogMessage::Win32ErrorLogMessage(const char* function,
 }
 
 Win32ErrorLogMessage::~Win32ErrorLogMessage() {
+  AppendError();
+}
+
+void Win32ErrorLogMessage::AppendError() {
   stream() << ": " << SystemErrorCodeToString(err_);
 }
 
+Win32ErrorLogMessageFatal::~Win32ErrorLogMessageFatal() {
+  AppendError();
+  Flush();
+  base::ImmediateCrash();
+}
+
 #elif BUILDFLAG(IS_POSIX)
 
 ErrnoLogMessage::ErrnoLogMessage(const char* function,
@@ -506,13 +521,23 @@ ErrnoLogMessage::ErrnoLogMessage(const char* function,
 }
 
 ErrnoLogMessage::~ErrnoLogMessage() {
+  AppendError();
+}
+
+void ErrnoLogMessage::AppendError() {
   stream() << ": "
            << base::safe_strerror(err_)
            << " ("
            << err_
            << ")";
 }
 
+ErrnoLogMessageFatal::~ErrnoLogMessageFatal() {
+  AppendError();
+  Flush();
+  base::ImmediateCrash();
+}
+
 #endif  // BUILDFLAG(IS_POSIX)
 
 }  // namespace logging

diff --git a/base/logging.h b/base/logging.h
@@ -123,6 +123,12 @@ class LogMessage {
   LogSeverity severity_;
 };
 
+class LogMessageFatal final : public LogMessage {
+ public:
+  using LogMessage::LogMessage;
+  [[noreturn]] ~LogMessageFatal() override;
+};
+
 class LogMessageVoidify {
  public:
   LogMessageVoidify() {}
@@ -144,9 +150,19 @@ class Win32ErrorLogMessage : public LogMessage {
 
   ~Win32ErrorLogMessage();
 
+ protected:
+  void AppendError();
+
  private:
   unsigned long err_;
 };
+
+class Win32ErrorLogMessageFatal final : public Win32ErrorLogMessage {
+ public:
+  using Win32ErrorLogMessage::Win32ErrorLogMessage;
+  [[noreturn]] ~Win32ErrorLogMessageFatal() override;
+};
+
 #elif BUILDFLAG(IS_POSIX)
 class ErrnoLogMessage : public LogMessage {
  public:
@@ -161,9 +177,18 @@ class ErrnoLogMessage : public LogMessage {
 
   ~ErrnoLogMessage();
 
+ protected:
+  void AppendError();
+
  private:
   int err_;
 };
+
+class ErrnoLogMessageFatal final : public ErrnoLogMessage {
+ public:
+  using ErrnoLogMessage::ErrnoLogMessage;
+  [[noreturn]] ~ErrnoLogMessageFatal() override;
+};
 #endif
 
 }  // namespace logging
@@ -187,8 +212,11 @@ class ErrnoLogMessage : public LogMessage {
     logging::ClassName(FUNCTION_SIGNATURE, __FILE__, __LINE__, \
                        logging::LOG_ERROR_REPORT, ## __VA_ARGS__)
 #define COMPACT_GOOGLE_LOG_EX_FATAL(ClassName, ...) \
-    logging::ClassName(FUNCTION_SIGNATURE, __FILE__, __LINE__, \
-                       logging::LOG_FATAL, ## __VA_ARGS__)
+  logging::ClassName##Fatal(FUNCTION_SIGNATURE,     \
+                            __FILE__,               \
+                            __LINE__,               \
+                            logging::LOG_FATAL,     \
+                            ##__VA_ARGS__)
 #define COMPACT_GOOGLE_LOG_EX_DFATAL(ClassName, ...) \
     logging::ClassName(FUNCTION_SIGNATURE, __FILE__, __LINE__, \
                        logging::LOG_DFATAL, ## __VA_ARGS__)
@@ -230,8 +258,11 @@ const LogSeverity LOG_0 = LOG_ERROR;
 #define LAZY_STREAM(stream, condition) \
     !(condition) ? (void) 0 : ::logging::LogMessageVoidify() & (stream)
 
-#define LOG_IS_ON(severity) \
-    ((::logging::LOG_ ## severity) >= ::logging::GetMinLogLevel())
+// FATAL is always enabled and required to be resolved in compile time for
+// LOG(FATAL) to be properly understood as [[noreturn]].
+#define LOG_IS_ON(severity)                               \
+  ((::logging::LOG_##severity) == ::logging::LOG_FATAL || \
+   (::logging::LOG_##severity) >= ::logging::GetMinLogLevel())
 #define VLOG_IS_ON(verbose_level) \
     ((verbose_level) <= ::logging::GetVlogLevel(__FILE__))
 

diff --git a/base/notreached.h b/base/notreached.h
@@ -5,14 +5,9 @@
 #ifndef MINI_CHROMIUM_BASE_NOTREACHED_H_
 #define MINI_CHROMIUM_BASE_NOTREACHED_H_
 
-#include <stdlib.h>
-
 #include "base/check.h"
+#include "base/logging.h"
 
-// Using abort() for NOTREACHED() doesn't support streaming arguments. For a
-// more complete implementation we could use LOG(FATAL) but that is currently
-// not annotated as [[noreturn]] because ~LogMessage is not. See TODO in
-// base/logging.h.
-#define NOTREACHED() abort()
+#define NOTREACHED() LOG(FATAL) << "NOTREACHED hit. "
 
 #endif  // MINI_CHROMIUM_BASE_NOTREACHED_H_