Messages in this thread | | | Date | Tue, 1 Apr 2014 16:33:46 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 3/7] uprobes/x86: Conditionalize the usage of handle_riprel_insn() |
| |
On 04/01, Masami Hiramatsu wrote: > > (2014/04/01 4:44), Oleg Nesterov wrote: > > arch_uprobe_analyze_insn() calls handle_riprel_insn() at the start, > > but only "0xff" and "default" cases need the UPROBE_FIX_RIP_ logic. > > Move the callsite into "default" case and change the "0xff" case to > > fall-through. > > > > We are going to add the various hooks to handle the rip-relative > > jmp/call instructions (and more), we need this change to enforce the > > fact that the new code can't conflict with is_riprel_insn() code. > > > > Signed-off-by: Oleg Nesterov <oleg@redhat.com> > > Hmm, this seems not obviously reasonable at this point. > However, the code itself is not wrong. Could you merge > this change to that new hooks?
Good point, I'll send v2.
I'd still prefer to do this in a separate patch if you do not object, and this patch should come after "add uprobe_xol_ops". In this case it should be more clear why do we need this change, and the changelog can tell more. Say, it can mention that otherwise uprobe_abort_xol() can be confused until at least we add uprobe_xol_ops->abort().
And probably I'll add another patch into this series, "restart if ->post_xol() fails" (see "TODO:" in 7/7). We need this change anyway to emulate the "call" insn. We could fix this before we add the hooks, but after 7/7 the change will be more simple/clear.
Thanks!
Oleg.
| |