Re: Patches for remote rom68k debugging with IDP board

[Jim Blandy]

Grzegorz Jablonski <address@hidden> writes:
> I have found a bug occuring with version 3.0 of IDP rom, concerning the
> readout of registers from the board. I attach the patches to the 5.1
> sources for this case. I am afraid, that monitor.c patch may break other
> targets.

Yes, that's the danger of messing with monitor.c.

I think I fixed this problem with the patch below.  It seems to have
been committed to the public sources, but it didn't make it onto the
5.1 branch.

2001-08-29  Jim Blandy  <address@hidden>

        Correctly parse register values provided by the monitor.
        * rom68k-rom.c: #include "value.h".
        (is_hex_digit, hex_digit_value, is_whitespace,
        rom68k_supply_one_register): New static functions.
        (rom68k_supply_register): Call rom68k_supply_one_register, instead
        of monitor_supply_register; the latter was incorrectly parsing 
        the values.
        * Makefile.in (rom68k-rom.o): Note that this now #includes value.h.

Index: gdb/Makefile.in
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/Makefile.in,v
retrieving revision 1.789.2.2.4.1
diff -c -r1.789.2.2.4.1 Makefile.in
*** gdb/Makefile.in     2001/02/07 02:08:52     1.789.2.2.4.1
--- gdb/Makefile.in     2001/08/30 05:04:58
***************
*** 1777,1783 ****
        $(inferior_h) $(remote_utils_h) symfile.h terminal.h
  
  rom68k-rom.o: rom68k-rom.c monitor.h $(bfd_h) gdb_wait.h $(defs_h) 
$(gdbcmd_h) \
!       $(inferior_h) target.h serial.h terminal.h
  
  rs6000-nat.o: rs6000-nat.c $(bfd_h) $(defs_h) $(inferior_h) target.h \
        xcoffsolib.h
--- 1777,1783 ----
        $(inferior_h) $(remote_utils_h) symfile.h terminal.h
  
  rom68k-rom.o: rom68k-rom.c monitor.h $(bfd_h) gdb_wait.h $(defs_h) 
$(gdbcmd_h) \
!       $(inferior_h) $(value_h) target.h serial.h terminal.h
  
  rs6000-nat.o: rs6000-nat.c $(bfd_h) $(defs_h) $(inferior_h) target.h \
        xcoffsolib.h
Index: gdb/rom68k-rom.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/rom68k-rom.c,v
retrieving revision 2.24.66.1
diff -c -r2.24.66.1 rom68k-rom.c
*** gdb/rom68k-rom.c    2001/08/09 01:19:45     2.24.66.1
--- gdb/rom68k-rom.c    2001/08/30 05:05:02
***************
*** 23,31 ****
--- 23,110 ----
  #include "target.h"
  #include "monitor.h"
  #include "serial.h"
+ #include "value.h"   /* for supply_register */
  
+ 
  static void rom68k_open (char *args, int from_tty);
  
+ /* Return true if C is a hex digit.
+    We can't use isxdigit here: that is affected by the current locale;
+    ROM68K is not.  */
+ static int
+ is_hex_digit (int c)
+ {
+   return (('0' <= c && c <= '9')
+           || ('a' <= c && c <= 'f')
+           || ('A' <= c && c <= 'F'));
+ }
+ 
+ 
+ /* Convert hex digit A to a number.  */
+ static int
+ hex_digit_value (int a)
+ {
+   if (a >= '0' && a <= '9')
+     return a - '0';
+   else if (a >= 'a' && a <= 'f')
+     return a - 'a' + 10;
+   else if (a >= 'A' && a <= 'F')
+     return a - 'A' + 10;
+   else
+     error ("Invalid hex digit %d", a);
+ }
+ 
+ 
+ /* Return true iff C is a whitespace character.
+    We can't use isspace here: that is affected by the current locale;
+    ROM68K is not.  */
+ static int
+ is_whitespace (int c)
+ {
+   return (c == ' '
+           || c == '\r'
+           || c == '\n'
+           || c == '\t'
+           || c == '\f');
+ }
+ 
+ 
+ /* Parse a string of hex digits starting at HEX, supply them as the
+    value of register REGNO, skip any whitespace, and return a pointer
+    to the next character.
+ 
+    There is a function in monitor.c, monitor_supply_register, which is
+    supposed to do this job.  However, there is some rather odd stuff
+    in there (whitespace characters don't terminate numbers, for
+    example) that is incorrect for ROM68k.  It's basically impossible
+    to safely tweak monitor_supply_register --- it's used by a zillion
+    other monitors; who knows what behaviors they're depending on.  So
+    instead, we'll just use our own function, which can behave exactly
+    the way we want it to.  */
+ static char *
+ rom68k_supply_one_register (int regno, unsigned char *hex)
+ {
+   ULONGEST value;
+   unsigned char regbuf[MAX_REGISTER_RAW_SIZE];
+ 
+   value = 0;
+   while (*hex != '\0')
+     if (is_hex_digit (*hex))
+       value = (value * 16) + hex_digit_value (*hex++);
+     else
+       break;
+ 
+   /* Skip any whitespace.  */
+   while (is_whitespace (*hex))
+     hex++;
+ 
+   store_unsigned_integer (regbuf, REGISTER_RAW_SIZE (regno), value);
+   supply_register (regno, regbuf);
+ 
+   return hex;
+ }
+ 
+ 
  static void
  rom68k_supply_register (char *regname, int regnamelen, char *val, int vallen)
  {
***************
*** 69,75 ****
  
    if (regno >= 0)
      while (numregs-- > 0)
!       val = monitor_supply_register (regno++, val);
  }
  
  /* This array of registers need to match the indexes used by GDB.
--- 148,154 ----
  
    if (regno >= 0)
      while (numregs-- > 0)
!       val = rom68k_supply_one_register (regno++, val);
  }
  
  /* This array of registers need to match the indexes used by GDB.