From 6415dddecb81f59996e422e87e1d3da266d743e8 Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Fri, 31 Jul 2009 18:46:47 +0200 Subject: PR10458. User actual breakpoint address for [ku]probe[ret]. Setup the pt_regs REG_IP to the actual breakpoint address before entering a probe handler for [ku]probe[ret] (and restore it after returning). This helps getting symbol resolution and backtraces more correct and makes it more conform with other probe handlers like the iutrace and profile timers that also provide pt_regs (which untill now exhibited off-by-one errors while unwinding). * tapsets.cxx (dwarf_derived_probe_group::emit_module_decls): Setup REG_IP correctly before calling enter_kprobe_probe and enter_kretprobe_probe, and restore afterwards. (uprobe_derived_probe_group::emit_module_decls): Likewise for enter_uprobe_probe and enter_uretprobe_probe. (kprobe_derived_probe_group::emit_module_decls): Likewise for enter_kprobe2_probe and enter_kretprobe2_probe. * runtime/unwind/i386.h (arch_unw_init_frame_info): Initialize info->call_frame to zero. * runtime/unwind/x86_64.h (arch_unw_init_frame_info): Likewise. --- runtime/unwind/i386.h | 2 +- runtime/unwind/x86_64.h | 2 +- tapsets.cxx | 66 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/runtime/unwind/i386.h b/runtime/unwind/i386.h index 9f488f07..b19df584 100644 --- a/runtime/unwind/i386.h +++ b/runtime/unwind/i386.h @@ -92,7 +92,7 @@ static inline void arch_unw_init_frame_info(struct unwind_frame_info *info, #endif } - info->call_frame = 1; + info->call_frame = 0; } static inline void arch_unw_init_blocked(struct unwind_frame_info *info) diff --git a/runtime/unwind/x86_64.h b/runtime/unwind/x86_64.h index 3c70f206..8860b8ee 100644 --- a/runtime/unwind/x86_64.h +++ b/runtime/unwind/x86_64.h @@ -107,7 +107,7 @@ static inline void arch_unw_init_frame_info(struct unwind_frame_info *info, /*const*/ struct pt_regs *regs) { info->regs = *regs; - info->call_frame = 1; + info->call_frame = 0; } static inline void arch_unw_init_blocked(struct unwind_frame_info *info) diff --git a/tapsets.cxx b/tapsets.cxx index 2d68ddd4..bd33fb0b 100644 --- a/tapsets.cxx +++ b/tapsets.cxx @@ -2986,7 +2986,18 @@ dwarf_derived_probe_group::emit_module_decls (systemtap_session& s) s.op->line() << "];"; common_probe_entryfn_prologue (s.op, "STAP_SESSION_RUNNING", "sdp->pp"); s.op->newline() << "c->regs = regs;"; + + // Make it look like the IP is set as it wouldn't have been replaced + // by a breakpoint instruction when calling real probe handler. Reset + // IP regs on return, so we don't confuse kprobes. PR10458 + s.op->newline() << "{"; + s.op->indent(1); + s.op->newline() << "unsigned long kprobes_ip = REG_IP(c->regs);"; + s.op->newline() << "REG_IP(regs) = (unsigned long) inst->addr;"; s.op->newline() << "(*sdp->ph) (c);"; + s.op->newline() << "REG_IP(regs) = kprobes_ip;"; + s.op->newline(-1) << "}"; + common_probe_entryfn_epilogue (s.op); s.op->newline() << "return 0;"; s.op->newline(-1) << "}"; @@ -3009,7 +3020,18 @@ dwarf_derived_probe_group::emit_module_decls (systemtap_session& s) common_probe_entryfn_prologue (s.op, "STAP_SESSION_RUNNING", "sdp->pp"); s.op->newline() << "c->regs = regs;"; s.op->newline() << "c->pi = inst;"; // for assisting runtime's backtrace logic + + // Make it look like the IP is set as it wouldn't have been replaced + // by a breakpoint instruction when calling real probe handler. Reset + // IP regs on return, so we don't confuse kprobes. PR10458 + s.op->newline() << "{"; + s.op->indent(1); + s.op->newline() << "unsigned long kprobes_ip = REG_IP(c->regs);"; + s.op->newline() << "REG_IP(regs) = (unsigned long) inst->rp->kp.addr;"; s.op->newline() << "(*sdp->ph) (c);"; + s.op->newline() << "REG_IP(regs) = kprobes_ip;"; + s.op->newline(-1) << "}"; + common_probe_entryfn_epilogue (s.op); s.op->newline() << "return 0;"; s.op->newline(-1) << "}"; @@ -4381,7 +4403,18 @@ uprobe_derived_probe_group::emit_module_decls (systemtap_session& s) s.op->newline() << "if (sup->spec_index < 0 ||" << "sup->spec_index >= " << probes.size() << ") return;"; // XXX: should not happen s.op->newline() << "c->regs = regs;"; + + // Make it look like the IP is set as it would in the actual user + // task when calling real probe handler. Reset IP regs on return, so + // we don't confuse uprobes. PR10458 + s.op->newline() << "{"; + s.op->indent(1); + s.op->newline() << "unsigned long uprobes_ip = REG_IP(c->regs);"; + s.op->newline() << "REG_IP(regs) = inst->vaddr;"; s.op->newline() << "(*sups->ph) (c);"; + s.op->newline() << "REG_IP(regs) = uprobes_ip;"; + s.op->newline(-1) << "}"; + common_probe_entryfn_epilogue (s.op); s.op->newline(-1) << "}"; @@ -4393,7 +4426,18 @@ uprobe_derived_probe_group::emit_module_decls (systemtap_session& s) << "sup->spec_index >= " << probes.size() << ") return;"; // XXX: should not happen // XXX: kretprobes saves "c->pi = inst;" too s.op->newline() << "c->regs = regs;"; + + // Make it look like the IP is set as it would in the actual user + // task when calling real probe handler. Reset IP regs on return, so + // we don't confuse uprobes. PR10458 + s.op->newline() << "{"; + s.op->indent(1); + s.op->newline() << "unsigned long uprobes_ip = REG_IP(c->regs);"; + s.op->newline() << "REG_IP(regs) = inst->rp->u.vaddr;"; s.op->newline() << "(*sups->ph) (c);"; + s.op->newline() << "REG_IP(regs) = uprobes_ip;"; + s.op->newline(-1) << "}"; + common_probe_entryfn_epilogue (s.op); s.op->newline(-1) << "}"; @@ -4882,7 +4926,18 @@ kprobe_derived_probe_group::emit_module_decls (systemtap_session& s) s.op->line() << "];"; common_probe_entryfn_prologue (s.op, "STAP_SESSION_RUNNING", "sdp->pp"); s.op->newline() << "c->regs = regs;"; + + // Make it look like the IP is set as it wouldn't have been replaced + // by a breakpoint instruction when calling real probe handler. Reset + // IP regs on return, so we don't confuse kprobes. PR10458 + s.op->newline() << "{"; + s.op->indent(1); + s.op->newline() << "unsigned long kprobes_ip = REG_IP(c->regs);"; + s.op->newline() << "REG_IP(regs) = (unsigned long) inst->addr;"; s.op->newline() << "(*sdp->ph) (c);"; + s.op->newline() << "REG_IP(regs) = kprobes_ip;"; + s.op->newline(-1) << "}"; + common_probe_entryfn_epilogue (s.op); s.op->newline() << "return 0;"; s.op->newline(-1) << "}"; @@ -4905,7 +4960,18 @@ kprobe_derived_probe_group::emit_module_decls (systemtap_session& s) common_probe_entryfn_prologue (s.op, "STAP_SESSION_RUNNING", "sdp->pp"); s.op->newline() << "c->regs = regs;"; s.op->newline() << "c->pi = inst;"; // for assisting runtime's backtrace logic + + // Make it look like the IP is set as it wouldn't have been replaced + // by a breakpoint instruction when calling real probe handler. Reset + // IP regs on return, so we don't confuse kprobes. PR10458 + s.op->newline() << "{"; + s.op->indent(1); + s.op->newline() << "unsigned long kprobes_ip = REG_IP(c->regs);"; + s.op->newline() << "REG_IP(regs) = (unsigned long) inst->rp->kp.addr;"; s.op->newline() << "(*sdp->ph) (c);"; + s.op->newline() << "REG_IP(regs) = kprobes_ip;"; + s.op->newline(-1) << "}"; + common_probe_entryfn_epilogue (s.op); s.op->newline() << "return 0;"; s.op->newline(-1) << "}"; -- cgit From 793e611dc277b6943100a5f81274961d526f9b02 Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Fri, 31 Jul 2009 19:54:53 +0200 Subject: Add testcase for PR10458, PR10459 and PR10454. Last test currently disabled because PR10454 is still open. * testsuite/systemtap.context/uprobe_stmt_num.exp: New file. * testsuite/systemtap.context/uprobe_stmt_num.stp: Likewise. * testsuite/systemtap.context/uprobe_stmt_num.c: Likewise. --- testsuite/systemtap.context/uprobe_stmt_num.c | 20 +++++++ testsuite/systemtap.context/uprobe_stmt_num.exp | 78 +++++++++++++++++++++++++ testsuite/systemtap.context/uprobe_stmt_num.stp | 4 ++ 3 files changed, 102 insertions(+) create mode 100644 testsuite/systemtap.context/uprobe_stmt_num.c create mode 100644 testsuite/systemtap.context/uprobe_stmt_num.exp create mode 100644 testsuite/systemtap.context/uprobe_stmt_num.stp diff --git a/testsuite/systemtap.context/uprobe_stmt_num.c b/testsuite/systemtap.context/uprobe_stmt_num.c new file mode 100644 index 00000000..887e572a --- /dev/null +++ b/testsuite/systemtap.context/uprobe_stmt_num.c @@ -0,0 +1,20 @@ +static int +func2 (int x, int y) +{ + return x + y; +} + +static int +func (int arg) +{ + int x = 16; + int y = arg - x; + int z = func2(x, y); + return x + y + z; +} + +int +main (int argc, char *argv[], char *envp[]) +{ + return func(42); +} diff --git a/testsuite/systemtap.context/uprobe_stmt_num.exp b/testsuite/systemtap.context/uprobe_stmt_num.exp new file mode 100644 index 00000000..fbb1126a --- /dev/null +++ b/testsuite/systemtap.context/uprobe_stmt_num.exp @@ -0,0 +1,78 @@ +# Tests whether we can put statement probes on all lines of a function, +# even without debuginfo around (in guru mode currently). PR10454. + +set test "uprobe_stmt_num" + +# Only run on make installcheck and utrace present. +if {! [installtest_p]} { untested "$test"; return } +if {! [utrace_p]} { untested "$test"; return } + +set testpath "$srcdir/$subdir" +set testsrc "$testpath/$test.c" +set testexe "[pwd]/$test" +# We want debug info and no optimization (every line counts). +set testflags "additional_flags=-g additional_flags=-O0" +set teststp "$testpath/$test.stp" + +set res [target_compile $testsrc $testexe executable $testflags] +if { $res != "" } { + verbose "target_compile failed: $res" 2 + fail "unable to compile $testsrc" + return +} + +set cmd [concat stap -c $testexe $teststp] +send_log "cmd: $cmd\n" +catch {eval exec $cmd} output +send_log "cmd output: $output\n" + +# There should be at least 6 lines probes +# Function entry, 4 actual source lines and function exit. +set output_lines [split $output "\n"] +set lines [llength $output_lines] +if { $lines >= 6 } { + pass "$test-run-one" +} else { + fail "$test-run-one ($lines)" +} + +# Expect the same output for next runs +set ::result_string $output + +# Sanity check, just run again... +stap_run3 $test-run-two $testpath/$test.stp -c $testexe + +# create a script based on the given statements, +# probe all of them individually. +set fp [open $test-run-statements.stp "w"] +foreach line $output_lines { + puts $fp "probe process(\"uprobe_stmt_num\").statement($line)" + puts $fp "{" + puts $fp " printf(\"0x%x\\n\", uaddr());" + puts $fp "}" +} +close $fp +stap_run3 $test-run-statements $test-run-statements.stp -c $testexe + +# Now strip away the line info and try again (with -g). +set strip_cmd [list "objcopy" "-R" ".debug_line"] +lappend strip_cmd "$testexe" +send_log "Executing: $strip_cmd\n" +eval exec $strip_cmd + +stap_run3 $test-run-statements-nolines -w -g $test-run-statements.stp -c $testexe + +# XXX Last test still fails. PR10454. +return + +# Now strip away all debuginfo. Since the script doesn't need any it +# should still work. +set strip_cmd [list "strip" "-g"] +lappend strip_cmd "$testexe" +send_log "Executing: $strip_cmd\n" +eval exec $strip_cmd + +stap_run3 $test-run-statements-nodebuginfo -w -g $test-run-statements.stp -c $testexe + +# cleanup +eval exec rm $testexe $test-run-statements.stp diff --git a/testsuite/systemtap.context/uprobe_stmt_num.stp b/testsuite/systemtap.context/uprobe_stmt_num.stp new file mode 100644 index 00000000..c2e8d5ba --- /dev/null +++ b/testsuite/systemtap.context/uprobe_stmt_num.stp @@ -0,0 +1,4 @@ +probe process("uprobe_stmt_num").statement("func@uprobe_stmt_num.c:*") +{ + printf("0x%x\n", uaddr()); +} -- cgit