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

Only do per-frame-blocklist-check when frame-pointer is enabled #172

Merged
merged 2 commits into from
Nov 3, 2022

Conversation

mornyx
Copy link
Contributor

@mornyx mornyx commented Nov 3, 2022

Signed-off-by: mornyx mornyx.z@gmail.com

Related Issues

Background

We introduced frame-pointer based stacktrace in 567c5d0. But we cannot guarantee that all shared libraries have frame pointers enabled, and performing stacktracing directly based on frame pointers is dangerous and may cause crash.

So, we provide some protection. In general, there are two means of protection: addr_validate and blocklist. addr_validate ensures that we do not try to access unreadable addresses, and blocklist helps us exclude stack frames belonging to specified shared libraries.

Issue

But the commit above introduces a problem, here is a minimal reproduction:

# Issue since: pprof = { git = "https://github.com/tikv/pprof-rs.git", rev = "567c5d01c9bd8deb9ccbe5515122d5aa9faef0ad", default-features = false, features = ["flamegraph", "protobuf-codec"] }
pprof = { version = "0.10", default-features = false, features = ["flamegraph", "protobuf-codec"] }
use pprof::ProfilerGuardBuilder;
use std::fs::File;

fn main() {
    let g = ProfilerGuardBuilder::default()
        .blocklist(&["libc", "libgcc", "pthread", "vdso"])
        .build()
        .unwrap();
    workload();
    g.report()
        .build()
        .unwrap()
        .flamegraph(File::create("/tmp/r.svg").unwrap())
        .unwrap();
}

fn workload() -> Vec<i32> {
    let mut rs = vec![];
    for _ in 0..100_000_000 {
        let mut v = vec![];
        for n in 100_000_000..0 {
            v.push(n);
        }
        v.sort();
        rs.push(v.iter().sum());
    }
    rs
}

To avoid deadlocks, we will block shared libraries like libc, pthread, etc, but this causes an additional problem. Let's see the generated svg:

r2

It only contains samples inside the signal handler, all stack frames before the signal frame are lost.

Reason

ac2cace5-c2be-4c4d-8723-d28c4c21b85e

The code in the RED BOX is newly introduced by the commit above, with frame-pointer based stacktracing. Its purpose is to do extra protection in the callback of each stack frame. But when doing stacktraces with backtrace-rs, there are some magical dependencies on libc in order to handle signal frames on the stack. So they are blocked due to the existence of blocklist.

But why does it works well with frame-pointer feature? Because frame-pointer based stacktracing starts from ucontext provided by OS kernel, which is an argument of perf_signal_handler. ucontext holds the top stack frame before SIGPROF triggered, so we don't have to deal with signal frames at all.

Solve

Only do per-frame-blocklist-check when frame-pointer is enabled. We will get the correct flamegraph:

r

Signed-off-by: mornyx <mornyx.z@gmail.com>
Signed-off-by: mornyx <mornyx.z@gmail.com>
Copy link
Member

@YangKeao YangKeao left a comment

Choose a reason for hiding this comment

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

LGTM

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