Moves the generic bug utility to QUICHE.
Protected by refactoring, no functional change; not protected.
PiperOrigin-RevId: 712628602
diff --git a/build/source_list.bzl b/build/source_list.bzl
index 32e4def..ac160d7 100644
--- a/build/source_list.bzl
+++ b/build/source_list.bzl
@@ -10,6 +10,8 @@
]
quiche_core_hdrs = [
"common/btree_scheduler.h",
+ "common/bug_utils.h",
+ "common/bug_utils_test_helper.h",
"common/capsule.h",
"common/http/http_header_block.h",
"common/http/http_header_storage.h",
@@ -409,6 +411,7 @@
"web_transport/web_transport_priority_scheduler.h",
]
quiche_core_srcs = [
+ "common/bug_utils.cc",
"common/capsule.cc",
"common/http/http_header_block.cc",
"common/http/http_header_storage.cc",
@@ -1077,6 +1080,7 @@
"balsa/simple_buffer_test.cc",
"binary_http/binary_http_message_test.cc",
"common/btree_scheduler_test.cc",
+ "common/bug_utils_test.cc",
"common/capsule_test.cc",
"common/http/http_header_block_test.cc",
"common/http/http_header_storage_test.cc",
diff --git a/build/source_list.gni b/build/source_list.gni
index 3b75a6b..a8bf15a 100644
--- a/build/source_list.gni
+++ b/build/source_list.gni
@@ -10,6 +10,8 @@
]
quiche_core_hdrs = [
"src/quiche/common/btree_scheduler.h",
+ "src/quiche/common/bug_utils.h",
+ "src/quiche/common/bug_utils_test_helper.h",
"src/quiche/common/capsule.h",
"src/quiche/common/http/http_header_block.h",
"src/quiche/common/http/http_header_storage.h",
@@ -409,6 +411,7 @@
"src/quiche/web_transport/web_transport_priority_scheduler.h",
]
quiche_core_srcs = [
+ "src/quiche/common/bug_utils.cc",
"src/quiche/common/capsule.cc",
"src/quiche/common/http/http_header_block.cc",
"src/quiche/common/http/http_header_storage.cc",
@@ -1078,6 +1081,7 @@
"src/quiche/balsa/simple_buffer_test.cc",
"src/quiche/binary_http/binary_http_message_test.cc",
"src/quiche/common/btree_scheduler_test.cc",
+ "src/quiche/common/bug_utils_test.cc",
"src/quiche/common/capsule_test.cc",
"src/quiche/common/http/http_header_block_test.cc",
"src/quiche/common/http/http_header_storage_test.cc",
diff --git a/build/source_list.json b/build/source_list.json
index 5eb9364..ab414ec 100644
--- a/build/source_list.json
+++ b/build/source_list.json
@@ -9,6 +9,8 @@
],
"quiche_core_hdrs": [
"quiche/common/btree_scheduler.h",
+ "quiche/common/bug_utils.h",
+ "quiche/common/bug_utils_test_helper.h",
"quiche/common/capsule.h",
"quiche/common/http/http_header_block.h",
"quiche/common/http/http_header_storage.h",
@@ -408,6 +410,7 @@
"quiche/web_transport/web_transport_priority_scheduler.h"
],
"quiche_core_srcs": [
+ "quiche/common/bug_utils.cc",
"quiche/common/capsule.cc",
"quiche/common/http/http_header_block.cc",
"quiche/common/http/http_header_storage.cc",
@@ -1077,6 +1080,7 @@
"quiche/balsa/simple_buffer_test.cc",
"quiche/binary_http/binary_http_message_test.cc",
"quiche/common/btree_scheduler_test.cc",
+ "quiche/common/bug_utils_test.cc",
"quiche/common/capsule_test.cc",
"quiche/common/http/http_header_block_test.cc",
"quiche/common/http/http_header_storage_test.cc",
diff --git a/quiche/common/bug_utils.cc b/quiche/common/bug_utils.cc
new file mode 100644
index 0000000..68bcb6b
--- /dev/null
+++ b/quiche/common/bug_utils.cc
@@ -0,0 +1,64 @@
+#include "quiche/common/bug_utils.h"
+
+#include <atomic>
+#include <cstdint>
+#include <limits>
+#include <string>
+
+#include "absl/base/log_severity.h"
+#include "absl/strings/str_cat.h"
+#include "absl/strings/string_view.h"
+
+namespace quiche {
+namespace internal {
+
+GenericBugStreamHandler::GenericBugStreamHandler(
+ const char* prefix, const char* bug_id, const GenericBugOptions& options)
+ : bug_id_(bug_id), options_(options) {
+ if (options_.condition_str.empty()) {
+ absl::StrAppend(&str_, prefix, "(", bug_id, "): ");
+ } else {
+ absl::StrAppend(&str_, prefix, "_IF(", bug_id, ", ", options_.condition_str,
+ "): ");
+ }
+}
+
+GenericBugStreamHandler::~GenericBugStreamHandler() {
+ GenericBugStreamHandler::OverrideFunction override_function =
+ GetOverrideFunction();
+ if (options_.bug_listener != nullptr) {
+ options_.bug_listener->OnBug(bug_id_, options_.file_name, options_.line,
+ str_);
+ }
+ if (override_function != nullptr) {
+ override_function(options_.severity, options_.file_name, options_.line,
+ str_);
+ }
+}
+
+// static
+void GenericBugStreamHandler::SetOverrideFunction(
+ GenericBugStreamHandler::OverrideFunction override_function) {
+ atomic_override_function_.store(override_function);
+}
+
+// static
+GenericBugStreamHandler::OverrideFunction
+GenericBugStreamHandler::GetOverrideFunction() {
+ return atomic_override_function_.load(std::memory_order_relaxed);
+}
+
+// static
+std::atomic<GenericBugStreamHandler::OverrideFunction>
+ GenericBugStreamHandler::atomic_override_function_ = nullptr;
+
+void GenericBugWithoutLog(const char* bug_id,
+ const GenericBugOptions& options) {
+ if (options.bug_listener != nullptr) {
+ options.bug_listener->OnBug(bug_id, options.file_name, options.line,
+ /*No bug message*/ "");
+ }
+}
+
+} // namespace internal
+} // namespace quiche
diff --git a/quiche/common/bug_utils.h b/quiche/common/bug_utils.h
new file mode 100644
index 0000000..ea6fbf8
--- /dev/null
+++ b/quiche/common/bug_utils.h
@@ -0,0 +1,151 @@
+#ifndef QUICHE_COMMON_BUG_UTILS_H_
+#define QUICHE_COMMON_BUG_UTILS_H_
+
+// This file contains macros for emitting bug log events when invariants are
+// violated.
+//
+// Each instance of a QUICHE_BUG and friends has an associated id, which can be
+// helpful for quickly finding the associated source code.
+//
+// The IDs are free form, but are expected to be unique. Best practice is to
+// provide a *short* description of the condition being detected, without
+// quotes, e.g.,
+//
+// QUICHE_BUG(http2_decoder_invalid_parse_state) << "...";
+//
+// QUICHE_BUG is defined in platform/api/quiche_bug_tracker.h.
+//
+
+#include <atomic>
+#include <cstdint>
+#include <sstream>
+#include <string>
+#include <tuple>
+#include <type_traits>
+
+#include "absl/base/log_severity.h"
+#include "absl/strings/str_cat.h"
+#include "absl/strings/string_view.h"
+#include "quiche/common/platform/api/quiche_export.h"
+
+namespace quiche {
+namespace internal {
+
+QUICHE_EXPORT class GenericBugListener {
+ public:
+ virtual ~GenericBugListener() = default;
+ virtual void OnBug(const char* bug_id, const char* file, int line,
+ absl::string_view bug_message) = 0;
+};
+
+QUICHE_NO_EXPORT struct GenericBugOptions {
+ explicit GenericBugOptions(absl::LogSeverity log_severity,
+ const char* file_name, int line)
+ : severity(log_severity), file_name(file_name), line(line) {}
+
+ GenericBugOptions& SetCondition(absl::string_view condition) {
+ this->condition_str = condition;
+ return *this;
+ }
+
+ GenericBugOptions& SetBugListener(GenericBugListener* listener) {
+ this->bug_listener = listener;
+ return *this;
+ }
+
+ absl::LogSeverity severity;
+ const char* const file_name;
+ const int line;
+ // !empty() for conditional GENERIC_BUGs.
+ absl::string_view condition_str;
+ // If not nullptr, |bug_listener| will be notified of this GENERIC_BUG hit.
+ // Since GenericBugListener may be a temporary object, this is only safe to
+ // access from GenericBugStreamHandler, whose scope is strictly narrower.
+ GenericBugListener* bug_listener = nullptr;
+};
+
+QUICHE_EXPORT inline GenericBugOptions DefaultBugOptions(const char* file_name,
+ int line) {
+ return GenericBugOptions(absl::kLogDebugFatal, file_name, line);
+}
+
+// Called if a GENERIC_BUG is hit, but logging is omitted.
+QUICHE_EXPORT void GenericBugWithoutLog(const char* bug_id,
+ const GenericBugOptions& options);
+
+// GenericBugStreamHandler exposes an interface similar to Abseil log objects,
+// and is used by GENERIC_BUG to trigger a function which can be overridden in
+// tests. By default, this class performs no action. SetOverrideFunction must be
+// called to accomplish anything interesting.
+class QUICHE_EXPORT GenericBugStreamHandler {
+ public:
+ // |prefix| and |bug_id| must be literal strings. They are used in the
+ // destructor.
+ explicit GenericBugStreamHandler(const char* prefix, const char* bug_id,
+ const GenericBugOptions& options);
+
+ ~GenericBugStreamHandler();
+
+ template <typename T,
+ std::enable_if_t<absl::HasAbslStringify<T>::value, bool> = true>
+ QUICHE_EXPORT GenericBugStreamHandler& operator<<(const T& v) {
+ absl::StrAppend(&str_, v);
+ return *this;
+ }
+
+ // For types that support only operator<<. There's a better solution in
+ // Abseil, but unfortunately OStringStream is in a namespace marked internal.
+ template <typename T,
+ std::enable_if_t<!absl::HasAbslStringify<T>::value, bool> = true>
+ QUICHE_EXPORT GenericBugStreamHandler& operator<<(const T& v) {
+ absl::StrAppend(&str_, (std::ostringstream() << v).view());
+ return *this;
+ }
+
+ // Interface similar to Abseil log objects.
+ GenericBugStreamHandler& stream() { return *this; }
+
+ using OverrideFunction = void (*)(absl::LogSeverity severity,
+ const char* file, int line,
+ absl::string_view log_message);
+
+ // Allows overriding the internal implementation. Call with nullptr to make
+ // this class a no-op. This getter and setter are thread-safe.
+ static void SetOverrideFunction(OverrideFunction override_function);
+ static OverrideFunction GetOverrideFunction();
+
+ private:
+ static std::atomic<OverrideFunction> atomic_override_function_;
+
+ const char* bug_id_;
+ std::string str_;
+ const GenericBugOptions& options_;
+};
+
+} // namespace internal
+} // namespace quiche
+
+// The GNU compiler emits a warning for code like:
+//
+// if (foo)
+// if (bar) { } else baz;
+//
+// because it thinks you might want the else to bind to the first if. This
+// leads to problems with code like:
+//
+// if (do_expr) GENERIC_BUG(bug_id) << "Some message";
+//
+// The "switch (0) case 0:" idiom is used to suppress this.
+#define GENERIC_BUG_UNBRACED_ELSE_BLOCKER \
+ switch (0) \
+ case 0: \
+ default:
+
+#define GENERIC_BUG_IMPL(prefix, bug_id, skip_log_condition, options) \
+ if (skip_log_condition) { \
+ ::quiche::internal::GenericBugWithoutLog(#bug_id, (options)); \
+ } else /* NOLINT */ \
+ ::quiche::internal::GenericBugStreamHandler(prefix, #bug_id, (options)) \
+ .stream()
+
+#endif // QUICHE_COMMON_BUG_UTILS_H_
diff --git a/quiche/common/bug_utils_test.cc b/quiche/common/bug_utils_test.cc
new file mode 100644
index 0000000..0bddb0d
--- /dev/null
+++ b/quiche/common/bug_utils_test.cc
@@ -0,0 +1,201 @@
+#include "quiche/common/bug_utils.h"
+
+#include <string.h>
+
+#include <algorithm>
+#include <memory>
+#include <string>
+
+#include "absl/base/log_severity.h"
+#include "absl/strings/string_view.h"
+#include "quiche/common/bug_utils_test_helper.h"
+#include "quiche/common/platform/api/quiche_test.h"
+
+namespace quiche {
+namespace internal {
+namespace {
+
+using ::testing::_;
+using ::testing::AnyNumber;
+using ::testing::EndsWith;
+using ::testing::InSequence;
+
+class BugHandler {
+ public:
+ virtual ~BugHandler() = default;
+ virtual void OnBug(absl::string_view file, int line,
+ absl::string_view message) = 0;
+};
+
+// This class provides a convenient way to write expectations for the bug
+// override function.
+class MockBugHandler : public BugHandler {
+ public:
+ MockBugHandler() = default;
+
+ MOCK_METHOD(void, OnBug,
+ (absl::string_view file, int line, absl::string_view message),
+ (override));
+};
+
+MockBugHandler* mock_bug_handler = nullptr;
+
+MockBugHandler* GetInstance() {
+ if (mock_bug_handler == nullptr) {
+ mock_bug_handler = new MockBugHandler;
+ }
+ return mock_bug_handler;
+}
+
+void ResetInstance() {
+ delete mock_bug_handler;
+ mock_bug_handler = nullptr;
+}
+
+class BugUtilsTest : public ::quiche::test::QuicheTest {
+ public:
+ void SetUp() override {
+ fn_ = GenericBugStreamHandler::GetOverrideFunction();
+ GenericBugStreamHandler::SetOverrideFunction(
+ [](absl::LogSeverity /*severity*/, const char* file, int line,
+ absl::string_view log_message) {
+ GetInstance()->OnBug(absl::string_view(file, strlen(file)), line,
+ log_message);
+ });
+ }
+
+ void TearDown() override {
+ GenericBugStreamHandler::SetOverrideFunction(fn_);
+ ResetInstance();
+ }
+
+ inline static GenericBugStreamHandler::OverrideFunction fn_ = nullptr;
+};
+
+// Tests several permutations.
+TEST_F(BugUtilsTest, TestsEverythingUsing23And26) {
+ InSequence seq;
+ EXPECT_CALL(*GetInstance(), OnBug(EndsWith("bug_utils_test_helper.h"), 23,
+ EndsWith("Here on line 23")));
+ LogBugLine23();
+
+ EXPECT_CALL(*GetInstance(), OnBug(EndsWith("bug_utils_test_helper.h"), 23,
+ EndsWith("Here on line 23")));
+ LogBugLine23();
+
+ EXPECT_CALL(*GetInstance(), OnBug(EndsWith("bug_utils_test_helper.h"), 26,
+ EndsWith("Here on line 26")));
+ EXPECT_CALL(*GetInstance(), OnBug(EndsWith("bug_utils_test_helper.h"), 27,
+ EndsWith("And 27!")));
+ LogBugLine26();
+}
+
+TEST_F(BugUtilsTest, TestBugIf) {
+ InSequence seq;
+
+ // Verify that we don't invoke the function for a false condition.
+ LogIfBugLine31(false);
+
+ // The first true should trigger an invocation.
+ EXPECT_CALL(*GetInstance(), OnBug(EndsWith("bug_utils_test_helper.h"), 31,
+ EndsWith("Here on line 31")));
+ LogIfBugLine31(true);
+
+ // It's always a no-op if the condition is false.
+ LogIfBugLine31(false); // no-op
+ LogIfBugLine31(false); // no-op
+}
+
+TEST_F(BugUtilsTest, TestBugIfMessage) {
+ int i;
+
+ // Check success
+ LogIfBugNullCheckLine35(&i);
+
+ // Check failure
+ EXPECT_CALL(
+ *GetInstance(),
+ OnBug(
+ EndsWith("bug_utils_test_helper.h"), 35,
+ EndsWith(
+ "QUICHE_TEST_BUG_IF(Bug 35, ptr == nullptr): Here on line 35")));
+ LogIfBugNullCheckLine35(nullptr);
+}
+
+// Don't actually need to crash, just cause a side effect the test can assert
+// on.
+int num_times_called = 0;
+bool BadCondition() {
+ ++num_times_called;
+ return true;
+}
+
+TEST_F(BugUtilsTest, BadCondition) {
+ InSequence seq;
+
+ EXPECT_EQ(num_times_called, 0);
+
+ EXPECT_CALL(*GetInstance(), OnBug(_, _, EndsWith("Called BadCondition")));
+ QUICHE_TEST_BUG_IF(id, BadCondition()) << "Called BadCondition";
+ EXPECT_EQ(num_times_called, 1);
+}
+
+TEST_F(BugUtilsTest, NoDanglingElse) {
+ auto unexpected_bug_message = [] {
+ ADD_FAILURE() << "This should not be called";
+ return "bad";
+ };
+
+ if (false) QUICHE_TEST_BUG(dangling_else) << unexpected_bug_message();
+
+ bool expected_else_reached = false;
+ if (false)
+ QUICHE_TEST_BUG(dangling_else_2) << unexpected_bug_message();
+ else
+ expected_else_reached = true;
+
+ EXPECT_TRUE(expected_else_reached);
+}
+
+TEST_F(BugUtilsTest, BugListener) {
+ class TestListener : public GenericBugListener {
+ public:
+ explicit TestListener(bool expect_log_message)
+ : expect_log_message_(expect_log_message) {}
+
+ ~TestListener() override { EXPECT_EQ(hit_count_, 1); }
+
+ void OnBug(const char* bug_id, const char* file, int line,
+ absl::string_view bug_message) override {
+ ++hit_count_;
+ EXPECT_EQ(bug_id, "bug_listener_test");
+ EXPECT_EQ(file, __FILE__);
+ EXPECT_GT(line, 0);
+ if (expect_log_message_) {
+ EXPECT_EQ(bug_message, "TEST_BUG(bug_listener_test): Bug listener msg");
+ } else {
+ EXPECT_EQ(bug_message, "");
+ }
+ }
+
+ TestListener* self() { return this; }
+
+ private:
+ int hit_count_ = 0;
+ const bool expect_log_message_;
+ };
+
+ GENERIC_BUG_IMPL("TEST_BUG", bug_listener_test, /*skip_log_condition=*/false,
+ QUICHE_TEST_BUG_OPTIONS().SetBugListener(
+ TestListener(/*expect_log_message=*/true).self()))
+ << "Bug listener msg";
+
+ GENERIC_BUG_IMPL("TEST_BUG", bug_listener_test, /*skip_log_condition=*/true,
+ QUICHE_TEST_BUG_OPTIONS().SetBugListener(
+ TestListener(/*expect_log_message=*/false).self()))
+ << "Bug listener msg";
+}
+
+} // namespace
+} // namespace internal
+} // namespace quiche
diff --git a/quiche/common/bug_utils_test_helper.h b/quiche/common/bug_utils_test_helper.h
new file mode 100644
index 0000000..6497bca
--- /dev/null
+++ b/quiche/common/bug_utils_test_helper.h
@@ -0,0 +1,41 @@
+#ifndef QUICHE_COMMON_BUG_UTILS_TEST_HELPER_H_
+#define QUICHE_COMMON_BUG_UTILS_TEST_HELPER_H_
+
+#include "quiche/common/bug_utils.h"
+
+// Sticking various logging functions used by the test in a separate file,
+// so their line numbers are unlikely to change as we modify the test file
+// itself, as the expectations we set take the file + line numbers into account.
+
+#define QUICHE_TEST_BUG(bug_id) \
+ GENERIC_BUG_UNBRACED_ELSE_BLOCKER \
+ GENERIC_BUG_IMPL("QUICHE_TEST_BUG", bug_id, false, \
+ ::quiche::internal::DefaultBugOptions(__FILE__, __LINE__))
+
+#define QUICHE_TEST_BUG_IF(bug_id, condition) \
+ GENERIC_BUG_UNBRACED_ELSE_BLOCKER \
+ if (!(condition)) { /* Do nothing */ \
+ } else /* NOLINT */ \
+ GENERIC_BUG_IMPL("QUICHE_TEST_BUG", bug_id, false, \
+ ::quiche::internal::DefaultBugOptions(__FILE__, __LINE__) \
+ .SetCondition(#condition))
+
+inline void LogBugLine23() { QUICHE_TEST_BUG(Bug 23) << "Here on line 23"; }
+
+inline void LogBugLine26() {
+ QUICHE_TEST_BUG(Bug 26) << "Here on line 26";
+ QUICHE_TEST_BUG(Bug 27) << "And 27!";
+}
+
+inline void LogIfBugLine31(bool condition) {
+ QUICHE_TEST_BUG_IF(Bug 31, condition) << "Here on line 31";
+}
+
+inline void LogIfBugNullCheckLine35(int *ptr) {
+ QUICHE_TEST_BUG_IF(Bug 35, ptr == nullptr) << "Here on line 35";
+}
+
+#define QUICHE_TEST_BUG_OPTIONS() \
+ ::quiche::internal::DefaultBugOptions(__FILE__, __LINE__)
+
+#endif // QUICHE_COMMON_BUG_UTILS_TEST_HELPER_H_
