[V3,0/8] Remove gdbserver dependency on xml files

Message ID 757A8B89-2EF0-46BD-BAA6-6E668538B17F@arm.com
Headers show
Series
  • Remove gdbserver dependency on xml files
Related show

Message

Alan Hayward March 1, 2018, 11:38 a.m.
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(-)

Comments

Alan Hayward March 9, 2018, 8:21 a.m. | #1
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(-)

>
Omair Javaid March 12, 2018, 2:04 p.m. | #2
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(-)

> >

>

>
Philipp Rudo March 12, 2018, 5:19 p.m. | #3
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(-)

> >   

>
Alan Hayward March 13, 2018, 10:16 a.m. | #4
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(-)

>>> 

>> 

>
Philipp Rudo March 13, 2018, 5:58 p.m. | #5
Hi Alan,

i managed to track the bug down (cannot say i understood what the problem is
though). It's in patch #6 please see the comments there.

> Could you please try gdb.gdb/unittest.exp and gdb.server/unittest.exp

> As that’ll test every xml file your build uses.


For the unittests you have to register the target description first. That's not
done for s390 currently.  However i hacked something together and it helped me
find an other bug in patch #6.

Thanks
Philipp

> 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(-)

> >>>   

> >>   

> >   

>