Re: [patch] Fix i386 memory-by-register access on amd64

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Thu, 25 Jun 2009 18:33:43 +0200, Tom Tromey wrote:
> 
> Jan> I believe gdb.x86_64 debugging gdb.i386 inferior should behave exactly as
> Jan> gdb.i386 debugging gdb.i386 inferior.
> 
> This sounds reasonable to me.  If I'm debugging a 32-bit inferior, it
> seems weird to see a 64-bit address.

Found out now the original patch
	http://sourceware.org/ml/gdb-patches/2009-04/msg00786.html

had a regression on x86_64-fedora-linux-gnu for `--target_board unix/-m32':
	+FAIL: gdb.base/dump.exp: array copy, srec; value restored ok
	+FAIL: gdb.base/dump.exp: array copy, ihex; value restored ok
	+FAIL: gdb.base/dump.exp: array copy, tekhex; value restored ok
	+FAIL: gdb.base/dump.exp: array partial with expressions; value restored ok

Updated the patch to do on 64bit hosts exactly the same what 32bit hosts do.
32bit hosts do all the CORE_ADDR calculations 64bit, just the final ptrace
call strips the width to 32bits.

Created a new (PASSing) test `highmem-debugger' to ensure ptrace will still
behave as expected (returning EIO and not just stripping the address width):
	http://sourceware.org/systemtap/wiki/utrace/tests

There is some suspection a similiar patch would be appropriate for theses
functions but I have no such test OSes/machines available:
	config/pa/hpux.mh	inf-ttrace.c	inf_ttrace_xfer_memory
	config/powerpc/aix.mh	rs6000-nat.c	rs6000_xfer_partial

As I changed the patch requesting new approval.  Sorry for the wrong patch.

Regression tested on:
x86_64-fedora-linux-gnu with `--target_board unix/-m64'.
x86_64-fedora-linux-gnu with `--target_board unix/-m32'.
  i386-fedora-linux-gnu with `--target_board unix/-m32'.


Thanks,
Jan


2009-07-06  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix signed 32bit inferior registers on 64bit GDB.
	* inf-ptrace.c (inf_ptrace_xfer_partial): New variables gdbarch
	and addr_bit.  Mask OFFSET by the ADDR_BIT width.

2009-07-06  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.arch/amd64-i386-address.exp, gdb.arch/amd64-i386-address.S: New.

--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -452,8 +452,20 @@ inf_ptrace_xfer_partial (struct target_ops *ops, enum target_object object,
 			 const gdb_byte *writebuf,
 			 ULONGEST offset, LONGEST len)
 {
+  struct gdbarch *gdbarch = target_thread_architecture (inferior_ptid);
+  int addr_bit = gdbarch_addr_bit (gdbarch);
   pid_t pid = ptid_get_pid (inferior_ptid);
 
+  /* 32-bit host will pass only the lower 32-bits of OFFSET to the ptrace
+     syscall.  64-bit host debugging 32-bit inferior would get EIO for non-zero
+     higher 32-bits in the same case.  Match the behavior of 32-bit host GDB
+     for 64-bit host GDB debugging 32-bit inferior.
+
+     Compare ADDR_BIT first to avoid a compiler warning on shift overflow.  */
+  gdb_assert (sizeof (offset) == sizeof (ULONGEST));
+  if (addr_bit < (sizeof (ULONGEST) * HOST_CHAR_BIT))
+    offset &= ((ULONGEST) 1 << addr_bit) - 1;
+
   switch (object)
     {
     case TARGET_OBJECT_MEMORY:
new file mode 100644
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-i386-address.S
@@ -0,0 +1,25 @@
+/* Copyright 2009 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+   This file is part of the gdb testsuite.  */
+
+_start:	.globl	_start
+	nop
+	int3
+	movl	%esp, %ebx
+	/* Examining memory from $ebx fails, from $esp it succeeds.  */
+	int3
+	nop
+	nop
new file mode 100644
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-i386-address.exp
@@ -0,0 +1,45 @@
+# Copyright 2009 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Test UNsigned extension of the 32-bit inferior address on a 64-bit host.
+# On native 32-bit host the test always PASSed.
+
+if {![istarget "x86_64-*-*"] && ![istarget "i?86-*-*"]} then {
+    verbose "Skipping amd64->i386 adress test."
+    return
+}
+
+if [prepare_for_testing amd64-i386-address.exp amd64-i386-address amd64-i386-address.S [list debug "additional_flags=-m32 -nostdlib"]] {
+    return -1
+}
+
+gdb_run_cmd
+
+set test "trap stop"
+gdb_test_multiple "" $test {
+    -re "Program received signal SIGTRAP,.*_start .*$gdb_prompt $" {
+	pass $test
+    }
+}
+
+gdb_test "stepi" ".*_start .*int3.*"
+
+gdb_test "x/x \$esp" "0x\[0-9a-f\]*:\t0x0*1"
+
+# Failure case would be:
+# 	0xff8d7f00:     Cannot access memory at address 0xff8d7f00
+gdb_test "x/x \$ebx" "0x\[0-9a-f\]*:\t0x0*1"