Re: [PATCH v4 15/16] ARM: add uprobes support

[Rabin Vincent]

2013/12/20 Jon Medhurst (Tixy) <tixy@xxxxxxxxxx>
> 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 and so the code above would believe it
is free.  Using ffs instead of fls would thus lead to R1 being used to
substitute PC.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/