On Thu, Mar 17, 2011 at 10:46 PM, Nicolas Pitre nicolas.pitre@linaro.org wrote:
On Thu, 17 Mar 2011, Dave Martin wrote:
On Fri, Mar 11, 2011 at 5:01 PM, Tixy tixy@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...
Accept the situation as described.
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 :-)
- 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.
We could also insert a conditional undefined instruction for the probe -- this may reduce the number of false positives, but it depends on the hardware implementation. Since kprobes needs to do explicit condition code checks anyway, use of conditional undefs doesn't add any functionality, however.
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.
Well, fair enough -- if there's appetite for tidying this up, and we don't think it will break anything anyone else is doing, then I'd say fixing it is a good idea.
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.
Note that in Thumb, not everything that's conditional is inside an IT block: there are a couple of conditional branch encodings which have an explicit condition code.
It's still a pretty simple decoding task, but not quite as simple as the ARM case.
Cheers ---Dave