Re: [PATCH V3 0/8] Remove gdbserver dependency on xml files

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 6459 bytes --]

Could you please try gdb.gdb/unittest.exp and gdb.server/unittest.exp
As that’ll test every xml file your build uses.
And, if there are any errors could you please send me the .log file contents - think I
should have enough info from there to debug (don’t have an s390 to try this myself).

Your review comments on the other patches make sense. Haven’t had a chance to
try the changes yet, but I’ll reply there if I have any issues with them.

Many thanks for the review!

Alan.

> On 12 Mar 2018, at 17:19, Philipp Rudo <prudo@linux.vnet.ibm.com> wrote:
> 
> Hi Alan,
> 
> sorry for the late response.  Here are my first findings.  I'm afraid there is
> a bug somewhere in you patch set.  When running the testsuite there are quite a
> lot of test from gdb.server failing (at least on s390). I have to take a closer
> look at it tomorrow.
> 
> Nevertheless there are some comments i already have to your patches.
> 
> Thanks
> Philipp
> 
> 
> On Fri, 9 Mar 2018 08:21:36 +0000
> Alan Hayward <Alan.Hayward@arm.com> wrote:
> 
>> Ping for this series please.
>> 
>> Alan.
>> 
>>> On 1 Mar 2018, at 11:38, Alan Hayward <Alan.Hayward@arm.com> wrote:
>>> 
>>> V3 builds on previous review comments, and the additional patches I've
>>> already pushed. Complete patch series pushed to branch users/ahayward/xml3.
>>> 
>>> Summary:
>>> 
>>> For those targets that use new style target descriptions, this set of patches
>>> removes the dependency on xml files. Namely:
>>> * Removes inclusion of xml files within gdbserver.
>>> * Removes the requirement for the .c files in features/ to be generated from
>>> cached xml files.
>>> This is made possible by changing xml descriptions generated by gdbserver, so
>>> that instead of including xml file names, gdbserver now generate a complete
>>> xml description.
>>> 
>>> The second point will be required for aarch64 SVE support, where the register
>>> size are variable. Creating SVE xml files for every possible vector length
>>> would not be feasible. Instead the plan for aarch64 SVE is to hand write the
>>> features/ .c code that would normally be generated from xml.
>>> 
>>> 
>>> XML Generation:
>>> 
>>> In existing code, gdbserver uses C code auto generated from xml files to
>>> create target descriptions. When sending an xml description to GDB, the
>>> function tdesc_get_features_xml () creates an xml containing the name of the
>>> original xml file(s). For example:
>>> 
>>> <!DOCTYPE target SYSTEM "gdb-target.dtd">
>>> <target>
>>> <architecture>i386</architecture>
>>> <osabi>GNU/Linux</osabi>
>>> <xi:include href="32bit-core.xml"/>
>>> <xi:include href="32bit-sse.xml"/>
>>> <xi:include href="32bit-linux.xml"/>
>>> <xi:include href="32bit-avx.xml"/>
>>> </target>
>>> 
>>> Upon receipt, GDB then makes requests to gdbserver for the contents of the
>>> xml files. Gdbserver keeps full copies all the xml files inside the binary.
>>> 
>>> This patch series adds common code that allows gdbserver (and gdb) to turn
>>> a C target description structure into xml.
>>> Now when asked fort an xml description to gdb, gdbserver turns the entire
>>> target description structure back into xml, without using any cached files.
>>> Producing, for example:
>>> 
>>> <!DOCTYPE target SYSTEM "gdb-target.dtd">
>>> <target>
>>> <architecture>i386</architecture>
>>> <osabi>GNU/Linux</osabi>
>>> <feature name="org.gnu.gdb.i386.core">
>>>   <flags id="i386_eflags" size="4">
>>>     <field name="CF" start="0" end="0"/>
>>>     <field name="" start="1" end="1"/>
>>>     <field name="PF" start="2" end="2"/>
>>>     <field name="AF" start="4" end="4"/>
>>> ...etc...
>>> 
>>> 
>>> Patch Contents:
>>> 
>>> Patches 2-4 commonise the various target descriptor functionality, allowing
>>> gdbserver to parse target descriptions in the same way as gdb. This series
>>> does not commonise target_desc, but this is hopefully a long term goal.
>>> 
>>> The sixth patch adds the xml printer, which iterates through the parsing
>>> generated in the previous patches.
>>> 
>>> The other patches are clean up patches.
>>> 
>>> 
>>> 
>>> Patches have been tested on a make check on x86 targets=all build with
>>> target board unix native-gdbserver. Also tested aarch64. Built for power
>>> (because it does not use new target descriptions), but am unable to test.
>>> In addition, patch six adds new test cases to unit test.
>>> 
>>> Alan.
>>> 
>>> gdb/Makefile.in                    |   2 +
>>> gdb/common/tdesc.c                 | 445 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> gdb/common/tdesc.h                 | 313 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>> gdb/features/aarch64-core.c        |   2 +-
>>> gdb/features/aarch64-fpu.c         |   2 +-
>>> gdb/features/i386/32bit-avx.c      |   2 +-
>>> gdb/features/i386/32bit-avx512.c   |   2 +-
>>> gdb/features/i386/32bit-core.c     |   2 +-
>>> gdb/features/i386/32bit-linux.c    |   2 +-
>>> gdb/features/i386/32bit-mpx.c      |   2 +-
>>> gdb/features/i386/32bit-pkeys.c    |   2 +-
>>> gdb/features/i386/32bit-sse.c      |   2 +-
>>> gdb/features/i386/64bit-avx.c      |   2 +-
>>> gdb/features/i386/64bit-avx512.c   |   2 +-
>>> gdb/features/i386/64bit-core.c     |   2 +-
>>> gdb/features/i386/64bit-linux.c    |   2 +-
>>> gdb/features/i386/64bit-mpx.c      |   2 +-
>>> gdb/features/i386/64bit-pkeys.c    |   2 +-
>>> gdb/features/i386/64bit-segments.c |   2 +-
>>> gdb/features/i386/64bit-sse.c      |   2 +-
>>> gdb/features/i386/x32-core.c       |   2 +-
>>> gdb/features/tic6x-c6xp.c          |   2 +-
>>> gdb/features/tic6x-core.c          |   2 +-
>>> gdb/features/tic6x-gp.c            |   2 +-
>>> gdb/gdbserver/Makefile.in          |   3 +
>>> gdb/gdbserver/configure.srv        |  36 -------
>>> gdb/gdbserver/tdesc.c              | 240 ++++++++++++++++++------------------------
>>> gdb/gdbserver/tdesc.h              |  57 ++--------
>>> gdb/regformats/regdat.sh           |   5 +-
>>> gdb/target-descriptions.c          | 596 ++++++++++++--------------------------------------------------------------------------------------------
>>> gdb/xml-tdesc.c                    |   9 ++
>>> gdb/xml-tdesc.h                    |   5 +
>>> 32 files changed, 974 insertions(+), 779 deletions(-)
>>> 
>> 
> 

\x16º&Öéj×!zÊÞ¶êç׎}ß¹b²Ö«r\x18\x1dn–­r\x17¬