Roll-forward: Honor ABSL_MIN_LOG_LEVEL in CHECK_XX, CHECK_STRXX, CHEC…

…K_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: 588181755
Change-Id: I95db3bae39f8dadb52a307ca3b80775db23de766

absl/base/log_severity.h

@@ -99,23 +99,24 @@ 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()
 //
 // 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));

absl/log/BUILD.bazel

@@ -654,6 +654,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",

absl/log/CMakeLists.txt

@@ -1084,6 +1084,7 @@ absl_cc_test(
     absl::log
     absl::log_internal_test_helpers
     absl::log_severity
+    absl::status
     absl::strerror
     absl::strings
     absl::str_format

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 {

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)

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) \

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

absl/status/internal/status_internal.h

@@ -118,6 +118,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);