Re: [RFC] dwarf2_read_address(): sign extend as appropriate

[Kevin Buettner <kevinb@redhat.com>]

On Fri, 27 Apr 2007 17:48:30 -0400
Daniel Jacobowitz <drow@false.org> wrote:

> On Fri, Apr 27, 2007 at 02:10:18PM -0700, Kevin Buettner wrote:
> > I decided to give the `value_as_address' approach a try.  It worked
> > as expected when I tried running it against a MIPS target.
> > 
> > Comments?
> > 
> > 	* dwarf2expr.c (unsigned_address_type): Add forward declaration.
> > 	(dwarf2_read_address): Sign extend return address as required by
> > 	target architecture.
> 
> Fine with me, although I would request you not to put code in the
> comments; we can figure it out again later, and	I find commented out
> code too confusing.

Thanks for the quick review.  I've committed my patch minus the code
that I had placed in the comments.  (See below for the patch that I
ended up committing.)

> Do you happen to have more MIPS patches lying around?  I'm in the
> middle of enabling CFI, based on some old patches of yours; I
> encountered yet another new address sign / size confusion.

I think we do have some other old MIPS patches lying about.  (I am not
the author of all of them.)  I'll make an effort to go through them
early next week and post the ones that still seem relevant.

I also noticed in my debugging earlier this week that registers are
not being correctly displayed for mips64 targets.  I know I had an old
patch that dealt with that problem.  Perhaps I'll dust off that patch
too.

Kevin


	* dwarf2expr.c (unsigned_address_type): Add forward declaration.
	(dwarf2_read_address): Sign extend return address as required by
	target architecture.

Index: dwarf2expr.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2expr.c,v
retrieving revision 1.19
diff -u -p -r1.19 dwarf2expr.c
--- dwarf2expr.c	9 Jan 2007 17:58:50 -0000	1.19
+++ dwarf2expr.c	27 Apr 2007 22:32:28 -0000
@@ -33,6 +33,7 @@
 
 static void execute_stack_op (struct dwarf_expr_context *,
 			      gdb_byte *, gdb_byte *);
+static struct type *unsigned_address_type (void);
 
 /* Create a new context for the expression evaluator.  */
 
@@ -205,9 +206,35 @@ dwarf2_read_address (gdb_byte *buf, gdb_
     error (_("dwarf2_read_address: Corrupted DWARF expression."));
 
   *bytes_read = TARGET_ADDR_BIT / TARGET_CHAR_BIT;
-  /* NOTE: cagney/2003-05-22: This extract is assuming that a DWARF 2
-     address is always unsigned.  That may or may not be true.  */
-  result = extract_unsigned_integer (buf, TARGET_ADDR_BIT / TARGET_CHAR_BIT);
+
+  /* For most architectures, calling extract_unsigned_integer() alone
+     is sufficient for extracting an address.  However, some
+     architectures (e.g. MIPS) use signed addresses and using
+     extract_unsigned_integer() will not produce a correct
+     result.  Turning the unsigned integer into a value and then
+     decomposing that value as an address will cause
+     gdbarch_integer_to_address() to be invoked for those
+     architectures which require it.  Thus, using value_as_address()
+     will produce the correct result for both types of architectures.
+
+     One concern regarding the use of values for this purpose is
+     efficiency.  Obviously, these extra calls will take more time to
+     execute and creating a value takes more space, space which will
+     have to be garbage collected at a later time.  If constructing
+     and then decomposing a value for this purpose proves to be too
+     inefficient, then gdbarch_integer_to_address() can be called
+     directly.
+
+     The use of `unsigned_address_type' in the code below refers to
+     the type of buf and has no bearing on the signedness of the
+     address being returned.  */
+
+  result = value_as_address (value_from_longest 
+			      (unsigned_address_type (),
+			       extract_unsigned_integer 
+				 (buf,
+				  TARGET_ADDR_BIT / TARGET_CHAR_BIT)));
+
   return result;
 }