iovisor / bcc

BCC - Tools for BPF-based Linux IO analysis, networking, monitoring, and more
Apache License 2.0
20.59k stars 3.88k forks source link

libbpf-tools/trace_helpers: Fix incorrect DSO information in stacktrace #4862

Closed ekyooo closed 6 months ago

ekyooo commented 10 months ago

offcputime may display inaccurate DSO information in the stacktrace. Here's an example of the issue:

It shows the same DSO offset for different addresses, which is incorrect.

  $ ./offcputime -v
    ..
    #14 0x00007f8b912a8c (/usr/lib/libabc.so_0x22afa8c)
    #15 0x000044000a3ee0 (/usr/lib/libabc.so_0x22afa8c)
    #16 0x000044001fc56c (/usr/lib/libabc.so_0x22afa8c)

This is why symsmap_addr_dso simply returns NULL when symsfind_dso also returns NULL. In that case, the values of dso_name and dso_offset are not changed. If the dso_name and dso_offset variables have a garbage value before calling symsmap_addr_dso, those garbage values are maintained after calling symsmap_addr_dso.

To ensure consistent usage of DSO info and symbol info, the prototype of syms__map_addr_dso has been modified to be similar to dladdr[1].

This is the prototype of dladdr:

  int dladdr(void *addr, Dl_info *info);

The information is returned in a Dl_info structure. If no symbol matching addr could be found, then dli_sname and dli_saddr are set to NULL.

  typedef struct {
      const char *dli_fname;  /* Pathname of shared object that
                                contains address */
      void       *dli_fbase;  /* Base address at which shared
                                object is loaded */
      const char *dli_sname;  /* Name of symbol whose definition
                                overlaps addr */
      void       *dli_saddr;  /* Exact address of symbol named
                                in dli_sname */
   } Dl_info;

Similarly, if no symbol matching the addr could be found, then sym_name and sym_offset are set to NULL in syms__map_addr_dso of this patch.

Also, apply the modified API usage to offcputime, futexctn, and memleak.

[1] https://man7.org/linux/man-pages/man3/dladdr.3.html

chenhengqi commented 10 months ago

@ekyooo Please resolve conflict and rebase to master.

@ethercflow Could you please help reviewing this PR ? Thanks.

ekyooo commented 10 months ago

It's rebased to master. Thank you.

@ethercflow Could you please help reviewing this PR? If you don't like this change, it's enough to let me know. Since there is an issue with offcputime and futexctn, I will solve it with a minor modification by initializing the dso_name and dso_offset variables on the caller side. Thank you.

ethercflow commented 7 months ago

Hi @ekyooo,thank you so much for the fix. My apologies for the delayed response as I have not been checking this email for a long time. cc @chenhengqi

chenhengqi commented 7 months ago

@ekyooo Please rebase to latest master.

ekyooo commented 7 months ago

@chenhengqi I rebased it to latest master branch. Thank you.

Are you in OSS NA? If so, I'd like to meet you for a moment.