Messages in this thread | | | Subject | Re: [PATCH v4 15/16] ARM: add uprobes support | From | "Jon Medhurst (Tixy)" <> | Date | Fri, 20 Dec 2013 19:47:18 +0000 |
| |
On Fri, 2013-12-20 at 20:00 +0100, Rabin Vincent wrote: > 2013/12/20 Jon Medhurst (Tixy) <tixy@linaro.org> > > On Sun, 2013-12-15 at 23:08 -0500, David Long wrote: > > > +static int uprobes_substitute_pc(unsigned long *pinsn, u32 oregs) > > > +{ > > > + probes_opcode_t insn = __mem_to_opcode_arm(*pinsn); > > > + probes_opcode_t temp; > > > + probes_opcode_t mask; > > > + int freereg; > > > + u32 free = 0xffff; > > > + u32 regs; > > > + > > > + for (regs = oregs; regs; regs >>= 4, insn >>= 4) { > > > + if ((regs & 0xf) == REG_TYPE_NONE) > > > + continue; > > > + > > > + free &= ~(1 << (insn & 0xf)); > > > + } > > > + > > > + /* No PC, no problem */ > > > + if (free & (1 << 15)) > > > + return 15; > > > + > > > + if (!free) > > > + return -1; > > > + > > > + /* > > > + * fls instead of ffs ensures that for "ldrd r0, r1, [pc]" we would > > > + * pick LR instead of R1. > > > > Do we know why this is desirable, i.e. preferring the higher numbered > > registers? If there isn't a preference, then no need for comment really. > > > > Also, the comment as is is wrong, should be "...pick LR instead of R2" > > because R1 wouldn't be chosen as the instruction already uses it. > > The second destination register of LDRD (R1 in the example above) is > not encoded in the instruction
Ah, that's the fact I'd missed, so the code and comment in this patch is correct.
Thanks
-- Tixy
| |