[RFC] Implementation of DW_OP_deref and DW_OP_deref_size operators

[RFC] Implementation of DW_OP_deref and DW_OP_deref_size operators

[Mark Kettenis]

After staring for a while at some compiler warnings resulting from an
attempt to make an i386 x x86-64 cross-debugger, and failing to
understand the code that implements the DW_OP_deref and
DW_OP_deref_size operators I think I've found out what's wrong.

These expressions are supposed to dereference pointers.  In GDB this
means that they should look at the address indicated by the pointer
they're dereferencing *in the target*.  The current code is trying to
dereference a pointer in GDB.  If we look at the history of this code
it is perfectly understandable.  It is largely copied from the
unwinder that comes with GCC.  This unwinder is supposed to be linked
into the program that contains the DWARF2 code, so in that context it
makes sense to lookup pointers in the program this code is running in.

Does this sound reasonable?

To fix this, I propose the following patch.

Ok, to check this in?

Mark


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* dwarf2cfi.c (execute_stack_op): Fix implementation of the
	DW_OP_deref and DW_OP_deref_size operators by letting do their
	lookup in the target.

Index: dwarf2cfi.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2cfi.c,v
retrieving revision 1.12
diff -u -p -r1.12 dwarf2cfi.c
--- dwarf2cfi.c 4 Jul 2002 14:43:51 -0000 1.12
+++ dwarf2cfi.c 4 Jul 2002 21:50:39 -0000
@@ -21,6 +21,7 @@
    Boston, MA 02111-1307, USA.  */
 
 #include "defs.h"
+#include "gdbcore.h"
 #include "symtab.h"
 #include "symfile.h"
 #include "objfiles.h"
@@ -1119,32 +1120,21 @@ execute_stack_op (struct objfile *objfil
 	    {
 	    case DW_OP_deref:
 	      {
-		char *ptr = (char *) result;
-		result = read_pointer (objfile->obfd, &ptr);
+		int len = TARGET_ADDR_BIT / TARGET_CHAR_BIT;
+		if (len != 4 && len != 8)
+		  internal_error (__FILE__, __LINE__,
+				  "execute_stack_op error");
+		result = read_memory_unsigned_integer (result, len);
 	      }
 	      break;
 
 	    case DW_OP_deref_size:
 	      {
-		char *ptr = (char *) result;
-		switch (*op_ptr++)
-		  {
-		  case 1:
-		    result = read_1u (objfile->obfd, &ptr);
-		    break;
-		  case 2:
-		    result = read_2u (objfile->obfd, &ptr);
-		    break;
-		  case 4:
-		    result = read_4u (objfile->obfd, &ptr);
-		    break;
-		  case 8:
-		    result = read_8u (objfile->obfd, &ptr);
-		    break;
-		  default:
-		    internal_error (__FILE__, __LINE__,
-				    "execute_stack_op error");
-		  }
+		int len = *op_ptr++;
+		if (len != 1 && len != 2 && len != 4 && len !=8)
+		  internal_error (__FILE__, __LINE__,
+				  "execute_stack_op error");
+		result = read_memory_unsigned_integer (result, len);
 	      }
 	      break;
 

Re: [RFC] Implementation of DW_OP_deref and DW_OP_deref_size operators

[Michal Ludvig]

On Fri, 5 Jul 2002, Mark Kettenis wrote:

> These expressions are supposed to dereference pointers.  In GDB this
> means that they should look at the address indicated by the pointer
> they're dereferencing *in the target*.  The current code is trying to
> dereference a pointer in GDB.  If we look at the history of this code
> it is perfectly understandable.  It is largely copied from the
> unwinder that comes with GCC.  This unwinder is supposed to be linked
> into the program that contains the DWARF2 code, so in that context it
> makes sense to lookup pointers in the program this code is running in.
>
> Does this sound reasonable?

Yes it does. I have already found this kind of flaws in there and
I bet that there are some more yet :-(

Michal Ludvig
-- 
* SuSE CR, s.r.o     * mludvig@suse.cz
* +420 2 9654 5373   * http://www.suse.cz

Re: [RFC] Implementation of DW_OP_deref and DW_OP_deref_size operators

[Jim Blandy]

Mark Kettenis <kettenis@chello.nl> writes:
> After staring for a while at some compiler warnings resulting from an
> attempt to make an i386 x x86-64 cross-debugger, and failing to
> understand the code that implements the DW_OP_deref and
> DW_OP_deref_size operators I think I've found out what's wrong.
> 
> These expressions are supposed to dereference pointers.  In GDB this
> means that they should look at the address indicated by the pointer
> they're dereferencing *in the target*.  The current code is trying to
> dereference a pointer in GDB.  If we look at the history of this code
> it is perfectly understandable.  It is largely copied from the
> unwinder that comes with GCC.  This unwinder is supposed to be linked
> into the program that contains the DWARF2 code, so in that context it
> makes sense to lookup pointers in the program this code is running in.
> 
> Does this sound reasonable?

Yes!  However, pointers are not always unsigned.  I think on some
32-bit ABIs for 64-bit MIPS chips they need to be signed.  Andrew will
know the facts.

Re: [RFC] Implementation of DW_OP_deref and DW_OP_deref_size operators

[Jim Blandy]

Jim Blandy <jimb@redhat.com> writes:
> Yes!  However, pointers are not always unsigned.  I think on some
> 32-bit ABIs for 64-bit MIPS chips they need to be signed.  Andrew will
> know the facts.

But it doesn't matter whether pointers are unsigned.  The Dwarf spec
says that DW_OP_deref pushes a value which is "the size of an address
on the target machine", which is the full width of a Dwarf
expression stack element; it doesn't need to be extended.  And it
further says that DW_OP_deref_size zero-extends its argument.  So the
patch looks correct to me.

Re: [RFC] Implementation of DW_OP_deref and DW_OP_deref_size operators

[Mark Kettenis]

   Sender: jimb@zwingli.cygnus.com
   Cc: gdb@sources.redhat.com, gdb-patches@sources.redhat.com
   From: Jim Blandy <jimb@redhat.com>
   Date: 04 Jul 2002 22:27:02 -0500
   User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.1
   Content-Type: text/plain; charset=us-ascii


   Jim Blandy <jimb@redhat.com> writes:
   > Yes!  However, pointers are not always unsigned.  I think on some
   > 32-bit ABIs for 64-bit MIPS chips they need to be signed.  Andrew will
   > know the facts.

   But it doesn't matter whether pointers are unsigned.  The Dwarf spec
   says that DW_OP_deref pushes a value which is "the size of an address
   on the target machine", which is the full width of a Dwarf
   expression stack element; it doesn't need to be extended.  And it
   further says that DW_OP_deref_size zero-extends its argument.  So the
   patch looks correct to me.

Yup, I came to the same conclusion.  The code that was origionally
there also used unsigned pointers.

I'll check it in.

Thanks,

Mark