Re: Testing insn.block probe point uncovers possible utrace bug

Re: Testing insn.block probe point uncovers possible utrace bug

[Roland McGrath]

The bug is in your module's unconditional use of UTRACE_*STEP.
It's documented that it's invalid to use them unless arch_has_*_step()
has returned true.  (The utrace_resume_action description refers you 
to utrace_control(), where this is documented.)

It's a bug to use these at all when the corresponding arch_has_*_step()
check returns false.  utrace_control() helps you out with this a bit more
by using WARN_ON, and returns -EOPNOTSTUPP if even single-step is not
there.  The warning spew is there to make sure you know this is a bug in
your module and it's just being overly extra nice not to crash on you.
(You would be entirely wrong to expect this return value and think it
was ever valid to make this call.)

There is no place for post-callback processing to return an error.  I've
now made it use WARN_ON there consistent with what utrace_control() does.
For UTRACE_BLOCKSTEP, you'll get the WARN_ON spew and then it will fall
back to treating it as UTRACE_SINGLESTEP.  (This is what utrace_control()
always did.)

What your module needs to do is check arch_has_*_step() correctly.  When
the feature you want is not there, gracefully tell your users you can't do
it.  Your module today is wrong on every architecture, even 32-bit x86
where it happens to work on every chip you are likely to test.

As to the powerpc implementation, that is a subject for the powerpc
maintainers (linuxppc-dev@ozlabs.org et al) and is orthogonal to utrace.
Take it up with them.  I gave them a patch long ago and it got stalled
waiting for them to figure out which chips really have the feature so the
arch_has_block_step() definition could be made perfect.


Thanks,
Roland