Merge pull request ceph#62294 from rzarzynski/wip-bypassable-ceph-assert

rzarzynski · web-flow · commit 04f76e072046 · 2025-04-02T20:27:35.000+02:00
common, *: make the abort() in ceph_assert() bypassable

Reviewed-by: Radoslaw Zarzynski &lt;rzarzyns@redhat.com&gt;
Reviewed-by: Adam Kupczyk &lt;akupczyk@redhat.com&gt;
Reviewed-by: Laura Flores &lt;lflores@redhat.com&gt;
Reviewed-by: Matan Breizman &lt;mbreizma@redhat.com&gt;
diff --git a/src/common/assert.cc b/src/common/assert.cc
@@ -17,6 +17,7 @@
 #include <sstream>
 
 #include "include/compat.h"
+#include "include/str_list.h"
 #include "common/BackTrace.h"
 #include "common/Clock.h" // for ceph_clock_now()
 #include "common/debug.h"
@@ -42,6 +43,12 @@ namespace ceph {
   {
     ceph_assert(!g_assert_context);
     g_assert_context = cct;
+    const auto supressions = get_str_list(
+      g_assert_context->_conf.get_val<std::string>("ceph_assert_supresssions"));
+    if (!supressions.empty()) {
+      lderr(g_assert_context) << "WARNING: supressions for ceph_assert present: "
+			      << supressions << dendl;
+    }
   }
 
   [[gnu::cold]] void __ceph_assert_fail(const char *assertion,
@@ -70,6 +77,7 @@ namespace ceph {
     oss << ClibBackTrace(1);
     dout_emergency(oss.str());
 
+    bool should_abort = true;
     if (g_assert_context) {
       lderr(g_assert_context) << g_assert_msg << std::endl;
       *_dout << oss.str() << dendl;
@@ -78,9 +86,22 @@ namespace ceph {
       if (!g_assert_context->_conf->fatal_signal_handlers) {
 	g_assert_context->_log->dump_recent();
       }
-    }
 
-    abort();
+      // bypass the abort?
+      const auto supressions = get_str_list(
+	g_assert_context->_conf.get_val<std::string>("ceph_assert_supresssions"));
+      should_abort = std::none_of(
+	std::begin(supressions), std::end(supressions),
+	[file, line](const auto& supression) {
+	  return supression == fmt::format("{}:{}", file, line);
+        });
+    }
+    if (should_abort) {
+      abort();
+    } else {
+      dout_emergency("WARNING: ceph_assert() does NOT abort() due "
+		     "to ceph_assert_supresssions");
+    }
   }
 
   [[gnu::cold]] void __ceph_assert_fail(const assert_data &ctx)
@@ -151,6 +172,7 @@ namespace ceph {
     oss << *bt;
     dout_emergency(oss.str());
 
+    bool should_abort = true;
     if (g_assert_context) {
       lderr(g_assert_context) << g_assert_msg << std::endl;
       *_dout << oss.str() << dendl;
@@ -159,9 +181,22 @@ namespace ceph {
       if (!g_assert_context->_conf->fatal_signal_handlers) {
 	g_assert_context->_log->dump_recent();
       }
-    }
 
-    abort();
+      // bypass the abort?
+      const auto supressions = get_str_list(
+	g_assert_context->_conf.get_val<std::string>("ceph_assert_supresssions"));
+      should_abort = std::none_of(
+	std::begin(supressions), std::end(supressions),
+	[file, line](const auto& supression) {
+	  return supression == fmt::format("{}:{}", file, line);
+        });
+    }
+    if (should_abort) {
+      abort();
+    } else {
+      dout_emergency("WARNING: ceph_assertf() does NOT abort() due to"
+		     "to ceph_assert_supresssions");
+    }
   }
 
   [[gnu::cold]] void __ceph_abort(const char *file, int line,
diff --git a/src/common/options/global.yaml.in b/src/common/options/global.yaml.in
@@ -6718,3 +6718,10 @@ options:
   desc: Enables exception throwing instead of process abort on transaction submission error.
   default: false
   with_legacy: false
+- name: ceph_assert_supresssions
+  type: str
+  desc: Suppress specific ceph_assert instances to let the execution continue with unknown, potentially DESTRUCTIVE consequences.
+  fmt_desc: Comma-separated list of locations of ceph_assert in the source code.
+    The format is ``{file}:{line} [, {file}:{line}]``
+  level: dev
+  with_legacy: false
diff --git a/src/common/perf_histogram.cc b/src/common/perf_histogram.cc
@@ -33,7 +33,7 @@ void PerfHistogramCommon::dump_formatted_axis(
       f->dump_string("scale_type", "log2");
       break;
     default:
-      ceph_assert(false && "Invalid scale type");
+      ceph_abort_msg("Invalid scale type");
   }
 
   {
@@ -63,7 +63,7 @@ int64_t get_quants(int64_t i, PerfHistogramCommon::scale_type_d st) {
     case PerfHistogramCommon::SCALE_LOG2:
       return int64_t(1) << (i - 1);
   }
-  ceph_assert(false && "Invalid scale type");
+  ceph_abort_msg("Invalid scale type");
 }
 
 int64_t PerfHistogramCommon::get_bucket_for_axis(
@@ -87,7 +87,7 @@ int64_t PerfHistogramCommon::get_bucket_for_axis(
       }
       return ac.m_buckets - 1;
   }
-  ceph_assert(false && "Invalid scale type");
+  ceph_abort_msg("Invalid scale type");
 }
 
 std::vector<std::pair<int64_t, int64_t>>
diff --git a/src/crimson/os/alienstore/alien_store.cc b/src/crimson/os/alienstore/alien_store.cc
@@ -638,7 +638,7 @@ auto AlienStore::omap_get_header(CollectionRef ch,
         return crimson::ct_error::enoent::make();
       } else if (r < 0) {
         logger().error("omap_get_header: {}", r);
-        ceph_assert(0 == "impossible");
+        ceph_abort_msg("impossible");
       } else {
         return get_attr_errorator::make_ready_future<ceph::bufferlist>(
 	  std::move(bl));
diff --git a/src/crimson/osd/scheduler/mclock_scheduler.cc b/src/crimson/osd/scheduler/mclock_scheduler.cc
@@ -123,12 +123,12 @@ item_t mClockScheduler::dequeue()
   } else {
     mclock_queue_t::PullReq result = scheduler.pull_request();
     if (result.is_future()) {
-      ceph_assert(
-	0 == "Not implemented, user would have to be able to be woken up");
+      ceph_abort_msg(
+	"Not implemented, user would have to be able to be woken up");
       return std::move(*(item_t*)nullptr);
     } else if (result.is_none()) {
-      ceph_assert(
-	0 == "Impossible, must have checked empty() first");
+      ceph_abort_msg(
+	"Impossible, must have checked empty() first");
       return std::move(*(item_t*)nullptr);
     } else {
       ceph_assert(result.is_retn());
diff --git a/src/include/ceph_assert.h b/src/include/ceph_assert.h
@@ -50,13 +50,14 @@ struct assert_data {
   const char *function;
 };
 
-extern void __ceph_assert_fail(const char *assertion, const char *file, int line, const char *function)
-  __attribute__ ((__noreturn__));
-extern void __ceph_assert_fail(const assert_data &ctx)
-  __attribute__ ((__noreturn__));
+extern void __ceph_assert_fail(const char *assertion, const char *file, int line, const char *function);
+extern void __ceph_assert_fail(const assert_data &ctx);
+template <const assert_data* AssertCtxV>
+[[gnu::noinline, gnu::cold]] static void __ceph_assert_fail()  {
+  __ceph_assert_fail(*AssertCtxV);
+}
 
-extern void __ceph_assertf_fail(const char *assertion, const char *file, int line, const char *function, const char* msg, ...)
-  __attribute__ ((__noreturn__));
+extern void __ceph_assertf_fail(const char *assertion, const char *file, int line, const char *function, const char* msg, ...);
 extern void __ceph_assert_warn(const char *assertion, const char *file, int line, const char *function);
 
 [[noreturn]] void __ceph_abort(const char *file, int line, const char *func,
@@ -101,11 +102,14 @@ using namespace ceph;
   } while (false)
 #else
 #define ceph_assert(expr)							\
-  do { static const ceph::assert_data assert_data_ctx = \
-   {__STRING(expr), __FILE__, __LINE__, __CEPH_ASSERT_FUNCTION}; \
-   ((expr) \
-   ? _CEPH_ASSERT_VOID_CAST (0) \
-    : ::ceph::__ceph_assert_fail(assert_data_ctx)); } while(false)
+  do { \
+    static const auto func_name = __CEPH_ASSERT_FUNCTION; \
+    [] (const bool eval) { \
+    static const ceph::assert_data assert_data_ctx = \
+    {__STRING(expr), __FILE__, __LINE__, func_name}; \
+    ((eval) \
+    ? _CEPH_ASSERT_VOID_CAST (0) \
+    : ::ceph::__ceph_assert_fail<&assert_data_ctx>()); }((bool)(expr)); } while(false)
 #endif
 
 // this variant will *never* get compiled out to NDEBUG in the future.
@@ -119,11 +123,14 @@ using namespace ceph;
   } while(false)
 #else
 #define ceph_assert_always(expr)							\
-  do { static const ceph::assert_data assert_data_ctx = \
-   {__STRING(expr), __FILE__, __LINE__, __CEPH_ASSERT_FUNCTION}; \
-   ((expr) \
-   ? _CEPH_ASSERT_VOID_CAST (0) \
-    : ::ceph::__ceph_assert_fail(assert_data_ctx)); } while(false)
+  do { \
+    static const auto func_name = __CEPH_ASSERT_FUNCTION; \
+    [] (const bool eval) { \
+    static const ceph::assert_data assert_data_ctx = \
+    {__STRING(expr), __FILE__, __LINE__, func_name}; \
+    ((eval) \
+    ? _CEPH_ASSERT_VOID_CAST (0) \
+    : ::ceph::__ceph_assert_fail<&assert_data_ctx>()); }((bool)(expr)); } while(false)
 #endif
 
 // Named by analogy with printf.  Along with an expression, takes a format
diff --git a/src/librbd/crypto/CryptoContextPool.h b/src/librbd/crypto/CryptoContextPool.h
@@ -57,7 +57,7 @@ class CryptoContextPool : public DataCryptor<T>  {
         case CIPHER_MODE_DEC:
           return m_decrypt_contexts;
         default:
-          ceph_assert(false);
+          ceph_abort();
       }
     }
 };
diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc
@@ -4193,10 +4193,10 @@ struct AnonConnection : public Connection {
   entity_addr_t socket_addr;
 
   int send_message(Message *m) override {
-    ceph_assert(!"send_message on anonymous connection");
+    ceph_abort_msg("send_message on anonymous connection");
   }
   void send_keepalive() override {
-    ceph_assert(!"send_keepalive on anonymous connection");
+    ceph_abort_msg("send_keepalive on anonymous connection");
   }
   void mark_down() override {
     // silently ignore
diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
@@ -10175,7 +10175,7 @@ class ShallowFSCKThreadPool : public ThreadPool
     }
     /// Check whether there is anything to do.
     bool _empty() override {
-      ceph_assert(false);
+      ceph_abort();
     }
 
     /// Get the next work item to process.
@@ -10242,7 +10242,7 @@ class ShallowFSCKThreadPool : public ThreadPool
      * so at most one copy will execute simultaneously for a given thread pool.
      * It can be used for non-thread-safe finalization. */
     void _void_process_finish(void*) override {
-      ceph_assert(false);
+      ceph_abort();
     }
 
     bool queue(
diff --git a/src/os/bluestore/bluestore_types.h b/src/os/bluestore/bluestore_types.h
@@ -1052,7 +1052,7 @@ struct bluestore_blob_t {
       }
       loc -= e.length;
     }
-    ceph_assert(false);
+    ceph_abort();
   };
 
   /// updates blob's pextents container and return unused pextents eligible
diff --git a/src/osd/scheduler/OpScheduler.cc b/src/osd/scheduler/OpScheduler.cc
@@ -43,7 +43,7 @@ OpSchedulerRef make_scheduler(
       mClockScheduler>(cct, whoami, num_shards, shard_id, is_rotational,
         op_queue_cut_off, monc);
   } else {
-    ceph_assert("Invalid choice of wq" == 0);
+    ceph_abort_msg("Invalid choice of wq");
   }
 }
 
diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt
@@ -1023,3 +1023,11 @@ add_executable(test_nvmeof_mon_encoding
 target_link_libraries(test_nvmeof_mon_encoding
   mon ceph-common global-static
   )
+
+if(NOT WIN32)
+# unittest_ceph_assert
+add_executable(unittest_ceph_assert
+  ceph_assert.cc)
+add_ceph_unittest(unittest_ceph_assert)
+target_link_libraries(unittest_ceph_assert ceph-common global)
+endif()
diff --git a/src/test/ceph_assert.cc b/src/test/ceph_assert.cc
@@ -0,0 +1,51 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+
+#include <fmt/format.h>
+
+#include "common/ceph_argparse.h"
+#include "common/ceph_context.h"
+#include "global/global_context.h"
+#include "global/global_init.h"
+
+#include "include/ceph_assert.h"
+
+#include "gtest/gtest.h"
+
+static void non_supressed_assert_line16() {
+  ceph_assert(42 == 41); // LINE16
+}
+static void supressed_assert_line19() {
+  ceph_assert(42 == 40); // LINE19
+}
+static void supressed_assertf_line22() {
+  ceph_assertf(42 == 39, "FAILED ceph_assertf"); // LINE22
+}
+static void non_supressed_assertf_line25() {
+  ceph_assertf(42 == 38, "FAILED ceph_assertf"); // LINE25
+}
+
+TEST(CephAssertDeathTest, ceph_assert_supresssions) {
+  ASSERT_DEATH(non_supressed_assert_line16(), "FAILED ceph_assert");
+  supressed_assert_line19();
+  supressed_assertf_line22();
+  ASSERT_DEATH(non_supressed_assertf_line25(), "FAILED ceph_assertf");
+}
+
+int main(int argc, char **argv) {
+
+  auto args = argv_to_vec(argc, argv);
+  auto cct = global_init(
+    nullptr,
+    args,
+    CEPH_ENTITY_TYPE_CLIENT,
+    CODE_ENVIRONMENT_UTILITY,
+    CINIT_FLAG_NO_DEFAULT_CONFIG_FILE);
+  g_ceph_context->_conf.set_val(
+    "ceph_assert_supresssions",
+    fmt::format(
+      "{}:{}, {}:{}", __FILE__, /* LINE19 */19, __FILE__, /* LINE22 */22));
+  common_init_finish(g_ceph_context);
+  testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}
diff --git a/src/test/crimson/seastore/test_extent_allocator.cc b/src/test/crimson/seastore/test_extent_allocator.cc
@@ -38,7 +38,7 @@ struct allocator_test_t :
       allocator.reset(new AvlAllocator(false));
       return seastar::now();
     } 
-    ceph_assert(0 == "no support");
+    ceph_abort("no support");
   }
   seastar::future<> tear_down_fut() final {
     if (allocator) {
diff --git a/src/tools/immutable_object_cache/ObjectCacheStore.cc b/src/tools/immutable_object_cache/ObjectCacheStore.cc
@@ -276,7 +276,7 @@ int ObjectCacheStore::lookup_object(std::string pool_nspace, uint64_t pool_id,
       return ret;
     default:
       lderr(m_cct) << "unrecognized object cache status" << dendl;
-      ceph_assert(0);
+      ceph_abort();
   }
 }
 

Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ void PerfHistogramCommon::dump_formatted_axis(`
`33`	`33`	`f->dump_string("scale_type", "log2");`
`34`	`34`	`break;`
`35`	`35`	`default:`
`36`		`- ceph_assert(false && "Invalid scale type");`
	`36`	`+ ceph_abort_msg("Invalid scale type");`
`37`	`37`	`}`
`38`	`38`
`39`	`39`	`{`
`@@ -63,7 +63,7 @@ int64_t get_quants(int64_t i, PerfHistogramCommon::scale_type_d st) {`
`63`	`63`	`case PerfHistogramCommon::SCALE_LOG2:`
`64`	`64`	`return int64_t(1) << (i - 1);`
`65`	`65`	`}`
`66`		`- ceph_assert(false && "Invalid scale type");`
	`66`	`+ ceph_abort_msg("Invalid scale type");`
`67`	`67`	`}`
`68`	`68`
`69`	`69`	`int64_t PerfHistogramCommon::get_bucket_for_axis(`
`@@ -87,7 +87,7 @@ int64_t PerfHistogramCommon::get_bucket_for_axis(`
`87`	`87`	`}`
`88`	`88`	`return ac.m_buckets - 1;`
`89`	`89`	`}`
`90`		`- ceph_assert(false && "Invalid scale type");`
	`90`	`+ ceph_abort_msg("Invalid scale type");`
`91`	`91`	`}`
`92`	`92`
`93`	`93`	`std::vector<std::pair<int64_t, int64_t>>`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ class CryptoContextPool : public DataCryptor<T> {`
`57`	`57`	`case CIPHER_MODE_DEC:`
`58`	`58`	`return m_decrypt_contexts;`
`59`	`59`	`default:`
`60`		`- ceph_assert(false);`
	`60`	`+ ceph_abort();`
`61`	`61`	`}`
`62`	`62`	`}`
`63`	`63`	`};`
Original file line number	Diff line number	Diff line change
`@@ -4193,10 +4193,10 @@ struct AnonConnection : public Connection {`
`4193`	`4193`	`entity_addr_t socket_addr;`
`4194`	`4194`
`4195`	`4195`	`int send_message(Message *m) override {`
`4196`		`- ceph_assert(!"send_message on anonymous connection");`
	`4196`	`+ ceph_abort_msg("send_message on anonymous connection");`
`4197`	`4197`	`}`
`4198`	`4198`	`void send_keepalive() override {`
`4199`		`- ceph_assert(!"send_keepalive on anonymous connection");`
	`4199`	`+ ceph_abort_msg("send_keepalive on anonymous connection");`
`4200`	`4200`	`}`
`4201`	`4201`	`void mark_down() override {`
`4202`	`4202`	`// silently ignore`
Original file line number	Diff line number	Diff line change
`@@ -10175,7 +10175,7 @@ class ShallowFSCKThreadPool : public ThreadPool`
`10175`	`10175`	`}`
`10176`	`10176`	`/// Check whether there is anything to do.`
`10177`	`10177`	`bool _empty() override {`
`10178`		`- ceph_assert(false);`
	`10178`	`+ ceph_abort();`
`10179`	`10179`	`}`
`10180`	`10180`
`10181`	`10181`	`/// Get the next work item to process.`
`@@ -10242,7 +10242,7 @@ class ShallowFSCKThreadPool : public ThreadPool`
`10242`	`10242`	`* so at most one copy will execute simultaneously for a given thread pool.`
`10243`	`10243`	`* It can be used for non-thread-safe finalization. */`
`10244`	`10244`	`void _void_process_finish(void*) override {`
`10245`		`- ceph_assert(false);`
	`10245`	`+ ceph_abort();`
`10246`	`10246`	`}`
`10247`	`10247`
`10248`	`10248`	`bool queue(`
Original file line number	Diff line number	Diff line change
`@@ -1052,7 +1052,7 @@ struct bluestore_blob_t {`
`1052`	`1052`	`}`
`1053`	`1053`	`loc -= e.length;`
`1054`	`1054`	`}`
`1055`		`- ceph_assert(false);`
	`1055`	`+ ceph_abort();`
`1056`	`1056`	`};`
`1057`	`1057`
`1058`	`1058`	`/// updates blob's pextents container and return unused pextents eligible`