2014-04-02 19:53:27

by Jim Keniston

Subject: Re: [PATCH 5/7] uprobes/x86: Gather "riprel" functions together

On Mon, 2014-03-31 at 21:44 +0200, Oleg Nesterov wrote:
...
> +static void
> +handle_riprel_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs, long *correction)
> +{
> + if (auprobe->fixups & (UPROBE_FIX_RIP_AX | UPROBE_FIX_RIP_CX)) {
> + struct arch_uprobe_task *autask;
> +
> + autask = &current->utask->autask;
> + if (auprobe->fixups & UPROBE_FIX_RIP_AX)
> + regs->ax = autask->saved_scratch_register;
> + else
> + regs->cx = autask->saved_scratch_register;
> +
> + /*
> + * The original instruction includes a displacement, and so
> + * is 4 bytes longer than what we've just single-stepped.
> + * Fall through to handle stuff like "jmpq *...(%rip)" and
> + * "callq *...(%rip)".
> + */
> + if (correction)
> + *correction += 4;
> + }
> +}

This is another case of a glitch in the code being moved. Since this
code was moved to its own function, the "Fall through" comment makes no
sense. Maybe
* Caller may need to apply other fixups to handle stuff
* like "jmpq *...(%rip)" and "callq *...(%rip)".

Jim

2014-04-03 20:09:43

by Oleg Nesterov

Subject: Re: [PATCH 5/7] uprobes/x86: Gather "riprel" functions together

On 04/02, Jim Keniston wrote:
>
> On Mon, 2014-03-31 at 21:44 +0200, Oleg Nesterov wrote:
> ...
> > +static void
> > +handle_riprel_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs, long *correction)
> > +{
> > + if (auprobe->fixups & (UPROBE_FIX_RIP_AX | UPROBE_FIX_RIP_CX)) {
> > + struct arch_uprobe_task *autask;
> > +
> > + autask = &current->utask->autask;
> > + if (auprobe->fixups & UPROBE_FIX_RIP_AX)
> > + regs->ax = autask->saved_scratch_register;
> > + else
> > + regs->cx = autask->saved_scratch_register;
> > +
> > + /*
> > + * The original instruction includes a displacement, and so
> > + * is 4 bytes longer than what we've just single-stepped.
> > + * Fall through to handle stuff like "jmpq *...(%rip)" and
> > + * "callq *...(%rip)".
> > + */
> > + if (correction)
> > + *correction += 4;
> > + }
> > +}
>
> This is another case of a glitch in the code being moved. Since this
> code was moved to its own function, the "Fall through" comment makes no
> sense. Maybe
> * Caller may need to apply other fixups to handle stuff
> * like "jmpq *...(%rip)" and "callq *...(%rip)".

Thanks. updated the comment in v2.

Oleg.