Improve user symbol resolution #2386

lenticularis39 · 2022-10-11T08:40:36Z

This PR contains three related improvements of usym resolution (edit: added purpose of the changes):

Caching BCC symcache per process if ASLR is enabled (edit: optional after changes, by default disabled)
- Enables faster symbol resolution and symcache preloading (see below)
Resolving symbols directly from ELF (only if ASLR is disabled)
- Enables resolving symbols that are not dynamically loaded even after process exit
Preloading BCC symcache for running instances of program
- Enables resolving symbols after program exists even with ASLR enabled, given that the process exists at probe attachment time.

The latter two provide a partial fix of #2284.

Checklist

Language changes are updated in man/adoc/bpftrace.adoc and if needed in docs/reference_guide.md
User-visible and non-trivial changes updated in CHANGELOG.md
The new behaviour is covered by tests

viktormalik · 2022-10-14T06:34:31Z

CI failed, can you debug it?

lenticularis39 · 2022-10-14T07:54:26Z

I looked into that, one issue is codegen tests, because there is now one more attribute of the usym event (that one is easily fixable), other issue is ustack runtime tests failing with segfault (that one will be a little bit more difficult, since it doesn't happen on my system).

Also there is another problem with the PR, that is that the necessary BCC fix was reverted in iovisor/bcc#4270 (it turned that /proc/.../exe is not a simple symlink, but it also reflects mount namespaces, meaning the patch broke symbol resolution for processes running in a different namespace). That has to be resolved in order for the last of this PR to work.

viktormalik

Overall, these are very nice changes!

What I'm missing here, though, are better explanations of why the given approaches are necessary - this should appear in the PR description, in commit messages, and in code comments, too. Perhaps even a single-place summary of all the considered scenarios and corresponding solutions could be useful.

src/bpftrace.cpp

man/adoc/bpftrace.adoc

lenticularis39 · 2022-11-07T10:04:42Z

I opened iovisor/bcc#4319 to implement necessary functionality for the symcache preloading to work correctly.

lenticularis39 · 2023-01-31T09:51:32Z

Rebased and changed caching by PID, which is required for per-process symbol cache preloading, to optional (see the last commit for details).

tests/runtime/builtin

viktormalik

Looks good but it's quite a complex change so I'd like to have another review before merging.

ajor · 2023-04-06T19:22:50Z

I love this PR, thanks so much for doing it - it solves so many problems I've had with user symbols over the years!

I'd suggest that we make "PER_PID" the default caching behaviour when ASLR is enabled, and leave it up to users to disable the cache if they feel it's using too much memory.

Having ASLR enabled is not too uncommon, and often users will only be tracing a single process. I'd think the large amount of forking described in Collapse bcc symbol resolvers by process executable #816 is a less common scenario
It's simpler for users to understand disabling a cache than it is to work out which type of caching to enable

ajor

I also updated the ELF symbol resolution logic to work with addresses with non-zero offsets

Did you have an example of where this was causing symbolication problems? It'd be useful to check that in as a test-case.

I have a slight concern about the impact this pre-caching will have on startup time when tracing huge programs. I'll do a little bit of testing on this to satisfy myself.

src/ast/passes/codegen_llvm.cpp

src/utils.cpp

src/types.cpp

src/bpftrace.h

tests/runtime/builtin

lenticularis39 · 2023-05-17T07:52:14Z

I also updated the ELF symbol resolution logic to work with addresses with non-zero offsets

Did you have an example of where this was causing symbolication problems? It'd be useful to check that in as a test-case.

The issue is in general ustack addresses that are not resolvable using bcc_symcache_resolve, but are resolvable using symbol_table_cache_. More specifically, it is ustack addresses in exited processes with ASLR disabled - I didn't write any tests for this because of the requirement of disabling ASLR (also for usym resolution), now when I'm thinking about it I could use personality(ADDR_NO_RANDOMIZE) to disable ASLR in a test program and test both.

I have a slight concern about the impact this pre-caching will have on startup time when tracing huge programs. I'll do a little bit of testing on this to satisfy myself.

OK. The bcc symcache pre-loading is not a issue, since it only loads the symbols when bcc_symcache_resolve is called (since e.g. dynamically loaded modules might not be present at the time of calling bcc_symcache_new), what could impact the startup time is symbol_table_cache_, it might be useful to have an option to disable it, like we do for the symcache.

tests/testprogs/uprobe_symres_exited_process.c

tests/testprogs/disable_aslr.c

lenticularis39 · 2023-05-22T13:55:48Z

Oops, I forgot to change the main offset in the test to a more generic pattern, causing the test to fail with a different compiler.

ajor

Looks great, thank you!

Extend caching usym tables by program (used when ASLR is disabled) with caching by PID, which is used when ASLR is enabled. Without ASLR all instances of one program share the same memory layout, hence it can be stored once per-program.

In addition to resolving usyms from the running process, an alternative is implemented that reads the program binary via libelf. This is useful for cases when the usym resolution happens after the process exists. This approach does not work for dynamically loaded symbols or processes with ASLR enabled, in these cases usym resolution falls back to using BCC symcache. Implementation notes: - To access the program binary, a "probe id" (distinct from the AST probe id) is generated in resource analyzer, and passed via usym/ustack events into the userspace when the associated perf event fires. If the probe contains only one attach point, and therefore one program, this allows the binary to be identified even for exited processes. - ustack perf event was changed from pid/stack id packed integer to struct to fit the probe id.

While attaching a uprobe using symbol resultions, look for running instances of the program targeted by the uprobe and create a BCC symcache for each of them. This enables bpftrace to resolve usyms for processes running at the time of attaching the probe even when ASLR is on and the process is gone at the time when the print event fires.

Change BPFTRACE_CACHE_USER_SYMBOLS values from 0 and 1 to PER_PID, PER_PROGRAM, and NONE. The values 0 and 1 are still supported, meaning NONE and default (see below), for compatibility. The new default is PER_PROGRAM when ASLR is disabled and PER_PID if ASLR is enabled. NONE option can be used to save memory in case the number of traced processes is high.

Note: test for ASLR enabled is disabled because of race condition, see iovisor/bcc#4319.

Better symbol resolution

lenticularis39 · 2023-06-12T06:42:20Z

Rebased and fixed failing test added by #2625.

Pulls the following versions of BPF packages: - bpftrace: 0.19.1 - libbpf: 1.2.2 - bcc: 0.28-gb8b943a1 (commit b8b943a1) This version of bpftrace speeds up symbol resolution [1] and fixes ustack symbolication on 32-bit systems [2]. [1] bpftrace/bpftrace#2386 [2] iovisor/bcc#4775

lenticularis39 force-pushed the even-better-symbol-resolution branch from 8133e9d to 6f969a6 Compare October 14, 2022 08:54

viktormalik reviewed Oct 18, 2022

View reviewed changes

src/bpftrace.cpp Outdated Show resolved Hide resolved

man/adoc/bpftrace.adoc Outdated Show resolved Hide resolved

lenticularis39 force-pushed the even-better-symbol-resolution branch from 6f969a6 to bc4d07e Compare October 19, 2022 14:27

lenticularis39 marked this pull request as draft November 7, 2022 10:04

lenticularis39 force-pushed the even-better-symbol-resolution branch 2 times, most recently from 58756c1 to 618cd0e Compare December 20, 2022 09:56

lenticularis39 force-pushed the even-better-symbol-resolution branch 2 times, most recently from e5f628d to 5af39fa Compare January 31, 2023 09:26

lenticularis39 marked this pull request as ready for review January 31, 2023 09:49

lenticularis39 requested review from ajor, danobi, fbs and BurntBrunch as code owners January 31, 2023 09:49

lenticularis39 requested review from viktormalik and removed request for BurntBrunch, fbs, danobi and ajor January 31, 2023 09:56

viktormalik reviewed Feb 6, 2023

View reviewed changes

tests/runtime/builtin Outdated Show resolved Hide resolved

lenticularis39 force-pushed the even-better-symbol-resolution branch from 5af39fa to ae62e0b Compare February 7, 2023 07:48

viktormalik approved these changes Feb 7, 2023

View reviewed changes

lenticularis39 mentioned this pull request Feb 15, 2023

ustack is not readable #2503

Closed

ajor added the waiting-for-feedback label Apr 6, 2023

ajor self-requested a review April 6, 2023 13:08

ajor self-assigned this Apr 6, 2023

ajor reviewed May 16, 2023

View reviewed changes

src/ast/passes/codegen_llvm.cpp Outdated Show resolved Hide resolved

src/utils.cpp Outdated Show resolved Hide resolved

src/types.cpp Show resolved Hide resolved

src/bpftrace.h Outdated Show resolved Hide resolved

tests/runtime/builtin Outdated Show resolved Hide resolved

lenticularis39 force-pushed the even-better-symbol-resolution branch 3 times, most recently from bcff44b to 90dddb6 Compare May 22, 2023 13:30

github-advanced-security bot found potential problems May 22, 2023

View reviewed changes

tests/testprogs/uprobe_symres_exited_process.c Dismissed Show dismissed Hide dismissed

tests/testprogs/disable_aslr.c Dismissed Show resolved Hide resolved

lenticularis39 force-pushed the even-better-symbol-resolution branch from 90dddb6 to ae9abd0 Compare May 22, 2023 14:22

lenticularis39 requested a review from ajor May 25, 2023 07:24

danobi added the waiting-for-feedback label Jun 3, 2023

ajor approved these changes Jun 9, 2023

View reviewed changes

ajor removed the waiting-for-feedback label Jun 9, 2023

Add usym caching by PID

e201229

Extend caching usym tables by program (used when ASLR is disabled) with caching by PID, which is used when ASLR is enabled. Without ASLR all instances of one program share the same memory layout, hence it can be stored once per-program.

lenticularis39 force-pushed the even-better-symbol-resolution branch from ae9abd0 to b37abdd Compare June 12, 2023 05:45

lenticularis39 added 6 commits June 12, 2023 08:16

Mention usym() limitations in manual

6e5b341

Add tests for usym cache preload

99ed7af

Note: test for ASLR enabled is disabled because of race condition, see iovisor/bcc#4319.

Update CHANGELOG.md

1588195

Better symbol resolution

lenticularis39 force-pushed the even-better-symbol-resolution branch from b37abdd to 1588195 Compare June 12, 2023 06:16

ajor merged commit 87ee523 into bpftrace:master Jun 13, 2023
21 checks passed

lenticularis39 mentioned this pull request Jun 19, 2023

Fix func builtin in kretprobes and uretprobes #2645

Merged

3 tasks

tnovak mentioned this pull request Nov 3, 2023

Update bpftools to latest versions facebookexperimental/ExtendedAndroidTools#75

Merged

This was referenced Feb 6, 2024

Builtin function uaddr() return bad address #2983

Open

uaddr(), usym(), ustack to support PIE ASLR #75

Closed

Rtoax mentioned this pull request Apr 15, 2024

Improve user symbol resolution Rtoax/ulpatch#11

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve user symbol resolution #2386

Improve user symbol resolution #2386

lenticularis39 commented Oct 11, 2022 •

edited

Loading

viktormalik commented Oct 14, 2022

lenticularis39 commented Oct 14, 2022 •

edited

Loading

viktormalik left a comment

lenticularis39 commented Nov 7, 2022 •

edited

Loading

lenticularis39 commented Jan 31, 2023

viktormalik left a comment

ajor commented Apr 6, 2023

ajor left a comment

lenticularis39 commented May 17, 2023

lenticularis39 commented May 22, 2023

ajor left a comment

lenticularis39 commented Jun 12, 2023

Improve user symbol resolution #2386

Improve user symbol resolution #2386

Conversation

lenticularis39 commented Oct 11, 2022 • edited Loading

Checklist

viktormalik commented Oct 14, 2022

lenticularis39 commented Oct 14, 2022 • edited Loading

viktormalik left a comment

Choose a reason for hiding this comment

lenticularis39 commented Nov 7, 2022 • edited Loading

lenticularis39 commented Jan 31, 2023

viktormalik left a comment

Choose a reason for hiding this comment

ajor commented Apr 6, 2023

ajor left a comment

Choose a reason for hiding this comment

lenticularis39 commented May 17, 2023

lenticularis39 commented May 22, 2023

ajor left a comment

Choose a reason for hiding this comment

lenticularis39 commented Jun 12, 2023

lenticularis39 commented Oct 11, 2022 •

edited

Loading

lenticularis39 commented Oct 14, 2022 •

edited

Loading

lenticularis39 commented Nov 7, 2022 •

edited

Loading