[RFC] Make version attribute of <target> really optional

[RFC] Make version attribute of <target> really optional

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 2959 bytes --]

Hello,

I'm trying to fix the type of the "lr" register in the XML features
files for rs6000, but I have hit a strange problem: The XML parser
is complaining that the version number in the <target> element is
missing. On the other hand, the documentation says that it is
optional (but recommended).

Looking back at the discussion, it seems that the attribute was
meant to be optional for now. But on the other hand, the implementation
was such that the attribute is NOT optional.

I thought it'd quickly give it a go at making the attribute optional,
and the following seems to be working, but now I'm having other issues
when trying to read the XML file from GDB:

    (gdb) set tdesc filename rs6000/powerpc-32.xml 
    warning: while parsing target description: unbound prefix
    warning: Could not load XML target description; ignoring

This error seems independent of the change I made, as I can reproduce
the same error without my change. It looks like the problem is happening
during the processing of the xi:include tag, but I can't figure out
better what is going on for lack of familiarity with the XML parser.

Just for the record, I'm using expat-2.0.1.

"set debug xml" reveals:

    (gdb) set debug xml 1
    (gdb) set tdesc filename rs6000/powerpc-32.xml
    target description (line 1): Starting:
    <?xml version="1.0"?>
    [snip]
    
    <!DOCTYPE target SYSTEM "gdb-target.dtd">
    <target>
      <architecture>powerpc:common</architecture>
      <xi:include href="power-core.xml"/>
      <xi:include href="power-fpu.xml"/>
      <xi:include href="power-altivec.xml"/>
    </target>
    
    target description (line 12): Entering element <target>
    target description (line 13): Entering element <architecture>
    target description (line 13): Leaving element <architecture>
    warning: while parsing target description: unbound prefix
    warning: Could not load XML target description; ignoring

We raised the failure because we weren't able to find element
"target" in the list of elements passed to us by the parser:

  /* Find this element in the list of the current scope's allowed
     children.  Record that we've seen it.  */

  seen = 1;
  for (element = scope->elements; element && element->name;
       element++, seen <<= 1)
    if (strcmp (element->name, name) == 0)
      break;

It feels like I'm missing something, here... Does it work for someone
else? Daniel, or Ulrich, maybe? You regenerated the files not long ago,
so perhaps it works for you...

The following is the untested patch that I wrote in an attempt to make
the version optional for now:

2008-02-28  Joel Brobecker  <brobecker@adacore.com>

        * xml-tdesc.c (tdesc_start_target): Handle the "version" attribute
        as optional.
        (target_attributes): Make the "version" attribute optional.

Note that another approach would be to keep this attribute mandatory,
update the XML files, and update the documentation as well.

-- 
Joel

[-- Attachment #2: xml-tdesc.c.diff --]
[-- Type: text/plain, Size: 1046 bytes --]

Index: xml-tdesc.c
===================================================================
RCS file: /cvs/src/src/gdb/xml-tdesc.c,v
retrieving revision 1.10
diff -u -p -r1.10 xml-tdesc.c
--- xml-tdesc.c	1 Jan 2008 22:53:13 -0000	1.10
+++ xml-tdesc.c	29 Feb 2008 03:36:06 -0000
@@ -114,7 +114,14 @@ tdesc_start_target (struct gdb_xml_parse
 		    void *user_data, VEC(gdb_xml_value_s) *attributes)
 {
   struct tdesc_parsing_data *data = user_data;
-  char *version = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+  int length;
+  char *version;
+  
+  length = VEC_length (gdb_xml_value_s, attributes);
+  if (length == 0)
+    return;
+
+  version = VEC_index (gdb_xml_value_s, attributes, 0)->value;
 
   if (strcmp (version, "1.0") != 0)
     gdb_xml_error (parser,
@@ -338,7 +345,7 @@ static const struct gdb_xml_element feat
 };
 
 static const struct gdb_xml_attribute target_attributes[] = {
-  { "version", GDB_XML_AF_NONE, NULL, NULL },
+  { "version", GDB_XML_AF_OPTIONAL, NULL, NULL },
   { NULL, GDB_XML_AF_NONE, NULL, NULL }
 };
 

Re: [RFC] Make version attribute of <target> really optional

[Daniel Jacobowitz]

On Thu, Feb 28, 2008 at 07:57:44PM -0800, Joel Brobecker wrote:
> Hello,
> 
> I'm trying to fix the type of the "lr" register in the XML features
> files for rs6000, but I have hit a strange problem: The XML parser
> is complaining that the version number in the <target> element is
> missing. On the other hand, the documentation says that it is
> optional (but recommended).
> 
> Looking back at the discussion, it seems that the attribute was
> meant to be optional for now. But on the other hand, the implementation
> was such that the attribute is NOT optional.

The DTD says:

version         CDATA   #FIXED "1.0"

What's supposed to happen is that expat fills in this value when you
parse the document.  It will complain if version is set to something
besides 1.0, but add version="1.0" if it was not specified.

It looks to me like something is wrong with your expat, or we are
depending on a configuration knob of expat that you've got turned the
other way and we should improve the configure test, or else we are
using expat in a way that broke with 2.0.1 (I usually use 1.95.8 since
that's what Debian has).

> I thought it'd quickly give it a go at making the attribute optional,
> and the following seems to be working, but now I'm having other issues
> when trying to read the XML file from GDB:
> 
>     (gdb) set tdesc filename rs6000/powerpc-32.xml 
>     warning: while parsing target description: unbound prefix
>     warning: Could not load XML target description; ignoring

Ditto.  When this error occurs, which parser is running?  There's two,
one for the gdb-target.dtd and one preprocessor for xi:include.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] Make version attribute of <target> really optional

[Joel Brobecker]

Thanks, Daniel.

> The DTD says:
> 
> version         CDATA   #FIXED "1.0"
> 
> What's supposed to happen is that expat fills in this value when you
> parse the document.  It will complain if version is set to something
> besides 1.0, but add version="1.0" if it was not specified.

Aha, looks like I need to get a crash course in XML :-/. I have
a coworker who wrote XMLAda (an XML parser in Ada), so I'll ask him
when I see him this summer.

In the meantime...

> It looks to me like something is wrong with your expat [...]
> [...] (I usually use 1.95.8 since that's what Debian has).

I don't really understand what is going on. I tried 1.95.8 and 2.0.1
on our ppc-aix machine, and no luck. However, I tried identical
packages on my x86-linux laptop (running Debian GNU/Linux) by configuring
GDB for powerpc-elf, and this time, no problem with either version!

I'm running out of time for this activity, I want to start the 6.8
branch today. So I've reverted to what I did before, which is generate
the new files on my linux machine with ppc-elf debugger, and copy
the new files over to my AiX build for testing.

If I find the time later this summer after I get more litterate about
XML, I'll try to find out where the actual problem is.

-- 
Joel

Re: [RFC] Make version attribute of <target> really optional

[Daniel Jacobowitz]

On Fri, Feb 29, 2008 at 09:23:13AM -0800, Joel Brobecker wrote:
> I don't really understand what is going on. I tried 1.95.8 and 2.0.1
> on our ppc-aix machine, and no luck. However, I tried identical
> packages on my x86-linux laptop (running Debian GNU/Linux) by configuring
> GDB for powerpc-elf, and this time, no problem with either version!

Boo.  Expat may be broken on AIX.  Or maybe its configure script picks
different options?

-- 
Daniel Jacobowitz
CodeSourcery