Re: [PATCH 00/25 V2] Make GDB builtin target descriptions more flexible

[Alan Hayward <Alan.Hayward@arm.com>]

> On 26 Jun 2017, at 15:45, Tedeschi, Walfred <walfred.tedeschi@intel.com> wrote:
> 
> 
> 
> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Simon Marchi
> Sent: Monday, June 19, 2017 9:00 PM
> To: Yao Qi <qiyaoltc@gmail.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 00/25 V2] Make GDB builtin target descriptions more flexible
> 
> On 2017-06-12 10:41, Yao Qi wrote:
>> This patch series is to change GDB and GDBserver builtin target 
>> descriptions more flexible, by removing pre-generated ones.  Instead, 
>> these builtin target descriptions can be got lazily and dynamically.
>> GDB builtin target descriptions are created from initialize_tdesc_* 
>> functions in features/*.c files, while GDBserver builtin target 
>> descriptions are generated from regformats/*.dat files.
>> 
>> This patch series changes both GDB and GDBserver to create target 
>> description dynamically from features, instead of using pre-generated 
>> target descriptions.  This patch series only convert x86-linux ( 
>> including i386-linux, amd64-linux and x32-linux) target description to 
>> demonstrate the usefulness of the change.
>> 
>> Once one target architecture switches to the new flexible target 
>> description,
>> 
>> - only need xml feature files under gdb/features directory.  All  
>> existing target description xml files can be kept for the tests.
>> Add new xml feature file if we want to support the new feature,  but 
>> don't need to add new target description xml files.
>> 
>> - All existing gdb/regformats/*.dat are not used, but kept for  the 
>> tests.
>> 
>> This is the V2, and V1 is here
>> https://sourceware.org/ml/gdb-patches/2017-05/msg00291.html,
>> the differences are,
>> 
>> - Change target descriptions for both GDB and GDBserver,
>> - Generate functions creating features from xml feature file
>>   instead of feature name, so that don't have to worry about
>>   different features with the same name (different features
>>   with the same name still have different file names).
>> - Extend the changes for i386-linux to x86-linux (including,
>>   {i386,amd64,x32}-linux)
>> 
>> The big design change in V2 is that use generate c files from xml 
>> feature files, and use generate c files in both GDB and GDBserver.
>> 
>> In next step,  I want to remove the duplication of target descriptions 
>> in GDB and GDBserver, and share more code on creating x86-linux target 
>> descriptions in GDB and GDBserver.
>> I also want people give comments on how to do unit/self tests in 
>> GDBserver, see patch 14.  The purpose of this patch series is still to 
>> demonstrate the design, so the changelog, NEWS entry, and doc may be 
>> incomplete.  I'll complete them later.
>> 
>> Regression tested on x86_64-linux (both -m64 and -m32), native and 
>> gdbserver, on aarch64-linux native and gdbserver.
>> ppc64-linux, native.
> 
> Hi Yao,
> 
> Do you have a git branch we can pull from to look at this change?
> 
> Thanks,
> 
> Simon
> 
> 
> Hello Yao,
> 
> I did some tests and they are looking fine for remote and native gdb.
> I could tweak the XCR0 in GDB and GDBSERVER and see tests that also use cpu-id to detect features failing, as expected! Very nice!
> Thanks again!
> 
> As an observation, in the gdbserver side there are calls to _get_ipa_tdesc. I suppose we could rename this to a more generic name.
> About the design: GDBserver still uses the combination of cpu features to build the target description. 
>                                     On the other hand, the improvement was huge, i.e. I would go to accept the patch and consider an additional patch series to address gdbserver tdesc creation. 
> 
> 

Yao -

I’ve been looking at this set using the branch (qiyao/target-desc-2),
instead of patch by patch.

Parts of gdbserver/linux-x86-tdesc.c is duplicated in i386-linux-tdep.c,
and the same for other -tdesc.c files.
Would it be better to move the -tdesc.c files into a common directory,
maybe nat/ ?

How is linux-x86-tdesc-ipa.o built? I can see the entry in configure.srv,
but I can’t find a .c either in my src or build directory.


Alan.