Re: [PATCH 1/2] This patch fixes GDBServer's run control for single stepping

[Pedro Alves <palves@redhat.com>]

On 02/17/2017 01:41 AM, Antoine Tremblay wrote:
> 
> Pedro Alves writes:
> 
>> On 01/30/2017 01:28 PM, Antoine Tremblay wrote:
>>
>>>> We don't change anything when setting breakpoint inside IT block.
>>>
>>> Well that's a problem if we write a 32 bit thumb2 breakpoint aligned on
>>> 2 bytes like discussed before.
>>
> 
> Sorry for the delay I just saw your mail...
> 
>> Can we restrict the stopping-all-threads to the case that
>> needs it, only?
> 
> Possibly but I would like to avoid introducing 2 execution paths in the
> run control, it's already hard to follow as it is and it would introduce
> a lot of code in the arch independant code just for this case, that's
> something I would like to avoid too.

I don't immediately see how this would imply introducing lots of code
in the run control.  We shouldn't be imposing this stop-all-threads
on all archs, since it adds a lot of slowness (and
the more threads the inferior has, the worse it gets).  So if
we already need the 2 execution paths, making the condition that
determines whether to pause all threads consider more state
doesn't seem to add that much complexity to the run control
part itself.

>> An optimization that would avoid this would be to use a
>> hardware watchpoint/breakpoint (if available) for single-stepping.
>> IIRC, you can ARM a breakpoint (or was it a watchpoint) on ARM for
>> triggering when the PC is different from the current PC (or really,
>> some specified address).
> 
> I did not know that but I'm worried about the limited number of hardware
> watchpoints available. IIRC on my board I had only 4, if GDBServer can't
> find one available would it refuse to single step ? That would be
> weird... ?

My thought was that we'd give preference to user-requested
watchpoints.  I.e., treat this as an optimization.  We always need to
pause all threads in order to install watchpoints in all
threads (watchpoints must be inserted with the thread paused, and
we do that on thread resume).  So if a request for a user-watchpoints
comes in, we'd just interrupt the current single-step as we currently
do, install the watchpoints, and go back to single-stepping, if it didn't
manage to finish, as we currently do.  At this point, we notice that we
don't have free hardware watchpoints left, and fallback to do
the slow software single-step as before.

> 
> It's an interesting approch however I'll dig about it more.
> 
>>
>> In IT blocks, this would probably make the thread stop on
>> instructions that aren't really executed (e.g., the then/else
>> branches when the condition is correspondingly false/true),
> 
> Really ? I need to find something about that in the arch man...

AFAIK, in IT blocks, all instructions always "execute", but, when
the condition is false, the instruction becomes as-if a nop.
The only reason breakpoints don't stop there currently is that
breakpoints are just another instruction (actually an undefined
instruction the kernel is aware of, that causes an undefined
instruction exception that the kernel translates to a SIGTRAP
instead of a SIGILL), and when the condition is false, the breakpoint
instruction becomes a nop too...  If you have a hardware trap
set to trap at such an address though, then it should trap, I believe,
as if you had armed a hardware breakpoint to trap on the address
of a real nop instruction.

Thanks,
Pedro Alves