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

[Omair Javaid <omair.javaid@linaro.org>]

Hi Alan,

Your patch series look good overall. I wanted to have a look at the overall
code after applying this series but it still doesnt work.

First 2 patches apply just fine. Patch 3 fails.

Thanks!



On 9 March 2018 at 13:21, 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(-)
> >
>
>