summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Peter Boström <pbos@google.com>2024-03-22 12:33:04 -0700
committerGravatar Copybara-Service <copybara-worker@google.com>2024-03-22 12:33:53 -0700
commit9a9502bfd19981bace5e93597db9ec1fade48005 (patch)
tree46c1f1e63854a9126de5e9cba0efb8b9293bdd5c
parent76f8011beabdaee872b5fde7546e02407b220cb1 (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.cc1
-rw-r--r--absl/log/internal/log_impl.h44
-rw-r--r--absl/log/internal/log_message.cc46
-rw-r--r--absl/log/internal/log_message.h19
-rw-r--r--absl/log/internal/strip.h21
-rw-r--r--absl/log/log_basic_test.cc1
-rw-r--r--absl/log/log_basic_test_impl.inc26
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());