Roll-forward: 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: 588181755
Change-Id: I95db3bae39f8dadb52a307ca3b80775db23de766

6 files changed, 137 insertions, 39 deletions

diff --git a/absl/log/BUILD.bazel b/absl/log/BUILD.bazel
index cb069381..0e9078a4 100644
--- a/absl/log/BUILD.bazel
+++ b/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",
diff --git a/absl/log/CMakeLists.txt b/absl/log/CMakeLists.txt
index 916b5bea..c4ac59a4 100644
--- a/absl/log/CMakeLists.txt
+++ b/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
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