[rfa, gdbserver] Fix breakage due to XML rework

[rfa, gdbserver] Fix breakage due to XML rework

[Ulrich Weigand]

Hello Dan,

I just noticed that my XML target rework patch introduced a stupid
error in get_features_xml that causes gdbserver to never actually
report a target.xml file.

Once this was fixed, it still didn't work on powerpc as the xmltarget
statement in the regformats file referred to "rs6000/powerpc-32.xml"
while the generated xml-builtins.c file only supported the file
name "powerpc-32.xml" without the subdirectory name.  As all the 
secondary XML files were already using just the base name, I've
now changes the regformats xmltarget statements to also use the
base name only.

The third problem shows up only in --multi mode.  Here it can happen
that on the first call to the target's arch_setup routine, register
settings without any XML target are installed, but on a later call
arch_setup, XML target setting are added.  However, as the target
initially did not support any target.xml file, gdbserver common
code replied to GDB's qSupported query that it does not support
the qXfer:features:read command.  As it appears the qSupported
query is never repeated, GDB will actually never notice that the
target now does provide an target.xml file ...   I've simply changed
handle_query to always claim to support qXfer:features:read; and as
gdbserver actually *does* always handle it, this cannot hurt anything.


I'm not sure how I missed those problems in my original testing.
Interestingly enough, even with target.xml completely broken like
this, gdbserver still passes the altivec-regs.exp test cases ...

Sorry for the breakage.

The patch below fixes all three problems; tested on powerpc-linux
and powerpc64-linux using local gdbserver testing.

OK to apply?

Bye,
Ulrich


ChangeLog:

	* features/Makefile (%.dat): Set xmltarget to the base filename
	of the XML source, without subdirectory.
	* regformats/rs6000/powerpc-32.dat: Regenerate.
	* regformats/rs6000/powerpc-64.dat: Regenerate.
	* regformats/rs6000/powerpc-e500.dat: Regenerate.

gdbserver/ChangeLog:

	* server.c (get_features_xml): Fix inverted condition.
	(handle_query): Always support qXfer:feature:read.


diff -urNp gdb-orig/gdb/features/Makefile gdb-head/gdb/features/Makefile
--- gdb-orig/gdb/features/Makefile	2008-03-27 02:29:55.625448991 +0100
+++ gdb-head/gdb/features/Makefile	2008-03-27 02:29:40.979424568 +0100
@@ -55,7 +55,7 @@ all: $(OUTPUTS)
 $(outdir)/%.dat: %.xml number-regs.xsl sort-regs.xsl gdbserver-regs.xsl
 	echo "# DO NOT EDIT: generated from $<" > $(outdir)/$*.tmp
 	echo "name:`echo $(notdir $*) | sed 's/-/_/g'`" >> $(outdir)/$*.tmp
-	echo "xmltarget:$<" >> $(outdir)/$*.tmp
+	echo "xmltarget:$(<F)" >> $(outdir)/$*.tmp
 	echo "expedite:$($*-expedite)" >> $(outdir)/$*.tmp
 	$(XSLTPROC) --path "$(PWD)" --xinclude number-regs.xsl $< | \
 	  $(XSLTPROC) sort-regs.xsl - | \
diff -urNp gdb-orig/gdb/gdbserver/server.c gdb-head/gdb/gdbserver/server.c
--- gdb-orig/gdb/gdbserver/server.c	2008-03-27 02:29:55.632447986 +0100
+++ gdb-head/gdb/gdbserver/server.c	2008-03-27 00:49:00.822578882 +0100
@@ -276,7 +276,7 @@ get_features_xml (const char *annex)
      init_registers_... routine for the current target.  */
 
   if (gdbserver_xmltarget
-      && strcmp (annex, "target.xml") != 0)
+      && strcmp (annex, "target.xml") == 0)
     {
       if (*gdbserver_xmltarget == '@')
 	return gdbserver_xmltarget + 1;
@@ -618,8 +618,11 @@ handle_query (char *own_buf, int packet_
       if (the_target->qxfer_spu != NULL)
 	strcat (own_buf, ";qXfer:spu:read+;qXfer:spu:write+");
 
-      if (get_features_xml ("target.xml") != NULL)
-	strcat (own_buf, ";qXfer:features:read+");
+      /* We always report qXfer:features:read, as targets may
+	 install XML files on a subsequent call to arch_setup.
+	 If we reported to GDB on startup that we don't support
+	 qXfer:feature:read at all, we will never be re-queried.  */
+      strcat (own_buf, ";qXfer:features:read+");
 
       return;
     }
diff -urNp gdb-orig/gdb/regformats/rs6000/powerpc-32.dat gdb-head/gdb/regformats/rs6000/powerpc-32.dat
--- gdb-orig/gdb/regformats/rs6000/powerpc-32.dat	2008-03-27 02:29:55.637447268 +0100
+++ gdb-head/gdb/regformats/rs6000/powerpc-32.dat	2008-03-27 02:29:41.098407486 +0100
@@ -1,6 +1,6 @@
 # DO NOT EDIT: generated from rs6000/powerpc-32.xml
 name:powerpc_32
-xmltarget:rs6000/powerpc-32.xml
+xmltarget:powerpc-32.xml
 expedite:r1,pc
 32:r0
 32:r1
diff -urNp gdb-orig/gdb/regformats/rs6000/powerpc-64.dat gdb-head/gdb/regformats/rs6000/powerpc-64.dat
--- gdb-orig/gdb/regformats/rs6000/powerpc-64.dat	2008-03-27 02:29:55.640446837 +0100
+++ gdb-head/gdb/regformats/rs6000/powerpc-64.dat	2008-03-27 02:29:41.102406912 +0100
@@ -1,6 +1,6 @@
 # DO NOT EDIT: generated from rs6000/powerpc-64.xml
 name:powerpc_64
-xmltarget:rs6000/powerpc-64.xml
+xmltarget:powerpc-64.xml
 expedite:r1,pc
 64:r0
 64:r1
diff -urNp gdb-orig/gdb/regformats/rs6000/powerpc-e500.dat gdb-head/gdb/regformats/rs6000/powerpc-e500.dat
--- gdb-orig/gdb/regformats/rs6000/powerpc-e500.dat	2008-03-27 02:29:55.644446263 +0100
+++ gdb-head/gdb/regformats/rs6000/powerpc-e500.dat	2008-03-26 23:35:00.476149379 +0100
@@ -1,6 +1,6 @@
 # DO NOT EDIT: generated from rs6000/powerpc-e500.xml
 name:powerpc_e500
-xmltarget:rs6000/powerpc-e500.xml
+xmltarget:powerpc-e500.xml
 expedite:r1,pc
 32:r0
 32:r1
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfa, gdbserver] Fix breakage due to XML rework

[Daniel Jacobowitz]

On Thu, Mar 27, 2008 at 02:53:26AM +0100, Ulrich Weigand wrote:
> OK to apply?

Yes, this all looks OK.  Thanks.  Is this needed for 6.8 too?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfa, gdbserver] Fix breakage due to XML rework

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> On Thu, Mar 27, 2008 at 02:53:26AM +0100, Ulrich Weigand wrote:
> > OK to apply?
> 
> Yes, this all looks OK.  Thanks.  Is this needed for 6.8 too?

Checked in, thanks.  The XML rework was introduced after the
branch point, so these fixes are not needed for 6.8.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfa, gdbserver] Fix breakage due to XML rework

[Luis Machado]

Hi Uli,

On Thu, 2008-03-27 at 14:13 +0100, Ulrich Weigand wrote:
> Daniel Jacobowitz wrote:
> > On Thu, Mar 27, 2008 at 02:53:26AM +0100, Ulrich Weigand wrote:
> > > OK to apply?
> > 
> > Yes, this all looks OK.  Thanks.  Is this needed for 6.8 too?
> 
> Checked in, thanks.  The XML rework was introduced after the
> branch point, so these fixes are not needed for 6.8.

This regressed two testcases for PowerPC (not sure if any other archs
are affected), ext-run.exp and server-run.exp. This might have something
to do with this message that shows up during the execution:

"Protocol error: qXfer:features:read (target-features) conflicting
enabled responses."

Reverting the patch fixes the problem.

Regards,

-- 
Luis Machado
IBM Linux Technology Center

Re: [rfa, gdbserver] Fix breakage due to XML rework

[Ulrich Weigand]

Hi Luis,

> This regressed two testcases for PowerPC (not sure if any other archs
> are affected), ext-run.exp and server-run.exp. This might have something
> to do with this message that shows up during the execution:
> 
> "Protocol error: qXfer:features:read (target-features) conflicting
> enabled responses."
> 
> Reverting the patch fixes the problem.

Doh.  If gdbserver always reports qXfer:features:read as supported,
it must of course also never return "unsupported" status when that
query is executed.  I thought this was already the case, but I 
overlooked this extra check ...   Sorry.

The following patch fixes the regressions on ppc.
OK for mainline?

Bye,
Ulrich


ChangeLog:

	* server.c (handle_query): Never return "unsupported" for
	qXfer:features:read queries.


diff -urNp gdb-orig/gdb/gdbserver/server.c gdb-head/gdb/gdbserver/server.c
--- gdb-orig/gdb/gdbserver/server.c	2008-03-27 14:11:52.000000000 +0100
+++ gdb-head/gdb/gdbserver/server.c	2008-04-04 20:39:26.703101250 +0200
@@ -497,14 +497,6 @@ handle_query (char *own_buf, int packet_
 
       require_running (own_buf);
 
-      /* Check for support.  */
-      document = get_features_xml ("target.xml");
-      if (document == NULL)
-	{
-	  own_buf[0] = '\0';
-	  return;
-	}
-
       /* Grab the annex, offset, and length.  */
       if (decode_xfer_read (own_buf + 20, &annex, &ofs, &len) < 0)
 	{


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfa, gdbserver] Fix breakage due to XML rework

[Daniel Jacobowitz]

On Fri, Apr 04, 2008 at 09:09:50PM +0200, Ulrich Weigand wrote:
> Doh.  If gdbserver always reports qXfer:features:read as supported,
> it must of course also never return "unsupported" status when that
> query is executed.  I thought this was already the case, but I 
> overlooked this extra check ...   Sorry.
> 
> The following patch fixes the regressions on ppc.
> OK for mainline?

Yes, thanks.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfa, gdbserver] Fix breakage due to XML rework

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> On Fri, Apr 04, 2008 at 09:09:50PM +0200, Ulrich Weigand wrote:
> > Doh.  If gdbserver always reports qXfer:features:read as supported,
> > it must of course also never return "unsupported" status when that
> > query is executed.  I thought this was already the case, but I 
> > overlooked this extra check ...   Sorry.
> > 
> > The following patch fixes the regressions on ppc.
> > OK for mainline?
> 
> Yes, thanks.

Applied, thanks!

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com