From 436e5bf634020bcb9f98967891508db21f9e6cbd Mon Sep 17 00:00:00 2001 From: Jim Keniston Date: Mon, 20 Apr 2009 16:39:26 -0700 Subject: [PATCH] PR10078: uretprobes on functions returning structs/unions arch_predict_sp_at_ret() for x86_32 now accommodates ret $4. Added bz10078 regression test. --- runtime/uprobes/uprobes_i386.c | 13 ++++++++++- runtime/uprobes/uprobes_x86.c | 13 ++++++++++- runtime/uprobes2/uprobes_x86.h | 13 ++++++++++- testsuite/systemtap.base/bz10078.c | 22 +++++++++++++++++ testsuite/systemtap.base/bz10078.exp | 35 ++++++++++++++++++++++++++++ testsuite/systemtap.base/bz10078.stp | 4 ++++ 6 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 testsuite/systemtap.base/bz10078.c create mode 100644 testsuite/systemtap.base/bz10078.exp create mode 100644 testsuite/systemtap.base/bz10078.stp diff --git a/runtime/uprobes/uprobes_i386.c b/runtime/uprobes/uprobes_i386.c index c43f87bf8..7743f4007 100644 --- a/runtime/uprobes/uprobes_i386.c +++ b/runtime/uprobes/uprobes_i386.c @@ -301,9 +301,20 @@ unsigned long arch_hijack_uret_addr(unsigned long trampoline_address, return orig_ret_addr; } +/* + * On x86_32, if a function returns a struct or union, the return + * value is copied into an area created by the caller. The address + * of this area is passed on the stack as a "hidden" first argument. + * When such a function returns, it uses a "ret $4" instruction to pop + * not only the return address but also the hidden arg. To accommodate + * such functions, we add 4 bytes of slop when predicting the return + * address. See PR #10078. + */ +#define STRUCT_RETURN_SLOP 4 + static unsigned long arch_predict_sp_at_ret(struct pt_regs *regs, struct task_struct *tsk) { - return (unsigned long) (regs->esp + 4); + return (unsigned long) (regs->esp + 4 + STRUCT_RETURN_SLOP); } diff --git a/runtime/uprobes/uprobes_x86.c b/runtime/uprobes/uprobes_x86.c index 404c95182..933317154 100644 --- a/runtime/uprobes/uprobes_x86.c +++ b/runtime/uprobes/uprobes_x86.c @@ -716,12 +716,23 @@ unsigned long arch_hijack_uret_addr(unsigned long trampoline_address, return orig_ret_addr; } +/* + * On x86_32, if a function returns a struct or union, the return + * value is copied into an area created by the caller. The address + * of this area is passed on the stack as a "hidden" first argument. + * When such a function returns, it uses a "ret $4" instruction to pop + * not only the return address but also the hidden arg. To accommodate + * such functions, we add 4 bytes of slop when predicting the return + * address. See PR #10078. + */ +#define STRUCT_RETURN_SLOP 4 + static unsigned long arch_predict_sp_at_ret(struct pt_regs *regs, struct task_struct *tsk) { if (test_tsk_thread_flag(tsk, TIF_IA32)) - return (unsigned long) (REGS_SP + 4); + return (unsigned long) (REGS_SP + 4 + STRUCT_RETURN_SLOP); else return (unsigned long) (REGS_SP + 8); } diff --git a/runtime/uprobes2/uprobes_x86.h b/runtime/uprobes2/uprobes_x86.h index ca3f48734..a07fa0d3b 100644 --- a/runtime/uprobes2/uprobes_x86.h +++ b/runtime/uprobes2/uprobes_x86.h @@ -93,11 +93,22 @@ static inline unsigned long arch_get_cur_sp(struct pt_regs *regs) return (unsigned long) regs->sp; } +/* + * On x86_32, if a function returns a struct or union, the return + * value is copied into an area created by the caller. The address + * of this area is passed on the stack as a "hidden" first argument. + * When such a function returns, it uses a "ret $4" instruction to pop + * not only the return address but also the hidden arg. To accommodate + * such functions, we add 4 bytes of slop when predicting the return + * address. See PR #10078. + */ +#define STRUCT_RETURN_SLOP 4 + static inline unsigned long arch_predict_sp_at_ret(struct pt_regs *regs, struct task_struct *tsk) { if (test_tsk_thread_flag(tsk, TIF_IA32)) - return (unsigned long) (regs->sp + 4); + return (unsigned long) (regs->sp + 4 + STRUCT_RETURN_SLOP); else return (unsigned long) (regs->sp + 8); } diff --git a/testsuite/systemtap.base/bz10078.c b/testsuite/systemtap.base/bz10078.c new file mode 100644 index 000000000..9075fbc7f --- /dev/null +++ b/testsuite/systemtap.base/bz10078.c @@ -0,0 +1,22 @@ +#include +#include + +struct point { int x, y; }; + +struct point mkpoint2(void) +{ + struct point p = { 1, 2 }; + return p; +} + +struct point mkpoint1(void) +{ + return mkpoint2(); +} + +main() +{ + struct point p = mkpoint1(); + printf("%d,%d\n", p.x, p.y); + exit(0); +} diff --git a/testsuite/systemtap.base/bz10078.exp b/testsuite/systemtap.base/bz10078.exp new file mode 100644 index 000000000..cad3a3a87 --- /dev/null +++ b/testsuite/systemtap.base/bz10078.exp @@ -0,0 +1,35 @@ +set test bz10078 + +catch {exec gcc -g -o $test $srcdir/$subdir/$test.c} err +if {$err == "" && [file exists $test]} then { pass "$test compile" } else { fail "$test compile" } + +if {![utrace_p]} { + catch {exec rm -f $test} + untested "$test -p4" + untested "$test -p5" + return +} + +set rc [stap_run_batch $srcdir/$subdir/$test.stp] +if {$rc == 0} then { pass "$test -p4" } else { fail "$test -p4" } + +if {! [installtest_p]} { + catch {exec rm -f $test} + untested "$test -p5" + return +} + +# Pick up the stap being tested. +set stapexe [exec /usr/bin/which stap] +spawn sudo $stapexe $srcdir/$subdir/$test.stp -c ./$test +set ok 0 +expect { + -timeout 60 + -re {mkpoint[^\r\n]* returns\r\n} { incr ok; exp_continue } + -re {1,2\r\n} { incr ok; exp_continue } + timeout { fail "$test (timeout)" } + eof { } +} +wait +if {$ok == 3} then { pass "$test -p5" } else { fail "$test -p5 ($ok)" } +exec rm -f $test diff --git a/testsuite/systemtap.base/bz10078.stp b/testsuite/systemtap.base/bz10078.stp new file mode 100644 index 000000000..0318e4e9c --- /dev/null +++ b/testsuite/systemtap.base/bz10078.stp @@ -0,0 +1,4 @@ +#! stap -p4 +probe process("./bz10078").function("mkpoint*").return { + printf("%s returns\n", probefunc()) +} -- 2.43.5