[RFC] kprobes with thumb2 conditional code

[Nicolas Pitre]

On Thu, 17 Mar 2011, Dave Martin wrote:

> On Fri, Mar 11, 2011 at 5:01 PM, Tixy <tixy at yxit.co.uk> wrote:
> > Hello All
> >
> > I'm about to start work on getting kprobes working with thumb2.
> >
> > One of the issues I have is that when probes are placed onto
> > conditionally executed instructions in a IT block, they may not fire if
> > the condition is not met. This is because on ARM we use invalid
> > instructions for breakpoints, and the ARM ARM says:
> >
> >  "it is IMPLEMENTATION DEFINED whether the instruction executes as a
> >   NOP or causes an Undefined Instruction exception"
> >
> > This is a similar issue to that already encountered by GDB [1][2]
> >
> > If we take this code as an example...
> >
> > probe1:
> > if(condition) {
> > probe2:
> >    some_code;
> > }
> >
> > It seams reasonable at first sight that probe2 would only fire if the
> > condition is true. This will always be the case if the compiler
> > generates test-and-branch code, but if it generates conditionally
> > executed instructions for 'some_code' then it gets complicated...
> >
> > The current arm kprobes implementation will always fire probe2 because
> 
> Well, actually it's somewhat unpredictable whether/when a probe on
> probe2 will fire, because it depends on the code generated by the
> compiler.  The compiler could choose to use a conditional instruction
> at probe2, or it could conditionally branch round it, even in ARM.

And we definitely don't want that.

> Or, the whole conditional might disappear to be replaced with an
> unconditional instruction sequence that has the same overall effect.

Sure, but this is an extreme case which is not specific to ARM or 
Thumb2.  Such a situation could happen on x86 as well.  Let's not worry 
about that and keep ourselves to those cases we have control over.

> > it uses unconditional instructions for its breakpoints. With thumb
> > instructions we can't force unconditional execution, so we would have an
> > 'implementation defined' situation whether it would fire or not when the
> > condition is false. (Thought you would hope it would be consistent on a
> > particular device.)
> >
> > Some options for dealing with this, in increasing order of
> > complexity...
> >
> > 1. Accept the situation as described.
> >
> > 2. Change arm probes to use conditional instructions so we would
> > (hopefully) have consistent undefined behaviour in both arm and thumb
> > code. (If that isn't an oxymoron :-)
> >
> > 3. Do 2, and modify kprobe_handler to test for false firings (breakpoint
> > when condition false) and not execute the probe's callback functions in
> > these cases. E.g. consistently make probe2 appear to not fire when
> > condition is false.
> 
> My preference would be for option (1) or (3).  For (3), we could
> choose to extend this behaviour to cover the existing ARM
> implementation as well as Thumb, which could be a tidier and more
> consistent approach.  This seems to be the "right" thing, since it
> means kprobes never fire for real on instructions which are logically
> not executed, no matter how the compiler (or human) implemented the
> conditionality.  Currently, ARM kprobes can fire on instructions which
> would fail their condition check, which might be considered wrong.

Definitely #3.

In the ARM case, this can be achieved by saving the condition code 
of the replaced instruction when installing a probe, and test it 
against the CPSR value from the interrupted state.  If the condition is 
false then just skip the native emulation of the resulting NOP and 
ignore the probe callback.  In the Thumb2 case the IT state in the CPSR 
would indicate right away if the probe should be ignored.

> Alternatively, we could avoid fixing what isn't broken and leave the
> ARM implementation unchanged.

I wouldn't call the ARM implementation not broken.  It just happens that 
probes are typically installed at the beginning of functions and no 
conditional instructions are normally placed there.  This would explain 
why no one reported this issue so far.

> Since the compiler will implement some
> different instruction-level optimisations in Thumb compared with ARM,
> it may not make sense to try to make the kprobe firing behaviour
> behave exactly the same around conditional code in both instruction
> sets -- because the code generation differences may always cause the
> observed behaviour to be different anyway, just as might happen if you
> switch to a newer compiler or use a different -march/-mtune
> combination.

That doesn't matter.  We should respect the expected execution flow at 
the probe location irrespective of any compiler optimization.  It is 
true that code generation might be different between different compilers 
and options, but that's not our problem as long as we don't introduce 
more randomness.  And it happens that making this work properly is 
trivial with Thumb2.  And in ARM mode, the most complex task is to 
determine if a given instruction is conditionally executable or not.


Nicolas