Commit ab78817c authored by Mark Drayton's avatar Mark Drayton Committed by 4ast

ProcSyms: fix off-by-ones, use binary search to resolve addresses (#594)

* libbcc: fix off-by-one errors in resolving adjacent modules/symbols, add test

* libbcc: use binary search in ProcSyms::Module::find_addr()
parent e51985ef
...@@ -93,7 +93,7 @@ int bcc_perf_map_foreach_sym(const char *path, bcc_perf_map_symcb callback, ...@@ -93,7 +93,7 @@ int bcc_perf_map_foreach_sym(const char *path, bcc_perf_map_symcb callback,
if (newline) if (newline)
newline[0] = '\0'; newline[0] = '\0';
callback(cursor, begin, len - 1, 0, payload); callback(cursor, begin, len, 0, payload);
} }
free(line); free(line);
......
...@@ -111,7 +111,7 @@ bool ProcSyms::resolve_addr(uint64_t addr, struct bcc_symbol *sym) { ...@@ -111,7 +111,7 @@ bool ProcSyms::resolve_addr(uint64_t addr, struct bcc_symbol *sym) {
sym->offset = 0x0; sym->offset = 0x0;
for (Module &mod : modules_) { 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 mod.find_addr(addr, sym);
} }
return false; return false;
...@@ -152,6 +152,8 @@ void ProcSyms::Module::load_sym_table() { ...@@ -152,6 +152,8 @@ void ProcSyms::Module::load_sym_table() {
bcc_perf_map_foreach_sym(name_.c_str(), _add_symbol, this); bcc_perf_map_foreach_sym(name_.c_str(), _add_symbol, this);
else else
bcc_elf_foreach_sym(name_.c_str(), _add_symbol, this); 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) { 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) { ...@@ -174,13 +176,19 @@ bool ProcSyms::Module::find_addr(uint64_t addr, struct bcc_symbol *sym) {
sym->module = name_.c_str(); sym->module = name_.c_str();
sym->offset = offset; sym->offset = offset;
for (Symbol &s : syms_) { auto it = std::upper_bound(syms_.begin(), syms_.end(), Symbol("", offset, 0));
if (offset >= s.start && offset <= (s.start + s.size)) { if (it != syms_.begin())
sym->name = s.name.c_str(); --it;
sym->offset = (offset - s.start); else
return true; 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; return false;
} }
......
...@@ -69,6 +69,10 @@ class ProcSyms : SymbolCache { ...@@ -69,6 +69,10 @@ class ProcSyms : SymbolCache {
uint64_t start; uint64_t start;
uint64_t size; uint64_t size;
int flags; int flags;
bool operator<(const struct Symbol& rhs) const {
return start < rhs.start;
}
}; };
struct Module { struct Module {
......
...@@ -129,6 +129,7 @@ static int child_func(void *arg) { ...@@ -129,6 +129,7 @@ static int child_func(void *arg) {
return -1; return -1;
} }
fprintf(file, "%llx 10 dummy_fn\n", map_addr); fprintf(file, "%llx 10 dummy_fn\n", map_addr);
fprintf(file, "%llx 10 right_next_door_fn\n", map_addr + 0x10);
fclose(file); fclose(file);
sleep(5); sleep(5);
...@@ -172,6 +173,12 @@ TEST_CASE("resolve symbols using /tmp/perf-pid.map", "[c_api]") { ...@@ -172,6 +173,12 @@ TEST_CASE("resolve symbols using /tmp/perf-pid.map", "[c_api]") {
REQUIRE(sym.module); REQUIRE(sym.module);
REQUIRE(string(sym.module) == perf_map_path(child)); REQUIRE(string(sym.module) == perf_map_path(child));
REQUIRE(string("dummy_fn") == sym.name); 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") { SECTION("separate namespace") {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment