From 511b6ff97346ae01c5d7d09791325f94d9b84ea7 Mon Sep 17 00:00:00 2001 From: Mark Drayton Date: Fri, 1 Jul 2016 14:35:49 +0100 Subject: [PATCH 1/2] libbcc: fix off-by-one errors in resolving adjacent modules/symbols, add test --- src/cc/bcc_perf_map.c | 2 +- src/cc/bcc_syms.cc | 4 ++-- src/cc/syms.h | 4 ++++ tests/cc/test_c_api.cc | 7 +++++++ 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/cc/bcc_perf_map.c b/src/cc/bcc_perf_map.c index b6173a01a71c..58f53ccf8636 100644 --- a/src/cc/bcc_perf_map.c +++ b/src/cc/bcc_perf_map.c @@ -93,7 +93,7 @@ int bcc_perf_map_foreach_sym(const char *path, bcc_perf_map_symcb callback, if (newline) newline[0] = '\0'; - callback(cursor, begin, len - 1, 0, payload); + callback(cursor, begin, len, 0, payload); } free(line); diff --git a/src/cc/bcc_syms.cc b/src/cc/bcc_syms.cc index 8df8c78f2902..3399d324fd71 100644 --- a/src/cc/bcc_syms.cc +++ b/src/cc/bcc_syms.cc @@ -111,7 +111,7 @@ bool ProcSyms::resolve_addr(uint64_t addr, struct bcc_symbol *sym) { sym->offset = 0x0; for (Module &mod : modules_) { - if (addr >= mod.start_ && addr <= mod.end_) + if (addr >= mod.start_ && addr < mod.end_) return mod.find_addr(addr, sym); } return false; @@ -175,7 +175,7 @@ bool ProcSyms::Module::find_addr(uint64_t addr, struct bcc_symbol *sym) { sym->offset = offset; for (Symbol &s : syms_) { - if (offset >= s.start && offset <= (s.start + s.size)) { + if (offset >= s.start && offset < (s.start + s.size)) { sym->name = s.name.c_str(); sym->offset = (offset - s.start); return true; diff --git a/src/cc/syms.h b/src/cc/syms.h index 022226042127..eb34f2c910e8 100644 --- a/src/cc/syms.h +++ b/src/cc/syms.h @@ -69,6 +69,10 @@ class ProcSyms : SymbolCache { uint64_t start; uint64_t size; int flags; + + bool operator<(const struct Symbol& rhs) const { + return start < rhs.start; + } }; struct Module { diff --git a/tests/cc/test_c_api.cc b/tests/cc/test_c_api.cc index 52b2d95e0a6d..dae7aa988978 100644 --- a/tests/cc/test_c_api.cc +++ b/tests/cc/test_c_api.cc @@ -129,6 +129,7 @@ static int child_func(void *arg) { return -1; } fprintf(file, "%llx 10 dummy_fn\n", map_addr); + fprintf(file, "%llx 10 right_next_door_fn\n", map_addr + 0x10); fclose(file); sleep(5); @@ -172,6 +173,12 @@ TEST_CASE("resolve symbols using /tmp/perf-pid.map", "[c_api]") { REQUIRE(sym.module); REQUIRE(string(sym.module) == perf_map_path(child)); REQUIRE(string("dummy_fn") == sym.name); + + REQUIRE(bcc_symcache_resolve(resolver, (unsigned long long)map_addr + 0x10, + &sym) == 0); + REQUIRE(sym.module); + REQUIRE(string(sym.module) == perf_map_path(child)); + REQUIRE(string("right_next_door_fn") == sym.name); } SECTION("separate namespace") { From 2d19ed42073cb64a5d25e2e5cd128eb91b9bf8b8 Mon Sep 17 00:00:00 2001 From: Mark Drayton Date: Fri, 1 Jul 2016 14:49:46 +0100 Subject: [PATCH 2/2] libbcc: use binary search in ProcSyms::Module::find_addr() --- src/cc/bcc_syms.cc | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/cc/bcc_syms.cc b/src/cc/bcc_syms.cc index 3399d324fd71..b13cb88df1bc 100644 --- a/src/cc/bcc_syms.cc +++ b/src/cc/bcc_syms.cc @@ -152,6 +152,8 @@ void ProcSyms::Module::load_sym_table() { bcc_perf_map_foreach_sym(name_.c_str(), _add_symbol, this); else bcc_elf_foreach_sym(name_.c_str(), _add_symbol, this); + + std::sort(syms_.begin(), syms_.end()); } bool ProcSyms::Module::find_name(const char *symname, uint64_t *addr) { @@ -174,13 +176,19 @@ bool ProcSyms::Module::find_addr(uint64_t addr, struct bcc_symbol *sym) { sym->module = name_.c_str(); sym->offset = offset; - for (Symbol &s : syms_) { - if (offset >= s.start && offset < (s.start + s.size)) { - sym->name = s.name.c_str(); - sym->offset = (offset - s.start); - return true; - } + auto it = std::upper_bound(syms_.begin(), syms_.end(), Symbol("", offset, 0)); + if (it != syms_.begin()) + --it; + else + it = syms_.end(); + + if (it != syms_.end() + && offset >= it->start && offset < it->start + it->size) { + sym->name = it->name.c_str(); + sym->offset = (offset - it->start); + return true; } + return false; }