the same usdt probe appearing in multiple binary files not working

[yonghong-song]

Here, the same usdt probe refers to probes with the same <provider>:<probe_name>.

Current bcc implementation assumes probes with a particular <probe_name> only appears in one binary. If a probe is defined in a header files, it is possible that eventually, the probe may end up in different binaries. For example,

  define.h: contains FOLLY_SDT(foo, bar, ...);

a.c includes define.h and eventually lands in liba.so and liba.so have foo:bar probe.
b.c include define.h and eventually links with liba.so to produce the binary a.out and
a.out also have foo:bar probe.

In this case, foo:bar appears in two different binary files. I hit this issue when I debug a FB internal issue.

I hacked bcc and the following code change seems working:

diff --git a/src/cc/usdt/usdt.cc b/src/cc/usdt/usdt.cc
index dec0646..98d5935 100644
--- a/src/cc/usdt/usdt.cc
+++ b/src/cc/usdt/usdt.cc
@@ -238,7 +238,8 @@ int Context::_each_module(const char *modpath, uint64_t, uint64_t, uint64_t,
 
 void Context::add_probe(const char *binpath, const struct bcc_elf_usdt *probe) {
   for (auto &p : probes_) {
-    if (p->provider_ == probe->provider && p->name_ == probe->name) {
+    if (p->bin_path_ == binpath && p->provider_ == probe->provider &&
+        p->name_ == probe->name) {
       p->add_location(probe->pc, probe->arg_fmt);
       return;
     }
@@ -277,8 +278,15 @@ bool Context::enable_probe(const std::string &probe_name,
   if (pid_stat_ && pid_stat_->is_stale())
     return false;
 
-  auto p = get(probe_name);
-  return p && p->enable(fn_name);
+  bool found = false;
+  for (auto &p : probes_) {
+    if (p->name_ == probe_name) {
+      p->enable(fn_name);
+      found = true;
+    }
+  }
+
+  return found;
 }
 
 void Context::each(each_cb callback) {
diff --git a/tools/tplist.py b/tools/tplist.py
index db4b68b..04ce05b 100755
--- a/tools/tplist.py
+++ b/tools/tplist.py
@@ -88,10 +88,11 @@ def print_usdt(pid, lib):
         probes_seen = []
         for probe in reader.enumerate_probes():
                 probe_name = probe.short_name()
+                binary_probe_name = probe.bin_path + "_" + probe_name
                 if not args.filter or fnmatch.fnmatch(probe_name, args.filter):
-                        if probe_name in probes_seen:
+                        if binary_probe_name in probes_seen:
                                 continue
-                        probes_seen.append(probe_name)
+                        probes_seen.append(binary_probe_name)
                         print_usdt_details(probe)
 
 if __name__ == "__main__":

Before I go ahead for a formal patch with test cases, any concern for this that I may not foresee?

[goldshtn]

Yes, that certainly makes sense. When testing your patch, can you check if the trace/argdist/funccount etc. support still works without specifying the binary path? It is very convenient to do something like trace u::gc__start -p $(pidof node) without having to specify the full path. Perhaps if the user did not specify a library, we could just search for an appropriate probe without a library name?

[yonghong-song]

Right. If user did not specify a library, we could just search for the probe across all file-backed modules given a pid.

[palmtenor]

Looking at the patch, those two same probes from different binary will still only generate one readarg_X helper, right? What if this same probe has different format in those two different binaries (and have conflicting virtual address in the switch-case in the readarg_X helper)?

[yonghong-song]

Looking at the patch, those two same probes from different binary will still only generate one readarg_X helper, right?
Right.

What if this same probe has different format in those two different binaries (and have conflicting virtual address in the switch-case in the readarg_X helper)?
Then we even have issues when the same probe appears more than once in the same binary. I guess we could add logic in the code to check format as well and return error, right? it is really not a good practice to have different formats for the same probe name.

The virtual address should be okay as they are all under the same pid address space. However, the virtual address in the .so note section cannot be directly used for code generation ...