[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