Use bcc_symbol_option in bcc_resolve_symname

This commit makes `bcc_resolve_symname` to take an `bcc_symbol_option`
parameter, and pass it to underlying calls to control symboling behavior.
When `NULL` is passed, it will fallback to default which is current
behavior that uses debug file, verify debug file CRC, and check all
types of symbols.

This commit also removes the unneccesary intermediate
`bcc_find_symbol_addr`.

Added documentation for usage of the API, updated most call sites to use
default with `NULL`, and fixed some memory leaks at call sites.

diff --git a/src/cc/BPF.cc b/src/cc/BPF.cc
index dda6fd7a1cd6bdba4a2a075d0d67b4a0dde93f31..98febc4f295463ab287aa2a000677c098c26e355 100644 (file)
--- a/src/cc/BPF.cc
+++ b/src/cc/BPF.cc
@@ -469,8 +469,9 @@ StatusTuple BPF::unload_func(const std::string& func_name) {
 StatusTuple BPF::check_binary_symbol(const std::string& binary_path,
                                      const std::string& symbol,
                                      uint64_t symbol_addr, bcc_symbol* output) {
+  // TODO: Fix output.module memory leak here
   int res = bcc_resolve_symname(binary_path.c_str(), symbol.c_str(),
-                                symbol_addr, 0, output);
+                                symbol_addr, 0, nullptr, output);
   if (res < 0)
     return StatusTuple(
         -1, "Unable to find offset for binary %s symbol %s address %lx",

diff --git a/src/cc/bcc_syms.cc b/src/cc/bcc_syms.cc
index 627913f919e4caa6132dd48fb883f53188dd4f06..c68b68ca9aa0d406b33a1e6147854477e3dbd2f0 100644 (file)
--- a/src/cc/bcc_syms.cc
+++ b/src/cc/bcc_syms.cc
@@ -438,21 +438,6 @@ int bcc_resolve_global_addr(int pid, const char *module, const uint64_t address,
   return 0;
 }
 
-static int _find_sym(const char *symname, uint64_t addr, uint64_t end,
-                     int flags, void *payload) {
-  struct bcc_symbol *sym = (struct bcc_symbol *)payload;
-  // TODO: check for actual function symbol in flags
-  if (!strcmp(sym->name, symname)) {
-    sym->offset = addr;
-    return -1;
-  }
-  return 0;
-}
-
-int bcc_find_symbol_addr(struct bcc_symbol *sym) {
-  return bcc_elf_foreach_sym(sym->module, _find_sym, sym);
-}
-
 static int _sym_cb_wrapper(const char *symname, uint64_t addr, uint64_t end,
                            int flags, void *payload) {
   SYM_CB cb = (SYM_CB) payload;
@@ -473,17 +458,32 @@ int bcc_foreach_function_symbol(const char *module, SYM_CB cb) {
       module, _sym_cb_wrapper, &default_option, (void *)cb);
 }
 
+static int _find_sym(const char *symname, uint64_t addr, uint64_t end,
+                     int flags, void *payload) {
+  struct bcc_symbol *sym = (struct bcc_symbol *)payload;
+  if (!strcmp(sym->name, symname)) {
+    sym->offset = addr;
+    return -1;
+  }
+  return 0;
+}
+
 int bcc_resolve_symname(const char *module, const char *symname,
-                        const uint64_t addr, int pid, struct bcc_symbol *sym) {
+                        const uint64_t addr, int pid,
+                        struct bcc_symbol_option *option,
+                        struct bcc_symbol *sym) {
   uint64_t load_addr;
-
-  sym->module = NULL;
-  sym->name = NULL;
-  sym->offset = 0x0;
+  static struct bcc_symbol_option default_option = {
+    .use_debug_file = 1,
+    .check_debug_file_crc = 1,
+    .use_symbol_type = BCC_SYM_ALL_TYPES,
+  };
 
   if (module == NULL)
     return -1;
 
+  memset(sym, 0, sizeof(bcc_symbol));
+
   if (strchr(module, '/')) {
     sym->module = strdup(module);
   } else {
@@ -501,8 +501,11 @@ int bcc_resolve_symname(const char *module, const char *symname,
   sym->name = symname;
   sym->offset = addr;
 
+  if (option == NULL)
+    option = &default_option;
+
   if (sym->name && sym->offset == 0x0)
-    if (bcc_find_symbol_addr(sym) < 0)
+    if (bcc_elf_foreach_sym(sym->module, _find_sym, option, sym) < 0)
       goto invalid_module;
 
   if (sym->offset == 0x0)

diff --git a/src/cc/bcc_syms.h b/src/cc/bcc_syms.h
index 886cc060f43f1ea5c3f2aa8043544c492ad9a926..8e6984ea8e0220a3f1e578db4c486ac6fb571d58 100644 (file)
--- a/src/cc/bcc_syms.h
+++ b/src/cc/bcc_syms.h
@@ -62,9 +62,22 @@ int bcc_resolve_global_addr(int pid, const char *module, const uint64_t address,
 // Will prefer use debug file and check debug file CRC when reading the module.
 int bcc_foreach_function_symbol(const char *module, SYM_CB cb);
 
-int bcc_find_symbol_addr(struct bcc_symbol *sym);
+// Find the offset of a symbol in a module binary. If addr is not zero, will
+// calculate the offset using the provided addr and the module's load address.
+//
+// If pid is provided, will use it to help lookup the module in the Process and
+// enter the Process's mount Namespace.
+//
+// If option is not NULL, will respect the specified options for lookup.
+// Otherwise default option will apply, which is to use debug file, verify
+// checksum, and try all types of symbols.
+//
+// Return 0 on success and -1 on failure. Output will be write to sym. After
+// use, sym->module need to be freed if it's not empty.
 int bcc_resolve_symname(const char *module, const char *symname,
-                        const uint64_t addr, int pid, struct bcc_symbol *sym);
+                        const uint64_t addr, int pid,
+                        struct bcc_symbol_option* option,
+                        struct bcc_symbol *sym);
 
 void *bcc_enter_mount_ns(int pid);
 void bcc_exit_mount_ns(void **guard);

diff --git a/src/cc/usdt_args.cc b/src/cc/usdt_args.cc
index e7d75c88b1a5708ba63a3199c254427bcbc89be5..d54b1e5d77fd82ee03c0824c7c3d621f5b78aa25 100644 (file)
--- a/src/cc/usdt_args.cc
+++ b/src/cc/usdt_args.cc
@@ -40,9 +40,11 @@ bool Argument::get_global_address(uint64_t *address, const std::string &binpath,
   }
 
   if (!bcc_elf_is_shared_obj(binpath.c_str())) {
-    struct bcc_symbol sym = {deref_ident_->c_str(), binpath.c_str(), 0x0};
-    if (!bcc_find_symbol_addr(&sym) && sym.offset) {
+    struct bcc_symbol sym;
+    if (bcc_resolve_symname(binpath.c_str(), deref_ident_->c_str(), 0x0, -1, nullptr, &sym) == 0) {
       *address = sym.offset;
+      if (sym.module)
+        ::free(const_cast<char*>(sym.module));
       return true;
     }
   }

diff --git a/src/lua/bcc/libbcc.lua b/src/lua/bcc/libbcc.lua
index b00277ec09154e22ffc599e2cb84526cd92f182d..e41881cbc0cf3386a897a98cb9f9e835ee46231f 100644 (file)
--- a/src/lua/bcc/libbcc.lua
+++ b/src/lua/bcc/libbcc.lua
@@ -121,7 +121,8 @@ struct bcc_symbol_option {
 };
 
 int bcc_resolve_symname(const char *module, const char *symname, const uint64_t addr,
-               int pid, struct bcc_symbol *sym);
+                        int pid, struct bcc_symbol_option *option,
+                        struct bcc_symbol *sym);
 void bcc_procutils_free(const char *ptr);
 void *bcc_symcache_new(int pid);
 void bcc_symbol_free_demangle_name(struct bcc_symbol *sym);

diff --git a/src/lua/bcc/sym.lua b/src/lua/bcc/sym.lua
index 978999ac264a4813b250ae73d24de34b992b8de2..8d88d4e2f72afac58126b353f5c4f73d89bf89e3 100644 (file)
--- a/src/lua/bcc/sym.lua
+++ b/src/lua/bcc/sym.lua
@@ -35,7 +35,7 @@ end
 local function check_path_symbol(module, symname, addr, pid)
   local sym = SYM()
   local module_path
-  if libbcc.bcc_resolve_symname(module, symname, addr or 0x0, pid or 0, sym) < 0 then
+  if libbcc.bcc_resolve_symname(module, symname, addr or 0x0, pid or 0, nil, sym) < 0 then
     if sym[0].module == nil then
       error("could not find library '%s' in the library path" % module)
     else

diff --git a/src/python/bcc/__init__.py b/src/python/bcc/__init__.py
index ee0dce53d41ad0b9dbdffebad989d3ca5838459d..745e1c2ec6c5f4fbf43abf13758c0153987abb14 100644 (file)
--- a/src/python/bcc/__init__.py
+++ b/src/python/bcc/__init__.py
@@ -24,7 +24,7 @@ import errno
 import sys
 basestring = (unicode if sys.version_info[0] < 3 else str)
 
-from .libbcc import lib, _CB_TYPE, bcc_symbol, _SYM_CB_TYPE
+from .libbcc import lib, _CB_TYPE, bcc_symbol, bcc_symbol_option, _SYM_CB_TYPE
 from .table import Table
 from .perf import Perf
 from .utils import get_online_cpus
@@ -607,11 +607,12 @@ class BPF(object):
         sym = bcc_symbol()
         psym = ct.pointer(sym)
         c_pid = 0 if pid == -1 else pid
-        if lib.bcc_resolve_symname(module.encode("ascii"),
-                symname.encode("ascii"), addr or 0x0, c_pid, psym) < 0:
-            if not sym.module:
-                raise Exception("could not find library %s" % module)
-            lib.bcc_procutils_free(sym.module)
+        if lib.bcc_resolve_symname(
+            module.encode("ascii"), symname.encode("ascii"),
+            addr or 0x0, c_pid,
+            ct.cast(None, ct.POINTER(bcc_symbol_option)),
+            psym,
+        ) < 0:
             raise Exception("could not determine address of symbol %s" % symname)
         module_path = ct.cast(sym.module, ct.c_char_p).value.decode()
         lib.bcc_procutils_free(sym.module)

diff --git a/src/python/bcc/libbcc.py b/src/python/bcc/libbcc.py
index cbecf0d2248410b381c33e0d23e2d62d93fcf841..4fb2939c7bf53290ee74ac7831afc06bd837aa83 100644 (file)
--- a/src/python/bcc/libbcc.py
+++ b/src/python/bcc/libbcc.py
@@ -149,7 +149,7 @@ lib.bcc_procutils_language.argtypes = [ct.c_int]
 
 lib.bcc_resolve_symname.restype = ct.c_int
 lib.bcc_resolve_symname.argtypes = [
-    ct.c_char_p, ct.c_char_p, ct.c_ulonglong, ct.c_int, ct.POINTER(bcc_symbol)]
+    ct.c_char_p, ct.c_char_p, ct.c_ulonglong, ct.c_int, ct.POINTER(bcc_symbol_option), ct.POINTER(bcc_symbol)]
 
 _SYM_CB_TYPE = ct.CFUNCTYPE(ct.c_int, ct.c_char_p, ct.c_ulonglong)
 lib.bcc_foreach_function_symbol.restype = ct.c_int

diff --git a/tests/cc/test_c_api.cc b/tests/cc/test_c_api.cc
index 8b73b3392cbb6ccf95b0649bd9c4c5b66c5d1454..c3a8d3c01342c89b526257059fdca616b6004d67 100644 (file)
--- a/tests/cc/test_c_api.cc
+++ b/tests/cc/test_c_api.cc
@@ -92,7 +92,7 @@ TEST_CASE("file-backed mapping identification") {
 TEST_CASE("resolve symbol name in external library", "[c_api]") {
   struct bcc_symbol sym;
 
-  REQUIRE(bcc_resolve_symname("c", "malloc", 0x0, 0, &sym) == 0);
+  REQUIRE(bcc_resolve_symname("c", "malloc", 0x0, 0, nullptr, &sym) == 0);
   REQUIRE(string(sym.module).find("libc.so") != string::npos);
   REQUIRE(sym.module[0] == '/');
   REQUIRE(sym.offset != 0);
@@ -102,7 +102,7 @@ TEST_CASE("resolve symbol name in external library", "[c_api]") {
 TEST_CASE("resolve symbol name in external library using loaded libraries", "[c_api]") {
   struct bcc_symbol sym;
 
-  REQUIRE(bcc_resolve_symname("bcc", "bcc_procutils_which", 0x0, getpid(), &sym) == 0);
+  REQUIRE(bcc_resolve_symname("bcc", "bcc_procutils_which", 0x0, getpid(), nullptr, &sym) == 0);
   REQUIRE(string(sym.module).find("libbcc.so") != string::npos);
   REQUIRE(sym.module[0] == '/');
   REQUIRE(sym.offset != 0);