Re: [PATCH 1/2] [gdbserver] Rename supports_conditional_breakpoints to supports_hardware_single_step

[Antoine Tremblay <antoine.tremblay@ericsson.com>]

On 09/03/2015 11:13 AM, Antoine Tremblay wrote:
>
>
> On 09/02/2015 03:22 PM, Antoine Tremblay wrote:
>>
>> On 09/01/2015 04:41 AM, Yao Qi wrote:
>>> In my patch https://sourceware.org/ml/gdb-patches/2015-04/msg01110.html
>>> a new target_ops hook supports_conditional_breakpoints was added to
>>> disable conditional breakpoints if target doesn't have hardware single
>>> step.  This patch is to generalize this hook from
>>> supports_conditional_breakpoints to supports_hardware_single_step,
>>> so that the following patch can use it.
>>>
>>
>> Could we generalize this even more to supports_single_step like your
>> next patch ?
>>
>> Since I'm working on software single stepping for ARM, if this patch
>> goes in I'll need to implement a supports_software_single_step and
>> enable ConditionalBreakpoints for this case too...
>>
>> Is there a need to know that it's really hardware or just knowing that
>> it can single step would be ok ?
>>
>
> Note also that this way would force supports_conditional_breakpoints to
> check for more than can_hardware_single_step and require a target
> function set manually to 1 on targets that we know have a proper
> software or hardware single step like :
>
> static int
> linux_supports_conditional_breakpoints (void)
> {
>    return the_low_target.supports_conditional_breakpoints ();
> }
>
> I had initially added that in my set but we could change it for
> the_low_target_.can_single_step () ?
>
> This way targets that have proper software single step can be handled.
> Otherwise every software single step implementation is deemed too simple
> to be used...
>
> Would that seem ok ?
>
>

After more analysis, I think ConditionalBreakpoints should be enabled 
for software/hardware single step.

But for the vCont s:S I don't think we can assume that gdbserver's 
software single step is better then GDB's, it should be the opposite. 
So it should only be enabled when hardware single step is present so we 
can't do away with supports_hardware_single_step.

So to support all features I suggest we have in GDBServer :

supports_hardware_single_step
supports_software_single_step

Enable ConditionalBreakpoints for supports_hardware_single_step || 
supports_software_single_step

And enable vCont s:s only for supports_hardware_single_step

I can add the supports_software_single_step in my patchset..

Does that sounds right ? So your current patch would be fine..