Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit core file

["H.J. Lu" <hongjiu.lu@intel.com>]

On Sat, Apr 10, 2010 at 03:27:42PM -0700, H.J. Lu wrote:
> On Sat, Apr 10, 2010 at 03:19:43PM -0700, H.J. Lu wrote:
> > Hi,
> > 
> > I am checking in this patch to support 32bit core note sections on
> > Linux/x86-64. I will submit a separate gdb patch.
> > 
> > 
> > H.J.
> > ---
> > 2010-04-10  H.J. Lu  <hongjiu.lu@intel.com>
> > 
> > 	PR corefiles/11467
> > 	* configure.in (CORE_HEADER): New. Set to hosts/x86-64linux.h
> > 	for x86_64-*-linux*.
> > 	* config.in: Regenerated.
> > 	* configure: Likewise.
> > 
> > 	* elf.c: Include CORE_HEADER if it is defined.
> > 
> > 2010-04-10  H.J. Lu  <hongjiu.lu@intel.com>
> > 	    Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	* hosts/x86-64linux.h: New.
> 
> Hi,
> 
> Hi,
> 
> Here is the gdb patch to properly generate 32bit coredumps on
> Linux/x86-64. The key here is to use the right register offset
> for gcore. OK to install?
> 

Here is an updated patch.  We shouldn't zero-extended 32bit registers
to 64 bits for gcore since they are stored as 32bit.


H.J.
---
gdb/

2010-04-10  H.J. Lu  <hongjiu.lu@intel.com>

	PR corefiles/11467
	* amd64-linux-nat.c (fill_gregset): Pass 1 as gcore to
	amd64_collect_native_gregset.
	(amd64_linux_store_inferior_registers): Pass 0 as gcore to
	amd64_native_gregset_reg_offset.
	(_initialize_amd64_linux_nat): Set amd64_gcore_gregset32_reg_offset
	to amd64_linux_gcore_gregset32_reg_offset,

	* amd64-nat.c (amd64_gcore_gregset32_reg_offset): New.
	(amd64_native_gregset_reg_offset): Add an argument, gcore.
	Use amd64_gcore_gregset32_reg_offset if gcore isn't 0.
	(amd64_native_gregset_supplies_p): Pass 0 as gcore to
	amd64_native_gregset_reg_offset.
	(amd64_supply_native_gregset): Likewise.
	(amd64_collect_native_gregset): Add an argument, gcore, and
	pass it to amd64_native_gregset_reg_offset.  Don't
	zero-extended to 64 bits for gcore.

	* amd64-nat.h (amd64_gcore_gregset32_reg_offset): New.
	(amd64_native_gregset_reg_offset): Add an argument, gcore.

2010-04-10  H.J. Lu  <hongjiu.lu@intel.com>
	    Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR corefiles/11467
	* amd64-linux-nat.c (amd64_linux_gcore_gregset32_reg_offset): New.

gdb/testsuite/

2010-04-10  H.J. Lu  <hongjiu.lu@intel.com>

	PR corefiles/11467
	* gdb.arch/amd64-gcore32.exp: New.
	* gdb.arch/gcore.c: Likewise.

diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index 9812610..39c4786 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -102,6 +102,36 @@ static int amd64_linux_gregset64_reg_offset[] =
    GNU/Linux i386 registers are all 32-bit, but since we're
    little-endian we get away with that.  */
 
+/* This info is not reusable from "i386-linux-nat.c" as gdb itself runs in
+   64-bit mode and so ptrace(2) has 64-bit structure layout.
+   Just the corefile being generated has 32-bit layout so we need to do
+   a conversion specific to the i386-on-amd64 compatibility mode.  */
+static int amd64_linux_gcore_gregset32_reg_offset[] =
+{
+  6 * 4,			/* %eax */
+  1 * 4,			/* %ecx */
+  2 * 4,			/* %edx */
+  0 * 4,			/* %ebx */
+  15 * 4,			/* %esp */
+  5 * 4,			/* %ebp */
+  3 * 4,			/* %esi */
+  4 * 4,			/* %edi */
+  12 * 4,			/* %eip */
+  14 * 4,			/* %eflags */
+  13 * 4,			/* %cs */
+  16 * 4,			/* %ss */
+  7 * 4,			/* %ds */
+  8 * 4,			/* %es */
+  9 * 4,			/* %fs */
+  10 * 4,			/* %gs */
+  -1, -1, -1, -1, -1, -1, -1, -1,
+  -1, -1, -1, -1, -1, -1, -1, -1,
+  -1, -1, -1, -1, -1, -1, -1, -1,
+  -1,
+  -1, -1, -1, -1, -1, -1, -1, -1,
+  11 * 4			/* "orig_eax" */
+};
+
 /* From <sys/reg.h> on GNU/Linux i386.  */
 static int amd64_linux_gregset32_reg_offset[] =
 {
@@ -141,7 +171,7 @@ void
 fill_gregset (const struct regcache *regcache,
 	      elf_gregset_t *gregsetp, int regnum)
 {
-  amd64_collect_native_gregset (regcache, gregsetp, regnum);
+  amd64_collect_native_gregset (regcache, gregsetp, regnum, 1);
 }
 
 /* Transfering floating-point registers between GDB, inferiors and cores.  */
@@ -247,7 +277,7 @@ amd64_linux_store_inferior_registers (struct target_ops *ops,
       if (ptrace (PTRACE_GETREGS, tid, 0, (long) &regs) < 0)
 	perror_with_name (_("Couldn't get registers"));
 
-      amd64_collect_native_gregset (regcache, &regs, regnum);
+      amd64_collect_native_gregset (regcache, &regs, regnum, 0);
 
       if (ptrace (PTRACE_SETREGS, tid, 0, (long) &regs) < 0)
 	perror_with_name (_("Couldn't write registers"));
@@ -806,6 +836,7 @@ _initialize_amd64_linux_nat (void)
   struct target_ops *t;
 
   amd64_native_gregset32_reg_offset = amd64_linux_gregset32_reg_offset;
+  amd64_gcore_gregset32_reg_offset = amd64_linux_gcore_gregset32_reg_offset;
   amd64_native_gregset32_num_regs = I386_LINUX_NUM_REGS;
   amd64_native_gregset64_reg_offset = amd64_linux_gregset64_reg_offset;
   amd64_native_gregset64_num_regs = AMD64_LINUX_NUM_REGS;
diff --git a/gdb/amd64-nat.c b/gdb/amd64-nat.c
index bcf303e..167a22f 100644
--- a/gdb/amd64-nat.c
+++ b/gdb/amd64-nat.c
@@ -43,6 +43,7 @@
 
 /* General-purpose register mapping for native 32-bit code.  */
 int *amd64_native_gregset32_reg_offset;
+int *amd64_gcore_gregset32_reg_offset;
 int amd64_native_gregset32_num_regs = I386_NUM_GREGS;
 
 /* General-purpose register mapping for native 64-bit code.  */
@@ -53,7 +54,8 @@ int amd64_native_gregset64_num_regs = AMD64_NUM_GREGS;
    general-purpose register set.  */
 
 static int
-amd64_native_gregset_reg_offset (struct gdbarch *gdbarch, int regnum)
+amd64_native_gregset_reg_offset (struct gdbarch *gdbarch, int regnum,
+				 int gcore)
 {
   int *reg_offset = amd64_native_gregset64_reg_offset;
   int num_regs = amd64_native_gregset64_num_regs;
@@ -62,7 +64,10 @@ amd64_native_gregset_reg_offset (struct gdbarch *gdbarch, int regnum)
 
   if (gdbarch_ptr_bit (gdbarch) == 32)
     {
-      reg_offset = amd64_native_gregset32_reg_offset;
+      if (gcore && amd64_gcore_gregset32_reg_offset)
+	reg_offset = amd64_gcore_gregset32_reg_offset;
+      else
+	reg_offset = amd64_native_gregset32_reg_offset;
       num_regs = amd64_native_gregset32_num_regs;
     }
 
@@ -81,7 +86,7 @@ amd64_native_gregset_reg_offset (struct gdbarch *gdbarch, int regnum)
 int
 amd64_native_gregset_supplies_p (struct gdbarch *gdbarch, int regnum)
 {
-  return (amd64_native_gregset_reg_offset (gdbarch, regnum) != -1);
+  return (amd64_native_gregset_reg_offset (gdbarch, regnum, 0) != -1);
 }
 
 
@@ -107,7 +112,7 @@ amd64_supply_native_gregset (struct regcache *regcache,
     {
       if (regnum == -1 || regnum == i)
 	{
-	  int offset = amd64_native_gregset_reg_offset (gdbarch, i);
+	  int offset = amd64_native_gregset_reg_offset (gdbarch, i, 0);
 
 	  if (offset != -1)
 	    regcache_raw_supply (regcache, i, regs + offset);
@@ -121,14 +126,15 @@ amd64_supply_native_gregset (struct regcache *regcache,
 
 void
 amd64_collect_native_gregset (const struct regcache *regcache,
-			      void *gregs, int regnum)
+			      void *gregs, int regnum, int gcore)
 {
   char *regs = gregs;
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
   int num_regs = amd64_native_gregset64_num_regs;
   int i;
 
-  if (gdbarch_ptr_bit (gdbarch) == 32)
+  /* Don't zero-extended to 64 bits for gcore.  */
+  if (gdbarch_ptr_bit (gdbarch) == 32 && !gcore)
     {
       num_regs = amd64_native_gregset32_num_regs;
 
@@ -137,13 +143,17 @@ amd64_collect_native_gregset (const struct regcache *regcache,
       for (i = 0; i <= I386_EIP_REGNUM; i++)
 	{
 	  if (regnum == -1 || regnum == i)
-	    memset (regs + amd64_native_gregset_reg_offset (gdbarch, i), 0, 8);
+	    memset (regs + amd64_native_gregset_reg_offset (gdbarch, i,
+							    gcore),
+		    0, 8);
 	}
       /* Ditto for %cs, %ss, %ds, %es, %fs, and %gs.  */
       for (i = I386_CS_REGNUM; i <= I386_GS_REGNUM; i++)
 	{
 	  if (regnum == -1 || regnum == i)
-	    memset (regs + amd64_native_gregset_reg_offset (gdbarch, i), 0, 8);
+	    memset (regs + amd64_native_gregset_reg_offset (gdbarch, i,
+							    gcore),
+		    0, 8);
 	}
     }
 
@@ -154,7 +164,7 @@ amd64_collect_native_gregset (const struct regcache *regcache,
     {
       if (regnum == -1 || regnum == i)
 	{
-	  int offset = amd64_native_gregset_reg_offset (gdbarch, i);
+	  int offset = amd64_native_gregset_reg_offset (gdbarch, i, gcore);
 
 	  if (offset != -1)
 	    regcache_raw_collect (regcache, i, regs + offset);
diff --git a/gdb/amd64-nat.h b/gdb/amd64-nat.h
index d1f9199..2aaf0b1 100644
--- a/gdb/amd64-nat.h
+++ b/gdb/amd64-nat.h
@@ -25,6 +25,7 @@ struct regcache;
 
 /* General-purpose register set description for native 32-bit code.  */
 extern int *amd64_native_gregset32_reg_offset;
+extern int *amd64_gcore_gregset32_reg_offset;
 extern int amd64_native_gregset32_num_regs;
 
 /* General-purpose register set description for native 64-bit code.  */
@@ -48,7 +49,8 @@ extern void amd64_supply_native_gregset (struct regcache *regcache,
    registers.  */
 
 extern void amd64_collect_native_gregset (const struct regcache *regcache,
-					  void *gregs, int regnum);
+					  void *gregs, int regnum,
+					  int gcore);
 
 /* Create a prototype *BSD/amd64 target.  The client can override it
    with local methods.  */
diff --git a/gdb/testsuite/gdb.arch/amd64-gcore32.exp b/gdb/testsuite/gdb.arch/amd64-gcore32.exp
new file mode 100644
index 0000000..83dad1e
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-gcore32.exp
@@ -0,0 +1,230 @@
+# Copyright 2010
+# 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/>.
+
+if $tracelevel then {
+	strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+if { ![istarget x86_64-*-linux* ] } {
+    verbose "Skipping amd64-linux 32bit gcore tests."
+    return
+}
+
+set testfile "amd64-gcore32"
+set srcfile  gcore.c
+set binfile  ${objdir}/${subdir}/${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "additional_flags=-m32"]] != "" } {
+     untested amd64-gcore32.exp
+     return -1
+}
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# Does this gdb support gcore?
+send_gdb "help gcore\n"
+gdb_expect {
+    -re "Undefined command: .gcore.*$gdb_prompt $" {
+	# gcore command not supported -- nothing to test here.
+	unsupported "gdb does not support gcore on this target"
+	return -1;
+    }
+    -re "Save a core file .*$gdb_prompt $" {
+	pass "help gcore"
+    }
+    -re ".*$gdb_prompt $" {
+	fail "help gcore"
+    }
+    timeout {
+	fail "help gcore (timeout)"
+    }
+}
+
+if { ! [ runto_main ] } then {
+    untested amd64-gcore32.exp
+    return -1
+}
+
+proc capture_command_output { command prefix } {
+    global gdb_prompt
+    global expect_out
+
+    set output_string ""
+    gdb_test_multiple "$command" "capture_command_output for $command" {
+	-re "${command}\[\r\n\]+${prefix}(.*)\[\r\n\]+$gdb_prompt $" {
+	    set output_string $expect_out(1,string)
+	}
+    }
+    return $output_string
+}
+
+gdb_test "break terminal_func" "Breakpoint .* at .*${srcfile}, line .*" \
+	"set breakpoint at terminal_func"
+
+gdb_test "continue" "Breakpoint .* terminal_func.*" \
+	"continue to terminal_func"
+
+set print_prefix ".\[0123456789\]* = "
+
+set pre_corefile_backtrace [capture_command_output "backtrace" ""]
+set pre_corefile_regs [capture_command_output "info registers" ""]
+set pre_corefile_allregs [capture_command_output "info all-reg" ""]
+set pre_corefile_static_array \
+	[capture_command_output "print static_array" "$print_prefix"]
+set pre_corefile_uninit_array \
+	[capture_command_output "print un_initialized_array" "$print_prefix"]
+set pre_corefile_heap_string \
+	[capture_command_output "print heap_string" "$print_prefix"]
+set pre_corefile_local_array \
+	[capture_command_output "print array_func::local_array" "$print_prefix"]
+set pre_corefile_extern_array \
+	[capture_command_output "print extern_array" "$print_prefix"]
+
+set escapedfilename [string_to_regexp ${objdir}/${subdir}/gcore.test]
+
+set core_supported 0
+gdb_test_multiple "gcore ${objdir}/${subdir}/gcore.test" \
+	"save a corefile" \
+{
+  -re "Saved corefile ${escapedfilename}\[\r\n\]+$gdb_prompt $" {
+    pass "save a corefile"
+    global core_supported
+    set core_supported 1
+  }
+  -re "Can't create a corefile\[\r\n\]+$gdb_prompt $" {
+    unsupported "save a corefile"
+    global core_supported
+    set core_supported 0
+  }
+}
+
+if {!$core_supported} {
+  return -1
+}
+
+# Now restart gdb and load the corefile.
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+send_gdb "core ${objdir}/${subdir}/gcore.test\n"
+gdb_expect {
+    -re ".* is not a core dump:.*$gdb_prompt $" {
+	fail "re-load generated corefile (bad file format)"
+	# No use proceeding from here.
+	return;	
+    }
+    -re ".*: No such file or directory.*$gdb_prompt $" {
+	fail "re-load generated corefile (file not found)"
+	# No use proceeding from here.
+	return;	
+    }
+    -re ".*Couldn't find .* registers in core file.*$gdb_prompt $" {
+	fail "re-load generated corefile (incomplete note section)"
+    }
+    -re "Core was generated by .*$gdb_prompt $" {
+	pass "re-load generated corefile"
+    }
+    -re ".*$gdb_prompt $" {
+	fail "re-load generated corefile"
+    }
+    timeout {
+	fail "re-load generated corefile (timeout)"
+    }
+}
+
+send_gdb "where\n"
+gdb_expect_list "where in corefile" ".*$gdb_prompt $" {
+    ".*\[\r\n\]+#0 .* terminal_func \\(\\) at "
+    ".*\[\r\n\]+#1 .* array_func \\(\\) at "
+    ".*\[\r\n\]+#2 .* factorial_func \\(value=1\\) at "
+    ".*\[\r\n\]+#3 .* factorial_func \\(value=2\\) at "
+    ".*\[\r\n\]+#4 .* factorial_func \\(value=3\\) at "
+    ".*\[\r\n\]+#5 .* factorial_func \\(value=4\\) at "
+    ".*\[\r\n\]+#6 .* factorial_func \\(value=5\\) at "
+    ".*\[\r\n\]+#7 .* factorial_func \\(value=6\\) at "
+    ".*\[\r\n\]+#8 .* main \\(.*\\) at "
+}
+
+set post_corefile_regs [capture_command_output "info registers" ""]
+if ![string compare $pre_corefile_regs $post_corefile_regs] then {
+    pass "corefile restored general registers"
+} else {
+    fail "corefile restored general registers"
+}
+
+set post_corefile_allregs [capture_command_output "info all-reg" ""]
+if ![string compare $pre_corefile_allregs $post_corefile_allregs] then {
+    pass "corefile restored all registers"
+} else {
+    fail "corefile restored all registers"
+}
+
+set post_corefile_extern_array \
+	[capture_command_output "print extern_array" "$print_prefix"]
+if ![string compare $pre_corefile_extern_array $post_corefile_extern_array]  {
+    pass "corefile restored extern array"
+} else {
+    fail "corefile restored extern array"
+}
+
+set post_corefile_static_array \
+	[capture_command_output "print static_array" "$print_prefix"]
+if ![string compare $pre_corefile_static_array $post_corefile_static_array]  {
+    pass "corefile restored static array"
+} else {
+    fail "corefile restored static array"
+}
+
+set post_corefile_uninit_array \
+	[capture_command_output "print un_initialized_array" "$print_prefix"]
+if ![string compare $pre_corefile_uninit_array $post_corefile_uninit_array]  {
+    pass "corefile restored un-initialized array"
+} else {
+    fail "corefile restored un-initialized array"
+}
+
+set post_corefile_heap_string \
+	[capture_command_output "print heap_string" "$print_prefix"]
+if ![string compare $pre_corefile_heap_string $post_corefile_heap_string]  {
+    pass "corefile restored heap array"
+} else {
+    fail "corefile restored heap array"
+}
+
+set post_corefile_local_array \
+	[capture_command_output "print array_func::local_array" "$print_prefix"]
+if ![string compare $pre_corefile_local_array $post_corefile_local_array]  {
+    pass "corefile restored stack array"
+} else {
+    fail "corefile restored stack array"
+}
+
+set post_corefile_backtrace [capture_command_output "backtrace" ""]
+if ![string compare $pre_corefile_backtrace $post_corefile_backtrace]  {
+    pass "corefile restored backtrace"
+} else {
+    fail "corefile restored backtrace"
+}
diff --git a/gdb/testsuite/gdb.arch/gcore.c b/gdb/testsuite/gdb.arch/gcore.c
new file mode 100644
index 0000000..3eb10b2
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/gcore.c
@@ -0,0 +1,70 @@
+/* Copyright 2002, 2004, 2007, 2008, 2009, 2010 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+/*
+ * Test GDB's ability to save and reload a corefile.
+ */
+
+#include <stdlib.h>
+#include <string.h>
+
+int extern_array[4] = {1, 2, 3, 4};
+static int static_array[4] = {5, 6, 7, 8};
+static int un_initialized_array[4];
+static char *heap_string;
+
+void 
+terminal_func ()
+{
+  return;
+}
+
+void
+array_func ()
+{
+  int local_array[4];
+  int i;
+
+  heap_string = (char *) malloc (80);
+  strcpy (heap_string, "I'm a little teapot, short and stout...");
+  for (i = 0; i < 4; i++)
+    {
+      un_initialized_array[i] = extern_array[i] + 8;
+      local_array[i] = extern_array[i] + 12;
+    }
+  terminal_func ();
+}
+
+#ifdef PROTOTYPES
+int factorial_func (int value)
+#else
+int factorial_func (value)
+     int value;
+#endif
+{
+  if (value > 1) {
+    value *= factorial_func (value - 1);
+  }
+  array_func ();
+  return (value);
+}
+
+main()
+{
+  factorial_func (6);
+  return 0;
+}