From 5878cbd768e0988a88adc84db81119873c17c36b Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Sat, 19 Jun 2021 12:00:54 -0400 Subject: [PATCH] common/BackTrace: refactor into Clib and Py implementations Signed-off-by: Sage Weil --- src/common/BackTrace.cc | 22 +++++++++++-- src/common/BackTrace.h | 52 +++++++++++++++++------------- src/common/assert.cc | 8 ++--- src/common/cmdparse.cc | 2 +- src/common/lockdep.cc | 8 ++--- src/global/signal_handler.cc | 2 +- src/os/filestore/FileStore.cc | 2 +- src/osd/PG.cc | 2 +- src/test/common/test_back_trace.cc | 2 +- 9 files changed, 62 insertions(+), 38 deletions(-) diff --git a/src/common/BackTrace.cc b/src/common/BackTrace.cc index 42a8da2aebd..03105d64ad9 100644 --- a/src/common/BackTrace.cc +++ b/src/common/BackTrace.cc @@ -11,7 +11,7 @@ namespace ceph { -void BackTrace::print(std::ostream& out) const +void ClibBackTrace::print(std::ostream& out) const { out << " " << pretty_version_to_str() << std::endl; for (size_t i = skip; i < size; i++) { @@ -19,7 +19,7 @@ void BackTrace::print(std::ostream& out) const } } -void BackTrace::dump(Formatter *f) const +void ClibBackTrace::dump(Formatter *f) const { f->open_array_section("backtrace"); for (size_t i = skip; i < size; i++) { @@ -29,7 +29,7 @@ void BackTrace::dump(Formatter *f) const f->close_section(); } -std::string BackTrace::demangle(const char* name) +std::string ClibBackTrace::demangle(const char* name) { // find the parentheses and address offset surrounding the mangled name #ifdef __FreeBSD__ @@ -71,4 +71,20 @@ std::string BackTrace::demangle(const char* name) } } +void PyBackTrace::dump(Formatter *f) const +{ + f->open_array_section("backtrace"); + for (auto& i : strings) { + f->dump_string("frame", i); + } + f->close_section(); +} + +void PyBackTrace::print(std::ostream& out) const +{ + for (auto& i : strings) { + out << i << std::endl; + } +} + } diff --git a/src/common/BackTrace.h b/src/common/BackTrace.h index ce3912f52cd..d89b34d3e8e 100644 --- a/src/common/BackTrace.h +++ b/src/common/BackTrace.h @@ -19,6 +19,18 @@ namespace ceph { class Formatter; struct BackTrace { + virtual ~BackTrace() {} + virtual void print(std::ostream& out) const = 0; + virtual void dump(Formatter *f) const = 0; +}; + +inline std::ostream& operator<<(std::ostream& out, const BackTrace& bt) { + bt.print(out); + return out; +} + + +struct ClibBackTrace : public BackTrace { const static int max = 32; int skip; @@ -26,20 +38,9 @@ struct BackTrace { size_t size; char **strings; - std::list src_strings; - - explicit BackTrace(std::list& s) - : skip(0), - size(s.size()) { - src_strings = s; - strings = (char **)malloc(sizeof(*strings) * src_strings.size()); - unsigned i = 0; - for (auto& s : src_strings) { - strings[i++] = (char *)s.c_str(); - } - } - explicit BackTrace(int s) : skip(s) { + explicit ClibBackTrace(int s) { #ifdef HAVE_EXECINFO_H + skip = s; size = backtrace(array, max); strings = backtrace_symbols(array, size); #else @@ -48,22 +49,29 @@ struct BackTrace { strings = nullptr; #endif } - ~BackTrace() { + ~ClibBackTrace() { free(strings); } - BackTrace(const BackTrace& other); - const BackTrace& operator=(const BackTrace& other); + ClibBackTrace(const ClibBackTrace& other); + const ClibBackTrace& operator=(const ClibBackTrace& other); + + void print(std::ostream& out) const override; + void dump(Formatter *f) const override; - void print(std::ostream& out) const; - void dump(Formatter *f) const; static std::string demangle(const char* name); }; -inline std::ostream& operator<<(std::ostream& out, const BackTrace& bt) { - bt.print(out); - return out; -} + +struct PyBackTrace : public BackTrace { + std::list strings; + + explicit PyBackTrace(std::list& s) : strings(s) {} + + void dump(Formatter *f) const override; + void print(std::ostream& out) const override; +}; + } diff --git a/src/common/assert.cc b/src/common/assert.cc index a663c16e907..7fb4c2d726b 100644 --- a/src/common/assert.cc +++ b/src/common/assert.cc @@ -59,7 +59,7 @@ namespace ceph { // TODO: get rid of this memory allocation. ostringstream oss; - oss << BackTrace(1); + oss << ClibBackTrace(1); dout_emergency(oss.str()); if (g_assert_context) { @@ -126,7 +126,7 @@ namespace ceph { sizeof(g_assert_thread_name)); BufAppender ba(g_assert_msg, sizeof(g_assert_msg)); - BackTrace *bt = new BackTrace(1); + BackTrace *bt = new ClibBackTrace(1); ba.printf("%s: In function '%s' thread %llx time %s\n" "%s: %d: FAILED ceph_assert(%s)\n", file, func, (unsigned long long)pthread_self(), tss.str().c_str(), @@ -171,7 +171,7 @@ namespace ceph { ceph_pthread_getname(pthread_self(), g_assert_thread_name, sizeof(g_assert_thread_name)); - BackTrace *bt = new BackTrace(1); + BackTrace *bt = new ClibBackTrace(1); snprintf(g_assert_msg, sizeof(g_assert_msg), "%s: In function '%s' thread %llx time %s\n" "%s: %d: ceph_abort_msg(\"%s\")\n", file, func, @@ -214,7 +214,7 @@ namespace ceph { sizeof(g_assert_thread_name)); BufAppender ba(g_assert_msg, sizeof(g_assert_msg)); - BackTrace *bt = new BackTrace(1); + BackTrace *bt = new ClibBackTrace(1); ba.printf("%s: In function '%s' thread %llx time %s\n" "%s: %d: abort()\n", file, func, (unsigned long long)pthread_self(), tss.str().c_str(), diff --git a/src/common/cmdparse.cc b/src/common/cmdparse.cc index 1d72320ef51..7e66869097d 100644 --- a/src/common/cmdparse.cc +++ b/src/common/cmdparse.cc @@ -438,7 +438,7 @@ handle_bad_get(CephContext *cct, const string& k, const char *tname) lderr(cct) << errstr.str() << dendl; ostringstream oss; - oss << BackTrace(1); + oss << ClibBackTrace(1); lderr(cct) << oss.str() << dendl; if (status == 0) diff --git a/src/common/lockdep.cc b/src/common/lockdep.cc index 18d0cde09c8..aa7d9e0f0e4 100644 --- a/src/common/lockdep.cc +++ b/src/common/lockdep.cc @@ -297,7 +297,7 @@ int lockdep_will_lock(const char *name, int id, bool force_backtrace, if (!recursive) { lockdep_dout(0) << "\n"; *_dout << "recursive lock of " << name << " (" << id << ")\n"; - auto bt = new ceph::BackTrace(BACKTRACE_SKIP); + auto bt = new ceph::ClibBackTrace(BACKTRACE_SKIP); bt->print(*_dout); if (p->second) { *_dout << "\npreviously locked at\n"; @@ -312,7 +312,7 @@ int lockdep_will_lock(const char *name, int id, bool force_backtrace, // did we just create a cycle? if (does_follow(id, p->first)) { - auto bt = new ceph::BackTrace(BACKTRACE_SKIP); + auto bt = new ceph::ClibBackTrace(BACKTRACE_SKIP); lockdep_dout(0) << "new dependency " << lock_names[p->first] << " (" << p->first << ") -> " << name << " (" << id << ")" << " creates a cycle at\n"; @@ -338,7 +338,7 @@ int lockdep_will_lock(const char *name, int id, bool force_backtrace, } else { ceph::BackTrace* bt = NULL; if (force_backtrace || lockdep_force_backtrace()) { - bt = new ceph::BackTrace(BACKTRACE_SKIP); + bt = new ceph::ClibBackTrace(BACKTRACE_SKIP); } follows[p->first].set(id); follows_bt[p->first][id] = bt; @@ -363,7 +363,7 @@ int lockdep_locked(const char *name, int id, bool force_backtrace) lockdep_dout(20) << "_locked " << name << dendl; if (force_backtrace || lockdep_force_backtrace()) - held[p][id] = new ceph::BackTrace(BACKTRACE_SKIP); + held[p][id] = new ceph::ClibBackTrace(BACKTRACE_SKIP); else held[p][id] = 0; out: diff --git a/src/global/signal_handler.cc b/src/global/signal_handler.cc index f4e41a5cf97..055763eee46 100644 --- a/src/global/signal_handler.cc +++ b/src/global/signal_handler.cc @@ -322,7 +322,7 @@ static void handle_oneshot_fatal_signal(int signum) // TODO: don't use an ostringstream here. It could call malloc(), which we // don't want inside a signal handler. // Also fix the backtrace code not to allocate memory. - BackTrace bt(1); + ClibBackTrace bt(1); ostringstream oss; bt.print(oss); dout_emergency(oss.str()); diff --git a/src/os/filestore/FileStore.cc b/src/os/filestore/FileStore.cc index 399fa47e62c..0c143d57213 100644 --- a/src/os/filestore/FileStore.cc +++ b/src/os/filestore/FileStore.cc @@ -4134,7 +4134,7 @@ public: } void finish(int r) override { - BackTrace *bt = new BackTrace(1); + BackTrace *bt = new ClibBackTrace(1); generic_dout(-1) << "FileStore: sync_entry timed out after " << m_commit_timeo << " seconds.\n"; bt->print(*_dout); diff --git a/src/osd/PG.cc b/src/osd/PG.cc index 4fa4e634d23..aaa7c9324ea 100644 --- a/src/osd/PG.cc +++ b/src/osd/PG.cc @@ -136,7 +136,7 @@ uint64_t PG::get_with_id() ref++; std::lock_guard l(_ref_id_lock); uint64_t id = ++_ref_id; - BackTrace bt(0); + ClibBackTrace bt(0); stringstream ss; bt.print(ss); lgeneric_subdout(cct, refs, 5) << "PG::get " << this << " " << info.pgid diff --git a/src/test/common/test_back_trace.cc b/src/test/common/test_back_trace.cc index 43f9bc559aa..97db3268671 100644 --- a/src/test/common/test_back_trace.cc +++ b/src/test/common/test_back_trace.cc @@ -16,7 +16,7 @@ std::string foo() { std::ostringstream oss; - oss << ceph::BackTrace(1); + oss << ceph::ClibBackTrace(1); return oss.str(); } -- 2.39.5