[patch] Cut memory address width

[patch] Cut memory address width

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 1424 bytes --]

Hi,

`x/x $ebx' on gdb/amd64 debugging inferior/i386 causes Cannot access memory at
address 0xffffce70 (or so) as $ebx is considered `int' and sign-extended to
64-bit while the resulting address 0xffffffffffffce70 fails to be accessed.

$esp does not exhibit this problem as it is `builtin_type_void_data_ptr' not
`builtin_type_int' as $ebx is.  Therefore it gets extended as unsigned.

Simulate the part of paddress(); it is questionable how deep in the functions
calling stack the address width cut should be.


Regards,
Jan


As bugreported by John Reiser <jreiser(at)BitWagon.com>:

When debugging a 32-bit executable on x86_64, gdb does not allow examining the stack if pointed to by a non-$esp register.  For example,
-----foo.S
_start: .globl _start
        nop
        int3
        movl %esp,%ebx
        int3  # examining memory from $ebx fails, from $esp succeeds
        nop
        nop
-----
$ gcc -m32 -o foo -nostartfiles -nostdlib foo.S
$ gdb foo
Program received signal SIGTRAP, Trace/breakpoint trap.
0x08048076 in _start ()
(gdb) x/i $pc
0x8048076 <_start+2>:   mov    %esp,%ebx
(gdb) stepi
0x08048078 in _start ()
(gdb) x/x $esp
0xffffce70:     0x00000001
(gdb) x/x $ebx
0xffffce70:     Cannot access memory at address 0xffffce70
(gdb) x/x 0xffffce70
0xffffce70:     0x00000001

Expected Results:  "x/x $ebx" should have succeeded, too, when %ebx has the
same value as %esp and examining from $esp works.

[-- Attachment #2: gdb-6.5-memory-address-width.patch --]
[-- Type: text/plain, Size: 1672 bytes --]

2006-09-27  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* target.c (target_read_memory): Cut memory address to the target's
	address bit size, bugreport by John Reiser.
	(target_write_memory): Likewise.
		
		
Index: gdb/target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.126
diff -u -p -r1.126 target.c
--- gdb/target.c	21 Sep 2006 14:00:53 -0000	1.126
+++ gdb/target.c	27 Sep 2006 16:01:27 -0000
@@ -1032,6 +1032,16 @@ target_xfer_partial (struct target_ops *
 int
 target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, int len)
 {
+  /* `x/x $ebx' on gdb/amd64 debugging inferior/i386 causes
+     Cannot access memory at address 0xffffce70
+     as $ebx is considered `int' and sign-extended to 64-bit.
+     $esp does not exhibit this problem as it is `builtin_type_void_data_ptr',
+     not `builtin_type_int' as $ebx is.
+     Simulate the part of paddress() here.  */
+  int addr_bit = TARGET_ADDR_BIT;
+  if (addr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT))
+    memaddr &= ((CORE_ADDR) 1 << addr_bit) - 1;
+
   if (target_read (&current_target, TARGET_OBJECT_MEMORY, NULL,
 		   myaddr, memaddr, len) == len)
     return 0;
@@ -1042,6 +1052,11 @@ target_read_memory (CORE_ADDR memaddr, g
 int
 target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, int len)
 {
+  /* See `target_read_memory' above.  */
+  int addr_bit = TARGET_ADDR_BIT;
+  if (addr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT))
+    memaddr &= ((CORE_ADDR) 1 << addr_bit) - 1;
+
   if (target_write (&current_target, TARGET_OBJECT_MEMORY, NULL,
 		    myaddr, memaddr, len) == len)
     return 0;

Re: [patch] Cut memory address width

[Michael Snyder]

On Wed, 2006-09-27 at 18:15 +0200, Jan Kratochvil wrote:
> Hi,
> 
> `x/x $ebx' on gdb/amd64 debugging inferior/i386 causes Cannot access memory at
> address 0xffffce70 (or so) as $ebx is considered `int' and sign-extended to
> 64-bit while the resulting address 0xffffffffffffce70 fails to be accessed.
> 
> $esp does not exhibit this problem as it is `builtin_type_void_data_ptr' not
> `builtin_type_int' as $ebx is.  Therefore it gets extended as unsigned.
> 
> Simulate the part of paddress(); it is questionable how deep in the functions
> calling stack the address width cut should be.

Yes, but I think the assumption is that esp is most commonly used 
to hold an address, while ebx is most commonly used to hold an integer.
Hence the default types.

I would tend to say that what the user should do is use an explicit
cast.  As is, he is using an implicit cast and not getting what he
expects.



> 
> As bugreported by John Reiser <jreiser(at)BitWagon.com>:
> 
> When debugging a 32-bit executable on x86_64, gdb does not allow examining the stack if pointed to by a non-$esp register.  For example,
> -----foo.S
> _start: .globl _start
>         nop
>         int3
>         movl %esp,%ebx
>         int3  # examining memory from $ebx fails, from $esp succeeds
>         nop
>         nop
> -----
> $ gcc -m32 -o foo -nostartfiles -nostdlib foo.S
> $ gdb foo
> Program received signal SIGTRAP, Trace/breakpoint trap.
> 0x08048076 in _start ()
> (gdb) x/i $pc
> 0x8048076 <_start+2>:   mov    %esp,%ebx
> (gdb) stepi
> 0x08048078 in _start ()
> (gdb) x/x $esp
> 0xffffce70:     0x00000001
> (gdb) x/x $ebx
> 0xffffce70:     Cannot access memory at address 0xffffce70
> (gdb) x/x 0xffffce70
> 0xffffce70:     0x00000001
> 
> Expected Results:  "x/x $ebx" should have succeeded, too, when %ebx has the
> same value as %esp and examining from $esp works.

Re: [patch] Cut memory address width

[Daniel Jacobowitz]

On Wed, Sep 27, 2006 at 11:20:22AM -0700, Michael Snyder wrote:
> On Wed, 2006-09-27 at 18:15 +0200, Jan Kratochvil wrote:
> > Hi,
> > 
> > `x/x $ebx' on gdb/amd64 debugging inferior/i386 causes Cannot access memory at
> > address 0xffffce70 (or so) as $ebx is considered `int' and sign-extended to
> > 64-bit while the resulting address 0xffffffffffffce70 fails to be accessed.
> > 
> > $esp does not exhibit this problem as it is `builtin_type_void_data_ptr' not
> > `builtin_type_int' as $ebx is.  Therefore it gets extended as unsigned.
> > 
> > Simulate the part of paddress(); it is questionable how deep in the functions
> > calling stack the address width cut should be.
> 
> Yes, but I think the assumption is that esp is most commonly used 
> to hold an address, while ebx is most commonly used to hold an integer.
> Hence the default types.
> 
> I would tend to say that what the user should do is use an explicit
> cast.  As is, he is using an implicit cast and not getting what he
> expects.

What's interesting is why this behavior is different on x86_64 and
i386.  Where are we doing the sign extension - that's probably where it
should be fixed, if anywhere.


-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Cut memory address width

[Jan Kratochvil]

On Wed, 27 Sep 2006 20:22:11 +0200, Daniel Jacobowitz wrote:
> On Wed, Sep 27, 2006 at 11:20:22AM -0700, Michael Snyder wrote:
> > On Wed, 2006-09-27 at 18:15 +0200, Jan Kratochvil wrote:
> > > Hi,
> > > 
> > > `x/x $ebx' on gdb/amd64 debugging inferior/i386 causes Cannot access memory at
> > > address 0xffffce70 (or so) as $ebx is considered `int' and sign-extended to
> > > 64-bit while the resulting address 0xffffffffffffce70 fails to be accessed.
...
> What's interesting is why this behavior is different on x86_64 and
> i386.  Where are we doing the sign extension - that's probably where it
> should be fixed, if anywhere.

In such case `paddress' should print full 64-bit addresses.
Currently it is weird as it refuses to access memory while it will show you
that you were accessing 0xffffce70 - the already cut form - it lies.

If you type on i386 gdb that you want to `x/x 0xffffffffffffce70' it works - as
it will cut the address automatically as even the native C compiler would do.

With these two existing behaviors I was feeling it is more expected to cut the
address even for the access.  Still I like more the strict forbidding access
and so I would rather like to remove the existing address-cut in `paddress' and
also wherever it exists for the native i386.

Still the current state is inconsistent, no matter which way to go is the right
one.


Regards,
Jan

Re: [patch] Cut memory address width

[Daniel Jacobowitz]

On Wed, Sep 27, 2006 at 08:37:16PM +0200, Jan Kratochvil wrote:
> In such case `paddress' should print full 64-bit addresses.
> Currently it is weird as it refuses to access memory while it will show you
> that you were accessing 0xffffce70 - the already cut form - it lies.

Yes, I'm inclined to agree with this.  I've encountered it a time or
two, mostly while working on MIPS64, and it's really bewildering!
If we have garbage in the high bits, that's a problem already.

I don't know what else would break but unless someone else does, I
think we should change paddress to print what it's got.  One caveat:
if addresses are supposed to be sign extended, we should not print out
64-bit addresses for a 32-bit target just because they're sign
extended.  This will show up on MIPS, which sign extends addresses.

> If you type on i386 gdb that you want to `x/x 0xffffffffffffce70' it works - as
> it will cut the address automatically as even the native C compiler would do.

Probably because it's casting it to a 32-bit LONGEST somewhere.  I bet
it breaks if you use --enable-64-bit-bfd.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Cut memory address width

[Mark Kettenis]

> Date: Wed, 27 Sep 2006 18:15:01 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> Hi,
> 
> `x/x $ebx' on gdb/amd64 debugging inferior/i386 causes Cannot access memory at
> address 0xffffce70 (or so) as $ebx is considered `int' and sign-extended to
> 64-bit while the resulting address 0xffffffffffffce70 fails to be accessed.
> 
> $esp does not exhibit this problem as it is `builtin_type_void_data_ptr' not
> `builtin_type_int' as $ebx is.  Therefore it gets extended as unsigned.

We could change it into an unsigned type, but then "x /x -1" would
still fail, and a think the signed type is a bit more useful than an
unsigned type here.

> Simulate the part of paddress(); it is questionable how deep in the functions
> calling stack the address width cut should be.

Well, your proposed fix is defenitely the wrong place to do it.

This should almost certainly be handled in value.c:value_as_address().
You could add an i386-specific integer_to_address(), that would
truncate the address to 32 bits.  But in fact, I can't think of a
reason why truncating to the size of a pointer shouldn't be the
default behaviour.

Mark

Re: [patch] Cut memory address width

[Jim Blandy]

Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> `x/x $ebx' on gdb/amd64 debugging inferior/i386 causes Cannot access memory at
> address 0xffffce70 (or so) as $ebx is considered `int' and sign-extended to
> 64-bit while the resulting address 0xffffffffffffce70 fails to be accessed.
>
> $esp does not exhibit this problem as it is `builtin_type_void_data_ptr' not
> `builtin_type_int' as $ebx is.  Therefore it gets extended as unsigned.
>
> Simulate the part of paddress(); it is questionable how deep in the functions
> calling stack the address width cut should be.

Just as a sanity check: what does 'show architecture' say when you're
debugging an i386 inferior on gdb/amd64?

I'm guessing that x_command calls value_to_address, which passes the
value of $ebx to to gdbarch_integer_to_address, which is where the
conversion is happening.  Is there some code there assuming that host
== target?

Re: [patch] Cut memory address width

[Jim Blandy]

Daniel Jacobowitz <drow@false.org> writes:
> On Wed, Sep 27, 2006 at 08:37:16PM +0200, Jan Kratochvil wrote:
>> In such case `paddress' should print full 64-bit addresses.
>> Currently it is weird as it refuses to access memory while it will show you
>> that you were accessing 0xffffce70 - the already cut form - it lies.
>
> Yes, I'm inclined to agree with this.  I've encountered it a time or
> two, mostly while working on MIPS64, and it's really bewildering!
> If we have garbage in the high bits, that's a problem already.
>
> I don't know what else would break but unless someone else does, I
> think we should change paddress to print what it's got.

Definitely.  Having paddress "fix" things adds insult to injury.  :)

Re: [patch] Cut memory address width

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 990 bytes --]

Hi,

On Wed, 27 Sep 2006 21:01:11 +0200, Mark Kettenis wrote:
...
> This should almost certainly be handled in value.c:value_as_address().
> You could add an i386-specific integer_to_address(), that would
> truncate the address to 32 bits.  But in fact, I can't think of a
> reason why truncating to the size of a pointer shouldn't be the
> default behaviour.

Made the default way, I also do not see the reason to keep larger addresses.
There is some note about `ADDR_BITS_REMOVE' but I believe it is only about the
lowest (0-2 or so) bits and the high bits should not hurt anyone.


On Wed, 27 Sep 2006 21:24:27 +0200, Jim Blandy wrote:
...
> Just as a sanity check: what does 'show architecture' say when you're
> debugging an i386 inferior on gdb/amd64?

as expected:
	The target architecture is set automatically (currently i386)

...
> Is there some code there assuming that host == target?

I do not believe so, it is handled everything by `LONGEST' / `unpack_long'.



Regards,
Jan

[-- Attachment #2: gdb-6.5-memory-address-width-v2.patch --]
[-- Type: text/plain, Size: 3734 bytes --]

2006-09-28  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb/utils.c (paddress): Disable cutting of the printed addresses
	to the target's address bit size; user wants to see everything.
	* gdb/value.c (value_as_address_core): Original `value_as_address'.
	(value_as_address): New `value_as_address' wrapper - cut memory address
	to the target's address bit size, bugreport by John Reiser.


Index: gdb/utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.169
diff -u -p -r1.169 utils.c
--- gdb/utils.c	21 Sep 2006 13:50:51 -0000	1.169
+++ gdb/utils.c	28 Sep 2006 17:06:03 -0000
@@ -2596,6 +2596,14 @@ paddr_nz (CORE_ADDR addr)
 const char *
 paddress (CORE_ADDR addr)
 {
+  /* Do no cut the address as the user should see all the information
+     available.  Otherwise 64-bit gdb debugging 32-bit inferior would
+     report for `x/x 0xffffffffffffce70' error
+     `Cannot access memory at 0xffffce70' while the error occured just
+     because of the higher order bits 0xffffffff00000000 there.
+     This specific error no longer occurs as the address is now cut
+     during execution by `value_as_address'.  */
+#if 0
   /* Truncate address to the size of a target address, avoiding shifts
      larger or equal than the width of a CORE_ADDR.  The local
      variable ADDR_BIT stops the compiler reporting a shift overflow
@@ -2609,6 +2617,8 @@ paddress (CORE_ADDR addr)
 
   if (addr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT))
     addr &= ((CORE_ADDR) 1 << addr_bit) - 1;
+#endif
+
   return hex_string (addr);
 }
 
Index: gdb/value.c
===================================================================
RCS file: /cvs/src/src/gdb/value.c,v
retrieving revision 1.36
diff -u -p -r1.36 value.c
--- gdb/value.c	31 Mar 2006 10:36:18 -0000	1.36
+++ gdb/value.c	28 Sep 2006 17:06:03 -0000
@@ -950,11 +950,10 @@ value_as_double (struct value *val)
     error (_("Invalid floating value found in program."));
   return foo;
 }
-/* Extract a value as a C pointer. Does not deallocate the value.  
-   Note that val's type may not actually be a pointer; value_as_long
-   handles all the cases.  */
-CORE_ADDR
-value_as_address (struct value *val)
+
+/* See `value_as_address' below - core of value to C pointer extraction.  */
+static CORE_ADDR
+value_as_address_core (struct value *val)
 {
   /* Assume a CORE_ADDR can fit in a LONGEST (for now).  Not sure
      whether we want this to be true eventually.  */
@@ -1054,6 +1053,33 @@ value_as_address (struct value *val)
   return unpack_long (value_type (val), value_contents (val));
 #endif
 }
+
+/* Extract a value as a C pointer. Does not deallocate the value.  
+   Note that val's type may not actually be a pointer; value_as_long
+   handles all the cases.  */
+CORE_ADDR
+value_as_address (struct value *val)
+{
+  CORE_ADDR addr;
+
+  addr = value_as_address_core (val);
+
+  /* Truncate address to the size of a target address, avoiding shifts
+     larger or equal than the width of a CORE_ADDR.  The local
+     variable ADDR_BIT stops the compiler reporting a shift overflow
+     when it won't occur. */
+  /* NOTE: This assumes that the significant address information is
+     kept in the least significant bits of ADDR - the upper bits were
+     either zero or sign extended.  Should ADDRESS_TO_POINTER() or
+     some ADDRESS_TO_PRINTABLE() be used to do the conversion?  */
+
+  int addr_bit = TARGET_ADDR_BIT;
+
+  if (addr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT))
+    addr &= ((CORE_ADDR) 1 << addr_bit) - 1;
+
+  return addr;
+}
 \f
 /* Unpack raw data (copied from debugee, target byte order) at VALADDR
    as a long, or as a double, assuming the raw data is described

Re: [patch] Cut memory address width

[Daniel Jacobowitz]

On Thu, Sep 28, 2006 at 07:27:25PM +0200, Jan Kratochvil wrote:
> Hi,
> 
> On Wed, 27 Sep 2006 21:01:11 +0200, Mark Kettenis wrote:
> ...
> > This should almost certainly be handled in value.c:value_as_address().
> > You could add an i386-specific integer_to_address(), that would
> > truncate the address to 32 bits.  But in fact, I can't think of a
> > reason why truncating to the size of a pointer shouldn't be the
> > default behaviour.
> 
> Made the default way, I also do not see the reason to keep larger addresses.
> There is some note about `ADDR_BITS_REMOVE' but I believe it is only about the
> lowest (0-2 or so) bits and the high bits should not hurt anyone.

Please see the other message in this thread where I explained about
MIPS; there, all 0x80000000 - 0xFFFFFFFF addresses will be sign
extended.  Suddenly printing out the extra eight F's would be really
ugly IMHO.  I tried to come up with a good solution to this today, but
I couldn't find one; the flag we need is associated with the BFD and
not easily accessible here.  Maybe there should be a gdbarch flag
for the property; only MIPS and sh64 set it, so duplicating the
knowledge here isn't a big deal.

As for the other half of the bug, Mark was talking about
integer_to_address, which is separate from value_to_address where your
patch tried to fix this.  I think that either inside the
implementations of that function, or at the one call site, is where we
should fix it.  That will avoid creating such addresses in the first
place. There are two options: ignore high bits, or complain if the high
bits would affect the address (e.g. if the integer was not properly
sign or value extended).  Mark liked truncate, I don't really care
which.

Oh, and please don't #if 0 things.  Either they're right, or they're
wrong.  If we need them again, there's CVS history.  Thanks in advance.

-- 
Daniel Jacobowitz
CodeSourcery