This is the mail archive of the systemtap@sources.redhat.com mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC][PATCH] PPC64: return address probes


Jim Keniston wrote:
On Fri, 2005-05-20 at 05:15, Ananth N Mavinakayanahalli wrote:

Hi Jim,


Thanks for the comments...

Hi,

Here is the return address probes prototype for ppc64. This patch
applies on 2.6.12-rc4-mm2 + Rusty's arch_break_inst.patch and Hien's
kprobe_flush.patch.

...


Comments welcome!

Ananth

______________________________________________________________________
+
+void arch_kprobe_flush_task(struct task_struct *tk)
+{
+	struct kretprobe_instance *ri;
+
+	while ((ri = get_rp_inst_tsk(tk)) != NULL) {
+		*(ri->arch_retaddr.linkreg_ptr) = (unsigned long)ri->ret_addr;
+		recycle_rp_inst(ri);
+	}
+}
+
+void arch_prepare_kretprobe(struct kretprobe *rp, struct pt_regs *regs)
+{
+	struct kretprobe_instance *ri;
+	static void *orig_ret_addr;
+
+	/* Save the return address when this hits the first time */
+	if (regs->link != (unsigned long)kretprobe_trampoline)
+		orig_ret_addr = (void *)regs->link;
+
+	if ((ri = get_free_rp_inst(rp)) != NULL) {
+		ri->rp = rp;
+		ri->stack_addr = (void *)regs->gpr[1];
+		ri->ret_addr = orig_ret_addr;
+		ri->arch_retaddr.linkreg_ptr =
+			(kprobe_opcode_t *)&regs->link;


This looks wrong to me.  It seems to me that &regs->link is in the
middle of the pt_regs struct, which will disappear as soon as we return
from the trap.  To be of use to arch_kprobe_flush_task(),
ri->arch_retaddr.linkreg_ptr should point to the place in the
yet-to-be-constructed stack frame of the probed function where the link
register will be saved.

Yes, this is incorrect - I'll fix that (come to think of it, I am not sure how I could assume that :-). However, I don't see an easy way to determine where the location for restoring the value. Also, you may never take a stack frame unless there is another function call from within the return address probed function.

If this location is always predictable, and does indeed turn out to be
equal to &regs->link, then you should add a comment explaining the
coincidence.  I was under the impression that it's not predictable from
the information available to kprobes.  If that's the case, then I guess
that arch_kprobe_flush_task() can't do the restore-the-return-address
trick.  This would affect only return-probed functions that are active
when arch_kprobe_flush_task() is called, and actually return.  I think
that this is a fairly small class of functions; flush_old_exec() would
be one.

As I see in the code, do_exit() calls exit_thread() but doesn't "return" and as you mention, I see that flush_old_exec() is the only function
calls flush_tread() and does a return.


What do you suggest would be a good way to handle this? Do we put a
caveat that users can't have return probes on flush_old_exec()? Any
other ideas?

Ananth


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]