parca-agent icon indicating copy to clipboard operation
parca-agent copied to clipboard

Double check process mappings correctness

Open javierhonduco opened this issue 3 years ago • 4 comments

While reading the implementation and usage of the process mapping /proc/<pid>/maps implementation that we use I've noticed two things that we might want to double-check:

  • Caching the memory mappings might yield incorrect results. If a dynamic library is loaded after we parsed and cached the mappings, we won't account for it
  • Non-executable sections are skipped [1], which can be a problem when we need the base address where a library has been loaded

[1]:

[javierhonduco@fedora parca-agent]$ cat /proc/self/maps
562ed4619000-562ed461b000 r--p 00000000 00:21 3445                       /usr/bin/cat
562ed461b000-562ed461f000 r-xp 00002000 00:21 3445                       /usr/bin/cat

(note how the base address where cat has been loaded is non executable, so it will be skipped in the code below)

https://github.com/google/pprof/blob/70bd9ae97f40a5e0ef03e61bec8e90f3a5732191/profile/legacy_profile.go#L1060-L1063

javierhonduco avatar Sep 29 '22 10:09 javierhonduco

Just bumped into this which is semi-related to this issue:

level=debug name=parca-agent ts=2022-10-25T10:27:22.915483149Z caller=maps.go:109 msg="failed to read object build ID" object=/proc/2689308/root/usr/lib/locale/locale-archive err="failed to open elf: bad magic number '[9 1 2 222]' in record at byte 0x0"

What I think it's going on is that we are not properly filtering and not finding the right metadata for dynamically loaded executables. Will add some more information once I publish the code I am working on that aims to tackle this

javierhonduco avatar Oct 25 '22 10:10 javierhonduco

Caching the memory mappings might yield incorrect results. If a dynamic library is loaded after we parsed and cached the mappings, we won't account for it

I believe this is a problem, yes.

Non-executable sections are skipped [1], which can be a problem when we need the base address where a library has been loaded

I think because we use the binary to calculate the base address, this is probably correct.

brancz avatar Nov 22 '22 14:11 brancz

Leaving this to @Sylfrena @kakkoyun 🙇

javierhonduco avatar Jan 11 '24 15:01 javierhonduco

Note: we now refresh mappings if needed, so things should be significantly improved (https://github.com/parca-dev/parca-agent/commit/6130ef335b94ca5ea5bd5c2f028a2335717a8bd8)

javierhonduco avatar Jan 11 '24 15:01 javierhonduco