From ab78817c93732e1e8785cd5bde5906e93094dcce Mon Sep 17 00:00:00 2001
From: Mark Drayton <mark@markdrayton.info>
Date: Sat, 2 Jul 2016 00:47:39 +0100
Subject: [PATCH] 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()
---
 src/cc/bcc_perf_map.c  |  2 +-
 src/cc/bcc_syms.cc     | 22 +++++++++++++++-------
 src/cc/syms.h          |  4 ++++
 tests/cc/test_c_api.cc |  7 +++++++
 4 files changed, 27 insertions(+), 8 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..b13cb88df1bc 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;
@@ -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;
 }
 
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") {