problem with "info spu signal" command on spu-gdb

problem with "info spu signal" command on spu-gdb

[Hidetaka Takano]

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

Hello GDB maintainer,

I am working on Linux running on Cell/B.E. system.

I found a bugs in spu-gdb command "info spu signal".

Would you fix this bug?
I made a patch for it and I've attached it.

Details are as follows:

The "info spu signal" command displays wrong type.
It almost always returns "Override".
Especially, by collaborating Cell IDE Eclipse, the value is quite strange.

This problem is caused by what the GDB's internal function "strtoulst()" expects
 the entry of SPUFS should have the LF code (0x0a) and null (0x00) in its end.
The only signal_type doesn't have it.
 (event_status and event_mask are OK)

In order to fix this problem, of course, it is possible to modify the SPUFS.
Tentatively I would like GDB to work around it.

I will also post this problem report to SPUFS maintainer.

Best regard.
---
Hidetaka Takano

[-- Attachment #2: fix-info-spu-signal.diff --]
[-- Type: application/octet-stream, Size: 1913 bytes --]

diff -urpN src-org/gdb/spu-tdep.c src/gdb/spu-tdep.c
--- src-org/gdb/spu-tdep.c	2008-01-11 22:20:02.000000000 +0900
+++ src/gdb/spu-tdep.c	2008-02-15 10:24:20.000000000 +0900
@@ -1505,16 +1505,18 @@ info_spu_event_command (char *args, int 
 
   xsnprintf (annex, sizeof annex, "%d/event_status", id);
   len = target_read (&current_target, TARGET_OBJECT_SPU, annex,
-		     buf, 0, sizeof buf);
+		     buf, 0, (sizeof (buf) - 1));
   if (len <= 0)
     error (_("Could not read event_status."));
+  buf[len] = '\0';
   event_status = strtoulst (buf, NULL, 16);
  
   xsnprintf (annex, sizeof annex, "%d/event_mask", id);
   len = target_read (&current_target, TARGET_OBJECT_SPU, annex,
-		     buf, 0, sizeof buf);
+		     buf, 0, (sizeof (buf) - 1));
   if (len <= 0)
     error (_("Could not read event_mask."));
+  buf[len] = '\0';
   event_mask = strtoulst (buf, NULL, 16);
  
   chain = make_cleanup_ui_out_tuple_begin_end (uiout, "SPUInfoEvent");
@@ -1565,9 +1567,10 @@ info_spu_signal_command (char *args, int
     
   xsnprintf (annex, sizeof annex, "%d/signal1_type", id);
   len = target_read (&current_target, TARGET_OBJECT_SPU, annex,
-		     buf, 0, sizeof buf);
+		     buf, 0, (sizeof (buf) - 1));
   if (len <= 0)
     error (_("Could not read signal1_type."));
+  buf[len] = '\0';
   signal1_type = strtoulst (buf, NULL, 16);
 
   xsnprintf (annex, sizeof annex, "%d/signal2", id);
@@ -1582,9 +1585,10 @@ info_spu_signal_command (char *args, int
     
   xsnprintf (annex, sizeof annex, "%d/signal2_type", id);
   len = target_read (&current_target, TARGET_OBJECT_SPU, annex,
-		     buf, 0, sizeof buf);
+		     buf, 0, (sizeof (buf) - 1));
   if (len <= 0)
     error (_("Could not read signal2_type."));
+  buf[len] = '\0';
   signal2_type = strtoulst (buf, NULL, 16);
 
   chain = make_cleanup_ui_out_tuple_begin_end (uiout, "SPUInfoSignal");

Re: problem with "info spu signal" command on spu-gdb

[Ulrich Weigand]

Hello Takano-san,

> This problem is caused by what the GDB's internal function "strtoulst()" expects
>  the entry of SPUFS should have the LF code (0x0a) and null (0x00) in its end.
> The only signal_type doesn't have it.
>  (event_status and event_mask are OK)
> 
> In order to fix this problem, of course, it is possible to modify the SPUFS.
> Tentatively I would like GDB to work around it.

Thanks for reporting this problem.  I agree that GDB should not rely
on a null terminator being present when reading spufs files, so your
patch should go in.

Could you provide a ChangeLog entry describing the patch?

Thanks,
Ulrich

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

RE: problem with "info spu signal" command on spu-gdb

[Hidetaka Takano]

Hello Ulrich,

Sorry for my late response.

> Could you provide a ChangeLog entry describing the patch?

Is this OK?

2008-02-29  Hidetaka Takano <hidetaka.takano@glb.toshiba.co.jp>

	* spu-tdep.c (info_spu_event_command): insert a '\0' to the end
	of the data passing to strtoulst function.
	* spu-tdep.c (info_spu_signal_command): Likewise.

Best regard.
---
Hidetaka Takano / Toshiba

Re: problem with "info spu signal" command on spu-gdb

[Ulrich Weigand]

Hello Takano-san,

just some minor formatting nits:

> 2008-02-29  Hidetaka Takano <hidetaka.takano@glb.toshiba.co.jp>

There should be two spaces between the name and the e-mail address.

> 	* spu-tdep.c (info_spu_event_command): insert a '\0' to the end
> 	of the data passing to strtoulst function.

Start the sentence describing the change with a capital letter.

> 	* spu-tdep.c (info_spu_signal_command): Likewise.

For multiple changes to the same file, it suffices to name the
file only once.

I've made those changes and committed the patch; here's the
version of the ChangeLog entry I committed:

2008-03-10  Hidetaka Takano  <hidetaka.takano@glb.toshiba.co.jp>

	* spu-tdep.c (info_spu_event_command): Insert a '\0' to the end
	of the data passing to strtoulst function.
	(info_spu_signal_command): Likewise.

Thanks for fixing this problem!

Bye,
Ulrich

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