From 50aeaed4d101f22027eaf07186418ec8dcbab019 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Fri, 19 Jul 2019 14:58:19 +0200 Subject: [PATCH] Report proper module on kernel backtrace Raghavendra Rao reported that memleak does not display proper name of the related kernel module, but just the "kernel" string, like here for xfs module functions: 131072 bytes in 4 allocations from stack .. bvec_alloc+0x92 [kernel] bio_alloc_bioset+0x13f [kernel] xfs_add_to_ioend+0x2df [kernel] xfs_do_writepage+0x148 [kernel] write_cache_pages+0x171 [kernel] xfs_vm_writepages+0x59 [kernel] do_writepages+0x43 [kernel] ... The kernel resolver code is parsing /proc/kallsyms, which already has the module information in. This patch is adding support to parse the module info from /proc/kallsyms and initialize the module with proper value. Above memleak backtrace now looks like: 131072 bytes in 4 allocations from stack bvec_alloc+0x92 [kernel] bio_alloc_bioset+0x13f [kernel] xfs_add_to_ioend+0x2df [xfs] xfs_do_writepage+0x148 [xfs] write_cache_pages+0x171 [kernel] xfs_vm_writepages+0x59 [xfs] do_writepages+0x43 [kernel] ... Reported-by: Raghavendra Rao Signed-off-by: Jiri Olsa --- src/cc/bcc_proc.c | 20 ++++++++++++++++++-- src/cc/bcc_proc.h | 2 +- src/cc/bcc_syms.cc | 8 ++++---- src/cc/syms.h | 5 +++-- tests/cc/test_c_api.cc | 2 +- 5 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/cc/bcc_proc.c b/src/cc/bcc_proc.c index c8f4041ed27b..d6e17282eed6 100644 --- a/src/cc/bcc_proc.c +++ b/src/cc/bcc_proc.c @@ -208,7 +208,7 @@ int bcc_procutils_each_module(int pid, bcc_procutils_modulecb callback, int bcc_procutils_each_ksym(bcc_procutils_ksymcb callback, void *payload) { char line[2048]; - char *symname, *endsym; + char *symname, *endsym, *modname, *endmod = NULL; FILE *kallsyms; unsigned long long addr; @@ -237,7 +237,23 @@ int bcc_procutils_each_ksym(bcc_procutils_ksymcb callback, void *payload) { while (*endsym && !isspace(*endsym)) endsym++; *endsym = '\0'; - callback(symname, addr, payload); + // Parse module name if it's available + modname = endsym + 1; + while (*modname && isspace(*endsym)) modname++; + + if (*modname && *modname == '[') { + endmod = ++modname; + while (*endmod && *endmod != ']') endmod++; + if (*endmod) + *(endmod) = '\0'; + else + endmod = NULL; + } + + if (!endmod) + modname = "kernel"; + + callback(symname, modname, addr, payload); } fclose(kallsyms); diff --git a/src/cc/bcc_proc.h b/src/cc/bcc_proc.h index a56f680d5f56..368f27d9e0c3 100644 --- a/src/cc/bcc_proc.h +++ b/src/cc/bcc_proc.h @@ -42,7 +42,7 @@ typedef struct mod_info { typedef int (*bcc_procutils_modulecb)(mod_info *, int, void *); // Symbol name, address, payload -typedef void (*bcc_procutils_ksymcb)(const char *, uint64_t, void *); +typedef void (*bcc_procutils_ksymcb)(const char *, const char *, uint64_t, void *); char *bcc_procutils_which_so(const char *libname, int pid); char *bcc_procutils_which(const char *binpath); diff --git a/src/cc/bcc_syms.cc b/src/cc/bcc_syms.cc index a946fe1a1048..95ec8bad398a 100644 --- a/src/cc/bcc_syms.cc +++ b/src/cc/bcc_syms.cc @@ -47,9 +47,9 @@ bool ProcStat::is_stale() { ProcStat::ProcStat(int pid) : procfs_(tfm::format("/proc/%d/exe", pid)), inode_(getinode_()) {} -void KSyms::_add_symbol(const char *symname, uint64_t addr, void *p) { +void KSyms::_add_symbol(const char *symname, const char *modname, uint64_t addr, void *p) { KSyms *ks = static_cast(p); - ks->syms_.emplace_back(symname, addr); + ks->syms_.emplace_back(symname, modname, addr); } void KSyms::refresh() { @@ -67,13 +67,13 @@ bool KSyms::resolve_addr(uint64_t addr, struct bcc_symbol *sym, bool demangle) { if (syms_.empty()) goto unknown_symbol; - it = std::upper_bound(syms_.begin(), syms_.end(), Symbol("", addr)); + it = std::upper_bound(syms_.begin(), syms_.end(), Symbol("", "", addr)); if (it != syms_.begin()) { it--; sym->name = (*it).name.c_str(); if (demangle) sym->demangle_name = sym->name; - sym->module = "kernel"; + sym->module = (*it).mod.c_str(); sym->offset = addr - (*it).addr; return true; } diff --git a/src/cc/syms.h b/src/cc/syms.h index 351da1dffbf4..021b28aa4c3a 100644 --- a/src/cc/syms.h +++ b/src/cc/syms.h @@ -50,8 +50,9 @@ class SymbolCache { class KSyms : SymbolCache { struct Symbol { - Symbol(const char *name, uint64_t addr) : name(name), addr(addr) {} + Symbol(const char *name, const char *mod, uint64_t addr) : name(name), mod(mod), addr(addr) {} std::string name; + std::string mod; uint64_t addr; bool operator<(const Symbol &rhs) const { return addr < rhs.addr; } @@ -59,7 +60,7 @@ class KSyms : SymbolCache { std::vector syms_; std::unordered_map symnames_; - static void _add_symbol(const char *, uint64_t, void *); + static void _add_symbol(const char *, const char *, uint64_t, void *); public: virtual bool resolve_addr(uint64_t addr, struct bcc_symbol *sym, bool demangle = true) override; diff --git a/tests/cc/test_c_api.cc b/tests/cc/test_c_api.cc index 8b96c649b65c..e3744be4e16a 100644 --- a/tests/cc/test_c_api.cc +++ b/tests/cc/test_c_api.cc @@ -67,7 +67,7 @@ TEST_CASE("binary resolution with `which`", "[c_api]") { free(ld); } -static void _test_ksym(const char *sym, uint64_t addr, void *_) { +static void _test_ksym(const char *sym, const char *mod, uint64_t addr, void *_) { if (!strcmp(sym, "startup_64")) REQUIRE(addr != 0x0ull); }