From beeb31fb15ce3fba4fdec50d9b9f95323c0e534e Mon Sep 17 00:00:00 2001 From: Gordon Marler Date: Fri, 13 Sep 2024 14:21:51 -0400 Subject: [PATCH 1/5] Eliminate unwind stat error_catchall - added 3 actual stats that composed it --- src/bpf/profiler.bpf.c | 6 +++--- src/bpf/profiler.h | 12 +++++++----- src/bpf/profiler_bindings.rs | 4 +++- src/bpf/shared_maps.h | 7 ++++--- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/bpf/profiler.bpf.c b/src/bpf/profiler.bpf.c index a0f35f1..1aecaf6 100644 --- a/src/bpf/profiler.bpf.c +++ b/src/bpf/profiler.bpf.c @@ -470,7 +470,7 @@ int dwarf_unwind(struct bpf_perf_event_data *ctx) { // add it, it would be best to add it only during development. if (previous_rsp == 0) { LOG("[error] previous_rsp should not be zero."); - bump_unwind_error_catchall(); + bump_unwind_error_previous_rsp_zero(); return 1; } @@ -491,7 +491,7 @@ int dwarf_unwind(struct bpf_perf_event_data *ctx) { LOG("[error] previous_rip should not be zero. This can mean that the " "read failed, ret=%d while reading @ %llx.", err, previous_rip_addr); - bump_unwind_error_catchall(); + bump_unwind_error_previous_rip_zero(); } return 1; } @@ -510,7 +510,7 @@ int dwarf_unwind(struct bpf_perf_event_data *ctx) { LOG("[error] previous_rbp should not be zero. This can mean " "that the read has failed %d.", ret); - bump_unwind_error_catchall(); + bump_unwind_error_previous_rbp_zero(); return 1; } } diff --git a/src/bpf/profiler.h b/src/bpf/profiler.h index 69f9a59..c94c322 100644 --- a/src/bpf/profiler.h +++ b/src/bpf/profiler.h @@ -76,7 +76,9 @@ struct unwinder_stats_t { u64 error_unsupported_expression; u64 error_unsupported_frame_pointer_action; u64 error_unsupported_cfa_register; - u64 error_catchall; + u64 error_previous_rsp_zero; + u64 error_previous_rip_zero; + u64 error_previous_rbp_zero; u64 error_should_never_happen; u64 error_bp_should_be_zero_for_bottom_frame; u64 error_mapping_not_found; @@ -89,8 +91,8 @@ struct unwinder_stats_t { u64 jit_encountered; }; -const volatile struct lightswitch_config_t lightswitch_config = {.verbose_logging = - true}; +const volatile struct lightswitch_config_t lightswitch_config = { + .verbose_logging = true}; // A different stack produced the same hash. #define STACK_COLLISION(err) (err == -EEXIST) @@ -99,7 +101,7 @@ const volatile struct lightswitch_config_t lightswitch_config = {.verbose_loggin #define LOG(fmt, ...) \ ({ \ - if (lightswitch_config.verbose_logging) { \ + if (lightswitch_config.verbose_logging) { \ bpf_printk(fmt, ##__VA_ARGS__); \ } \ }) @@ -226,4 +228,4 @@ unsigned long long hash_stack(native_stack_t *stack) { } return hash; -} \ No newline at end of file +} diff --git a/src/bpf/profiler_bindings.rs b/src/bpf/profiler_bindings.rs index 52af506..66edc99 100644 --- a/src/bpf/profiler_bindings.rs +++ b/src/bpf/profiler_bindings.rs @@ -49,7 +49,9 @@ impl Add for unwinder_stats_t { + other.error_unsupported_frame_pointer_action, error_unsupported_cfa_register: self.error_unsupported_cfa_register + other.error_unsupported_cfa_register, - error_catchall: self.error_catchall + other.error_catchall, + error_previous_rsp_zero: self.error_previous_rsp_zero + other.error_previous_rsp_zero, + error_previous_rip_zero: self.error_previous_rip_zero + other.error_previous_rip_zero, + error_previous_rbp_zero: self.error_previous_rbp_zero + other.error_previous_rbp_zero, error_should_never_happen: self.error_should_never_happen + other.error_should_never_happen, error_bp_should_be_zero_for_bottom_frame: self.error_bp_should_be_zero_for_bottom_frame diff --git a/src/bpf/shared_maps.h b/src/bpf/shared_maps.h index b8c86c6..3c34c09 100644 --- a/src/bpf/shared_maps.h +++ b/src/bpf/shared_maps.h @@ -18,7 +18,6 @@ struct { __type(value, struct unwinder_stats_t); } percpu_stats SEC(".maps"); - #define DEFINE_COUNTER(__func__name) \ static void bump_unwind_##__func__name() { \ u32 zero = 0; \ @@ -35,7 +34,9 @@ DEFINE_COUNTER(error_truncated); DEFINE_COUNTER(error_unsupported_expression); DEFINE_COUNTER(error_unsupported_frame_pointer_action); DEFINE_COUNTER(error_unsupported_cfa_register); -DEFINE_COUNTER(error_catchall); +DEFINE_COUNTER(error_previous_rsp_zero); +DEFINE_COUNTER(error_previous_rip_zero); +DEFINE_COUNTER(error_previous_rbp_zero); DEFINE_COUNTER(error_should_never_happen); DEFINE_COUNTER(error_bp_should_be_zero_for_bottom_frame); DEFINE_COUNTER(error_mapping_not_found); @@ -47,4 +48,4 @@ DEFINE_COUNTER(error_cfa_offset_did_not_fit); DEFINE_COUNTER(vdso_encountered); DEFINE_COUNTER(jit_encountered); -#endif \ No newline at end of file +#endif From 167ce0ffed573fcb6f3d14fa01d8ae5cef13f222 Mon Sep 17 00:00:00 2001 From: Gordon Marler Date: Fri, 13 Sep 2024 15:18:28 -0400 Subject: [PATCH 2/5] Add tmate session for debugging --- .github/workflows/build.yml | 40 +++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 4f7e867..6d80555 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -2,8 +2,8 @@ name: ci on: pull_request: push: - branches: - - main + branches: + - main concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref || github.run_id }} @@ -13,21 +13,23 @@ jobs: ci: runs-on: ubuntu-22.04 permissions: - id-token: write - contents: read + id-token: write + contents: read steps: - - uses: actions/checkout@main - - uses: DeterminateSystems/nix-installer-action@main - - uses: DeterminateSystems/magic-nix-cache-action@main - - name: Set up nix dev env - run: nix develop --command echo 0 - - name: Run `cargo build` - run: nix develop --ignore-environment --command cargo build - - name: Run `cargo clippy` - run: nix develop --ignore-environment --command cargo clippy --all-targets -- -D warnings - - name: Run `cargo test` - run: nix develop --command cargo test - - name: Run `cargo fmt` - run: nix develop --ignore-environment --command cargo fmt --check - - name: Run `nix fmt` - run: nix fmt -- --check . \ No newline at end of file + - uses: actions/checkout@main + - uses: DeterminateSystems/nix-installer-action@main + - uses: DeterminateSystems/magic-nix-cache-action@main + - name: Set up nix dev env + run: nix develop --command echo 0 + - name: Run `cargo build` + run: nix develop --ignore-environment --command cargo build + - name: Setup tmate session + uses: mxschmitt/action-tmate@v3 + - name: Run `cargo clippy` + run: nix develop --ignore-environment --command cargo clippy --all-targets -- -D warnings + - name: Run `cargo test` + run: nix develop --command cargo test + - name: Run `cargo fmt` + run: nix develop --ignore-environment --command cargo fmt --check + - name: Run `nix fmt` + run: nix fmt -- --check . From 45c81a0fd9712efdc4efa412fe8f9407601643bb Mon Sep 17 00:00:00 2001 From: Gordon Marler Date: Mon, 16 Sep 2024 15:13:59 -0400 Subject: [PATCH 3/5] Remove --ignore-environment from Github workflow clippy step for now --- .github/workflows/build.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 6d80555..0330584 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -26,7 +26,8 @@ jobs: - name: Setup tmate session uses: mxschmitt/action-tmate@v3 - name: Run `cargo clippy` - run: nix develop --ignore-environment --command cargo clippy --all-targets -- -D warnings + # Removing --ignore-environment so this build will work for now + run: nix develop --command cargo clippy --all-targets -- -D warnings - name: Run `cargo test` run: nix develop --command cargo test - name: Run `cargo fmt` From 8db91530e1ea0159dc2d50ac16e269784ba0ca19 Mon Sep 17 00:00:00 2001 From: Gordon Marler Date: Mon, 16 Sep 2024 15:43:25 -0400 Subject: [PATCH 4/5] Remove tmate shell session step --- .github/workflows/build.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0330584..301a6f7 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -23,8 +23,9 @@ jobs: run: nix develop --command echo 0 - name: Run `cargo build` run: nix develop --ignore-environment --command cargo build - - name: Setup tmate session - uses: mxschmitt/action-tmate@v3 + # For when we need a shell session into the workflow + # - name: Setup tmate session + # uses: mxschmitt/action-tmate@v3 - name: Run `cargo clippy` # Removing --ignore-environment so this build will work for now run: nix develop --command cargo clippy --all-targets -- -D warnings From eaf7bdab3f5856fe3d55fa9fb7410168a5507c4f Mon Sep 17 00:00:00 2001 From: Gordon Marler Date: Tue, 17 Sep 2024 12:25:16 -0400 Subject: [PATCH 5/5] back to original CI workflow --- .github/workflows/build.yml | 42 +++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 301a6f7..4f7e867 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -2,8 +2,8 @@ name: ci on: pull_request: push: - branches: - - main + branches: + - main concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref || github.run_id }} @@ -13,25 +13,21 @@ jobs: ci: runs-on: ubuntu-22.04 permissions: - id-token: write - contents: read + id-token: write + contents: read steps: - - uses: actions/checkout@main - - uses: DeterminateSystems/nix-installer-action@main - - uses: DeterminateSystems/magic-nix-cache-action@main - - name: Set up nix dev env - run: nix develop --command echo 0 - - name: Run `cargo build` - run: nix develop --ignore-environment --command cargo build - # For when we need a shell session into the workflow - # - name: Setup tmate session - # uses: mxschmitt/action-tmate@v3 - - name: Run `cargo clippy` - # Removing --ignore-environment so this build will work for now - run: nix develop --command cargo clippy --all-targets -- -D warnings - - name: Run `cargo test` - run: nix develop --command cargo test - - name: Run `cargo fmt` - run: nix develop --ignore-environment --command cargo fmt --check - - name: Run `nix fmt` - run: nix fmt -- --check . + - uses: actions/checkout@main + - uses: DeterminateSystems/nix-installer-action@main + - uses: DeterminateSystems/magic-nix-cache-action@main + - name: Set up nix dev env + run: nix develop --command echo 0 + - name: Run `cargo build` + run: nix develop --ignore-environment --command cargo build + - name: Run `cargo clippy` + run: nix develop --ignore-environment --command cargo clippy --all-targets -- -D warnings + - name: Run `cargo test` + run: nix develop --command cargo test + - name: Run `cargo fmt` + run: nix develop --ignore-environment --command cargo fmt --check + - name: Run `nix fmt` + run: nix fmt -- --check . \ No newline at end of file