Add an API to clear the saved LogBacktraceAt location, and call it when setting an empty or invalid flag value.

PiperOrigin-RevId: 525065479
Change-Id: I3c0822db8301e0999b0669394b415df82edd445f

4 files changed, 43 insertions, 20 deletions

diff --git a/absl/log/flags.cc b/absl/log/flags.cc
index b5308881..215b7bd5 100644
--- a/absl/log/flags.cc
+++ b/absl/log/flags.cc
@@ -90,19 +90,27 @@ ABSL_FLAG(std::string, log_backtrace_at, "",
     .OnUpdate([] {
       const std::string log_backtrace_at =
           absl::GetFlag(FLAGS_log_backtrace_at);
-      if (log_backtrace_at.empty()) return;
+      if (log_backtrace_at.empty()) {
+        absl::ClearLogBacktraceLocation();
+        return;
+      }
 
       const size_t last_colon = log_backtrace_at.rfind(':');
-      if (last_colon == log_backtrace_at.npos) return;
+      if (last_colon == log_backtrace_at.npos) {
+        absl::ClearLogBacktraceLocation();
+        return;
+      }
 
       const absl::string_view file =
           absl::string_view(log_backtrace_at).substr(0, last_colon);
       int line;
-      if (absl::SimpleAtoi(
+      if (!absl::SimpleAtoi(
               absl::string_view(log_backtrace_at).substr(last_colon + 1),
               &line)) {
-        absl::SetLogBacktraceLocation(file, line);
+        absl::ClearLogBacktraceLocation();
+        return;
       }
+      absl::SetLogBacktraceLocation(file, line);
     });
 
 ABSL_FLAG(bool, log_prefix, true,
diff --git a/absl/log/flags_test.cc b/absl/log/flags_test.cc
index a0f6d763..1080ea11 100644
--- a/absl/log/flags_test.cc
+++ b/absl/log/flags_test.cc
@@ -92,23 +92,23 @@ TEST_F(LogFlagsTest, PrependLogPrefix) {
 
 TEST_F(LogFlagsTest, EmptyBacktraceAtFlag) {
   absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
-  absl::SetFlag(&FLAGS_log_backtrace_at, "");
   absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
 
   EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
 
   test_sink.StartCapturingLogs();
+  absl::SetFlag(&FLAGS_log_backtrace_at, "");
   LOG(INFO) << "hello world";
 }
 
 TEST_F(LogFlagsTest, BacktraceAtNonsense) {
   absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
-  absl::SetFlag(&FLAGS_log_backtrace_at, "gibberish");
   absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
 
   EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
 
   test_sink.StartCapturingLogs();
+  absl::SetFlag(&FLAGS_log_backtrace_at, "gibberish");
   LOG(INFO) << "hello world";
 }
 
@@ -116,13 +116,13 @@ TEST_F(LogFlagsTest, BacktraceAtWrongFile) {
   absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
   const int log_line = __LINE__ + 1;
   auto do_log = [] { LOG(INFO) << "hello world"; };
-  absl::SetFlag(&FLAGS_log_backtrace_at,
-                absl::StrCat("some_other_file.cc:", log_line));
   absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
 
   EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
 
   test_sink.StartCapturingLogs();
+  absl::SetFlag(&FLAGS_log_backtrace_at,
+                absl::StrCat("some_other_file.cc:", log_line));
   do_log();
 }
 
@@ -130,13 +130,13 @@ TEST_F(LogFlagsTest, BacktraceAtWrongLine) {
   absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
   const int log_line = __LINE__ + 1;
   auto do_log = [] { LOG(INFO) << "hello world"; };
-  absl::SetFlag(&FLAGS_log_backtrace_at,
-                absl::StrCat("flags_test.cc:", log_line + 1));
   absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
 
   EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
 
   test_sink.StartCapturingLogs();
+  absl::SetFlag(&FLAGS_log_backtrace_at,
+                absl::StrCat("flags_test.cc:", log_line + 1));
   do_log();
 }
 
@@ -144,12 +144,12 @@ TEST_F(LogFlagsTest, BacktraceAtWholeFilename) {
   absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
   const int log_line = __LINE__ + 1;
   auto do_log = [] { LOG(INFO) << "hello world"; };
-  absl::SetFlag(&FLAGS_log_backtrace_at, absl::StrCat(__FILE__, ":", log_line));
   absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
 
   EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
 
   test_sink.StartCapturingLogs();
+  absl::SetFlag(&FLAGS_log_backtrace_at, absl::StrCat(__FILE__, ":", log_line));
   do_log();
 }
 
@@ -157,13 +157,13 @@ TEST_F(LogFlagsTest, BacktraceAtNonmatchingSuffix) {
   absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
   const int log_line = __LINE__ + 1;
   auto do_log = [] { LOG(INFO) << "hello world"; };
-  absl::SetFlag(&FLAGS_log_backtrace_at,
-                absl::StrCat("flags_test.cc:", log_line, "gibberish"));
   absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
 
   EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
 
   test_sink.StartCapturingLogs();
+  absl::SetFlag(&FLAGS_log_backtrace_at,
+                absl::StrCat("flags_test.cc:", log_line, "gibberish"));
   do_log();
 }
 
@@ -171,13 +171,17 @@ TEST_F(LogFlagsTest, LogsBacktrace) {
   absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
   const int log_line = __LINE__ + 1;
   auto do_log = [] { LOG(INFO) << "hello world"; };
-  absl::SetFlag(&FLAGS_log_backtrace_at,
-                absl::StrCat("flags_test.cc:", log_line));
   absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
 
+  testing::InSequence seq;
   EXPECT_CALL(test_sink, Send(TextMessage(HasSubstr("(stacktrace:"))));
+  EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
 
   test_sink.StartCapturingLogs();
+  absl::SetFlag(&FLAGS_log_backtrace_at,
+                absl::StrCat("flags_test.cc:", log_line));
+  do_log();
+  absl::SetFlag(&FLAGS_log_backtrace_at, "");
   do_log();
 }
 
diff --git a/absl/log/globals.cc b/absl/log/globals.cc
index 6dfe81f0..f7c2ec42 100644
--- a/absl/log/globals.cc
+++ b/absl/log/globals.cc
@@ -123,7 +123,7 @@ namespace log_internal {
 
 bool ShouldLogBacktraceAt(absl::string_view file, int line) {
   const size_t flag_hash =
-      log_backtrace_at_hash.load(std::memory_order_acquire);
+      log_backtrace_at_hash.load(std::memory_order_relaxed);
 
   return flag_hash != 0 && flag_hash == HashSiteForLogBacktraceAt(file, line);
 }
@@ -132,7 +132,11 @@ bool ShouldLogBacktraceAt(absl::string_view file, int line) {
 
 void SetLogBacktraceLocation(absl::string_view file, int line) {
   log_backtrace_at_hash.store(HashSiteForLogBacktraceAt(file, line),
-                              std::memory_order_release);
+                              std::memory_order_relaxed);
+}
+
+void ClearLogBacktraceLocation() {
+  log_backtrace_at_hash.store(0, std::memory_order_relaxed);
 }
 
 bool ShouldPrependLogPrefix() {
diff --git a/absl/log/globals.h b/absl/log/globals.h
index 32b87db0..d56622ae 100644
--- a/absl/log/globals.h
+++ b/absl/log/globals.h
@@ -110,8 +110,8 @@ class ScopedStderrThreshold final {
 // Log Backtrace At
 //------------------------------------------------------------------------------
 //
-// Users can request backtrace to be logged at specific locations, specified
-// by file and line number.
+// Users can request an existing `LOG` statement, specified by file and line
+// number, to also include a backtrace when logged.
 
 // ShouldLogBacktraceAt()
 //
@@ -123,9 +123,16 @@ ABSL_MUST_USE_RESULT bool ShouldLogBacktraceAt(absl::string_view file,
 
 // SetLogBacktraceLocation()
 //
-// Sets the location the backtrace should be logged at.
+// Sets the location the backtrace should be logged at.  If the specified
+// location isn't a `LOG` statement, the effect will be the same as
+// `ClearLogBacktraceLocation` (but less efficient).
 void SetLogBacktraceLocation(absl::string_view file, int line);
 
+// ClearLogBacktraceLocation()
+//
+// Clears the set location so that backtraces will no longer be logged at it.
+void ClearLogBacktraceLocation();
+
 //------------------------------------------------------------------------------
 // Prepend Log Prefix
 //------------------------------------------------------------------------------