Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ProcSyms should treat the executable like any other mapped file when symbolizing #3498

Merged

Conversation

davemarchevsky
Copy link
Collaborator

As reported in #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.

While writing this I noticed that the remaining place get_pid_exe is used, the USDT code, potentially also has a bug due to get_pid_exe and shouldn't be using it either if so. Will follow up

I still need to validate this. Plan on doing so with libbpf-tools' offcputime and hopefully some example that can be committed

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.
void ProcSyms::load_modules() {
load_exe();
bcc_procutils_each_module(pid_, _add_module, this);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@yonghong-song
Copy link
Collaborator

[buildbot, test this please]

@davemarchevsky
Copy link
Collaborator Author

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.

@yonghong-song
Copy link
Collaborator

yonghong-song commented Jul 12, 2021

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants