Improve ProcSyms module path handling

[lenticularis39]

Another, better attempt at implementing loading symbols from exited processes, following cc44177. Unlike the previous one, this one should respect mount namespaces. I tried opening a file inside a container-specific tmpfs and it worked using the approach described below.

TODO list:

• Add tests to make sure everything works as intended

---

Instead of loading modules from /proc/<pid>/root/, open /proc/<pid>/root in advance with open(..., O_PATH) first, then use openat to read the module file. This enables the module file to be read even if <pid> is not running.

A class named ModulePath is added, which does the openat call and manages the returned file descriptor, enabling functions expecting a path to use /proc/self/fd/... as the path to the module.

Edit: prepend backslash to < and > symbols to fix display on GitHub

[chenhengqi]

The CI still failed.

[lenticularis39]

/tmp/perf-pid.map maps tests fixed with a workaround disabling the openat and /proc/self/fd/... trick for perf maps (they rely on the filename ending with .map, which does not work well with the latter).

Looking at failing Python resolve_name tests now.

[lenticularis39]

There is too much functionality which breaks after substituting /proc/.../root/<module-name> with /proc/self/fd/..., so I opted for only using the latter when the former is not accessible (e.g. when the process has already exited). This should avoid any regressions at the cost of being prone to a race condition (the process might exit in between the accessibility check and reading the module).

[davemarchevsky]

Sorry for the delayed review.

In general I think this openat strategy is a good idea. I had some concerns about whether procfs would behave as expected since it's a 'special' fs, but skimming the code makes me think it'll work.

At Meta we have a profiling daemon that splits its execution into a few conceptual phases:

• discovery: Gather information about processes running on the host for use in next phase
• data collection: Attach BPF prog(s) and collect data. Usually this includes collecting stacks and passing them back to userspace.
• post-processing: Symbolicate stacks, etc

We had a similar issue with symbolicating exited process' stacks in the past, as we were trying to grab /proc/PID/root/... files in post-processing, but the process may have died between beginning of data collection and beginning of post-processing. To solve this we do something similar to this PR - grab fd during discovery phase (similar to ProcStat::refresh_root), use it later during post-processing. This way processes which were alive during discovery but died during data collection didn't have the "exited process" issue.

I'm mentioning this all because I'm not aware of any similar 'discovery' phase in tools using ProcSyms. The cpp and python APIs are initializing ProcSyms for pids when the first stack for the pid is requested (BPFStackTable::get_stack_symbol and BPF._sym_cache, respectively). For the changes in this PR to be useful 'by default', IMO we should also modify the API to make separating 'discovery' and 'post-processing' easy for tools.

[lenticularis39]

I'm mentioning this all because I'm not aware of any similar 'discovery' phase in tools using ProcSyms. The cpp and python APIs are initializing ProcSyms for pids when the first stack for the pid is requested (BPFStackTable::get_stack_symbol and BPF._sym_cache, respectively). For the changes in this PR to be useful 'by default', IMO we should also modify the API to make separating 'discovery' and 'post-processing' easy for tools.

lenticularis39/bpftrace@4f07cc1, a part of a bpftrace PR I referenced this one from earlier, does a similar thing. In bpftrace, BPF code is not separated from userspace code, hence the "discovery phase" is triggered by symbol resolution being used in the code; perhaps in BCC it could be a function called by the user with a way to specify for which processes the symcache should be created.

[davemarchevsky]

Cool, sounds like we're on the same page. Since you have a concrete usecase already, I think this can be merged w/o waiting for implementation of 'discovery' phase in normal BCC framework