[server] add unused ENVOY_BUG implementation

source/common/common/assert.cc

@@ -28,15 +28,55 @@ class ActionRegistrationImpl : public ActionRegistration {
   static std::function<void()> debug_assertion_failure_record_action_;
 };
 
+class EnvoyBugRegistrationImpl : public ActionRegistration {
+public:
+  EnvoyBugRegistrationImpl(std::function<void()> action) {
+    ASSERT(envoy_bug_failure_record_action_ == nullptr);
+    envoy_bug_failure_record_action_ = action;
+  }
+
+  ~EnvoyBugRegistrationImpl() override {
+    ASSERT(envoy_bug_failure_record_action_ != nullptr);
+    envoy_bug_failure_record_action_ = nullptr;
+  }
+
+  static void invokeAction() {
+    if (envoy_bug_failure_record_action_ != nullptr) {
+      ++count_;
+      // ENVOY_BUG actions are only invoked on power of two increments.
+      if ((count_ & (count_ - 1)) == 0) {
+        envoy_bug_failure_record_action_();
+      }
+    }
+  }
+
+private:
+  // This implementation currently only handles one action being set at a time. This is currently
+  // sufficient. If multiple actions are ever needed, the actions should be chained when
+  // additional actions are registered.
+  static std::function<void()> envoy_bug_failure_record_action_;
+  static std::atomic<uint64_t> count_;
+};
+
 std::function<void()> ActionRegistrationImpl::debug_assertion_failure_record_action_;
+std::function<void()> EnvoyBugRegistrationImpl::envoy_bug_failure_record_action_;
+std::atomic<uint64_t> EnvoyBugRegistrationImpl::count_;
 
 ActionRegistrationPtr setDebugAssertionFailureRecordAction(const std::function<void()>& action) {
   return std::make_unique<ActionRegistrationImpl>(action);
 }
 
+ActionRegistrationPtr setEnvoyBugFailureRecordAction(const std::function<void()>& action) {
+  return std::make_unique<EnvoyBugRegistrationImpl>(action);
+}
+
 void invokeDebugAssertionFailureRecordAction_ForAssertMacroUseOnly() {
   ActionRegistrationImpl::invokeAction();
 }
 
+void invokeEnvoyBugFailureRecordAction_ForEnvoyBugMacroUseOnly() {
+  EnvoyBugRegistrationImpl::invokeAction();
+}
+
 } // namespace Assert
 } // namespace Envoy

source/common/common/assert.h

@@ -32,6 +32,22 @@ using ActionRegistrationPtr = std::unique_ptr<ActionRegistration>;
  */
 ActionRegistrationPtr setDebugAssertionFailureRecordAction(const std::function<void()>& action);
 
+/**
+ * Sets an action to be invoked when an ENVOY_BUG failure is detected in a release build. This
+ * action will be invoked each time an ENVOY_BUG failure is detected.
+ *
+ * This function is not thread-safe; concurrent calls to set the action are not allowed.
+ *
+ * The action may be invoked concurrently if two ENVOY_BUGs in different threads fail at the
+ * same time, so the action must be thread-safe.
+ *
+ * This has no effect in debug builds (envoy bug failure aborts the process).
+ *
+ * @param action The action to take when an envoy bug fails.
+ * @return A registration object. The registration is removed when the object is destructed.
+ */
+ActionRegistrationPtr setEnvoyBugFailureRecordAction(const std::function<void()>& action);
+
 /**
  * Invokes the action set by setDebugAssertionFailureRecordAction, or does nothing if
  * no action has been set.
@@ -40,6 +56,14 @@ ActionRegistrationPtr setDebugAssertionFailureRecordAction(const std::function<v
  */
 void invokeDebugAssertionFailureRecordAction_ForAssertMacroUseOnly();
 
+/**
+ * Invokes the action set by setEnvoyBugFailureRecordAction, or does nothing if
+ * no action has been set.
+ *
+ * This should only be called by ENVOY_BUG macros in this file.
+ */
+void invokeEnvoyBugFailureRecordAction_ForEnvoyBugMacroUseOnly();
+
 // CONDITION_STR is needed to prevent macros in condition from being expected, which obfuscates
 // the logged failure, e.g., "EAGAIN" vs "11".
 #define _ASSERT_IMPL(CONDITION, CONDITION_STR, ACTION, DETAILS)                                    \
@@ -82,10 +106,16 @@ void invokeDebugAssertionFailureRecordAction_ForAssertMacroUseOnly();
  */
 #define SECURITY_ASSERT(X, DETAILS) _ASSERT_IMPL(X, #X, abort(), DETAILS)
 
+// This is a workaround for fact that MSVC expands __VA_ARGS__ after passing them into a macro,
+// rather than before passing them into a macro. Without this, _ASSERT_SELECTOR does not work
+// correctly when compiled with MSVC
+#define EXPAND(X) X
+
 #if !defined(NDEBUG) || defined(ENVOY_LOG_DEBUG_ASSERT_IN_RELEASE)
 
 #if !defined(NDEBUG) // If this is a debug build.
 #define ASSERT_ACTION abort()
+#define ENVOY_BUG_ACTION abort()
 #else // If this is not a debug build, but ENVOY_LOG_DEBUG_ASSERT_IN_RELEASE is defined.
 #define ASSERT_ACTION Envoy::Assert::invokeDebugAssertionFailureRecordAction_ForAssertMacroUseOnly()
 #endif // !defined(NDEBUG)
@@ -94,11 +124,6 @@ void invokeDebugAssertionFailureRecordAction_ForAssertMacroUseOnly();
 #define _ASSERT_VERBOSE(X, Y) _ASSERT_IMPL(X, #X, ASSERT_ACTION, Y)
 #define _ASSERT_SELECTOR(_1, _2, ASSERT_MACRO, ...) ASSERT_MACRO
 
-// This is a workaround for fact that MSVC expands __VA_ARGS__ after passing them into a macro,
-// rather than before passing them into a macro. Without this, _ASSERT_SELECTOR does not work
-// correctly when compiled with MSVC
-#define EXPAND(X) X
-
 #if !defined(ENVOY_DISABLE_KNOWN_ISSUE_ASSERTS)
 /**
  * Assert wrapper for an as-yet unidentified issue. Even with ASSERTs compiled in, it may be
@@ -111,7 +136,7 @@ void invokeDebugAssertionFailureRecordAction_ForAssertMacroUseOnly();
 // This non-implementation ensures that its argument is a valid expression that can be statically
 // casted to a bool, but the expression is never evaluated and will be compiled away.
 #define KNOWN_ISSUE_ASSERT _NULL_ASSERT_IMPL
-#endif // defined(ENVOY_DEBUG_KNOWN_ISSUES)
+#endif // defined(ENVOY_DISABLE_KNOWN_ISSUE_ASSERTS)
 
 // If ASSERT is called with one argument, the ASSERT_SELECTOR will return
 // _ASSERT_ORIGINAL and this will call _ASSERT_ORIGINAL(__VA_ARGS__).
@@ -122,6 +147,7 @@ void invokeDebugAssertionFailureRecordAction_ForAssertMacroUseOnly();
 #else
 #define ASSERT _NULL_ASSERT_IMPL
 #define KNOWN_ISSUE_ASSERT _NULL_ASSERT_IMPL
+#define ENVOY_BUG_ACTION Envoy::Assert::invokeEnvoyBugFailureRecordAction_ForEnvoyBugMacroUseOnly()
 #endif // !defined(NDEBUG) || defined(ENVOY_LOG_DEBUG_ASSERT_IN_RELEASE)
 
 /**
@@ -134,6 +160,32 @@ void invokeDebugAssertionFailureRecordAction_ForAssertMacroUseOnly();
     abort();                                                                                       \
   } while (false)
 
+// CONDITION_STR is needed to prevent macros in condition from being expected, which obfuscates
+// the logged failure, e.g., "EAGAIN" vs "11".
+#define _ENVOY_BUG_IMPL(CONDITION, CONDITION_STR, ACTION, DETAILS)                                 \
+  do {                                                                                             \
+    if (!(CONDITION)) {                                                                            \
+      const std::string& details = (DETAILS);                                                      \
+      ENVOY_LOG_TO_LOGGER(Envoy::Logger::Registry::getLog(Envoy::Logger::Id::envoy_bug), error,    \
+                          "envoy bug failure: {}.{}{}", CONDITION_STR,                             \
+                          details.empty() ? "" : " Details: ", details);                           \
+      ACTION;                                                                                      \
+    }                                                                                              \
+  } while (false)
+
+#define _ENVOY_BUG_ORIGINAL(X) _ENVOY_BUG_IMPL(X, #X, ENVOY_BUG_ACTION, "")
+#define _ENVOY_BUG_VERBOSE(X, Y) _ENVOY_BUG_IMPL(X, #X, ENVOY_BUG_ACTION, Y)
+#define _ENVOY_BUG_SELECTOR(_1, _2, ENVOY_BUG_MACRO, ...) ENVOY_BUG_MACRO
+
+/**
+ * If ENVOY_BUG is called with one argument, the ENVOY_BUG_SELECTOR will return _ENVOY_BUG_ORIGINAL
+ * and this will call _ENVOY_BUG_ORIGINAL(__VA_ARGS__). If ENVOY_BUG is called with two arguments,
+ * ENVOY_BUG_SELECTOR will return _ENVOY_BUG_VERBOSE, and this will call
+ * _ENVOY_BUG_VERBOSE(__VA_ARGS__).
+ */
+#define ENVOY_BUG(...)                                                                             \
+  EXPAND(_ENVOY_BUG_SELECTOR(__VA_ARGS__, _ENVOY_BUG_VERBOSE, _ENVOY_BUG_ORIGINAL)(__VA_ARGS__))
+
 // NOT_IMPLEMENTED_GCOVR_EXCL_LINE is for overridden functions that are expressly not implemented.
 // The macro name includes "GCOVR_EXCL_LINE" to exclude the macro's usage from code coverage
 // reports.

source/common/common/logger.h

@@ -34,6 +34,7 @@ namespace Logger {
   FUNCTION(conn_handler)                                                                           \
   FUNCTION(decompression)                                                                          \
   FUNCTION(dubbo)                                                                                  \
+  FUNCTION(envoy_bug)                                                                              \
   FUNCTION(ext_authz)                                                                              \
   FUNCTION(rocketmq)                                                                               \
   FUNCTION(file)                                                                                   \

source/server/server.cc

@@ -332,6 +332,8 @@ void InstanceImpl::initialize(const Options& options,
 
   assert_action_registration_ = Assert::setDebugAssertionFailureRecordAction(
       [this]() { server_stats_->debug_assertion_failures_.inc(); });
+  envoy_bug_action_registration_ = Assert::setEnvoyBugFailureRecordAction(
+      [this]() { server_stats_->envoy_bug_failures_.inc(); });
 
   InstanceImpl::failHealthcheck(false);
 

source/server/server.h

@@ -54,6 +54,7 @@ namespace Server {
  */
 #define ALL_SERVER_STATS(COUNTER, GAUGE, HISTOGRAM)                                                \
   COUNTER(debug_assertion_failures)                                                                \
+  COUNTER(envoy_bug_failures)                                                                      \
   COUNTER(dynamic_unknown_fields)                                                                  \
   COUNTER(static_unknown_fields)                                                                   \
   GAUGE(concurrency, NeverImport)                                                                  \
@@ -317,6 +318,7 @@ class InstanceImpl final : Logger::Loggable<Logger::Id::main>,
   Stats::StoreRoot& stats_store_;
   std::unique_ptr<ServerStats> server_stats_;
   Assert::ActionRegistrationPtr assert_action_registration_;
+  Assert::ActionRegistrationPtr envoy_bug_action_registration_;
   ThreadLocal::Instance& thread_local_;
   Api::ApiPtr api_;
   Event::DispatcherPtr dispatcher_;

test/common/common/assert_test.cc

@@ -42,4 +42,29 @@ TEST(AssertDeathTest, VariousLogs) {
   EXPECT_EQ(expected_counted_failures, assert_fail_count);
 }
 
+TEST(EnvoyBugDeathTest, VariousLogs) {
+  int expected_counted_failures;
+  int envoy_bug_fail_count = 0;
+  // ENVOY_BUG actions only occur on power of two counts.
+  auto envoy_bug_action_registration =
+      Assert::setEnvoyBugFailureRecordAction([&]() { envoy_bug_fail_count++; });
+
+#ifndef NDEBUG
+  EXPECT_DEATH({ ENVOY_BUG(0); }, ".*envoy bug failure: 0.*");
+  EXPECT_DEATH({ ENVOY_BUG(0, ""); }, ".*envoy bug failure: 0.*");
+  EXPECT_DEATH({ ENVOY_BUG(0, "With some logs"); },
+               ".*envoy bug failure: 0. Details: With some logs.*");
+  expected_counted_failures = 0;
+#else
+  EXPECT_LOG_CONTAINS("error", "envoy bug failure: 0", ENVOY_BUG(0));
+  EXPECT_LOG_CONTAINS("error", "envoy bug failure: 0", ENVOY_BUG(0));
+  EXPECT_LOG_CONTAINS("error", "envoy bug failure: 0", ENVOY_BUG(0));
+  EXPECT_LOG_CONTAINS("error", "envoy bug failure: 0", ENVOY_BUG(0, ""));
+  EXPECT_LOG_CONTAINS("error", "envoy bug failure: 0. Details: With some logs",
+                      ENVOY_BUG(0, "With some logs"));
+  expected_counted_failures = 3;
+#endif
+  EXPECT_EQ(expected_counted_failures, envoy_bug_fail_count);
+}
+
 } // namespace Envoy

test/server/server_test.cc

@@ -473,11 +473,24 @@ TEST_P(ServerInstanceImplTest, Stats) {
   EXPECT_EQ(2L, TestUtility::findGauge(stats_store_, "server.concurrency")->value());
   EXPECT_EQ(3L, TestUtility::findGauge(stats_store_, "server.hot_restart_epoch")->value());
 
-// This stat only works in this configuration.
-#if defined(NDEBUG) && defined(ENVOY_LOG_DEBUG_ASSERT_IN_RELEASE)
+// The ENVOY_BUG stat only works in release mode.
+#if defined(NDEBUG)
+  ENVOY_BUG(false, "Testing envoy bug assertion failure detection in release build.");
+  EXPECT_EQ(1L, TestUtility::findCounter(stats_store_, "server.envoy_bug_failures")->value());
+  // Test power of two increments.
+  for (int i = 0; i < 16; i++) {
+    ENVOY_BUG(false);
+  }
+  EXPECT_EQ(5L, TestUtility::findCounter(stats_store_, "server.envoy_bug_failures")->value());
+
+// The ASSERT stat only works in this configuration.
+#if defined(ENVOY_LOG_DEBUG_ASSERT_IN_RELEASE)
   ASSERT(false, "Testing debug assertion failure detection in release build.");
   EXPECT_EQ(1L, TestUtility::findCounter(stats_store_, "server.debug_assertion_failures")->value());
+#endif // defined(ENVOY_LOG_DEBUG_ASSERT_IN_RELEASE)
 #else
+  // The ENVOY_BUG macro aborts in debug mode.
+  EXPECT_DEATH(ENVOY_BUG(false), "");
   EXPECT_EQ(0L, TestUtility::findCounter(stats_store_, "server.debug_assertion_failures")->value());
 #endif
 }