All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
To: Denys Vlasenko <vda.linux@googlemail.com>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>, Ingo Molnar <mingo@elte.hu>,
	Andrew Morton <akpm@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux-mm <linux-mm@kvack.org>, Andi Kleen <andi@firstfloor.org>,
	Christoph Hellwig <hch@infradead.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Roland McGrath <roland@hack.frob.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Arnaldo Carvalho de Melo <acme@infradead.org>,
	Anton Arapov <anton@redhat.com>,
	Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
	Jim Keniston <jkenisto@linux.vnet.ibm.com>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	yrl.pp-manager.tt@hitachi.com
Subject: Re: [PATCH v9 3.2 1/9] uprobes: Install and remove breakpoints.
Date: Thu, 26 Jan 2012 23:14:38 +0900	[thread overview]
Message-ID: <4F215FCE.8010209@hitachi.com> (raw)
In-Reply-To: <CAK1hOcMeYDrLpgbyQXFAkrqk3Le-rLkoVwCN3wjkT61YJg0G6Q@mail.gmail.com>

(2012/01/26 0:32), Denys Vlasenko wrote:
> On Wed, Jan 25, 2012 at 4:13 PM, Denys Vlasenko
> <vda.linux@googlemail.com> wrote:
>>> +       /*
>>> +        * Convert from rip-relative addressing to indirect addressing
>>> +        * via a scratch register.  Change the r/m field from 0x5 (%rip)
>>> +        * to 0x0 (%rax) or 0x1 (%rcx), and squeeze out the offset field.
>>> +        */
>>> +       reg = MODRM_REG(insn);
>>> +       if (reg == 0) {
>>> +               /*
>>> +                * The register operand (if any) is either the A register
>>> +                * (%rax, %eax, etc.) or (if the 0x4 bit is set in the
>>> +                * REX prefix) %r8.  In any case, we know the C register
>>> +                * is NOT the register operand, so we use %rcx (register
>>> +                * #1) for the scratch register.
>>> +                */
>>> +               uprobe->arch_info.fixups = UPROBES_FIX_RIP_CX;
>>> +               /* Change modrm from 00 000 101 to 00 000 001. */
>>> +               *cursor = 0x1;
> 
> Hmm. I think we have a bug here.
> 
> What if this instruction has REX.B = 1? Granted, REX.B = 1 has no effect on
> rip-relative addressing and therefore normally won't be generated by gcc/as,
> but still. If you replace md and r/m fields as above, you are trying to convert
> 0x12345678(%rip) reference to (%rcx), but if REX.B = 1, then you in fact
> converted it to (%r9)!

Right, thanks for finding :)
And %rax register reference encoding has same problem, doesn't it?

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
To: Denys Vlasenko <vda.linux@googlemail.com>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>, Ingo Molnar <mingo@elte.hu>,
	Andrew Morton <akpm@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux-mm <linux-mm@kvack.org>, Andi Kleen <andi@firstfloor.org>,
	Christoph Hellwig <hch@infradead.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Roland McGrath <roland@hack.frob.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Arnaldo Carvalho de Melo <acme@infradead.org>,
	Anton Arapov <anton@redhat.com>,
	Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
	Jim Keniston <jkenisto@linux.vnet.ibm.com>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	yrl.pp-manager.tt@hitachi.com
Subject: Re: [PATCH v9 3.2 1/9] uprobes: Install and remove breakpoints.
Date: Thu, 26 Jan 2012 23:14:38 +0900	[thread overview]
Message-ID: <4F215FCE.8010209@hitachi.com> (raw)
In-Reply-To: <CAK1hOcMeYDrLpgbyQXFAkrqk3Le-rLkoVwCN3wjkT61YJg0G6Q@mail.gmail.com>

(2012/01/26 0:32), Denys Vlasenko wrote:
> On Wed, Jan 25, 2012 at 4:13 PM, Denys Vlasenko
> <vda.linux@googlemail.com> wrote:
>>> +       /*
>>> +        * Convert from rip-relative addressing to indirect addressing
>>> +        * via a scratch register.  Change the r/m field from 0x5 (%rip)
>>> +        * to 0x0 (%rax) or 0x1 (%rcx), and squeeze out the offset field.
>>> +        */
>>> +       reg = MODRM_REG(insn);
>>> +       if (reg == 0) {
>>> +               /*
>>> +                * The register operand (if any) is either the A register
>>> +                * (%rax, %eax, etc.) or (if the 0x4 bit is set in the
>>> +                * REX prefix) %r8.  In any case, we know the C register
>>> +                * is NOT the register operand, so we use %rcx (register
>>> +                * #1) for the scratch register.
>>> +                */
>>> +               uprobe->arch_info.fixups = UPROBES_FIX_RIP_CX;
>>> +               /* Change modrm from 00 000 101 to 00 000 001. */
>>> +               *cursor = 0x1;
> 
> Hmm. I think we have a bug here.
> 
> What if this instruction has REX.B = 1? Granted, REX.B = 1 has no effect on
> rip-relative addressing and therefore normally won't be generated by gcc/as,
> but still. If you replace md and r/m fields as above, you are trying to convert
> 0x12345678(%rip) reference to (%rcx), but if REX.B = 1, then you in fact
> converted it to (%r9)!

Right, thanks for finding :)
And %rax register reference encoding has same problem, doesn't it?

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

  reply	other threads:[~2012-01-26 14:14 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-10 11:48 [PATCH v9 3.2 0/9] Uprobes patchset with perf probe support Srikar Dronamraju
2012-01-10 11:48 ` Srikar Dronamraju
2012-01-10 11:48 ` [PATCH v9 3.2 1/9] uprobes: Install and remove breakpoints Srikar Dronamraju
2012-01-10 11:48   ` Srikar Dronamraju
2012-01-25 14:39   ` Denys Vlasenko
2012-01-25 14:39     ` Denys Vlasenko
2012-01-25 16:31     ` Oleg Nesterov
2012-01-25 16:31       ` Oleg Nesterov
2012-01-25 15:13   ` Denys Vlasenko
2012-01-25 15:13     ` Denys Vlasenko
2012-01-25 15:32     ` Denys Vlasenko
2012-01-25 15:32       ` Denys Vlasenko
2012-01-26 14:14       ` Masami Hiramatsu [this message]
2012-01-26 14:14         ` Masami Hiramatsu
2012-01-26 18:28         ` Denys Vlasenko
2012-01-26 18:28           ` Denys Vlasenko
2012-01-30  9:51           ` Masami Hiramatsu
2012-01-30  9:51             ` Masami Hiramatsu
2012-01-26 13:38     ` Masami Hiramatsu
2012-01-26 13:38       ` Masami Hiramatsu
2012-01-26 13:45   ` Masami Hiramatsu
2012-01-26 13:45     ` Masami Hiramatsu
2012-01-10 11:48 ` [PATCH v9 3.2 2/9] uprobes: handle breakpoint and signal step exception Srikar Dronamraju
2012-01-10 11:48   ` Srikar Dronamraju
2012-01-18  8:39   ` Anton Arapov
2012-01-18  8:39     ` Anton Arapov
2012-01-18  9:02     ` Srikar Dronamraju
2012-01-18  9:02       ` Srikar Dronamraju
2012-01-18 10:18       ` Mike Frysinger
2012-01-18 10:47         ` Srikar Dronamraju
2012-01-18 10:47           ` Srikar Dronamraju
2012-01-18 11:01           ` Mike Frysinger
2012-01-20 22:57             ` Mike Frysinger
2012-01-20 22:57               ` Mike Frysinger
2012-01-25  8:12               ` Srikar Dronamraju
2012-01-25  8:12                 ` Srikar Dronamraju
2012-01-10 11:48 ` [PATCH v9 3.2 3/9] uprobes: slot allocation Srikar Dronamraju
2012-01-10 11:48   ` Srikar Dronamraju
2012-01-10 11:49 ` [PATCH v9 3.2 4/9] uprobes: counter to optimize probe hits Srikar Dronamraju
2012-01-10 11:49   ` Srikar Dronamraju
2012-01-10 11:49 ` [PATCH v9 3.2 5/9] tracing: modify is_delete, is_return from ints to bool Srikar Dronamraju
2012-01-10 11:49   ` Srikar Dronamraju
2012-01-10 11:49 ` [PATCH v9 3.2 6/9] tracing: Extract out common code for kprobes/uprobes traceevents Srikar Dronamraju
2012-01-10 11:49   ` Srikar Dronamraju
2012-01-10 11:49 ` [PATCH v9 3.2 7/9] tracing: uprobes trace_event interface Srikar Dronamraju
2012-01-10 11:49   ` Srikar Dronamraju
2012-01-16 13:11   ` Jiri Olsa
2012-01-16 13:11     ` Jiri Olsa
2012-01-16 14:45     ` Srikar Dronamraju
2012-01-16 14:45       ` Srikar Dronamraju
2012-01-16 15:33       ` Jiri Olsa
2012-01-16 15:33         ` Jiri Olsa
2012-01-16 16:41         ` Srikar Dronamraju
2012-01-16 16:41           ` Srikar Dronamraju
2012-01-17  9:28     ` Ingo Molnar
2012-01-17  9:28       ` Ingo Molnar
2012-01-17 10:22       ` Srikar Dronamraju
2012-01-17 10:22         ` Srikar Dronamraju
2012-01-17 10:59         ` Ingo Molnar
2012-01-17 10:59           ` Ingo Molnar
2012-01-17 11:03           ` Srikar Dronamraju
2012-01-17 11:03             ` Srikar Dronamraju
2012-01-17 11:22         ` Ingo Molnar
2012-01-17 11:22           ` Ingo Molnar
2012-01-17 11:57           ` Srikar Dronamraju
2012-01-17 11:57             ` Srikar Dronamraju
2012-01-17 12:23             ` Ingo Molnar
2012-01-17 12:23               ` Ingo Molnar
2012-01-17 12:27               ` Ingo Molnar
2012-01-17 12:27                 ` Ingo Molnar
2012-01-17 12:11         ` Ingo Molnar
2012-01-17 12:11           ` Ingo Molnar
2012-01-17 12:21         ` Ingo Molnar
2012-01-17 12:21           ` Ingo Molnar
2012-01-18 10:07           ` Srikar Dronamraju
2012-01-18 10:07             ` Srikar Dronamraju
2012-01-10 11:49 ` [PATCH v9 3.2 8/9] perf: rename target_module to target Srikar Dronamraju
2012-01-10 11:49   ` Srikar Dronamraju
2012-01-16  8:34 ` [PATCH v9 3.2 0/9] Uprobes patchset with perf probe support Ingo Molnar
2012-01-16  8:34   ` Ingo Molnar
2012-01-16 15:17   ` Srikar Dronamraju
2012-01-16 15:17     ` Srikar Dronamraju
2012-01-17  9:39     ` Ingo Molnar
2012-01-17  9:39       ` Ingo Molnar
2012-01-25 14:11       ` Peter Zijlstra
2012-01-25 14:11         ` Peter Zijlstra
2012-01-26 11:10         ` Ingo Molnar
2012-01-26 11:10           ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F215FCE.8010209@hitachi.com \
    --to=masami.hiramatsu.pt@hitachi.com \
    --cc=acme@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=ananth@in.ibm.com \
    --cc=andi@firstfloor.org \
    --cc=anton@redhat.com \
    --cc=hch@infradead.org \
    --cc=jkenisto@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@elte.hu \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=roland@hack.frob.com \
    --cc=rostedt@goodmis.org \
    --cc=sfr@canb.auug.org.au \
    --cc=srikar@linux.vnet.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=vda.linux@googlemail.com \
    --cc=yrl.pp-manager.tt@hitachi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.