Re: [PATCH] [gdbserver] Disable conditional breakpoints on no-hardware-single-step targets

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

On 05/07/2015 06:47 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> Of a random PC address no, but in gdbserver's case, I think that it
>> would work, because we need it to step over a breakpoint that is
>> at the current PC.  So we could:
>>
>>   #1 - Get the mode of the current PC from the thread's $cpsr register.
>>
>>   #2 - Get the mode of the next PC by looking at the instruction that is
>>        about to be executed (at current PC).  If bx and blx, which change
>>        modes, check the thumb bit of the destination address.
>>        For all other instructions, same mode as the current PC.
>>
>
> We can know the mode of the next PC in this way, but we don't know the
> address of the next PC.  In fact, we need to know the address of the
> next PC first, and then determine the mode of the next PC.  Probably, we
> need something as below,
>
>   1. Teach GDBserver to compute the address of the next PC,
>   2. Determine the mode of the next PC as you suggested,
>   3. Add breakpoint_from_pc hook in target_ops, so that the right
>      breakpoint instruction can be selected.
>

Just fyi, I'm working on doing this at the moment, my investigation is 
still incomplete...

So far I mainly plan to port the arm_get_next code to gdbserver, to 
accomplish 1. , the code doesn't have so many deps so it should be ok
2. by looking at $cpsr
3. should be fine as 1 and 2 are done...

I don't know however yet the best strategy to share the code but I'm 
guessing I could make the parts that don't have any deps to gdbarch etc 
in a shared function with gdb/gdbserver... Any pointers on this are 
welcome...


>>>
>>> After thinking about how to teach GDBserver inserting right breakpoint
>>> (arm or thumb) for a while, I reconsider it from a different direction
>>> that it may be unreasonable to run target-side conditional breakpoint for
>>> targets without hardware single step.  Pedro also pointed this out here
>>> https://sourceware.org/ml/gdb-patches/2015-04/msg00337.html
>>
>> In the end I was somewhat convinced that things ended up working.
>> But I certainly don't object to this patch.
>>
>>> +  /* Although win32-i386 has hardware single step, still disable this
>>> +     feature for win32, because it is quite GNU/Linux specific.  */
>>> +  NULL, /* supports_conditional_breakpoints */
>>
>> TBC, it's not that the feature is GNU/Linux specific (like something
>> related to system calls or some detail in glibc), but that the support
>> for conditional breakpoints is baked into linux-low.c instead of
>> in generic code.
>
> How about writing comments like this?
>
>    /* Although win32-i386 has hardware single step, still disable this
>       feature for win32, because it is implemented in linux-low.c instead
>       of in generic code.  */
>