Re: [PATCH 0/7] Support tracepoints and software breakpoints on ARM aarch32-linux in GDBServer.

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

On 09/14/2015 06:33 AM, Yao Qi wrote:
> Antoine Tremblay <antoine.tremblay@ericsson.com> writes:
>
> Hi Antoine,
> Thanks for the work!
>
>> This patch series adds support for tracepoints and software breakpoints on ARM
>> aarch32 on linux.
>
> Term "aarch32" is only used on armv8, means 32-bit execution state.
> What your patches do is to add software single step and tracepoint point
> for ARM (ARMv7 and pre-ARMv7).  Don't need to say "aarch32".
>

Yes I was a bit confused about that I thought aarch32 meant all old 
32-bit instruction sets where it seems to refer the the 32 bits 
instructions in armv8 only.

I could rephrase to ARM 32 bits or up to ARMv7 ? Or just ARM ? I don't 
want to bring confusion with aarch64 since it supports hardware single step.

> Your patch series include too much things, and we need to split them,
>
>>
>> Some background :
>>
>> Some specific issues for ARM aarch32 make it difficult for GDBServer to enable
>> these features namely : it has no hardware single step support and the
>> breakpoint data and size depends on the current instruction set used, thumb,
>> thumb2 or arm.
>>
>> So GDBServer needs to know how to set a breakpoint of different size based on
>> the current instruction and registers and it needs to be able to do software
>> single stepping.
>
> The first part can be teaching GDBserver to choose the right breakpiont
> instruction (of different size and encoding) based on the address.
> Then, the second part can be teaching GDBserver to do software single stepping.
>

Yes I could separate the two, my main reason to go with them both was 
that I could not test the breakpoints if I did not have software single 
step.

I'll separate them.

>>
>> This patchset teaches this to GDBServer and enables support for tracepoints and
>> software breakpoints.
>
> Support tracepoint on ARM (with software single step in GDBserver side)
> can be the third part.

This is already a separate patch, namely  7/7.

Did you meant to create a patchset for it ?

I guess I could separate the patch in maybe 2 or 3 but isn't it better 
to keep it inside the patchset otherwise we will have to sync 3 
patchsets versions since they depend on each other ?

>>
>> The patchset :
>>
>> This patchset applies over :
>> https://sourceware.org/ml/gdb-patches/2015-09/msg00004.html
>> to avoid an immediate conflict...
>
> I'll push it in, if there are no regressions on x86 and aarch64.
>

OK thanks :)

>>
>> This patchset was tested on Ubuntu 14.04 on ARMv7 and x86 no regressions found.
>
> How do you test them on ARMv7?  I assume you tested your patches against
> gdbserver, but did you test them against compile options -marm and
> -mthumb respectively?  I do it in this way,
>
>   $ make check RUNTESTFLAGS='--target_board=YOUR_BOARD\{-marm,-mthumb\}'

Yes indeed with native-gdbserver and native-extended-gdbserver, But I 
had only checked manually with -march and -mthumb with specific tests.

I'll add this to my buildbot in general and make it more clear what test 
configs were used.

>
> YOUR_BOARD can be unix, native-gdbserver or whatever you are using.
> Sometime, I also pass -march=armv4t to exercise thumb-1 instructions,
> but I don't know whether your gcc has such multi-lib or not.
>

It does support it at least according to the man so I'll add this one too.

Thanks !