diff options
author | Andy Getzendanner <durandal@google.com> | 2023-09-15 15:24:03 -0700 |
---|---|---|
committer | Copybara-Service <copybara-worker@google.com> | 2023-09-15 15:24:54 -0700 |
commit | 9356553a43ae09ba7760fe7051d7100a1d8efd0b (patch) | |
tree | 1f47573d261e8c05bff90973c8c39f5ace027007 /absl | |
parent | f44e2cac7b8b20e1a6f5457714003e6d7022e889 (diff) |
Honor ABSL_MIN_LOG_LEVEL in CHECK_XX, CHECK_STRXX, CHECK_OK, and the QCHECK flavors of these.
In particular, if ABSL_MIN_LOG_LEVEL exceeds kFatal, these should, upon failure, terminate the program without logging anything. The lack of logging should be visible to the optimizer so that it can strip string literals and stringified variable names from the object file.
Making some edge cases work under Clang required rewriting NormalizeLogSeverity to help make constraints on its return value more obvious to the optimizer.
PiperOrigin-RevId: 565792699
Change-Id: Ibb6a47d4956191bbbd0297e04492cddc354578e2
Diffstat (limited to 'absl')
-rw-r--r-- | absl/base/log_severity.h | 21 | ||||
-rw-r--r-- | absl/log/BUILD.bazel | 1 | ||||
-rw-r--r-- | absl/log/CMakeLists.txt | 1 | ||||
-rw-r--r-- | absl/log/internal/check_op.h | 42 | ||||
-rw-r--r-- | absl/log/internal/conditions.h | 40 | ||||
-rw-r--r-- | absl/log/internal/strip.h | 7 | ||||
-rw-r--r-- | absl/log/stripping_test.cc | 85 | ||||
-rw-r--r-- | absl/status/internal/status_internal.h | 1 |
8 files changed, 149 insertions, 49 deletions
diff --git a/absl/base/log_severity.h b/absl/base/log_severity.h index c8bcd2fd..d0795a2d 100644 --- a/absl/base/log_severity.h +++ b/absl/base/log_severity.h @@ -99,13 +99,13 @@ static constexpr absl::LogSeverity kLogDebugFatal = absl::LogSeverity::kFatal; // Returns the all-caps string representation (e.g. "INFO") of the specified // severity level if it is one of the standard levels and "UNKNOWN" otherwise. constexpr const char* LogSeverityName(absl::LogSeverity s) { - return s == absl::LogSeverity::kInfo - ? "INFO" - : s == absl::LogSeverity::kWarning - ? "WARNING" - : s == absl::LogSeverity::kError - ? "ERROR" - : s == absl::LogSeverity::kFatal ? "FATAL" : "UNKNOWN"; + switch (s) { + case absl::LogSeverity::kInfo: return "INFO"; + case absl::LogSeverity::kWarning: return "WARNING"; + case absl::LogSeverity::kError: return "ERROR"; + case absl::LogSeverity::kFatal: return "FATAL"; + default: return "UNKNOWN"; + } } // NormalizeLogSeverity() @@ -113,9 +113,10 @@ constexpr const char* LogSeverityName(absl::LogSeverity s) { // Values less than `kInfo` normalize to `kInfo`; values greater than `kFatal` // normalize to `kError` (**NOT** `kFatal`). constexpr absl::LogSeverity NormalizeLogSeverity(absl::LogSeverity s) { - return s < absl::LogSeverity::kInfo - ? absl::LogSeverity::kInfo - : s > absl::LogSeverity::kFatal ? absl::LogSeverity::kError : s; + absl::LogSeverity n = s; + if (n < absl::LogSeverity::kInfo) n = absl::LogSeverity::kInfo; + if (n > absl::LogSeverity::kFatal) n = absl::LogSeverity::kError; + return n; } constexpr absl::LogSeverity NormalizeLogSeverity(int s) { return absl::NormalizeLogSeverity(static_cast<absl::LogSeverity>(s)); diff --git a/absl/log/BUILD.bazel b/absl/log/BUILD.bazel index 2f393554..cf49ea3e 100644 --- a/absl/log/BUILD.bazel +++ b/absl/log/BUILD.bazel @@ -550,6 +550,7 @@ cc_test( "//absl/base:strerror", "//absl/flags:program_name", "//absl/log/internal:test_helpers", + "//absl/status", "//absl/strings", "//absl/strings:str_format", "@com_google_googletest//:gtest_main", diff --git a/absl/log/CMakeLists.txt b/absl/log/CMakeLists.txt index 1ca35a16..8316b686 100644 --- a/absl/log/CMakeLists.txt +++ b/absl/log/CMakeLists.txt @@ -1020,6 +1020,7 @@ absl_cc_test( absl::log absl::log_internal_test_helpers absl::log_severity + absl::status absl::strerror absl::strings absl::str_format diff --git a/absl/log/internal/check_op.h b/absl/log/internal/check_op.h index 20b01b5e..a75d4639 100644 --- a/absl/log/internal/check_op.h +++ b/absl/log/internal/check_op.h @@ -65,6 +65,7 @@ ::absl::log_internal::GetReferenceableValue(val2), \ ABSL_LOG_INTERNAL_STRIP_STRING_LITERAL(val1_text \ " " #op " " val2_text))) \ + ABSL_LOG_INTERNAL_CONDITION_FATAL(STATELESS, true) \ ABSL_LOG_INTERNAL_CHECK(*absl_log_internal_check_op_result).InternalStream() #define ABSL_LOG_INTERNAL_QCHECK_OP(name, op, val1, val1_text, val2, \ val2_text) \ @@ -74,6 +75,7 @@ ::absl::log_internal::GetReferenceableValue(val2), \ ABSL_LOG_INTERNAL_STRIP_STRING_LITERAL( \ val1_text " " #op " " val2_text))) \ + ABSL_LOG_INTERNAL_CONDITION_QFATAL(STATELESS, true) \ ABSL_LOG_INTERNAL_QCHECK(*absl_log_internal_qcheck_op_result).InternalStream() #define ABSL_LOG_INTERNAL_CHECK_STROP(func, op, expected, s1, s1_text, s2, \ s2_text) \ @@ -82,6 +84,7 @@ (s1), (s2), \ ABSL_LOG_INTERNAL_STRIP_STRING_LITERAL(s1_text " " #op \ " " s2_text))) \ + ABSL_LOG_INTERNAL_CONDITION_FATAL(STATELESS, true) \ ABSL_LOG_INTERNAL_CHECK(*absl_log_internal_check_strop_result) \ .InternalStream() #define ABSL_LOG_INTERNAL_QCHECK_STROP(func, op, expected, s1, s1_text, s2, \ @@ -91,6 +94,7 @@ (s1), (s2), \ ABSL_LOG_INTERNAL_STRIP_STRING_LITERAL(s1_text " " #op \ " " s2_text))) \ + ABSL_LOG_INTERNAL_CONDITION_QFATAL(STATELESS, true) \ ABSL_LOG_INTERNAL_QCHECK(*absl_log_internal_qcheck_strop_result) \ .InternalStream() // This one is tricky: @@ -113,6 +117,10 @@ // * As usual, no braces so we can stream into the expansion with `operator<<`. // * Also as usual, it must expand to a single (partial) statement with no // ambiguous-else problems. +// * When stripped by `ABSL_MIN_LOG_LEVEL`, we must discard the `<expr> is OK` +// string literal and abort without doing any streaming. We don't need to +// strip the call to stringify the non-ok `Status` as long as we don't log it; +// dropping the `Status`'s message text is out of scope. #define ABSL_LOG_INTERNAL_CHECK_OK(val, val_text) \ for (::std::pair<const ::absl::Status*, ::std::string*> \ absl_log_internal_check_ok_goo; \ @@ -126,22 +134,24 @@ ABSL_LOG_INTERNAL_STRIP_STRING_LITERAL(val_text \ " is OK")), \ !ABSL_PREDICT_TRUE(absl_log_internal_check_ok_goo.first->ok());) \ + ABSL_LOG_INTERNAL_CONDITION_FATAL(STATELESS, true) \ ABSL_LOG_INTERNAL_CHECK(*absl_log_internal_check_ok_goo.second) \ .InternalStream() -#define ABSL_LOG_INTERNAL_QCHECK_OK(val, val_text) \ - for (::std::pair<const ::absl::Status*, ::std::string*> \ - absl_log_internal_check_ok_goo; \ - absl_log_internal_check_ok_goo.first = \ - ::absl::log_internal::AsStatus(val), \ - absl_log_internal_check_ok_goo.second = \ - ABSL_PREDICT_TRUE(absl_log_internal_check_ok_goo.first->ok()) \ - ? nullptr \ - : ::absl::status_internal::MakeCheckFailString( \ - absl_log_internal_check_ok_goo.first, \ - ABSL_LOG_INTERNAL_STRIP_STRING_LITERAL(val_text \ - " is OK")), \ - !ABSL_PREDICT_TRUE(absl_log_internal_check_ok_goo.first->ok());) \ - ABSL_LOG_INTERNAL_QCHECK(*absl_log_internal_check_ok_goo.second) \ +#define ABSL_LOG_INTERNAL_QCHECK_OK(val, val_text) \ + for (::std::pair<const ::absl::Status*, ::std::string*> \ + absl_log_internal_qcheck_ok_goo; \ + absl_log_internal_qcheck_ok_goo.first = \ + ::absl::log_internal::AsStatus(val), \ + absl_log_internal_qcheck_ok_goo.second = \ + ABSL_PREDICT_TRUE(absl_log_internal_qcheck_ok_goo.first->ok()) \ + ? nullptr \ + : ::absl::status_internal::MakeCheckFailString( \ + absl_log_internal_qcheck_ok_goo.first, \ + ABSL_LOG_INTERNAL_STRIP_STRING_LITERAL(val_text \ + " is OK")), \ + !ABSL_PREDICT_TRUE(absl_log_internal_qcheck_ok_goo.first->ok());) \ + ABSL_LOG_INTERNAL_CONDITION_QFATAL(STATELESS, true) \ + ABSL_LOG_INTERNAL_QCHECK(*absl_log_internal_qcheck_ok_goo.second) \ .InternalStream() namespace absl { @@ -152,8 +162,8 @@ template <typename T> class StatusOr; namespace status_internal { -std::string* MakeCheckFailString(const absl::Status* status, - const char* prefix); +ABSL_ATTRIBUTE_PURE_FUNCTION std::string* MakeCheckFailString( + const absl::Status* status, const char* prefix); } // namespace status_internal namespace log_internal { diff --git a/absl/log/internal/conditions.h b/absl/log/internal/conditions.h index 41f67215..45a17b0d 100644 --- a/absl/log/internal/conditions.h +++ b/absl/log/internal/conditions.h @@ -68,7 +68,7 @@ !(condition) ? (void)0 : ::absl::log_internal::Voidify()&& // `ABSL_LOG_INTERNAL_STATEFUL_CONDITION` applies a condition like -// `ABSL_LOG_INTERNAL_CONDITION` but adds to that a series of variable +// `ABSL_LOG_INTERNAL_STATELESS_CONDITION` but adds to that a series of variable // declarations, including a local static object which stores the state needed // to implement the stateful macros like `LOG_EVERY_N`. // @@ -147,20 +147,20 @@ (::absl::kLogDebugFatal == ::absl::LogSeverity::kFatal && \ (::absl::log_internal::AbortQuietly(), false))))) -#define ABSL_LOG_INTERNAL_CONDITION_LEVEL(severity) \ - for (int log_internal_severity_loop = 1; log_internal_severity_loop; \ - log_internal_severity_loop = 0) \ - for (const absl::LogSeverity log_internal_severity = \ - ::absl::NormalizeLogSeverity(severity); \ - log_internal_severity_loop; log_internal_severity_loop = 0) \ +#define ABSL_LOG_INTERNAL_CONDITION_LEVEL(severity) \ + for (int absl_log_internal_severity_loop = 1; \ + absl_log_internal_severity_loop; absl_log_internal_severity_loop = 0) \ + for (const absl::LogSeverity absl_log_internal_severity = \ + ::absl::NormalizeLogSeverity(severity); \ + absl_log_internal_severity_loop; absl_log_internal_severity_loop = 0) \ ABSL_LOG_INTERNAL_CONDITION_LEVEL_IMPL -#define ABSL_LOG_INTERNAL_CONDITION_LEVEL_IMPL(type, condition) \ - ABSL_LOG_INTERNAL_##type##_CONDITION( \ - (condition) && \ - (log_internal_severity >= \ - static_cast<::absl::LogSeverity>(ABSL_MIN_LOG_LEVEL) || \ - (log_internal_severity == ::absl::LogSeverity::kFatal && \ - (::absl::log_internal::AbortQuietly(), false)))) +#define ABSL_LOG_INTERNAL_CONDITION_LEVEL_IMPL(type, condition) \ + ABSL_LOG_INTERNAL_##type##_CONDITION(( \ + (condition) && \ + (absl_log_internal_severity >= \ + static_cast<::absl::LogSeverity>(ABSL_MIN_LOG_LEVEL) || \ + (absl_log_internal_severity == ::absl::LogSeverity::kFatal && \ + (::absl::log_internal::AbortQuietly(), false))))) #else // ndef ABSL_MIN_LOG_LEVEL #define ABSL_LOG_INTERNAL_CONDITION_INFO(type, condition) \ ABSL_LOG_INTERNAL_##type##_CONDITION(condition) @@ -174,12 +174,12 @@ ABSL_LOG_INTERNAL_##type##_CONDITION(condition) #define ABSL_LOG_INTERNAL_CONDITION_DFATAL(type, condition) \ ABSL_LOG_INTERNAL_##type##_CONDITION(condition) -#define ABSL_LOG_INTERNAL_CONDITION_LEVEL(severity) \ - for (int log_internal_severity_loop = 1; log_internal_severity_loop; \ - log_internal_severity_loop = 0) \ - for (const absl::LogSeverity log_internal_severity = \ - ::absl::NormalizeLogSeverity(severity); \ - log_internal_severity_loop; log_internal_severity_loop = 0) \ +#define ABSL_LOG_INTERNAL_CONDITION_LEVEL(severity) \ + for (int absl_log_internal_severity_loop = 1; \ + absl_log_internal_severity_loop; absl_log_internal_severity_loop = 0) \ + for (const absl::LogSeverity absl_log_internal_severity = \ + ::absl::NormalizeLogSeverity(severity); \ + absl_log_internal_severity_loop; absl_log_internal_severity_loop = 0) \ ABSL_LOG_INTERNAL_CONDITION_LEVEL_IMPL #define ABSL_LOG_INTERNAL_CONDITION_LEVEL_IMPL(type, condition) \ ABSL_LOG_INTERNAL_##type##_CONDITION(condition) diff --git a/absl/log/internal/strip.h b/absl/log/internal/strip.h index adc86ffd..f8d27869 100644 --- a/absl/log/internal/strip.h +++ b/absl/log/internal/strip.h @@ -37,7 +37,7 @@ #define ABSL_LOGGING_INTERNAL_LOG_DFATAL \ ::absl::log_internal::NullStreamMaybeFatal(::absl::kLogDebugFatal) #define ABSL_LOGGING_INTERNAL_LOG_LEVEL(severity) \ - ::absl::log_internal::NullStreamMaybeFatal(log_internal_severity) + ::absl::log_internal::NullStreamMaybeFatal(absl_log_internal_severity) #define ABSL_LOG_INTERNAL_CHECK(failure_message) ABSL_LOGGING_INTERNAL_LOG_FATAL #define ABSL_LOG_INTERNAL_QCHECK(failure_message) \ ABSL_LOGGING_INTERNAL_LOG_QFATAL @@ -57,8 +57,9 @@ ::absl::log_internal::LogMessageQuietlyFatal(__FILE__, __LINE__) #define ABSL_LOGGING_INTERNAL_LOG_DFATAL \ ::absl::log_internal::LogMessage(__FILE__, __LINE__, ::absl::kLogDebugFatal) -#define ABSL_LOGGING_INTERNAL_LOG_LEVEL(severity) \ - ::absl::log_internal::LogMessage(__FILE__, __LINE__, log_internal_severity) +#define ABSL_LOGGING_INTERNAL_LOG_LEVEL(severity) \ + ::absl::log_internal::LogMessage(__FILE__, __LINE__, \ + absl_log_internal_severity) // 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) \ diff --git a/absl/log/stripping_test.cc b/absl/log/stripping_test.cc index 35357039..271fae1d 100644 --- a/absl/log/stripping_test.cc +++ b/absl/log/stripping_test.cc @@ -54,6 +54,7 @@ #include "absl/log/check.h" #include "absl/log/internal/test_helpers.h" #include "absl/log/log.h" +#include "absl/status/status.h" #include "absl/strings/escaping.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" @@ -414,4 +415,88 @@ TEST_F(StrippingTest, Check) { } } +TEST_F(StrippingTest, CheckOp) { + // See `StrippingTest.Check` for some hairy implementation notes. + const std::string var_needle1 = + absl::Base64Escape("StrippingTestCheckOpVar1"); + const std::string var_needle2 = + absl::Base64Escape("StrippingTestCheckOpVar2"); + const std::string msg_needle = absl::Base64Escape("StrippingTest.CheckOp"); + volatile int U3RyaXBwaW5nVGVzdENoZWNrT3BWYXIx = 0xFEED; + volatile int U3RyaXBwaW5nVGVzdENoZWNrT3BWYXIy = 0xCAFE; + if (kReallyDie) { + CHECK_EQ(U3RyaXBwaW5nVGVzdENoZWNrT3BWYXIx, U3RyaXBwaW5nVGVzdENoZWNrT3BWYXIy) + << "U3RyaXBwaW5nVGVzdC5DaGVja09w"; + } + + std::unique_ptr<FILE, std::function<void(FILE*)>> exe = OpenTestExecutable(); + ASSERT_THAT(exe, NotNull()); + + if (absl::LogSeverity::kFatal >= kAbslMinLogLevel) { + EXPECT_THAT(exe.get(), FileHasSubstr(var_needle1)); + EXPECT_THAT(exe.get(), FileHasSubstr(var_needle2)); + EXPECT_THAT(exe.get(), FileHasSubstr(msg_needle)); + } else { + EXPECT_THAT(exe.get(), Not(FileHasSubstr(var_needle1))); + EXPECT_THAT(exe.get(), Not(FileHasSubstr(var_needle2))); + EXPECT_THAT(exe.get(), Not(FileHasSubstr(msg_needle))); + } +} + +TEST_F(StrippingTest, CheckStrOp) { + // See `StrippingTest.Check` for some hairy implementation notes. + const std::string var_needle1 = + absl::Base64Escape("StrippingTestCheckStrOpVar1"); + const std::string var_needle2 = + absl::Base64Escape("StrippingTestCheckStrOpVar2"); + const std::string msg_needle = absl::Base64Escape("StrippingTest.CheckStrOp"); + const char *volatile U3RyaXBwaW5nVGVzdENoZWNrU3RyT3BWYXIx = "FEED"; + const char *volatile U3RyaXBwaW5nVGVzdENoZWNrU3RyT3BWYXIy = "CAFE"; + if (kReallyDie) { + CHECK_STREQ(U3RyaXBwaW5nVGVzdENoZWNrU3RyT3BWYXIx, + U3RyaXBwaW5nVGVzdENoZWNrU3RyT3BWYXIy) + << "U3RyaXBwaW5nVGVzdC5DaGVja1N0ck9w"; + } + + std::unique_ptr<FILE, std::function<void(FILE*)>> exe = OpenTestExecutable(); + ASSERT_THAT(exe, NotNull()); + + if (absl::LogSeverity::kFatal >= kAbslMinLogLevel) { + EXPECT_THAT(exe.get(), FileHasSubstr(var_needle1)); + EXPECT_THAT(exe.get(), FileHasSubstr(var_needle2)); + EXPECT_THAT(exe.get(), FileHasSubstr(msg_needle)); + } else { + EXPECT_THAT(exe.get(), Not(FileHasSubstr(var_needle1))); + EXPECT_THAT(exe.get(), Not(FileHasSubstr(var_needle2))); + EXPECT_THAT(exe.get(), Not(FileHasSubstr(msg_needle))); + } +} + +TEST_F(StrippingTest, CheckOk) { + // See `StrippingTest.Check` for some hairy implementation notes. + const std::string var_needle = absl::Base64Escape("StrippingTestCheckOkVar1"); + const std::string msg_needle = absl::Base64Escape("StrippingTest.CheckOk"); + volatile bool x = false; + auto U3RyaXBwaW5nVGVzdENoZWNrT2tWYXIx = absl::OkStatus(); + if (x) { + U3RyaXBwaW5nVGVzdENoZWNrT2tWYXIx = + absl::InvalidArgumentError("Stripping this is not my job!"); + } + if (kReallyDie) { + CHECK_OK(U3RyaXBwaW5nVGVzdENoZWNrT2tWYXIx) + << "U3RyaXBwaW5nVGVzdC5DaGVja09r"; + } + + std::unique_ptr<FILE, std::function<void(FILE*)>> exe = OpenTestExecutable(); + ASSERT_THAT(exe, NotNull()); + + if (absl::LogSeverity::kFatal >= kAbslMinLogLevel) { + EXPECT_THAT(exe.get(), FileHasSubstr(var_needle)); + EXPECT_THAT(exe.get(), FileHasSubstr(msg_needle)); + } else { + EXPECT_THAT(exe.get(), Not(FileHasSubstr(var_needle))); + EXPECT_THAT(exe.get(), Not(FileHasSubstr(msg_needle))); + } +} + } // namespace diff --git a/absl/status/internal/status_internal.h b/absl/status/internal/status_internal.h index 6198e726..f22c611d 100644 --- a/absl/status/internal/status_internal.h +++ b/absl/status/internal/status_internal.h @@ -80,6 +80,7 @@ absl::StatusCode MapToLocalCode(int value); // suitable for output as an error message in assertion/`CHECK()` failures. // // This is an internal implementation detail for Abseil logging. +ABSL_ATTRIBUTE_PURE_FUNCTION std::string* MakeCheckFailString(const absl::Status* status, const char* prefix); |