[rfc] [08/18] Cell multi-arch: Some address vs. pointer fixes

Ulrich Weigand uweigand@de.ibm.com
Sun Sep 7 21:14:00 GMT 2008


Hello,

this patch fixes a number of places to better cope with non-trivial
pointer/integer <-> address conversion.

- print_scalar_formatted converts an internal address back to a pointer
  value by truncating to gdbarch_addr_bits bits.  However, it omits to
  do that truncation for the /d format -- this isn't really appropriate,
  even with /d we do not want to see the GDB internal encoded address.

  The patch changes this to always truncate for unsigned types (which
  includes pointer and reference types).

- restore_command takes a "load_offset" parameter.  As this offset really
  defines an address in target memory where to restore to, it needs to
  be treated as a GDB-internal address, so it should be parsed in as
  an address, not a long (as stored as CORE_ADDR, not long).

  Even so, dumping as srec format doesn't work as the format does not
  support 64-bit addresses.  Thus I've changes the test case to handle
  SPU like 64-bit platforms and skip those tests.

- varobj_create attempts to identify a frame in the current frame chain
  based on its frame base address that the caller has retrieved from a
  prior "print $fp" call.  The problem is that printing $fp will convert
  the internal address to a pointer by truncating, but the frame base is
  of course still an internal address.

  The current attempt to solve this it to "revert" the truncation when
  reading in the requested frame base in string_to_core_addr.  However,
  this doesn't really work, as it only handles sign-extension based on
  the bfd_get_sign_extend_vma property of exec_bfd -- this does not
  support cases where we don't have an exec_bfd, or cases like Cell
  where the internal GDB address encodes non-trivial extra information
  and is not simply about sign- vs. zero-extension.

  This patch fixes that by removing that logic in string_to_core_addr,
  which causes varobj_create to be called with an external pointer value,
  and then converting the frame base addresses of all frames in the
  current frame chain *also* to (truncated) external pointer values
  before comparing them against the requested value.

  There might be some theoretical cases where this fails because two
  frames in the same frame chain have different internal addresses that
  truncate to the same external representation.  But this case cannot
  be fixed anyway as long as the MI interface uses external representations
  to identify frames -- this should be changed to use (internal) frame IDs.

Bye,
Ulrich


ChangeLog:

	* printcmd.c (print_scalar_formatted): Always truncate
	unsigned data types.

	* cli-dump.c (struct callback_data): Change type of load_offset
	to CORE_ADDR.
	(restore_binary_file): Update type casts.
	(restore_command): Parse load_offset as address, not long.

	* utils.c (string_to_core_addr): Do not sign-extend value.
	* varobj.c (find_frame_addr_in_frame_chain): Truncate frame_base
	before comparing against requested frame address.

testsuite/ChangeLog:

	* gdb.base/dump.exp: Handle SPU like 64-bit platforms.


Index: src/gdb/printcmd.c
===================================================================
--- src.orig/gdb/printcmd.c
+++ src/gdb/printcmd.c
@@ -373,7 +373,7 @@ print_scalar_formatted (const void *vala
   /* If we are printing it as unsigned, truncate it in case it is actually
      a negative signed value (e.g. "print/u (short)-1" should print 65535
      (if shorts are 16 bits) instead of 4294967295).  */
-  if (format != 'd')
+  if (format != 'd' || TYPE_UNSIGNED (type))
     {
       if (len < sizeof (LONGEST))
 	val_long &= ((LONGEST) 1 << HOST_CHAR_BIT * len) - 1;
Index: src/gdb/cli/cli-dump.c
===================================================================
--- src.orig/gdb/cli/cli-dump.c
+++ src/gdb/cli/cli-dump.c
@@ -441,7 +441,7 @@ add_dump_command (char *name, void (*fun
 
 /* Opaque data for restore_section_callback. */
 struct callback_data {
-  long load_offset;
+  CORE_ADDR load_offset;
   CORE_ADDR load_start;
   CORE_ADDR load_end;
 };
@@ -546,8 +546,8 @@ restore_binary_file (char *filename, str
   printf_filtered 
     ("Restoring binary file %s into memory (0x%lx to 0x%lx)\n", 
      filename, 
-     (unsigned long) data->load_start + data->load_offset, 
-     (unsigned long) data->load_start + data->load_offset + len);
+     (unsigned long) (data->load_start + data->load_offset),
+     (unsigned long) (data->load_start + data->load_offset + len));
 
   /* Now set the file pos to the requested load start pos.  */
   if (fseek (file, data->load_start, SEEK_SET) != 0)
@@ -597,7 +597,7 @@ restore_command (char *args, int from_tt
       /* Parse offset (optional). */
       if (args != NULL && *args != '\0')
       data.load_offset = 
-	parse_and_eval_long (scan_expression_with_cleanup (&args, NULL));
+	parse_and_eval_address (scan_expression_with_cleanup (&args, NULL));
       if (args != NULL && *args != '\0')
 	{
 	  /* Parse start address (optional). */
Index: src/gdb/utils.c
===================================================================
--- src.orig/gdb/utils.c
+++ src/gdb/utils.c
@@ -2859,7 +2859,6 @@ core_addr_to_string_nz (const CORE_ADDR 
 CORE_ADDR
 string_to_core_addr (const char *my_string)
 {
-  int addr_bit = gdbarch_addr_bit (current_gdbarch);
   CORE_ADDR addr = 0;
 
   if (my_string[0] == '0' && tolower (my_string[1]) == 'x')
@@ -2875,17 +2874,6 @@ string_to_core_addr (const char *my_stri
 	  else
 	    error (_("invalid hex \"%s\""), my_string);
 	}
-
-      /* Not very modular, but if the executable format expects
-         addresses to be sign-extended, then do so if the address was
-         specified with only 32 significant bits.  Really this should
-         be determined by the target architecture, not by the object
-         file.  */
-      if (i - 2 == addr_bit / 4
-	  && exec_bfd
-	  && bfd_get_sign_extend_vma (exec_bfd))
-	addr = (addr ^ ((CORE_ADDR) 1 << (addr_bit - 1)))
-	       - ((CORE_ADDR) 1 << (addr_bit - 1));
     }
   else
     {
Index: src/gdb/varobj.c
===================================================================
--- src.orig/gdb/varobj.c
+++ src/gdb/varobj.c
@@ -426,6 +426,8 @@ static struct frame_info *
 find_frame_addr_in_frame_chain (CORE_ADDR frame_addr)
 {
   struct frame_info *frame = NULL;
+  CORE_ADDR frame_base;
+  int addr_bit;
 
   if (frame_addr == (CORE_ADDR) 0)
     return NULL;
@@ -435,7 +437,17 @@ find_frame_addr_in_frame_chain (CORE_ADD
       frame = get_prev_frame (frame);
       if (frame == NULL)
 	return NULL;
-      if (get_frame_base_address (frame) == frame_addr)
+
+      /* The CORE_ADDR we get as argument was parsed from a string GDB
+	 output as $fp.  This output got truncated to gdbarch_addr_bit.
+	 Truncate the frame base address in the same manner before
+	 comparing it against our argument.  */
+      frame_base = get_frame_base_address (frame);
+      addr_bit = gdbarch_addr_bit (get_frame_arch (frame));
+      if (addr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT))
+	frame_base &= ((CORE_ADDR) 1 << addr_bit) - 1;
+
+      if (frame_base == frame_addr)
 	return frame;
     }
 }
Index: src/gdb/testsuite/gdb.base/dump.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/dump.exp
+++ src/gdb/testsuite/gdb.base/dump.exp
@@ -42,6 +42,12 @@ if {[istarget "ia64*-*-*"] || [istarget 
     set is64bitonly "yes"
 }
 
+if {[istarget "spu*-*-*"]} then {
+    # The internal address format used for the combined Cell/B.E.
+    # debugger required 64-bit.
+    set is64bitonly "yes"
+}
+
 if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable ${options}] != "" } {
      untested dump.exp
      return -1
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com



More information about the Gdb-patches mailing list