From 03c35e22123889f58ef24610d861e90a4af6c496 Mon Sep 17 00:00:00 2001 From: "Yichun Zhang (agentzh)" Date: Sat, 16 Dec 2023 20:49:46 -0800 Subject: [PATCH] Refactored some of the code for kernel.data(*).* probepoints to the stap runtime. This makes development easier by avoiding updating a lot of code in the translator all the time. This also makes the upcoming process.data(*) probepoints easier to implement. Additionally, we made it abort when the first hw breakpoint fails to register instead of going on registering the remaining hw breakpoints. Added tests to cover this change. --- runtime/linux/stap-hw-breakpoint.h | 126 ++++++++++++++++++ tapsets.cxx | 82 +----------- .../kernel-hw-breakpoint-addr-err.exp | 65 +++++++++ .../kernel-hw-breakpoint-addr-err_1.c | 5 + .../kernel-hw-breakpoint-addr-err_1.stp | 15 +++ .../kernel-hw-breakpoint-addr-err_2.stp | 15 +++ 6 files changed, 233 insertions(+), 75 deletions(-) create mode 100644 runtime/linux/stap-hw-breakpoint.h create mode 100644 testsuite/systemtap.base/kernel-hw-breakpoint-addr-err.exp create mode 100644 testsuite/systemtap.base/kernel-hw-breakpoint-addr-err_1.c create mode 100644 testsuite/systemtap.base/kernel-hw-breakpoint-addr-err_1.stp create mode 100644 testsuite/systemtap.base/kernel-hw-breakpoint-addr-err_2.stp diff --git a/runtime/linux/stap-hw-breakpoint.h b/runtime/linux/stap-hw-breakpoint.h new file mode 100644 index 000000000..91a3c77ce --- /dev/null +++ b/runtime/linux/stap-hw-breakpoint.h @@ -0,0 +1,126 @@ +/* -*- linux-c -*- + * Utility functions for handling haredware breakpoints (watchpoints). + * + * Copyright (C) 2023 by OpenResty Inc. + * This file is part of systemtap, and is free software. You can + * redistribute it and/or modify it under the terms of the GNU General + * Public License (GPL); either version 2, or (at your option) any + * later version. + */ + +#ifndef _STP_HW_BREAKPOINT_H_INCLUDED_ +#define _STP_HW_BREAKPOINT_H_INCLUDED_ + +struct stap_hwbkpt_probe { + bool registered_p; + // registered_p = false signifies a probe that is unregistered (or failed) + // registered_p = true signifies a probe that got registered successfully + + uint8_t atype; + unsigned int len; + + // Symbol Names are mostly small and uniform enough + // to justify putting const char*. + const char * const symbol; + + const unsigned long address; + const struct stap_probe * const probe; +}; + +static inline int +stap_hwbkpt_init(perf_overflow_handler_t triggered, struct stap_hwbkpt_probe *probes, int probe_count, + struct perf_event_attr *probe_array, struct perf_event **ret_array[], const char **probe_point_ptr) +{ + int rc = 0; + int i; + + if (unlikely(probe_count == 0)) /* do nothing */ + return 0; + + for (i=0; iaddress; + const char *hwbkpt_symbol_name = addr ? NULL : skp->symbol; + hw_breakpoint_init(hp); + if (addr) + hp->bp_addr = (unsigned long) addr; + else { + hp->bp_addr = kallsyms_lookup_name(hwbkpt_symbol_name); + if (!hp->bp_addr) { + _stp_error("Probe %s registration failed: invalid symbol '%s' ", + skp->probe->pp, hwbkpt_symbol_name); + rc = -EINVAL; + skp->registered_p = false; + break; + } + } + hp->bp_type = skp->atype; + + // Convert actual len to bp len. + switch(skp->len) { + case 1: + hp->bp_len = HW_BREAKPOINT_LEN_1; + break; + case 2: + hp->bp_len = HW_BREAKPOINT_LEN_2; + break; + case 3: + case 4: + hp->bp_len = HW_BREAKPOINT_LEN_4; + break; + case 5: + case 6: + case 7: + case 8: + default: // XXX: could instead reject + hp->bp_len = HW_BREAKPOINT_LEN_8; + break; + } + + *probe_point_ptr = skp->probe->pp; // for error messages + +#ifdef STAPCONF_HW_BREAKPOINT_CONTEXT + ret_array[i] = register_wide_hw_breakpoint(hp, triggered, NULL); +#else + ret_array[i] = register_wide_hw_breakpoint(hp, triggered); +#endif + if (unlikely(IS_ERR(ret_array[i]))) { + rc = PTR_ERR(ret_array[i]); + ret_array[i] = 0; + _stp_error("Hwbkpt probe %s: registration error [man warning::pass5] %d, addr %p, name %s", skp->probe->pp, rc, addr, hwbkpt_symbol_name); + skp->registered_p = false; + break; + } + else skp->registered_p = true; + } // for loop + + if (unlikely(rc)) { + if (unlikely(i >= probe_count)) { + i = probe_count - 1; + } + for (; i >= 0; i--) { + struct stap_hwbkpt_probe *skp = & probes[i]; + if (unlikely(!skp->registered_p)) continue; + unregister_wide_hw_breakpoint(ret_array[i]); + skp->registered_p = false; + } + } + + return rc; +} + +static inline void +stap_hwbkpt_exit(struct stap_hwbkpt_probe *probes, int probe_count, struct perf_event **ret_array[]) +{ + int i; + //Unregister hwbkpt probes. + for (i=0; iregistered_p)) continue; + unregister_wide_hw_breakpoint(ret_array[i]); + skp->registered_p = false; + } +} + +#endif /* _STP_HW_BREAKPOINT_H_INCLUDED_ */ diff --git a/tapsets.cxx b/tapsets.cxx index 9e87cee7c..58752c1b3 100644 --- a/tapsets.cxx +++ b/tapsets.cxx @@ -10863,6 +10863,7 @@ hwbkpt_derived_probe_group::emit_module_decls (systemtap_session& s) s.op->newline() << "#include "; s.op->newline() << "#include "; + s.op->newline() << "#include "; s.op->newline(); // Forward declare the main entry functions @@ -10884,20 +10885,7 @@ hwbkpt_derived_probe_group::emit_module_decls (systemtap_session& s) s.op->newline() << "static struct perf_event **"; s.op->newline() << "stap_hwbkpt_ret_array[" << hwbkpt_probes.size() << "];"; - s.op->newline() << "static struct stap_hwbkpt_probe {"; - s.op->newline() << "int registered_p:1;"; -// registered_p = 0 signifies a probe that is unregistered (or failed) -// registered_p = 1 signifies a probe that got registered successfully - - // Symbol Names are mostly small and uniform enough - // to justify putting const char*. - s.op->newline() << "const char * const symbol;"; - - s.op->newline() << "const unsigned long address;"; - s.op->newline() << "uint8_t atype;"; - s.op->newline() << "unsigned int len;"; - s.op->newline() << "const struct stap_probe * const probe;"; - s.op->newline() << "} stap_hwbkpt_probes[] = {"; + s.op->newline() << "static struct stap_hwbkpt_probe stap_hwbkpt_probes[] = {"; s.op->indent(1); for (unsigned int it = 0; it < hwbkpt_probes.size(); it++) @@ -10964,73 +10952,17 @@ hwbkpt_derived_probe_group::emit_module_decls (systemtap_session& s) void hwbkpt_derived_probe_group::emit_module_init (systemtap_session& s) { - s.op->newline() << "for (i=0; i<" << hwbkpt_probes.size() << "; i++) {"; - s.op->newline(1) << "struct stap_hwbkpt_probe *skp = & stap_hwbkpt_probes[i];"; - s.op->newline() << "struct perf_event_attr *hp = & stap_hwbkpt_probe_array[i];"; - s.op->newline() << "void *addr = (void *) skp->address;"; - s.op->newline() << "const char *hwbkpt_symbol_name = addr ? NULL : skp->symbol;"; - s.op->newline() << "hw_breakpoint_init(hp);"; - s.op->newline() << "if (addr)"; - s.op->newline(1) << "hp->bp_addr = (unsigned long) addr;"; - s.op->newline(-1) << "else { "; - s.op->newline(1) << "hp->bp_addr = kallsyms_lookup_name(hwbkpt_symbol_name);"; - s.op->newline() << "if (!hp->bp_addr) { "; - s.op->newline(1) << "_stp_warn(\"Probe %s registration skipped: invalid symbol %s \",skp->probe->pp,hwbkpt_symbol_name);"; - s.op->newline() << "continue;"; - s.op->newline(-1) << "}"; - s.op->newline(-1) << "}"; - s.op->newline() << "hp->bp_type = skp->atype;"; - - // Convert actual len to bp len. - s.op->newline() << "switch(skp->len) {"; - s.op->newline() << "case 1:"; - s.op->newline(1) << "hp->bp_len = HW_BREAKPOINT_LEN_1;"; - s.op->newline() << "break;"; - s.op->newline(-1) << "case 2:"; - s.op->newline(1) << "hp->bp_len = HW_BREAKPOINT_LEN_2;"; - s.op->newline() << "break;"; - s.op->newline(-1) << "case 3:"; - s.op->newline() << "case 4:"; - s.op->newline(1) << "hp->bp_len = HW_BREAKPOINT_LEN_4;"; - s.op->newline() << "break;"; - s.op->newline(-1) << "case 5:"; - s.op->newline() << "case 6:"; - s.op->newline() << "case 7:"; - s.op->newline() << "case 8:"; - s.op->newline() << "default:"; // XXX: could instead reject - s.op->newline(1) << "hp->bp_len = HW_BREAKPOINT_LEN_8;"; - s.op->newline() << "break;"; - s.op->newline(-1) << "}"; - - s.op->newline() << "probe_point = skp->probe->pp;"; // for error messages - s.op->newline() << "#ifdef STAPCONF_HW_BREAKPOINT_CONTEXT"; - s.op->newline() << "stap_hwbkpt_ret_array[i] = register_wide_hw_breakpoint(hp, &enter_hwbkpt_probe, NULL);"; - s.op->newline() << "#else"; - s.op->newline() << "stap_hwbkpt_ret_array[i] = register_wide_hw_breakpoint(hp, &enter_hwbkpt_probe);"; - s.op->newline() << "#endif"; - s.op->newline() << "rc = 0;"; - s.op->newline() << "if (IS_ERR(stap_hwbkpt_ret_array[i])) {"; - s.op->newline(1) << "rc = PTR_ERR(stap_hwbkpt_ret_array[i]);"; - s.op->newline() << "stap_hwbkpt_ret_array[i] = 0;"; - s.op->newline(-1) << "}"; - s.op->newline() << "if (rc) {"; - s.op->newline(1) << "_stp_warn(\"Hwbkpt probe %s: registration error [man warning::pass5] %d, addr %p, name %s\", probe_point, rc, addr, hwbkpt_symbol_name);"; - s.op->newline() << "skp->registered_p = 0;"; - s.op->newline(-1) << "}"; - s.op->newline() << " else skp->registered_p = 1;"; - s.op->newline(-1) << "}"; // for loop + s.op->newline() << "rc = stap_hwbkpt_init(&enter_hwbkpt_probe, stap_hwbkpt_probes, " + << hwbkpt_probes.size() << ", stap_hwbkpt_probe_array, " + << "stap_hwbkpt_ret_array, &probe_point);"; } void hwbkpt_derived_probe_group::emit_module_exit (systemtap_session& s) { //Unregister hwbkpt probes. - s.op->newline() << "for (i=0; i<" << hwbkpt_probes.size() << "; i++) {"; - s.op->newline(1) << "struct stap_hwbkpt_probe *skp = & stap_hwbkpt_probes[i];"; - s.op->newline() << "if (skp->registered_p == 0) continue;"; - s.op->newline() << "unregister_wide_hw_breakpoint(stap_hwbkpt_ret_array[i]);"; - s.op->newline() << "skp->registered_p = 0;"; - s.op->newline(-1) << "}"; + s.op->newline() << "stap_hwbkpt_exit(stap_hwbkpt_probes, " + << hwbkpt_probes.size() << ", stap_hwbkpt_ret_array);"; } diff --git a/testsuite/systemtap.base/kernel-hw-breakpoint-addr-err.exp b/testsuite/systemtap.base/kernel-hw-breakpoint-addr-err.exp new file mode 100644 index 000000000..581441157 --- /dev/null +++ b/testsuite/systemtap.base/kernel-hw-breakpoint-addr-err.exp @@ -0,0 +1,65 @@ +set test "kernel-hw-breakpoint-addr-err" +set testpath "$srcdir/$subdir" + +if {! [installtest_p]} { untested $test; return } +if {! [uretprobes_p]} { untested $test; return } + +# --- TEST 1 --- + +set subtest1 "TEST 1: kernel.data(ADDR).rw 2nd probe failed to register" + +set res [target_compile ${testpath}/${test}_1.c ./a.out executable \ + "additional_flags=-O additional_flags=-g additional_flags=-O0 additional_flags=-g additional_flags=-fno-PIC additional_flags=-no-pie"] +if {$res ne ""} { + verbose "target_compile failed: $res" 2 + fail "$test: $subtest1: unable to compile ${test}_1.c" +} else { + if {! [process_template_file "$srcdir/$subdir/${test}_1.stp" \ + "./${test}_1.stp" "./a.out" nm_err]} { + fail "$test: $subtest1: $nm_err" + } else { + foreach runtime [get_runtime_list] { + if {$runtime eq ""} { + set runtime "kernel" + } + set test_name "$test: $subtest1 ($runtime)" + set cmd "stap --runtime=$runtime -c ./a.out './${test}_1.stp'" + set exit_code [run_cmd_2way_as_root $cmd out stderr] + set exp_out "" + is "${test_name}: stdout" $out $exp_out + isnt "${test_name}: exit code" $exit_code 0 + set stderr_pat "^ERROR: Probe kernel\\.data\\(\"blah_no_such_symbol\"\\)\\.length\\(1\\)\\.rw registration failed: invalid symbol 'blah_no_such_symbol'" + like "${test_name}: stderr" $stderr $stderr_pat "-lineanchor" + } + } +} + +# --- TEST 2 --- + +set subtest2 "TEST 2: kernel.data(ADDR).rw 1st probe failed to register" + +set res [target_compile ${testpath}/${test}_1.c ./a.out executable \ + "additional_flags=-O additional_flags=-g additional_flags=-O0 additional_flags=-g additional_flags=-fno-PIC additional_flags=-no-pie"] +if {$res ne ""} { + verbose "target_compile failed: $res" 2 + fail "$test: $subtest2: unable to compile ${test}_1.c" +} else { + if {! [process_template_file "$srcdir/$subdir/${test}_2.stp" \ + "./${test}_2.stp" "./a.out" nm_err]} { + fail "$test: $subtest2: $nm_err" + } else { + foreach runtime [get_runtime_list] { + if {$runtime eq ""} { + set runtime "kernel" + } + set test_name "$test: $subtest2 ($runtime)" + set cmd "stap --runtime=$runtime -c ./a.out './${test}_2.stp'" + set exit_code [run_cmd_2way_as_root $cmd out stderr] + set exp_out "" + is "${test_name}: stdout" $out $exp_out + isnt "${test_name}: exit code" $exit_code 0 + set stderr_pat "^ERROR: Probe kernel\\.data\\(\"blah_no_such_symbol\"\\)\\.length\\(1\\)\\.write registration failed: invalid symbol 'blah_no_such_symbol'" + like "${test_name}: stderr" $stderr $stderr_pat "-lineanchor" + } + } +} diff --git a/testsuite/systemtap.base/kernel-hw-breakpoint-addr-err_1.c b/testsuite/systemtap.base/kernel-hw-breakpoint-addr-err_1.c new file mode 100644 index 000000000..f9b1ac4a6 --- /dev/null +++ b/testsuite/systemtap.base/kernel-hw-breakpoint-addr-err_1.c @@ -0,0 +1,5 @@ +int a = 0; +int main(void) { + a++; + return 0; +} diff --git a/testsuite/systemtap.base/kernel-hw-breakpoint-addr-err_1.stp b/testsuite/systemtap.base/kernel-hw-breakpoint-addr-err_1.stp new file mode 100644 index 000000000..e0dc2d512 --- /dev/null +++ b/testsuite/systemtap.base/kernel-hw-breakpoint-addr-err_1.stp @@ -0,0 +1,15 @@ +probe kernel.data(0x$^ADDR_a).rw { + if (pid() == target()) { + print_ubacktrace_fileline(); + } +} + +probe kernel.data("blah_no_such_symbol").rw { + if (pid() == target()) { + println("Hit"); + } +} + +probe timer.s(1) { + exit(); +} diff --git a/testsuite/systemtap.base/kernel-hw-breakpoint-addr-err_2.stp b/testsuite/systemtap.base/kernel-hw-breakpoint-addr-err_2.stp new file mode 100644 index 000000000..f7ac2097a --- /dev/null +++ b/testsuite/systemtap.base/kernel-hw-breakpoint-addr-err_2.stp @@ -0,0 +1,15 @@ +probe kernel.data("blah_no_such_symbol").write { + if (pid() == target()) { + println("Hit"); + } +} + +probe kernel.data(0x$^ADDR_a).rw { + if (pid() == target()) { + print_ubacktrace_fileline(); + } +} + +probe timer.s(1) { + exit(); +} -- 2.43.5