[RFC] New targets remote-rx and extended-remote-rx

[RFC] New targets remote-rx and extended-remote-rx

[Kevin Buettner]

The patch below adds two new targets, remote-rx and
extended-remote-rx, which provide a serial debug interface to
platforms using the Renesas RX architecture.  The interface is
identical to that defined by remote.c except that memory transfer
operations are redefined to do byte swapping under certain conditions.

The new file, remote-rx.c, inherits operations defined in remote.c. 
It overrides several of those operations in order to provide support
for some of the idiosyncracies of the RX architecture.  The long comment
near the beginning of remote-rx.c explains the motivation for this
patch and these new targets.

This work depends upon an additional patch to remote.c and remote.h
which creates the interface for inheriting remote.c's target
operations, the patch for which I've posted separately.

I'm still working on a documentation patch for the user manual.

Comments?

	* configure.tgt (rx-*-elf): Add remote-rx.o to `gdb_target_obs'.
	* remote-rx.c: New file.

Index: configure.tgt
===================================================================
RCS file: /cvs/src/src/gdb/configure.tgt,v
retrieving revision 1.232
diff -u -p -r1.232 configure.tgt
--- configure.tgt	27 Apr 2010 21:01:14 -0000	1.232
+++ configure.tgt	30 Apr 2010 19:55:43 -0000
@@ -413,7 +413,7 @@ s390*-*-*)
 
 rx-*-elf)
 	# Target: Renesas RX
-	gdb_target_obs="rx-tdep.o"
+	gdb_target_obs="rx-tdep.o remote-rx.o"
 	gdb_sim=../sim/rx/libsim.a
 	;;
 
Index: remote-rx.c
===================================================================
RCS file: remote-rx.c
diff -N remote-rx.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ remote-rx.c	30 Apr 2010 19:55:43 -0000
@@ -0,0 +1,721 @@
+/* Remote debugging interface for Renesas RX targets using the GDB remote
+   serial protocol.
+
+   Copyright (C) 2010 Free Software Foundation, Inc.
+
+   Contributed by Kevin Buettner of Red Hat, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "gdbtypes.h"
+#include "gdbcore.h"
+#include "remote.h"
+#include "target.h"
+#include "gdbcmd.h"
+#include "regcache.h"
+#include "readline/readline.h"
+
+#include <sys/types.h>
+#include <fcntl.h>
+#include "gdb_string.h"
+#include "gdb_stat.h"
+#include <ctype.h>
+
+/* When running in big-endian mode, a chip implementing the RX
+   architecture will byte-swap an aligned 32-bit word containing
+   instructions prior to executing those instructions.  When running
+   in little endian mode, no swapping takes place.  For the sake of
+   brevity, and unless otherwise specified, the remainder of the
+   discussion below will assume that the chip is operating in big
+   endian mode.
+   
+   Thus, to the execution unit, words stored in instruction memory are
+   out of order.  It is only due to the fact that the prefetch unit
+   byte-swaps them that they end up being correctly presented to the
+   instruction unit.  The RX architecture has some instructions which
+   are only one byte in width.  Therefore, it's possible to encode
+   up to four instructions in one memory word.  If this is the case,
+   and assuming that those instructions do not change the control
+   flow, the execution unit will execute the instruction at the high
+   address within the word, followed by the instruction at the next
+   highest address, and so on.  The RX architecture also has multi-byte
+   instructions.  These too are byte swapped on a word-by-word basis
+   prior to being presented to the instruction unit.
+
+   I will use the term `memory order' to refer to the order (or
+   endianess) of instruction bytes as they should appear in memory.  I
+   will use the term `execution unit order' to refer to the order of
+   instruction bytes as they appear when presented to the execution
+   unit.
+   
+   Within an executable file, words in code sections are stored in
+   in `memory order'.  Thus words in an executable file could be
+   directly transferred to an RX target's memory without requiring
+   any swapping.  `Memory order', however, is often not the most
+   natural order for the rest of the toolchain to operate on the
+   instructions.  E.g, when disassembling a sequence of instructions,
+   the disassembler will want to look at the instruction bytes in the
+   same order as that with which the execution unit of the chip sees
+   them, i.e, it'll want to look at them in `execution unit order'.
+   
+   The BFD function which fetches the contents of a section
+   (bfd_get_section_contents) has been written to byte-swap the words
+   in the big-endian code sections, thus re-ordering them to be in
+   `execution unit order'.  As shown above, this makes it easier to
+   implement some of the tools which use the BFD library, including
+   portions of GDB.
+
+   When communicating with a big-endian RX target using GDB's remote
+   serial protocol, we must ensure that the data representing
+   instructions is written to that target using `memory order'. 
+   Internally though, GDB is using `execution unit order' for
+   representing instructions.  This means that GDB must byte swap
+   aligned words representing one or more instructions, the intent
+   being to present the words to the debug agent in a format whereby
+   they may be written to memory without needing to be further
+   swapped.  (Keep in mind, however, that should the protocol used
+   mandate additional swapping, this swapping will be done uniformly
+   to all memory words, irrespective of whether they represent code or
+   data.)
+
+   Likewise, when reading memory from the debug agent, instruction
+   words in `memory order' must be swapped to be in `execution unit
+   order' for GDB's internal use.
+
+   To further complicate matters, the contents of memory associated
+   with "data" addresses should not be swapped.  Thus, when in big
+   endian mode, we must take care to byte swap memory words only
+   associated with code addresses.
+   
+   This file subclasses, in a sense, the code contained in remote.c,
+   overriding only the `to_open', `to_load', and `to_xfer_partial'
+   operations.
+
+   The `to_open' operation needs to be overridden in order to push
+   either the remote-rx or extended-remote-rx target vector onto the
+   target stack.
+
+   The `to_load' operation is overridden so that memory addresses
+   associated with code sections needing to be swapped are located and
+   remembered for future memory transfer operations.  The `to_load'
+   operation associated with remote.c is invoked at the end, thus
+   effecting the actual load operation.
+
+   The `to_xfer_partial' operation is overridden to effect the
+   byte-swapping of instruction memory found during `to_load'
+   operation.  Again, the `to_xfer_partial' operation of remote.c is
+   invoked to do the actual work of communicating with the target.  */
+
+
+/* Target vectors for standard remote and extended remote operation.  */
+
+static struct target_ops remote_rx_ops;
+static struct target_ops extended_remote_rx_ops;
+
+/* This module inherits the operations defined in remote.c and overrides
+   several of its operations.  However, we still need access to some of
+   the overridden operations.  The following variable declarations will
+   be set to refer to those remote.c operations that have been overridden
+   in the target vector of this module.  */
+
+static void (*remote_load) (char *, int);
+static LONGEST (*remote_xfer_partial) (struct target_ops * ops,
+				       enum target_object object,
+				       const char *annex, gdb_byte *readbuf,
+				       const gdb_byte *writebuf,
+				       ULONGEST offset, LONGEST len);
+
+/* Define a a struct representing an address range.  */
+
+struct address_range
+{
+  /* Start and end addresses.  The end address is actually one byte beyond
+     the last address in the range.  */
+  ULONGEST start, end;
+
+  /* Pointer to the next address range.  */
+  struct address_range *next;
+};
+
+/* Keep track of the address ranges which need to be byte-swapped.  */
+
+static struct address_range *swap_ranges;
+
+/* Open a connection to a remote debugger.
+   NAME is the filename used for communication.  */
+
+static void
+remote_rx_open (char *name, int from_tty)
+{
+  remote_open_target (name, from_tty, &remote_rx_ops, 0);
+}
+
+/* Open a connection to a remote debugger using the extended remote gdb
+   protocol.  NAME is the filename used for communication.  */
+
+static void
+extended_remote_rx_open (char *name, int from_tty)
+{
+  remote_open_target (name, from_tty, &extended_remote_rx_ops, 1);
+}
+
+
+/* Clear the address range denoted by RANGES.  */
+
+static void
+clear_ranges (struct address_range **ranges)
+{
+  struct address_range *r, *nr;
+
+  r = *ranges;
+  while (r)
+    {
+      nr = r->next;
+      xfree (r);
+      r = nr;
+    }
+  *ranges = NULL;
+}
+
+/* Add address range starting at START and ending at END to address
+   range(s) RP.  The list of address ranges is kept sorted, low to high,
+   by starting address.  When adding a range which overlaps a range
+   already in the list, the ranges are merged.  */
+
+static void
+add_range (struct address_range **rp, ULONGEST start, ULONGEST end)
+{
+  struct address_range *nr;
+
+  while (*rp)
+    {
+      if ((*rp)->end < start)
+	rp = &((*rp)->next);
+      else if (end < (*rp)->start)
+	break;
+      else
+	{
+	  /* Otherwise, there's some overlap.  Extend the range specification
+	     under consideration and return.  */
+
+	  if (start < (*rp)->start)
+	    (*rp)->start = start;
+	  if (end > (*rp)->end)
+	    (*rp)->end = end;
+	  return;
+	}
+    }
+
+  /* If we exit the above loop, we need to create a new address range.  */
+
+  nr = xmalloc (sizeof (struct address_range));
+  nr->start = start;
+  nr->end = end;
+  nr->next = *rp;
+  *rp = nr;
+}
+
+/* Iterate over an address range starting at START and ending at END.
+   For each iteration, choose the maximal range that is either
+   entirely within or not within one of the subranges partioned by RP. 
+   The start and end for the current iteration are stored in
+   ITER_START_PTR and ITER_END_PTR.  Start and end addresses for the
+   next iteration are stored in NEXT_START_PTR and NEXT_END_PTR. 
+   IN_RANGE_PTR will be set to 1 when the values stored at
+   ITER_START_PTR and ITER_END_PTR are within one of the ranges
+   specified by RP.  Return 0 when there are no more iterations left,
+   and 1 otherwise.  */
+
+static int
+range_iter (const struct address_range *rp,
+	    ULONGEST start, ULONGEST end,
+	    ULONGEST * next_start_ptr, ULONGEST * next_end_ptr,
+	    ULONGEST * iter_start_ptr, ULONGEST * iter_end_ptr,
+	    int *in_range_ptr)
+{
+  if (start >= end)
+    return 0;
+
+  while (rp)
+    {
+      if (start < rp->start)
+	{
+	  /* Start is outside of one of the saved regions.  */
+	  if (end <= rp->start)
+	    {
+	      /* End is also outside of one of the saved regions.  This case
+	         is handled by exiting the loop.  */
+	      break;
+	    }
+	  else
+	    {
+	      /* Address range intersects one or more of the saved regions.
+	         Handle the part before the region under consideration.  */
+	      *iter_start_ptr = start;
+	      *iter_end_ptr = rp->start;
+	      *next_start_ptr = rp->start;
+	      *next_end_ptr = end;
+	      *in_range_ptr = 0;
+	      return 1;
+	    }
+	}
+      else if (start < rp->end)
+	{
+	  /* Start is inside one of the saved regions.  */
+	  if (end <= rp->end)
+	    {
+	      /* End is also inside one of the regions.  */
+	      *iter_start_ptr = start;
+	      *iter_end_ptr = end;
+	      *next_start_ptr = end;
+	      *next_end_ptr = end;
+	      *in_range_ptr = 1;
+	      return 1;
+	    }
+	  else
+	    {
+	      /* End is outside of the region under consideration.  */
+	      *iter_start_ptr = start;
+	      *iter_end_ptr = rp->end;
+	      *next_start_ptr = rp->end;
+	      *next_end_ptr = end;
+	      *in_range_ptr = 1;
+	      return 1;
+	    }
+	}
+      else
+	{
+	  /* Advance to the next region.  */
+	  rp = rp->next;
+	}
+    }
+
+  /* Start and end are not within any region.  */
+  *iter_start_ptr = start;
+  *iter_end_ptr = end;
+  *next_start_ptr = end;
+  *next_end_ptr = end;
+  *in_range_ptr = 0;
+  return 1;
+}
+
+/* Byte swap, in groups of 4, the bytes in buffer BUFFER of length
+   LEN.  If the buffer length is not a multiple of 4, the LEN % 4
+   bytes at the end of the buffer are *not* swapped.  */
+
+static void
+byte_swap (gdb_byte *buffer, LONGEST len)
+{
+  LONGEST offset;
+
+  for (offset = 0; offset < len - 3; offset += 4)
+    {
+      gdb_byte b0, b1, b2, b3;
+      b0 = buffer[offset + 0];
+      b1 = buffer[offset + 1];
+      b2 = buffer[offset + 2];
+      b3 = buffer[offset + 3];
+
+      buffer[offset + 0] = b3;
+      buffer[offset + 1] = b2;
+      buffer[offset + 2] = b1;
+      buffer[offset + 3] = b0;
+    }
+}
+
+/* Opaque data for find_sections_to_swap_callback.  */
+
+struct find_sections_to_swap_data
+{
+  unsigned long load_offset;
+};
+
+/* Callback service function for remote_rx_load (bfd_map_over_sections).  */
+
+static void
+find_sections_to_swap_callback (bfd *abfd, asection *asec, void *data)
+{
+  struct find_sections_to_swap_data *args = data;
+  bfd_size_type size = bfd_get_section_size (asec);
+
+  if ((bfd_get_section_flags (abfd, asec) & SEC_LOAD) == 0)
+    return;
+
+  if (size == 0)
+    return;
+
+  if ((asec->flags & SEC_CODE) && bfd_big_endian (abfd))
+    {
+      ULONGEST lma = bfd_section_lma (abfd, asec) + args->load_offset;
+
+      add_range (&swap_ranges, lma, lma + size);
+    }
+}
+
+/* Implementation of to_load.  */
+
+static void
+remote_rx_load (char *args, int from_tty)
+{
+  bfd *loadfile_bfd;
+  char *filename;
+  struct cleanup *old_cleanups = make_cleanup (null_cleanup, 0);
+  struct find_sections_to_swap_data cbdata;
+
+  CORE_ADDR entry;
+  char **argv;
+
+  memset (&cbdata, 0, sizeof (cbdata));
+
+  if (args == NULL)
+    error_no_arg (_("file to load"));
+
+  argv = gdb_buildargv (args);
+  make_cleanup_freeargv (argv);
+
+  filename = tilde_expand (argv[0]);
+  make_cleanup (xfree, filename);
+
+  if (argv[1] != NULL)
+    {
+      char *endptr;
+
+      cbdata.load_offset = strtoul (argv[1], &endptr, 0);
+
+      /* If the last word was not a valid number then
+         treat it as a file name with spaces in.  */
+      if (argv[1] == endptr)
+	error (_("Invalid download offset:%s."), argv[1]);
+
+      if (argv[2] != NULL)
+	error (_("Too many parameters."));
+    }
+
+  /* Open the file for loading. */
+  loadfile_bfd = bfd_openr (filename, gnutarget);
+  if (loadfile_bfd == NULL)
+    {
+      perror_with_name (filename);
+      return;
+    }
+
+  /* FIXME: should be checking for errors from bfd_close (for one thing,
+     on error it does not free all the storage associated with the
+     bfd).  */
+  make_cleanup_bfd_close (loadfile_bfd);
+
+  if (!bfd_check_format (loadfile_bfd, bfd_object))
+    {
+      error (_("\"%s\" is not an object file: %s"), filename,
+	     bfd_errmsg (bfd_get_error ()));
+    }
+
+  /* Find the SEC_CODE sections in the executable file.  Put the address
+     ranges denoted by these sections into `swap_ranges'.  */
+  clear_ranges (&swap_ranges);
+  bfd_map_over_sections (loadfile_bfd, find_sections_to_swap_callback,
+			 &cbdata);
+
+  /* Clean up temporary storage allocated in this function.  */
+  do_cleanups (old_cleanups);
+
+  /* Invoke remote.c's to_load method.  This will cause the file to actually
+     be loaded.  Byte-swapping of SEC_CODE sections will take place during
+     as part of the transfer.  */
+  remote_load (args, from_tty);
+}
+
+/* Read or write OBJECT_NAME/ANNEX from the remote target using
+   remote_xfer_partial().  When OBJECT_NAME/ANNEX refers to
+   memory, data written to address ranges denoted by the global
+   SWAP_RANGES will be byte-swapped.
+
+   When READBUF is non-NULL, data at OFFSET of up to LEN bytes is
+   read into READBUF.  The number of bytes read is returned, or -1
+   indicating that an error occurred.
+
+   When WRITEBUF is non-NULL, data of length LEN in WRITEBUF is
+   written to the target starting at OFFSET.  (OFFSET is often a
+   memory address.)  The number of bytes written is returned, or
+   -1 indicating that an error has occurred.
+
+   Only one of READBUF or WRITEBUF should be non-NULL.  */
+
+static LONGEST
+remote_rx_xfer_partial (struct target_ops *ops, enum target_object object,
+			const char *annex, gdb_byte *readbuf,
+			const gdb_byte *writebuf, ULONGEST offset,
+			LONGEST len)
+{
+  if (object == TARGET_OBJECT_MEMORY)
+    {
+      ULONGEST start, end, istart, iend;
+      int do_swap;
+      LONGEST tot_len = 0;
+
+      start = offset;
+      end = offset + len;
+
+      while (range_iter (swap_ranges, start, end, &start, &end,
+			 &istart, &iend, &do_swap))
+	{
+	  if (do_swap)
+	    {
+	      if (readbuf)
+		{
+		  LONGEST fetch_len;
+
+		  /* Fetch and swap unaligned bytes at beginning.  */
+		  if ((istart & 0x3) != 0)
+		    {
+		      gdb_byte buf[4];
+		      LONGEST len;
+
+		      fetch_len =
+			remote_xfer_partial (ops, object, annex, buf, NULL,
+					     align_down (istart, 4), 4);
+		      if (fetch_len < 0)
+			return fetch_len;
+		      else if (fetch_len != 4)
+			return tot_len;
+
+		      byte_swap (buf, 4);
+
+		      len = 4 - (istart & 0x3);
+		      if (iend - istart < len)
+			len = iend - istart;
+
+		      memcpy (readbuf + (istart - offset),
+			      buf + (istart & 0x3), len);
+
+		      tot_len += len;
+
+		      istart = align_up (istart, 4);
+		    }
+
+
+		  /* Fetch and swap aligned bytes in the middle.  */
+		  if (istart < align_down (iend, 4))
+		    {
+		      fetch_len =
+			remote_xfer_partial (ops, object, annex,
+					     readbuf + (istart - offset),
+					     NULL, istart,
+					     align_down (iend, 4) - istart);
+
+		      if (fetch_len < 0)
+			return fetch_len;
+
+		      byte_swap (readbuf + (istart - offset),
+				 align_down (fetch_len, 4));
+
+		      tot_len += align_down (fetch_len, 4);
+
+		      if (fetch_len != align_down (iend, 4) - istart)
+			return tot_len;
+		    }
+
+		  /* Fetch and swap unaligned bytes at end.  */
+		  if (istart < iend && (iend & 0x3) != 0)
+		    {
+		      gdb_byte buf[4];
+
+		      fetch_len =
+			remote_xfer_partial (ops, object, annex, buf, NULL,
+					     align_down (iend, 4), 4);
+		      if (fetch_len < 0)
+			return fetch_len;
+		      else if (fetch_len != 4)
+			return tot_len;
+
+		      byte_swap (buf, 4);
+
+		      memcpy (readbuf + (align_down (iend, 4) - offset),
+			      buf, iend & 0x3);
+
+		      tot_len += (iend & 0x3);
+		    }
+		}
+	      else if (writebuf)
+		{
+		  gdb_byte *wbuf;
+		  LONGEST wbuflen, write_len;
+		  struct cleanup *old_chain;
+
+		  /* Byte swap and write unaligned bytes at beginning.  */
+		  if ((istart & 0x3) != 0)
+		    {
+		      gdb_byte buf[4];
+		      LONGEST fetch_len, len;
+
+		      fetch_len =
+			remote_xfer_partial (ops, object, annex, buf, NULL,
+					     align_down (istart, 4), 4);
+		      if (fetch_len < 0)
+			return fetch_len;
+		      else if (fetch_len != 4)
+			return tot_len;
+
+		      byte_swap (buf, 4);
+
+		      len = 4 - (istart & 0x3);
+		      if (iend - istart < len)
+			len = iend - istart;
+
+		      memcpy (buf + (istart & 0x3),
+			      writebuf + (istart - offset), len);
+
+		      byte_swap (buf, 4);
+
+		      write_len =
+			remote_xfer_partial (ops, object, annex, NULL, buf,
+					     align_down (istart, 4), 4);
+		      if (write_len < 0)
+			return write_len;
+		      else if (write_len != 4)
+			return tot_len;
+
+		      tot_len += len;
+
+		      istart = align_up (istart, 4);
+		    }
+
+		  /* Byte swap and write aligned bytes in the middle.  */
+		  if (istart < align_down (iend, 4))
+		    {
+		      wbuflen = align_down (iend, 4) - istart;
+		      wbuf = xmalloc (wbuflen);
+		      old_chain = make_cleanup (xfree, wbuf);
+
+		      memcpy (wbuf, writebuf + (istart - offset), wbuflen);
+		      byte_swap (wbuf, wbuflen);
+
+		      write_len =
+			remote_xfer_partial (ops, object, annex, NULL,
+					     wbuf, istart, wbuflen);
+
+		      do_cleanups (old_chain);
+
+		      if (write_len < 0)
+			return write_len;
+		      tot_len += write_len;
+		      if (write_len != wbuflen)
+			return tot_len;
+		    }
+
+		  /* Byte swap and write unaligned bytes at end.  */
+		  if (istart < iend && (iend & 0x03) != 0)
+		    {
+		      gdb_byte buf[4];
+		      LONGEST fetch_len;
+
+		      fetch_len =
+			remote_xfer_partial (ops, object, annex, buf, NULL,
+					     align_down (iend, 4), 4);
+		      if (fetch_len < 0)
+			return fetch_len;
+		      else if (fetch_len != 4)
+			return tot_len;
+
+		      byte_swap (buf, 4);
+
+		      memcpy (buf, writebuf + (align_down (iend, 4) - offset),
+			      iend & 0x3);
+
+		      byte_swap (buf, 4);
+
+		      write_len =
+			remote_xfer_partial (ops, object, annex, NULL, buf,
+					     align_down (iend, 4), 4);
+
+		      if (write_len < 0)
+			return write_len;
+		      else if (write_len != 4)
+			return tot_len;
+
+		      tot_len += (iend & 0x3);
+		    }
+		}
+	    }
+	  else
+	    {
+	      LONGEST xfer_len;
+
+	      xfer_len =
+		remote_xfer_partial (ops, object, annex,
+				     readbuf ? readbuf + (istart -
+							  offset) : NULL,
+				     writebuf ? writebuf + (istart -
+							    offset) : NULL,
+				     istart, iend - istart);
+
+	      if (xfer_len < 0)
+		return 0;
+
+	      tot_len += xfer_len;
+
+	      if (xfer_len != iend - istart)
+		return tot_len;
+
+	    }
+	}
+      return tot_len;
+    }
+  else
+    return remote_xfer_partial (ops, object, annex, readbuf, writebuf,
+				offset, len);
+}
+
+/* Initialize remote_rx_ops, extended_remote_rx_ops, remote_load, and
+   remote_xfer_partial.  */
+
+static void
+init_remote_rx_ops (void)
+{
+  init_remote_ops (&remote_rx_ops);
+
+  remote_rx_ops.to_shortname = "remote-rx";
+  remote_rx_ops.to_longname =
+    "Remote target of RX architecture using GDB remote serial protocol";
+  remote_rx_ops.to_open = remote_rx_open;
+
+  remote_load = remote_rx_ops.to_load;
+  remote_rx_ops.to_load = remote_rx_load;
+
+  remote_xfer_partial = remote_rx_ops.to_xfer_partial;
+  remote_rx_ops.to_xfer_partial = remote_rx_xfer_partial;
+
+  add_target (&remote_rx_ops);
+
+  init_extended_remote_ops (&extended_remote_rx_ops);
+  extended_remote_rx_ops.to_shortname = "extended-remote-rx";
+  extended_remote_rx_ops.to_longname =
+    "Extended remote target of RX architecture using GDB remote serial protocol";
+  extended_remote_rx_ops.to_open = extended_remote_rx_open;
+  extended_remote_rx_ops.to_load = remote_rx_load;
+  extended_remote_rx_ops.to_xfer_partial = remote_rx_xfer_partial;
+
+  add_target (&extended_remote_rx_ops);
+}
+
+/* Module specific initializations.  */
+
+void
+_initialize_remote_rx (void)
+{
+  init_remote_rx_ops ();
+}

Re: [RFC] New targets remote-rx and extended-remote-rx

[Pedro Alves]

On Friday 30 April 2010 22:42:18, Kevin Buettner wrote:
> The patch below adds two new targets, remote-rx and
> extended-remote-rx, which provide a serial debug interface to
> platforms using the Renesas RX architecture.  The interface is
> identical to that defined by remote.c except that memory transfer
> operations are redefined to do byte swapping under certain conditions.
> 
> The new file, remote-rx.c, inherits operations defined in remote.c. 
> It overrides several of those operations in order to provide support
> for some of the idiosyncracies of the RX architecture.  The long comment
> near the beginning of remote-rx.c explains the motivation for this
> patch and these new targets.

I'm not convinced this inheritance is a good idea.  Why not
handle this in the regular remote target instead?  Adding new
targets is evil.  :-)  That is, say, with a gdbarch flag telling
the remote.c target to handle this when needed.  (or a
qSupported feature, or  a new feature in the xml target
description if it is expected that stubs might handle this
themselves somehow)

This would remove the burden from the user/frontend, of
knowing upfront the idiosyncracies of this architecture,
and having to remember to connect with a special target.

Or, even, imagine that at some point you will have a native
gdb running on such architecture.  This raises the question of
whether it would make more sense to make the common memory reading
code handle this independent of target_ops instead.  On first
sight, it seems to.  I'm not sure I grasped it enough to understand
if this could benefit from a TARGET_OBJECT_MEMORY_CODE vs
TARGET_OBJECT_MEMORY_DATA request distinction, instead of just
calling everything TARGET_OBJECT_MEMORY_CODE?  That is switch
the swapping decision to the transfer intent, not to where
the code is in memory.  For example, what should happen
if I build a buffer of executable code in memory at runtime,
and I want to disassemble it with GDB?  I'll build the memory
buffer, with a layout as the compiler puts things in .text,
but the code will not be in .text, yet, don't I want for
GDB to read it in execute order, not memory order?

Lastly, is there any relation between the new address_range
structure, and struct addrmap?

-- 
Pedro Alves

Re: [RFC] New targets remote-rx and extended-remote-rx

[Joseph S. Myers]

On Fri, 30 Apr 2010, Kevin Buettner wrote:

> +   Within an executable file, words in code sections are stored in
> +   in `memory order'.  Thus words in an executable file could be
> +   directly transferred to an RX target's memory without requiring
> +   any swapping.  `Memory order', however, is often not the most
> +   natural order for the rest of the toolchain to operate on the
> +   instructions.  E.g, when disassembling a sequence of instructions,
> +   the disassembler will want to look at the instruction bytes in the
> +   same order as that with which the execution unit of the chip sees
> +   them, i.e, it'll want to look at them in `execution unit order'.

The issue of having memory order different from execution unit order is 
not unique to the RX, so one question would be what is the right general 
approach to handle such issues for other targets as well.

The case I know of is the TI C6X processors (C64X+ and C674X are the only 
ones affected by this issue).  The C64X+ and C674X have a compact 
instruction encoding where some instructions are 16 bits and some are 
32 bits; 256-bit fetch packets can have a header saying which 32-bit words 
are a single instruction and which are two 16-bit instructions.  In 
execution unit order, the least significant half of a pair of 16-bit 
instructions always comes before the most significant half, leading to 
them being out of order in memory order in the big-endian case.

On this processor, branch instructions work with logical addresses, where 
a 4-byte-aligned address pointing to a pair of 16-bit instructions always 
refers to the least-significant - first in execution unit order - of those 
instructions, rather than with physical addresses.  External code symbols 
are required to be 4-byte-aligned (most branch instructions only support 
4-byte-aligned target addresses).  (The ISA manuals don't make this clear, 
but it's been confirmed with TI that this is how the processors work and 
what the ABI should be.)

We haven't yet started the GDB port or the assembler support for 16-bit 
instructions, but for the disassembler the approach we've followed (see 
opcodes/tic6x-dis.c, which can handle 16-bit instructions to the extent of 
knowing what's 32-bit and what's 16-bit although they aren't yet in the 
opcode table), which is also the approach followed by TI's tools, is that 
when asked to disassemble the instruction at a given address it treats it 
as a logical rather than a physical address.  So BFD doesn't need to do 
anything special regarding this peculiarity, but the assembler will need 
to swap instructions at the last minute when it gets support for 16-bit 
instructions, and of course there may be GDB issues such as you found (but 
possibly a different set of issues because of not making BFD do the 
swapping).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC] New targets remote-rx and extended-remote-rx

[Kevin Buettner]

On Fri, 30 Apr 2010 23:35:44 +0100
Pedro Alves <pedro@codesourcery.com> wrote:

> On Friday 30 April 2010 22:42:18, Kevin Buettner wrote:
> > The patch below adds two new targets, remote-rx and
> > extended-remote-rx, which provide a serial debug interface to
> > platforms using the Renesas RX architecture.  The interface is
> > identical to that defined by remote.c except that memory transfer
> > operations are redefined to do byte swapping under certain conditions.
> > 
> > The new file, remote-rx.c, inherits operations defined in remote.c. 
> > It overrides several of those operations in order to provide support
> > for some of the idiosyncracies of the RX architecture.  The long comment
> > near the beginning of remote-rx.c explains the motivation for this
> > patch and these new targets.
> 
> I'm not convinced this inheritance is a good idea.  Why not
> handle this in the regular remote target instead?

I didn't want to complicate remote.c (or other parts of GDB) with code
which would be used for very few (possibly only one) target(s). 
However, Joseph's reply suggests that RX's idiosyncracies may not be
as uncommon as I first thought.  I'm willing to consider other
approaches to solving this problem.

> Adding new
> targets is evil.  :-)  That is, say, with a gdbarch flag telling
> the remote.c target to handle this when needed.  (or a
> qSupported feature, or  a new feature in the xml target
> description if it is expected that stubs might handle this
> themselves somehow)
> 
> This would remove the burden from the user/frontend, of
> knowing upfront the idiosyncracies of this architecture,
> and having to remember to connect with a special target.

This is a good point.  For RX, if the user were to use "target remote"
instead of "target remote-rx", things will definitely not work.

> Or, even, imagine that at some point you will have a native
> gdb running on such architecture.  This raises the question of
> whether it would make more sense to make the common memory reading
> code handle this independent of target_ops instead.  On first
> sight, it seems to.

Yes, this might make sense.  (Though, at present, the RX sim has
memory swapping code on the sim side of things.  But it'll be easy to
remove the swapping code from the sim.  And it would make sense to
have it done in one place in GDB.)

I'll start poking around in the common memory reading code.  (I
did take a quick look before starting remote-rx.c.  It turned
out to be a bit more complicated than I expected it to be.)

> I'm not sure I grasped it enough to understand
> if this could benefit from a TARGET_OBJECT_MEMORY_CODE vs
> TARGET_OBJECT_MEMORY_DATA request distinction, instead of just
> calling everything TARGET_OBJECT_MEMORY_CODE?  That is switch
> the swapping decision to the transfer intent, not to where
> the code is in memory.

I'll have to think about this.  Doesn't sound unreasonable though.

> For example, what should happen
> if I build a buffer of executable code in memory at runtime,
> and I want to disassemble it with GDB?  I'll build the memory
> buffer, with a layout as the compiler puts things in .text,
> but the code will not be in .text, yet, don't I want for
> GDB to read it in execute order, not memory order?

E.g, a just-in-time (JIT) compiler?  Yes, you'd want GDB to convert
dynamically compiled code from memory order to execute order.

> Lastly, is there any relation between the new address_range
> structure, and struct addrmap?

I probably could have used struct addrmap instead of introducing a new
data structure.

Kevin

Re: [RFC] New targets remote-rx and extended-remote-rx

[Kevin Buettner]

On Fri, 30 Apr 2010 23:23:07 +0000 (UTC)
"Joseph S. Myers" <joseph@codesourcery.com> wrote:

> On Fri, 30 Apr 2010, Kevin Buettner wrote:
> 
> > +   Within an executable file, words in code sections are stored in
> > +   in `memory order'.  Thus words in an executable file could be
> > +   directly transferred to an RX target's memory without requiring
> > +   any swapping.  `Memory order', however, is often not the most
> > +   natural order for the rest of the toolchain to operate on the
> > +   instructions.  E.g, when disassembling a sequence of instructions,
> > +   the disassembler will want to look at the instruction bytes in the
> > +   same order as that with which the execution unit of the chip sees
> > +   them, i.e, it'll want to look at them in `execution unit order'.
> 
> The issue of having memory order different from execution unit order is 
> not unique to the RX, so one question would be what is the right general 
> approach to handle such issues for other targets as well.
> 
> The case I know of is the TI C6X processors (C64X+ and C674X are the only 
> ones affected by this issue).  The C64X+ and C674X have a compact 
> instruction encoding where some instructions are 16 bits and some are 
> 32 bits; 256-bit fetch packets can have a header saying which 32-bit words 
> are a single instruction and which are two 16-bit instructions.  In 
> execution unit order, the least significant half of a pair of 16-bit 
> instructions always comes before the most significant half, leading to 
> them being out of order in memory order in the big-endian case.
> 
> On this processor, branch instructions work with logical addresses, where 
> a 4-byte-aligned address pointing to a pair of 16-bit instructions always 
> refers to the least-significant - first in execution unit order - of those 
> instructions, rather than with physical addresses.  External code symbols 
> are required to be 4-byte-aligned (most branch instructions only support 
> 4-byte-aligned target addresses).  (The ISA manuals don't make this clear, 
> but it's been confirmed with TI that this is how the processors work and 
> what the ABI should be.)
> 
> We haven't yet started the GDB port or the assembler support for 16-bit 
> instructions, but for the disassembler the approach we've followed (see 
> opcodes/tic6x-dis.c, which can handle 16-bit instructions to the extent of 
> knowing what's 32-bit and what's 16-bit although they aren't yet in the 
> opcode table), which is also the approach followed by TI's tools, is that 
> when asked to disassemble the instruction at a given address it treats it 
> as a logical rather than a physical address.  So BFD doesn't need to do 
> anything special regarding this peculiarity, but the assembler will need 
> to swap instructions at the last minute when it gets support for 16-bit 
> instructions, and of course there may be GDB issues such as you found (but 
> possibly a different set of issues because of not making BFD do the 
> swapping).

For RX, I have mixed feelings about having BFD do the swapping.  On
the one hand, it makes coding certain parts of the toolchain easier. 
But, on the other hand, it introduces the need to do memory-order-to-
execute-order swapping when reading memory from a target.  (And vice
versa, when writing.) But then I remind myself that any instruction
decoding code will have to perform this swapping instead, which then
makes me think that the swapping in BFD isn't so bad after all.  And,
in addition, non-aligned code addresses generally refer to execute
order, rather than memory order.  I honestly don't know what the
"right" way to do it should be.

Kevin

Re: [RFC] New targets remote-rx and extended-remote-rx

[Pedro Alves]

On Monday 03 May 2010 23:30:29, Kevin Buettner wrote:
> On Fri, 30 Apr 2010 23:35:44 +0100
> Pedro Alves <pedro@codesourcery.com> wrote:

> > I'm not sure I grasped it enough to understand
> > if this could benefit from a TARGET_OBJECT_MEMORY_CODE vs
> > TARGET_OBJECT_MEMORY_DATA request distinction, instead of just
> > calling everything TARGET_OBJECT_MEMORY?  That is switch
> > the swapping decision to the transfer intent, not to where
> > the code is in memory.
> 
> I'll have to think about this.  Doesn't sound unreasonable though.
> 
> > For example, what should happen
> > if I build a buffer of executable code in memory at runtime,
> > and I want to disassemble it with GDB?  I'll build the memory
> > buffer, with a layout as the compiler puts things in .text,
> > but the code will not be in .text, yet, don't I want for
> > GDB to read it in execute order, not memory order?
> 
> E.g, a just-in-time (JIT) compiler?  Yes, you'd want GDB to convert
> dynamically compiled code from memory order to execute order.

Yes, that's one example, although there are many more use cases
beyond a full fledged compiler.  Just staying in gdb land, think of
things like debugging the displaced stepping scratch pad (when you
add support for non-stop mode you'll want to debug this, probably).
Another example, debugging fast tracepoints support for RX.  In the latter,
we patch the code stream with a jump into small trampoline routine built
at runtime in the inferior's memory.  Just copying a piece of .text
code into a data buffer, and trying to disassemble it is another example.

The more I think of this, the more it seems that it's the thought/intent
that counts.

-- 
Pedro Alves