diff options
author | Peter Boström <pbos@google.com> | 2024-03-22 12:33:04 -0700 |
---|---|---|
committer | Copybara-Service <copybara-worker@google.com> | 2024-03-22 12:33:53 -0700 |
commit | 9a9502bfd19981bace5e93597db9ec1fade48005 (patch) | |
tree | 46c1f1e63854a9126de5e9cba0efb8b9293bdd5c | |
parent | 76f8011beabdaee872b5fde7546e02407b220cb1 (diff) |
Reland: Make DLOG(FATAL) not understood as [[noreturn]]
Having DLOG(FATAL) be [[noreturn]] in debug builds makes dead-code
warnings impossible to satisfy between the release and debug build.
PiperOrigin-RevId: 618257732
Change-Id: Ib1c2cb6a026401fbaa0a6e0770adfc166b964ece
-rw-r--r-- | absl/log/absl_log_basic_test.cc | 1 | ||||
-rw-r--r-- | absl/log/internal/log_impl.h | 44 | ||||
-rw-r--r-- | absl/log/internal/log_message.cc | 46 | ||||
-rw-r--r-- | absl/log/internal/log_message.h | 19 | ||||
-rw-r--r-- | absl/log/internal/strip.h | 21 | ||||
-rw-r--r-- | absl/log/log_basic_test.cc | 1 | ||||
-rw-r--r-- | absl/log/log_basic_test_impl.inc | 26 |
7 files changed, 118 insertions, 40 deletions
diff --git a/absl/log/absl_log_basic_test.cc b/absl/log/absl_log_basic_test.cc index 3a4b83c1..7378f5a8 100644 --- a/absl/log/absl_log_basic_test.cc +++ b/absl/log/absl_log_basic_test.cc @@ -16,6 +16,7 @@ #include "absl/log/absl_log.h" #define ABSL_TEST_LOG ABSL_LOG +#define ABSL_TEST_DLOG ABSL_DLOG #include "gtest/gtest.h" #include "absl/log/log_basic_test_impl.inc" diff --git a/absl/log/internal/log_impl.h b/absl/log/internal/log_impl.h index 99de6dbb..b44ed068 100644 --- a/absl/log/internal/log_impl.h +++ b/absl/log/internal/log_impl.h @@ -35,11 +35,11 @@ #ifndef NDEBUG #define ABSL_LOG_INTERNAL_DLOG_IMPL(severity) \ ABSL_LOG_INTERNAL_CONDITION##severity(STATELESS, true) \ - ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #else #define ABSL_LOG_INTERNAL_DLOG_IMPL(severity) \ ABSL_LOG_INTERNAL_CONDITION##severity(STATELESS, false) \ - ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #endif // The `switch` ensures that this expansion is the begnning of a statement (as @@ -58,7 +58,7 @@ switch (const int absl_logging_internal_verbose_level = (verbose_level)) \ case 0: \ default: \ - ABSL_LOG_INTERNAL_LOG_IF_IMPL( \ + ABSL_LOG_INTERNAL_DLOG_IF_IMPL( \ _INFO, ABSL_VLOG_IS_ON(absl_logging_internal_verbose_level)) \ .WithVerbosity(absl_logging_internal_verbose_level) #else @@ -66,7 +66,7 @@ switch (const int absl_logging_internal_verbose_level = (verbose_level)) \ case 0: \ default: \ - ABSL_LOG_INTERNAL_LOG_IF_IMPL( \ + ABSL_LOG_INTERNAL_DLOG_IF_IMPL( \ _INFO, false && ABSL_VLOG_IS_ON(absl_logging_internal_verbose_level)) \ .WithVerbosity(absl_logging_internal_verbose_level) #endif @@ -82,11 +82,11 @@ #ifndef NDEBUG #define ABSL_LOG_INTERNAL_DLOG_IF_IMPL(severity, condition) \ ABSL_LOG_INTERNAL_CONDITION##severity(STATELESS, condition) \ - ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #else #define ABSL_LOG_INTERNAL_DLOG_IF_IMPL(severity, condition) \ ABSL_LOG_INTERNAL_CONDITION##severity(STATELESS, false && (condition)) \ - ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #endif // ABSL_LOG_EVERY_N @@ -132,36 +132,36 @@ #ifndef NDEBUG #define ABSL_LOG_INTERNAL_DLOG_EVERY_N_IMPL(severity, n) \ ABSL_LOG_INTERNAL_CONDITION_INFO(STATEFUL, true) \ - (EveryN, n) ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + (EveryN, n) ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #define ABSL_LOG_INTERNAL_DLOG_FIRST_N_IMPL(severity, n) \ ABSL_LOG_INTERNAL_CONDITION_INFO(STATEFUL, true) \ - (FirstN, n) ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + (FirstN, n) ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #define ABSL_LOG_INTERNAL_DLOG_EVERY_POW_2_IMPL(severity) \ ABSL_LOG_INTERNAL_CONDITION_INFO(STATEFUL, true) \ - (EveryPow2) ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + (EveryPow2) ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #define ABSL_LOG_INTERNAL_DLOG_EVERY_N_SEC_IMPL(severity, n_seconds) \ ABSL_LOG_INTERNAL_CONDITION_INFO(STATEFUL, true) \ - (EveryNSec, n_seconds) ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + (EveryNSec, n_seconds) ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #else // def NDEBUG #define ABSL_LOG_INTERNAL_DLOG_EVERY_N_IMPL(severity, n) \ ABSL_LOG_INTERNAL_CONDITION_INFO(STATEFUL, false) \ - (EveryN, n) ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + (EveryN, n) ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #define ABSL_LOG_INTERNAL_DLOG_FIRST_N_IMPL(severity, n) \ ABSL_LOG_INTERNAL_CONDITION_INFO(STATEFUL, false) \ - (FirstN, n) ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + (FirstN, n) ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #define ABSL_LOG_INTERNAL_DLOG_EVERY_POW_2_IMPL(severity) \ ABSL_LOG_INTERNAL_CONDITION_INFO(STATEFUL, false) \ - (EveryPow2) ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + (EveryPow2) ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #define ABSL_LOG_INTERNAL_DLOG_EVERY_N_SEC_IMPL(severity, n_seconds) \ ABSL_LOG_INTERNAL_CONDITION_INFO(STATEFUL, false) \ - (EveryNSec, n_seconds) ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + (EveryNSec, n_seconds) ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #endif // def NDEBUG #define ABSL_LOG_INTERNAL_VLOG_EVERY_N_IMPL(verbose_level, n) \ @@ -243,40 +243,40 @@ #ifndef NDEBUG #define ABSL_LOG_INTERNAL_DLOG_IF_EVERY_N_IMPL(severity, condition, n) \ ABSL_LOG_INTERNAL_CONDITION##severity(STATEFUL, condition)(EveryN, n) \ - ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #define ABSL_LOG_INTERNAL_DLOG_IF_FIRST_N_IMPL(severity, condition, n) \ ABSL_LOG_INTERNAL_CONDITION##severity(STATEFUL, condition)(FirstN, n) \ - ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #define ABSL_LOG_INTERNAL_DLOG_IF_EVERY_POW_2_IMPL(severity, condition) \ ABSL_LOG_INTERNAL_CONDITION##severity(STATEFUL, condition)(EveryPow2) \ - ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #define ABSL_LOG_INTERNAL_DLOG_IF_EVERY_N_SEC_IMPL(severity, condition, \ n_seconds) \ ABSL_LOG_INTERNAL_CONDITION##severity(STATEFUL, condition)(EveryNSec, \ n_seconds) \ - ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #else // def NDEBUG #define ABSL_LOG_INTERNAL_DLOG_IF_EVERY_N_IMPL(severity, condition, n) \ ABSL_LOG_INTERNAL_CONDITION##severity(STATEFUL, false && (condition))( \ - EveryN, n) ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + EveryN, n) ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #define ABSL_LOG_INTERNAL_DLOG_IF_FIRST_N_IMPL(severity, condition, n) \ ABSL_LOG_INTERNAL_CONDITION##severity(STATEFUL, false && (condition))( \ - FirstN, n) ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + FirstN, n) ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #define ABSL_LOG_INTERNAL_DLOG_IF_EVERY_POW_2_IMPL(severity, condition) \ ABSL_LOG_INTERNAL_CONDITION##severity(STATEFUL, false && (condition))( \ - EveryPow2) ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + EveryPow2) ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #define ABSL_LOG_INTERNAL_DLOG_IF_EVERY_N_SEC_IMPL(severity, condition, \ n_seconds) \ ABSL_LOG_INTERNAL_CONDITION##severity(STATEFUL, false && (condition))( \ EveryNSec, n_seconds) \ - ABSL_LOGGING_INTERNAL_LOG##severity.InternalStream() + ABSL_LOGGING_INTERNAL_DLOG##severity.InternalStream() #endif // def NDEBUG #endif // ABSL_LOG_INTERNAL_LOG_IMPL_H_ diff --git a/absl/log/internal/log_message.cc b/absl/log/internal/log_message.cc index 10ac2453..58505267 100644 --- a/absl/log/internal/log_message.cc +++ b/absl/log/internal/log_message.cc @@ -578,6 +578,13 @@ template void LogMessage::CopyToEncodedBuffer<LogMessage::StringType::kLiteral>( template void LogMessage::CopyToEncodedBuffer< LogMessage::StringType::kNotLiteral>(char ch, size_t num); +// We intentionally don't return from these destructors. Disable MSVC's warning +// about the destructor never returning as we do so intentionally here. +#if defined(_MSC_VER) && !defined(__clang__) +#pragma warning(push) +#pragma warning(disable : 4722) +#endif + LogMessageFatal::LogMessageFatal(const char* file, int line) : LogMessage(file, line, absl::LogSeverity::kFatal) {} @@ -587,19 +594,29 @@ LogMessageFatal::LogMessageFatal(const char* file, int line, *this << "Check failed: " << failure_msg << " "; } -// ABSL_ATTRIBUTE_NORETURN doesn't seem to work on destructors with msvc, so -// disable msvc's warning about the d'tor never returning. -#if defined(_MSC_VER) && !defined(__clang__) -#pragma warning(push) -#pragma warning(disable : 4722) -#endif LogMessageFatal::~LogMessageFatal() { Flush(); FailWithoutStackTrace(); } -#if defined(_MSC_VER) && !defined(__clang__) -#pragma warning(pop) -#endif + +LogMessageDebugFatal::LogMessageDebugFatal(const char* file, int line) + : LogMessage(file, line, absl::LogSeverity::kFatal) {} + +LogMessageDebugFatal::~LogMessageDebugFatal() { + Flush(); + FailWithoutStackTrace(); +} + +LogMessageQuietlyDebugFatal::LogMessageQuietlyDebugFatal(const char* file, + int line) + : LogMessage(file, line, absl::LogSeverity::kFatal) { + SetFailQuietly(); +} + +LogMessageQuietlyDebugFatal::~LogMessageQuietlyDebugFatal() { + Flush(); + FailQuietly(); +} LogMessageQuietlyFatal::LogMessageQuietlyFatal(const char* file, int line) : LogMessage(file, line, absl::LogSeverity::kFatal) { @@ -608,17 +625,10 @@ LogMessageQuietlyFatal::LogMessageQuietlyFatal(const char* file, int line) LogMessageQuietlyFatal::LogMessageQuietlyFatal(const char* file, int line, absl::string_view failure_msg) - : LogMessage(file, line, absl::LogSeverity::kFatal) { - SetFailQuietly(); - *this << "Check failed: " << failure_msg << " "; + : LogMessageQuietlyFatal(file, line) { + *this << "Check failed: " << failure_msg << " "; } -// ABSL_ATTRIBUTE_NORETURN doesn't seem to work on destructors with msvc, so -// disable msvc's warning about the d'tor never returning. -#if defined(_MSC_VER) && !defined(__clang__) -#pragma warning(push) -#pragma warning(disable : 4722) -#endif LogMessageQuietlyFatal::~LogMessageQuietlyFatal() { Flush(); FailQuietly(); diff --git a/absl/log/internal/log_message.h b/absl/log/internal/log_message.h index 4ecb8a14..fa721214 100644 --- a/absl/log/internal/log_message.h +++ b/absl/log/internal/log_message.h @@ -357,6 +357,25 @@ class LogMessageFatal final : public LogMessage { ABSL_ATTRIBUTE_NORETURN ~LogMessageFatal(); }; +// `LogMessageDebugFatal` ensures the process will exit in failure after logging +// this message. It matches LogMessageFatal but is not [[noreturn]] as it's used +// for DLOG(FATAL) variants. +class LogMessageDebugFatal final : public LogMessage { + public: + LogMessageDebugFatal(const char* file, int line) ABSL_ATTRIBUTE_COLD; + ~LogMessageDebugFatal(); +}; + +class LogMessageQuietlyDebugFatal final : public LogMessage { + public: + // DLOG(QFATAL) calls this instead of LogMessageQuietlyFatal to make sure the + // destructor is not [[noreturn]] even if this is always FATAL as this is only + // invoked when DLOG() is enabled. + LogMessageQuietlyDebugFatal(const char* file, int line) ABSL_ATTRIBUTE_COLD; + ~LogMessageQuietlyDebugFatal(); +}; + +// Used for LOG(QFATAL) to make sure it's properly understood as [[noreturn]]. class LogMessageQuietlyFatal final : public LogMessage { public: LogMessageQuietlyFatal(const char* file, int line) ABSL_ATTRIBUTE_COLD; diff --git a/absl/log/internal/strip.h b/absl/log/internal/strip.h index f8d27869..dfde5786 100644 --- a/absl/log/internal/strip.h +++ b/absl/log/internal/strip.h @@ -38,6 +38,13 @@ ::absl::log_internal::NullStreamMaybeFatal(::absl::kLogDebugFatal) #define ABSL_LOGGING_INTERNAL_LOG_LEVEL(severity) \ ::absl::log_internal::NullStreamMaybeFatal(absl_log_internal_severity) + +// Fatal `DLOG`s expand a little differently to avoid being `[[noreturn]]`. +#define ABSL_LOGGING_INTERNAL_DLOG_FATAL \ + ::absl::log_internal::NullStreamMaybeFatal(::absl::LogSeverity::kFatal) +#define ABSL_LOGGING_INTERNAL_DLOG_QFATAL \ + ::absl::log_internal::NullStreamMaybeFatal(::absl::LogSeverity::kFatal) + #define ABSL_LOG_INTERNAL_CHECK(failure_message) ABSL_LOGGING_INTERNAL_LOG_FATAL #define ABSL_LOG_INTERNAL_QCHECK(failure_message) \ ABSL_LOGGING_INTERNAL_LOG_QFATAL @@ -60,6 +67,13 @@ #define ABSL_LOGGING_INTERNAL_LOG_LEVEL(severity) \ ::absl::log_internal::LogMessage(__FILE__, __LINE__, \ absl_log_internal_severity) + +// Fatal `DLOG`s expand a little differently to avoid being `[[noreturn]]`. +#define ABSL_LOGGING_INTERNAL_DLOG_FATAL \ + ::absl::log_internal::LogMessageDebugFatal(__FILE__, __LINE__) +#define ABSL_LOGGING_INTERNAL_DLOG_QFATAL \ + ::absl::log_internal::LogMessageQuietlyDebugFatal(__FILE__, __LINE__) + // These special cases dispatch to special-case constructors that allow us to // avoid an extra function call and shrink non-LTO binaries by a percent or so. #define ABSL_LOG_INTERNAL_CHECK(failure_message) \ @@ -69,4 +83,11 @@ failure_message) #endif // !defined(STRIP_LOG) || !STRIP_LOG +// This part of a non-fatal `DLOG`s expands the same as `LOG`. +#define ABSL_LOGGING_INTERNAL_DLOG_INFO ABSL_LOGGING_INTERNAL_LOG_INFO +#define ABSL_LOGGING_INTERNAL_DLOG_WARNING ABSL_LOGGING_INTERNAL_LOG_WARNING +#define ABSL_LOGGING_INTERNAL_DLOG_ERROR ABSL_LOGGING_INTERNAL_LOG_ERROR +#define ABSL_LOGGING_INTERNAL_DLOG_DFATAL ABSL_LOGGING_INTERNAL_LOG_DFATAL +#define ABSL_LOGGING_INTERNAL_DLOG_LEVEL ABSL_LOGGING_INTERNAL_LOG_LEVEL + #endif // ABSL_LOG_INTERNAL_STRIP_H_ diff --git a/absl/log/log_basic_test.cc b/absl/log/log_basic_test.cc index 7fc7111d..ef8967af 100644 --- a/absl/log/log_basic_test.cc +++ b/absl/log/log_basic_test.cc @@ -16,6 +16,7 @@ #include "absl/log/log.h" #define ABSL_TEST_LOG LOG +#define ABSL_TEST_DLOG DLOG #include "gtest/gtest.h" #include "absl/log/log_basic_test_impl.inc" diff --git a/absl/log/log_basic_test_impl.inc b/absl/log/log_basic_test_impl.inc index e2f33566..8083ff2c 100644 --- a/absl/log/log_basic_test_impl.inc +++ b/absl/log/log_basic_test_impl.inc @@ -25,6 +25,10 @@ #error ABSL_TEST_LOG must be defined for these tests to work. #endif +#ifndef ABSL_TEST_DLOG +#error ABSL_TEST_DLOG must be defined for these tests to work. +#endif + #include <cerrno> #include <sstream> #include <string> @@ -34,6 +38,7 @@ #include "absl/base/internal/sysinfo.h" #include "absl/base/log_severity.h" #include "absl/log/globals.h" +#include "absl/log/internal/globals.h" #include "absl/log/internal/test_actions.h" #include "absl/log/internal/test_helpers.h" #include "absl/log/internal/test_matchers.h" @@ -367,6 +372,27 @@ TEST_P(BasicLogDeathTest, DFatal) { } #endif +#ifndef NDEBUG +TEST_P(BasicLogTest, DFatalIsCancellable) { + // LOG(DFATAL) does not die when DFATAL death is disabled. + absl::log_internal::SetExitOnDFatal(false); + ABSL_TEST_LOG(DFATAL) << "hello world"; + absl::log_internal::SetExitOnDFatal(true); +} + +#if GTEST_HAS_DEATH_TEST +TEST_P(BasicLogDeathTest, DLogFatalIsNotCancellable) { + EXPECT_EXIT( + { + absl::log_internal::SetExitOnDFatal(false); + ABSL_TEST_DLOG(FATAL) << "hello world"; + absl::log_internal::SetExitOnDFatal(true); + }, + DiedOfFatal, ""); +} +#endif +#endif + TEST_P(BasicLogTest, Level) { absl::log_internal::ScopedMinLogLevel scoped_min_log_level(GetParam()); |