ProcSyms should treat the executable like any other mapped file when symbolizing #3498
Conversation
symbolizing As reported in iovisor#3487, when `/proc/PID/exe`'s symlink points to a mountns-relative path from a different mountns than the tracing process, we can fail to open it as we don't prepend `/proc/PID/root` . A few potential solutions were discussed in that issue, we settled on treating the main exe like any other map in `/proc/PID/maps`. Since it's always the first map we can reuse existing code and get rid of exe-specific helpers.
davemarchevsky
requested review from
4ast,
drzaeus77 and
yonghong-song
as code owners
| void ProcSyms::load_modules() { | ||
| load_exe(); | ||
| bcc_procutils_each_module(pid_, _add_module, this); |
There was a problem hiding this comment.
No code needs to be added in this PR because the _add_module callback passed to the /proc/PID/maps iterator compares module 'name's of /proc/PID/maps lines - the file path - with already-added Module objects. Since load_exe was adding to modules_ a Module with name = result of get_pid_exe, if the /proc/PID/maps entry is the same path, it'll just get ignored.
|
[buildbot, test this please] |
|
This is good to go, changes in functionality are covered by this existing test, which I had trouble getting good signal from on the github actions testrun, so recreated it locally to be sure. |
Does github actions work for you? What do you mean having trouble getting good signals? Anything we can improve here? |
As reported in #3487, when
/proc/PID/exe's symlink points to amountns-relative path from a different mountns than the tracing process,
we can fail to open it as we don't prepend
/proc/PID/root.A few potential solutions were discussed in that issue, we settled on
treating the main exe like any other map in
/proc/PID/maps. Since it'salways the first map we can reuse existing code and get rid of
exe-specific helpers.
While writing this I noticed that the remaining place
get_pid_exeis used, the USDT code, potentially also has a bug due toget_pid_exeand shouldn't be using it either if so. Will follow upI still need to validate this. Plan on doing so with
libbpf-tools' offcputime and hopefully some example that can be committed