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 <ragrao@...>
Signed-off-by: Jiri Olsa <jolsa@...>
---
src/cc/bcc_proc.c | 20 ++++++++++++++++++--
src/cc/bcc_proc.h | 2 +-
src/cc/bcc_syms.cc | 8 ++++----
src/cc/libbpf | 2 +-
src/cc/syms.h | 5 +++--
tests/cc/test_c_api.cc | 2 +-
6 files changed, 28 insertions(+), 11 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 @@ done:

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<KSyms *>(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/libbpf b/src/cc/libbpf
index 45ad8626010b..33b017498543 160000
--- a/src/cc/libbpf
+++ b/src/cc/libbpf
@@ -1 +1 @@
-Subproject commit 45ad8626010b384306bc277c46a4107f21414b22
+Subproject commit 33b017498543167b65fa948d3a0267794c78787f
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 @@ public:

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<Symbol> syms_;
std::unordered_map<std::string, uint64_t> 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);
}
--
2.21.0


------------
Links: You receive all messages sent to this group.

View/Reply Online (#1742): https://lists.iovisor.org/g/iovisor-dev/message/1742
Mute This Topic: https://lists.iovisor.org/mt/32556654/1132638
Group Owner: iovisor-dev+owner@...
Unsubscribe: https://lists.iovisor.org/g/iovisor-dev/unsub [ys114321@...]
------------