Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit core  file
@ 2010-04-10 22:19 H.J. Lu
  2010-04-10 22:27 ` H.J. Lu
  0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2010-04-10 22:19 UTC (permalink / raw)
  To: binutils, jan.kratochvil; +Cc: GDB

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.
diff --git a/bfd/config.in b/bfd/config.in
index 5881b8d..065c905 100644
--- a/bfd/config.in
+++ b/bfd/config.in
@@ -1,5 +1,8 @@
 /* config.in.  Generated from configure.in by autoheader.  */
 
+/* Name of host specific core header file to include in elf.c. */
+#undef CORE_HEADER
+
 /* Define to 1 if translation of program messages to the user's native
    language is requested. */
 #undef ENABLE_NLS
diff --git a/bfd/configure b/bfd/configure
index f4aba27..928f984 100755
--- a/bfd/configure
+++ b/bfd/configure
@@ -13810,6 +13810,7 @@ fi
 # If we are configured native, pick a core file support file.
 COREFILE=
 COREFLAG=
+CORE_HEADER=
 TRAD_HEADER=
 if test "${target}" = "${host}"; then
   case "${host}" in
@@ -14034,6 +14035,9 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
 	COREFILE=trad-core.lo
 	TRAD_HEADER='"hosts/vaxbsd.h"'
 	;;
+  x86_64-*-linux*)
+	CORE_HEADER='"hosts/x86-64linux.h"'
+	;;
   x86_64-*-netbsd* | x86_64-*-openbsd*)
 	COREFILE=netbsd-core.lo
 	;;
@@ -14701,6 +14705,13 @@ $as_echo "$bfd_cv_have_sys_procfs_type_win32_pstatus_t" >&6; }
 fi
 
 
+if test -n "$CORE_HEADER"; then
+
+cat >>confdefs.h <<_ACEOF
+#define CORE_HEADER $CORE_HEADER
+_ACEOF
+
+fi
 if test -n "$TRAD_HEADER"; then
 
 cat >>confdefs.h <<_ACEOF
diff --git a/bfd/configure.in b/bfd/configure.in
index dfbd0a2..28d5bdd 100644
--- a/bfd/configure.in
+++ b/bfd/configure.in
@@ -211,6 +211,7 @@ AM_ZLIB
 # If we are configured native, pick a core file support file.
 COREFILE=
 COREFLAG=
+CORE_HEADER=
 TRAD_HEADER=
 if test "${target}" = "${host}"; then
   case "${host}" in
@@ -443,6 +444,9 @@ changequote([,])dnl
 	COREFILE=trad-core.lo
 	TRAD_HEADER='"hosts/vaxbsd.h"'
 	;;
+  x86_64-*-linux*)
+	CORE_HEADER='"hosts/x86-64linux.h"'
+	;;
   x86_64-*-netbsd* | x86_64-*-openbsd*)
 	COREFILE=netbsd-core.lo
 	;;
@@ -487,6 +491,10 @@ changequote([,])dnl
 fi
 AC_SUBST(COREFILE)
 AC_SUBST(COREFLAG)
+if test -n "$CORE_HEADER"; then
+  AC_DEFINE_UNQUOTED(CORE_HEADER, $CORE_HEADER,
+    [Name of host specific core header file to include in elf.c.])
+fi
 if test -n "$TRAD_HEADER"; then
   AC_DEFINE_UNQUOTED(TRAD_HEADER, $TRAD_HEADER,
     [Name of host specific header file to include in trad-core.c.])
diff --git a/bfd/elf.c b/bfd/elf.c
index 8f8bd61..bcd238d 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -45,6 +45,10 @@ SECTION
 #include "libiberty.h"
 #include "safe-ctype.h"
 
+#ifdef CORE_HEADER
+#include CORE_HEADER
+#endif
+
 static int elf_sort_sections (const void *, const void *);
 static bfd_boolean assign_file_positions_except_relocs (bfd *, struct bfd_link_info *);
 static bfd_boolean prep_headers (bfd *);
diff --git a/bfd/hosts/x86-64linux.h b/bfd/hosts/x86-64linux.h
new file mode 100644
index 0000000..fdf17f1
--- /dev/null
+++ b/bfd/hosts/x86-64linux.h
@@ -0,0 +1,192 @@
+/* Copyright (C) 2006 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+/* This is somewhat modelled after the file of the same name on SVR4
+   systems.  It provides a definition of the core file format for ELF
+   used on Linux.  It doesn't have anything to do with the /proc file
+   system, even though Linux has one.
+
+   Anyway, the whole purpose of this file is for GDB and GDB only.
+   Don't read too much into it.  Don't use it for anything other than
+   GDB unless you know what you are doing.  */
+
+#include <features.h>
+#include <sys/time.h>
+#include <sys/types.h>
+
+/* We define here only the symbols differing from their 64-bit variant.  */
+#include <sys/procfs.h>
+
+#ifdef HAVE_STDINT_H
+#include <stdint.h>
+#else
+typedef unsigned int uint32_t;
+#endif
+
+#define HAVE_PRPSINFO32_T
+#define HAVE_PRSTATUS32_T
+
+/* These are the 32-bit x86 structures.  */
+
+struct user_fpregs32_struct
+{
+  int32_t cwd;
+  int32_t swd;
+  int32_t twd;
+  int32_t fip;
+  int32_t fcs;
+  int32_t foo;
+  int32_t fos;
+  int32_t st_space [20];
+};
+
+struct user_fpxregs32_struct
+{
+  unsigned short int cwd;
+  unsigned short int swd;
+  unsigned short int twd;
+  unsigned short int fop;
+  int32_t fip;
+  int32_t fcs;
+  int32_t foo;
+  int32_t fos;
+  int32_t mxcsr;
+  int32_t reserved;
+  int32_t st_space[32];   /* 8*16 bytes for each FP-reg = 128 bytes */
+  int32_t xmm_space[32];  /* 8*16 bytes for each XMM-reg = 128 bytes */
+  int32_t padding[56];
+};
+
+struct user_regs32_struct
+{
+  int32_t ebx;
+  int32_t ecx;
+  int32_t edx;
+  int32_t esi;
+  int32_t edi;
+  int32_t ebp;
+  int32_t eax;
+  int32_t xds;
+  int32_t xes;
+  int32_t xfs;
+  int32_t xgs;
+  int32_t orig_eax;
+  int32_t eip;
+  int32_t xcs;
+  int32_t eflags;
+  int32_t esp;
+  int32_t xss;
+};
+
+struct user32
+{
+  struct user_regs32_struct	regs;
+  int				u_fpvalid;
+  struct user_fpregs32_struct	i387;
+  uint32_t			u_tsize;
+  uint32_t			u_dsize;
+  uint32_t			u_ssize;
+  uint32_t			start_code;
+  uint32_t			start_stack;
+  int32_t			signal;
+  int				reserved;
+  struct user_regs32_struct*	u_ar0;
+  struct user_fpregs32_struct*	u_fpstate;
+  uint32_t			magic;
+  char				u_comm [32];
+  int				u_debugreg [8];
+};
+
+/* Type for a general-purpose register.  */
+typedef unsigned int elf_greg32_t;
+
+/* And the whole bunch of them.  We could have used `struct
+   user_regs_struct' directly in the typedef, but tradition says that
+   the register set is an array, which does have some peculiar
+   semantics, so leave it that way.  */
+#define ELF_NGREG32 (sizeof (struct user_regs32_struct) / sizeof(elf_greg32_t))
+typedef elf_greg32_t elf_gregset32_t[ELF_NGREG32];
+
+/* Register set for the floating-point registers.  */
+typedef struct user_fpregs32_struct elf_fpregset32_t;
+
+/* Register set for the extended floating-point registers.  Includes
+   the Pentium III SSE registers in addition to the classic
+   floating-point stuff.  */
+typedef struct user_fpxregs32_struct elf_fpxregset32_t;
+
+
+/* Definitions to generate Intel SVR4-like core files.  These mostly
+   have the same names as the SVR4 types with "elf_" tacked on the
+   front to prevent clashes with Linux definitions, and the typedef
+   forms have been avoided.  This is mostly like the SVR4 structure,
+   but more Linuxy, with things that Linux does not support and which
+   GDB doesn't really use excluded.  */
+
+struct prstatus32_timeval
+  {
+    int tv_sec;
+    int tv_usec;
+  };
+
+struct elf_prstatus32
+  {
+    struct elf_siginfo pr_info;		/* Info associated with signal.  */
+    short int pr_cursig;		/* Current signal.  */
+    unsigned int pr_sigpend;		/* Set of pending signals.  */
+    unsigned int pr_sighold;		/* Set of held signals.  */
+    __pid_t pr_pid;
+    __pid_t pr_ppid;
+    __pid_t pr_pgrp;
+    __pid_t pr_sid;
+    struct prstatus32_timeval pr_utime;		/* User time.  */
+    struct prstatus32_timeval pr_stime;		/* System time.  */
+    struct prstatus32_timeval pr_cutime;	/* Cumulative user time.  */
+    struct prstatus32_timeval pr_cstime;	/* Cumulative system time.  */
+    elf_gregset32_t pr_reg;		/* GP registers.  */
+    int pr_fpvalid;			/* True if math copro being used.  */
+  };
+
+
+struct elf_prpsinfo32
+  {
+    char pr_state;			/* Numeric process state.  */
+    char pr_sname;			/* Char for pr_state.  */
+    char pr_zomb;			/* Zombie.  */
+    char pr_nice;			/* Nice val.  */
+    unsigned int pr_flag;		/* Flags.  */
+    unsigned short int pr_uid;
+    unsigned short int pr_gid;
+    int pr_pid, pr_ppid, pr_pgrp, pr_sid;
+    /* Lots missing */
+    char pr_fname[16];			/* Filename of executable.  */
+    char pr_psargs[ELF_PRARGSZ];	/* Initial part of arg list.  */
+  };
+
+
+/* The rest of this file provides the types for emulation of the
+   Solaris <proc_service.h> interfaces that should be implemented by
+   users of libthread_db.  */
+
+/* Register sets.  Linux has different names.  */
+typedef elf_gregset_t prgregset32_t;
+typedef elf_fpregset_t prfpregset32_t;
+
+/* Process status and info.  In the end we do provide typedefs for them.  */
+typedef struct elf_prstatus32 prstatus32_t;
+typedef struct elf_prpsinfo32 prpsinfo32_t;


^ permalink raw reply	[flat|nested] 19+ messages in thread

* PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit core  file
  2010-04-10 22:19 PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit core file H.J. Lu
@ 2010-04-10 22:27 ` H.J. Lu
  2010-04-11  0:01   ` H.J. Lu
  2010-04-11 16:50   ` Mark Kettenis
  0 siblings, 2 replies; 19+ messages in thread
From: H.J. Lu @ 2010-04-10 22:27 UTC (permalink / raw)
  To: GDB; +Cc: jan.kratochvil

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?

Thanks.


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.

	* 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..ac49448 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,7 +126,7 @@ 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);
@@ -137,13 +142,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 +163,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;
+}


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit  core file
  2010-04-10 22:27 ` H.J. Lu
@ 2010-04-11  0:01   ` H.J. Lu
  2010-04-11 20:53     ` H.J. Lu
  2010-04-11 16:50   ` Mark Kettenis
  1 sibling, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2010-04-11  0:01 UTC (permalink / raw)
  To: GDB, jan.kratochvil

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;
+}


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit core  file
  2010-04-10 22:27 ` H.J. Lu
  2010-04-11  0:01   ` H.J. Lu
@ 2010-04-11 16:50   ` Mark Kettenis
  2010-04-11 17:33     ` H.J. Lu
  1 sibling, 1 reply; 19+ messages in thread
From: Mark Kettenis @ 2010-04-11 16:50 UTC (permalink / raw)
  To: hjl.tools; +Cc: gdb-patches, jan.kratochvil

> Date: Sat, 10 Apr 2010 15:27:42 -0700
> From: "H.J. Lu" <hongjiu.lu@intel.com>
> 
> 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?

No, this is wrong.  This should all be handled by -tdep.c code.  The
gcore code in linux-nat.c nly falls back on the fetch_gregset() if for
some reason the gdbarch_regset_from_core_section() function doesn't
return a valid register set.  I don't immediately see why, but that is
what you need to fix.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit   core file
  2010-04-11 16:50   ` Mark Kettenis
@ 2010-04-11 17:33     ` H.J. Lu
  0 siblings, 0 replies; 19+ messages in thread
From: H.J. Lu @ 2010-04-11 17:33 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches, jan.kratochvil

On Sun, Apr 11, 2010 at 9:49 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> Date: Sat, 10 Apr 2010 15:27:42 -0700
>> From: "H.J. Lu" <hongjiu.lu@intel.com>
>>
>> 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?
>
> No, this is wrong.  This should all be handled by -tdep.c code.  The
> gcore code in linux-nat.c nly falls back on the fetch_gregset() if for
> some reason the gdbarch_regset_from_core_section() function doesn't
> return a valid register set.  I don't immediately see why, but that is
> what you need to fix.
>

I will prepare a patch. I will move codes from i386-linux-nat.c to
i386-linux-tdep.c.  I will add i386_linux_regset_from_core_section
and call

 set_gdbarch_regset_from_core_section (gdbarch,
              i386_linux_regset_from_core_section);

in i386_linux_init_abi. i386_linux_regset_from_core_section will
call fill_gregset originally in i386-linux-nat.c.

Thanks.


-- 
H.J.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit  core file
  2010-04-11  0:01   ` H.J. Lu
@ 2010-04-11 20:53     ` H.J. Lu
  2010-04-12 13:22       ` H.J. Lu
  0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2010-04-11 20:53 UTC (permalink / raw)
  To: kettenis; +Cc: GDB, jan.kratochvil

Hi,

Thanks for Mark's pointer. Solution is very simple. We just need to
make sure that we call the right fill_gregset for 32bit executable
on both Linux/x86-64 and Linux/i386.  OK to install?

Thanks.


H.J.
--
gdb/

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

	PR corefiles/11467
	PR corefiles/11467
	* amd64-linux-nat.c (fill_gregset): Call i386_linux_fill_gregset
	for 32bit.

	* i386-linux-nat.c (fill_gregset): Call i386_linux_fill_gregset.

	* i386-linux-tdep.c (i386_linux_fill_gregset): New.
	* i386-linux-tdep.h (i386_linux_fill_gregset): Likewise.

gdb/testsuite/

2010-04-11  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..336232e 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -141,7 +141,11 @@ void
 fill_gregset (const struct regcache *regcache,
 	      elf_gregset_t *gregsetp, int regnum)
 {
-  amd64_collect_native_gregset (regcache, gregsetp, regnum);
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  if (gdbarch_ptr_bit (gdbarch) == 32)
+    i386_linux_fill_gregset (regcache, gregsetp, regnum);
+  else
+    amd64_collect_native_gregset (regcache, gregsetp, regnum);
 }
 
 /* Transfering floating-point registers between GDB, inferiors and cores.  */
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index d559811..2ba120b 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -245,18 +245,7 @@ void
 fill_gregset (const struct regcache *regcache,
 	      elf_gregset_t *gregsetp, int regno)
 {
-  elf_greg_t *regp = (elf_greg_t *) gregsetp;
-  int i;
-
-  for (i = 0; i < I386_NUM_GREGS; i++)
-    if (regno == -1 || regno == i)
-      regcache_raw_collect (regcache, i, regp + regmap[i]);
-
-  if ((regno == -1 || regno == I386_LINUX_ORIG_EAX_REGNUM)
-      && I386_LINUX_ORIG_EAX_REGNUM
-	   < gdbarch_num_regs (get_regcache_arch (regcache)))
-    regcache_raw_collect (regcache, I386_LINUX_ORIG_EAX_REGNUM,
-			  regp + ORIG_EAX);
+  i386_linux_fill_gregset (regcache, gregsetp, regno);
 }
 
 #ifdef HAVE_PTRACE_GETREGS
diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index 72aced5..ee277ac 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -567,6 +567,32 @@ static int i386_linux_sc_reg_offset[] =
   0 * 4				/* %gs */
 };
 
+/* Fill register REGNO (if it is a general-purpose register) in
+   *GREGSETPS with the value in GDB's register array.  If REGNO is -1,
+   do this for all registers.  */
+
+void
+i386_linux_fill_gregset (const struct regcache *regcache,
+			 void *gregs, int regnum)
+{
+  char *regs = gregs;
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  int i, num_regs = tdep->gregset_num_regs;
+
+  gdb_assert (num_regs == gdbarch_num_regs (gdbarch));
+  for (i = 0; i < num_regs; i++)
+    {
+      if (regnum == -1 || regnum == i)
+	{
+	  int offset = i386_linux_gregset_reg_offset[i];
+
+	  if (offset != -1)
+	    regcache_raw_collect (regcache, i, regs + offset);
+	}
+    }
+}
+
 /* Get XSAVE extended state xcr0 from core dump.  */
 
 uint64_t
diff --git a/gdb/i386-linux-tdep.h b/gdb/i386-linux-tdep.h
index 1228681..d1830b7 100644
--- a/gdb/i386-linux-tdep.h
+++ b/gdb/i386-linux-tdep.h
@@ -39,6 +39,11 @@
 extern uint64_t i386_linux_core_read_xcr0
   (struct gdbarch *gdbarch, struct target_ops *target, bfd *abfd);
 
+/* Fill general-purpose register with the value in GDB's register
+   array. */
+extern void i386_linux_fill_gregset (const struct regcache *regcache,
+				     void *gregs, int regnum);
+
 /* Linux target description.  */
 extern struct target_desc *tdesc_i386_linux;
 extern struct target_desc *tdesc_i386_mmx_linux;
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;
+}


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit  core file
  2010-04-11 20:53     ` H.J. Lu
@ 2010-04-12 13:22       ` H.J. Lu
  2010-04-12 18:24         ` Mark Kettenis
  2010-04-13 17:18         ` H.J. Lu
  0 siblings, 2 replies; 19+ messages in thread
From: H.J. Lu @ 2010-04-12 13:22 UTC (permalink / raw)
  To: mark.kettenis; +Cc: GDB, jan.kratochvil

On Sun, Apr 11, 2010 at 01:52:50PM -0700, H.J. Lu wrote:
> Hi,
> 
> Thanks for Mark's pointer. Solution is very simple. We just need to
> make sure that we call the right fill_gregset for 32bit executable
> on both Linux/x86-64 and Linux/i386.  OK to install?
> 
> Thanks.
> 
> 

Small update to use tdep->gregset_reg_offset instead of
i386_linux_gregset_reg_offset.  OK to install?

Thanks.


H.J.
---
gdb/

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

	PR corefiles/11467
	PR corefiles/11467
	* amd64-linux-nat.c (fill_gregset): Call i386_linux_fill_gregset
	for 32bit.

	* i386-linux-nat.c (fill_gregset): Call i386_linux_fill_gregset.

	* i386-linux-tdep.c (i386_linux_fill_gregset): New.
	* i386-linux-tdep.h (i386_linux_fill_gregset): Likewise.

gdb/testsuite/

2010-04-11  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..336232e 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -141,7 +141,11 @@ void
 fill_gregset (const struct regcache *regcache,
 	      elf_gregset_t *gregsetp, int regnum)
 {
-  amd64_collect_native_gregset (regcache, gregsetp, regnum);
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  if (gdbarch_ptr_bit (gdbarch) == 32)
+    i386_linux_fill_gregset (regcache, gregsetp, regnum);
+  else
+    amd64_collect_native_gregset (regcache, gregsetp, regnum);
 }
 
 /* Transfering floating-point registers between GDB, inferiors and cores.  */
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index d559811..2ba120b 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -245,18 +245,7 @@ void
 fill_gregset (const struct regcache *regcache,
 	      elf_gregset_t *gregsetp, int regno)
 {
-  elf_greg_t *regp = (elf_greg_t *) gregsetp;
-  int i;
-
-  for (i = 0; i < I386_NUM_GREGS; i++)
-    if (regno == -1 || regno == i)
-      regcache_raw_collect (regcache, i, regp + regmap[i]);
-
-  if ((regno == -1 || regno == I386_LINUX_ORIG_EAX_REGNUM)
-      && I386_LINUX_ORIG_EAX_REGNUM
-	   < gdbarch_num_regs (get_regcache_arch (regcache)))
-    regcache_raw_collect (regcache, I386_LINUX_ORIG_EAX_REGNUM,
-			  regp + ORIG_EAX);
+  i386_linux_fill_gregset (regcache, gregsetp, regno);
 }
 
 #ifdef HAVE_PTRACE_GETREGS
diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index 72aced5..513e4c1 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -567,6 +567,34 @@ static int i386_linux_sc_reg_offset[] =
   0 * 4				/* %gs */
 };
 
+/* Fill register REGNUM (if it is a general-purpose register) in
+   *GREGS with the value in GDB's register array.  If REGNUM is -1,
+   do this for all registers.  */
+
+void
+i386_linux_fill_gregset (const struct regcache *regcache,
+			 void *gregs, int regnum)
+{
+  char *regs = gregs;
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  int *reg_offset = tdep->gregset_reg_offset;
+  int num_regs = tdep->gregset_num_regs;
+  int i;
+
+  gdb_assert (num_regs == gdbarch_num_regs (gdbarch));
+  for (i = 0; i < num_regs; i++)
+    {
+      if (regnum == -1 || regnum == i)
+	{
+	  int offset = reg_offset[i];
+
+	  if (offset != -1)
+	    regcache_raw_collect (regcache, i, regs + offset);
+	}
+    }
+}
+
 /* Get XSAVE extended state xcr0 from core dump.  */
 
 uint64_t
diff --git a/gdb/i386-linux-tdep.h b/gdb/i386-linux-tdep.h
index 1228681..d1830b7 100644
--- a/gdb/i386-linux-tdep.h
+++ b/gdb/i386-linux-tdep.h
@@ -39,6 +39,11 @@
 extern uint64_t i386_linux_core_read_xcr0
   (struct gdbarch *gdbarch, struct target_ops *target, bfd *abfd);
 
+/* Fill general-purpose register with the value in GDB's register
+   array. */
+extern void i386_linux_fill_gregset (const struct regcache *regcache,
+				     void *gregs, int regnum);
+
 /* Linux target description.  */
 extern struct target_desc *tdesc_i386_linux;
 extern struct target_desc *tdesc_i386_mmx_linux;
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;
+}


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit  core file
  2010-04-12 13:22       ` H.J. Lu
@ 2010-04-12 18:24         ` Mark Kettenis
  2010-04-12 18:50           ` H.J. Lu
  2010-04-13 17:18         ` H.J. Lu
  1 sibling, 1 reply; 19+ messages in thread
From: Mark Kettenis @ 2010-04-12 18:24 UTC (permalink / raw)
  To: hjl.tools; +Cc: gdb-patches, jan.kratochvil

> Date: Mon, 12 Apr 2010 06:22:25 -0700
> From: "H.J. Lu" <hongjiu.lu@intel.com>
> 
> On Sun, Apr 11, 2010 at 01:52:50PM -0700, H.J. Lu wrote:
> > Hi,
> > 
> > Thanks for Mark's pointer. Solution is very simple. We just need to
> > make sure that we call the right fill_gregset for 32bit executable
> > on both Linux/x86-64 and Linux/i386.  OK to install?
> > 
> > Thanks.
> > 
> > 
> 
> Small update to use tdep->gregset_reg_offset instead of
> i386_linux_gregset_reg_offset.  OK to install?

No.  Like I explained in an earlier mail, we're not supposed to end up
in fetch_gregset() in the first place.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit   core file
  2010-04-12 18:24         ` Mark Kettenis
@ 2010-04-12 18:50           ` H.J. Lu
  2010-04-13 18:40             ` Mark Kettenis
  0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2010-04-12 18:50 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches, jan.kratochvil

On Mon, Apr 12, 2010 at 11:23 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> Date: Mon, 12 Apr 2010 06:22:25 -0700
>> From: "H.J. Lu" <hongjiu.lu@intel.com>
>>
>> On Sun, Apr 11, 2010 at 01:52:50PM -0700, H.J. Lu wrote:
>> > Hi,
>> >
>> > Thanks for Mark's pointer. Solution is very simple. We just need to
>> > make sure that we call the right fill_gregset for 32bit executable
>> > on both Linux/x86-64 and Linux/i386.  OK to install?
>> >
>> > Thanks.
>> >
>> >
>>
>> Small update to use tdep->gregset_reg_offset instead of
>> i386_linux_gregset_reg_offset.  OK to install?
>
> No.  Like I explained in an earlier mail, we're not supposed to end up
> in fetch_gregset() in the first place.
>

fetch_gregset is always defined and used to fill .reg section in
coredump on Linux/x86. i386-tdep.c has

const struct regset *
i386_regset_from_core_section (struct gdbarch *gdbarch,
                               const char *sect_name, size_t sect_size)
{
  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

  if (strcmp (sect_name, ".reg") == 0 && sect_size == tdep->sizeof_gregset)
    {
      if (tdep->gregset == NULL)
        tdep->gregset = regset_alloc (gdbarch, i386_supply_gregset,
                                      i386_collect_gregset);
      return tdep->gregset;
    }

For Linux,  tdep->sizeof_gregset != the size of .reg section. In fact,
they have nothing to do with each other. sizeof_gregset includes
SSE and AVX registers, which have offset == -1 since they aren't
general-purpose registers. I don't believe we should set tdep->gregset
since general-purpose registers is a special case for Linux/x86. They
are handled differently. I don't want to change it.

Please see i386-linux-nat.c for all the details.

Thanks.


-- 
H.J.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit  core file
  2010-04-12 13:22       ` H.J. Lu
  2010-04-12 18:24         ` Mark Kettenis
@ 2010-04-13 17:18         ` H.J. Lu
  2010-04-13 17:27           ` Mark Kettenis
  2010-04-13 17:48           ` H.J. Lu
  1 sibling, 2 replies; 19+ messages in thread
From: H.J. Lu @ 2010-04-13 17:18 UTC (permalink / raw)
  To: GDB; +Cc: mark.kettenis, jan.kratochvil

On Mon, Apr 12, 2010 at 06:22:25AM -0700, H.J. Lu wrote:
> On Sun, Apr 11, 2010 at 01:52:50PM -0700, H.J. Lu wrote:
> > Hi,
> > 
> > Thanks for Mark's pointer. Solution is very simple. We just need to
> > make sure that we call the right fill_gregset for 32bit executable
> > on both Linux/x86-64 and Linux/i386.  OK to install?
> > 
> > Thanks.
> > 
> > 
> 
> Small update to use tdep->gregset_reg_offset instead of
> i386_linux_gregset_reg_offset.  OK to install?
> 


Here is the updated patch. It calls set_gdbarch_regset_from_core_section
with i386_linux_regset_from_core_section.  OK to install?

Thanks.


H.J.
---
gdb/

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

	PR corefiles/11467
	* i386-linux-nat.c (regmap): Removed.
	(fetch_register): Replace regmap with tdep->gregset_reg_offset.
	(store_register): Likewise.
	(supply_gregset): Call i386_fetch_gregset.
	(fetch_regs): Likewise.
	(fill_gregset): Call i386_fill_gregset.
	(store_regs): Likewise.

	* i386-linux-tdep.c (i386_linux_supply_gregset): New.
	(i386_linux_collect_gregset): Likewise.
	(i386_linux_regset_from_core_section): Likewise.
	(i386_linux_init_abi): Call set_gdbarch_regset_from_core_section
	with i386_linux_regset_from_core_section.

	* i386-tdep.c (i386_fetch_gregset): New.
	(i386_supply_gregset): Use it.
	(i386_fill_gregset): New.
	(i386_collect_gregset): Use it.

	* i386-tdep.h (i386_fetch_gregset): New.
	(i386_fill_gregset): Likewise..

gdb/testsuite/

2010-04-13  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/i386-linux-nat.c b/gdb/i386-linux-nat.c
index d559811..023c471 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -99,26 +99,6 @@ static int have_ptrace_getregset = -1;
    those names are now used for the register sets used in the
    `mcontext_t' type, and have a different size and layout.  */
 
-/* Mapping between the general-purpose registers in `struct user'
-   format and GDB's register array layout.  */
-static int regmap[] = 
-{
-  EAX, ECX, EDX, EBX,
-  UESP, EBP, ESI, EDI,
-  EIP, EFL, CS, SS,
-  DS, ES, FS, GS,
-  -1, -1, -1, -1,		/* st0, st1, st2, st3 */
-  -1, -1, -1, -1,		/* st4, st5, st6, st7 */
-  -1, -1, -1, -1,		/* fctrl, fstat, ftag, fiseg */
-  -1, -1, -1, -1,		/* fioff, foseg, fooff, fop */
-  -1, -1, -1, -1,		/* xmm0, xmm1, xmm2, xmm3 */
-  -1, -1, -1, -1,		/* xmm4, xmm5, xmm6, xmm6 */
-  -1,				/* mxcsr */
-  -1, -1, -1, -1,		/* ymm0h, ymm1h, ymm2h, ymm3h */
-  -1, -1, -1, -1,		/* ymm4h, ymm5h, ymm6h, ymm6h */
-  ORIG_EAX
-};
-
 /* Which ptrace request retrieves which registers?
    These apply to the corresponding SET requests as well.  */
 
@@ -166,9 +146,12 @@ fetch_register (struct regcache *regcache, int regno)
 {
   int tid;
   int val;
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  int *reg_offset = tdep->gregset_reg_offset;
 
   gdb_assert (!have_ptrace_getregs);
-  if (regmap[regno] == -1)
+  if (reg_offset[regno] == -1)
     {
       regcache_raw_supply (regcache, regno, NULL);
       return;
@@ -180,7 +163,7 @@ fetch_register (struct regcache *regcache, int regno)
     tid = PIDGET (inferior_ptid); /* Not a threaded program.  */
 
   errno = 0;
-  val = ptrace (PTRACE_PEEKUSER, tid, 4 * regmap[regno], 0);
+  val = ptrace (PTRACE_PEEKUSER, tid, reg_offset[regno], 0);
   if (errno != 0)
     error (_("Couldn't read register %s (#%d): %s."), 
 	   gdbarch_register_name (get_regcache_arch (regcache), regno),
@@ -196,9 +179,12 @@ store_register (const struct regcache *regcache, int regno)
 {
   int tid;
   int val;
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  int *reg_offset = tdep->gregset_reg_offset;
 
   gdb_assert (!have_ptrace_getregs);
-  if (regmap[regno] == -1)
+  if (reg_offset[regno] == -1)
     return;
 
   /* GNU/Linux LWP ID's are process ID's.  */
@@ -208,7 +194,7 @@ store_register (const struct regcache *regcache, int regno)
 
   errno = 0;
   regcache_raw_collect (regcache, regno, &val);
-  ptrace (PTRACE_POKEUSER, tid, 4 * regmap[regno], val);
+  ptrace (PTRACE_POKEUSER, tid, reg_offset[regno], val);
   if (errno != 0)
     error (_("Couldn't write register %s (#%d): %s."),
 	   gdbarch_register_name (get_regcache_arch (regcache), regno),
@@ -225,16 +211,7 @@ store_register (const struct regcache *regcache, int regno)
 void
 supply_gregset (struct regcache *regcache, const elf_gregset_t *gregsetp)
 {
-  const elf_greg_t *regp = (const elf_greg_t *) gregsetp;
-  int i;
-
-  for (i = 0; i < I386_NUM_GREGS; i++)
-    regcache_raw_supply (regcache, i, regp + regmap[i]);
-
-  if (I386_LINUX_ORIG_EAX_REGNUM
-	< gdbarch_num_regs (get_regcache_arch (regcache)))
-    regcache_raw_supply (regcache, I386_LINUX_ORIG_EAX_REGNUM,
-			 regp + ORIG_EAX);
+  i386_fetch_gregset (regcache, gregsetp, -1);
 }
 
 /* Fill register REGNO (if it is a general-purpose register) in
@@ -245,18 +222,7 @@ void
 fill_gregset (const struct regcache *regcache,
 	      elf_gregset_t *gregsetp, int regno)
 {
-  elf_greg_t *regp = (elf_greg_t *) gregsetp;
-  int i;
-
-  for (i = 0; i < I386_NUM_GREGS; i++)
-    if (regno == -1 || regno == i)
-      regcache_raw_collect (regcache, i, regp + regmap[i]);
-
-  if ((regno == -1 || regno == I386_LINUX_ORIG_EAX_REGNUM)
-      && I386_LINUX_ORIG_EAX_REGNUM
-	   < gdbarch_num_regs (get_regcache_arch (regcache)))
-    regcache_raw_collect (regcache, I386_LINUX_ORIG_EAX_REGNUM,
-			  regp + ORIG_EAX);
+  i386_fill_gregset (regcache, gregsetp, -1);
 }
 
 #ifdef HAVE_PTRACE_GETREGS
@@ -283,7 +249,7 @@ fetch_regs (struct regcache *regcache, int tid)
       perror_with_name (_("Couldn't get registers"));
     }
 
-  supply_gregset (regcache, (const elf_gregset_t *) regs_p);
+  i386_fetch_gregset (regcache, (const void *) regs_p, -1);
 }
 
 /* Store all valid general-purpose registers in GDB's register array
@@ -297,7 +263,7 @@ store_regs (const struct regcache *regcache, int tid, int regno)
   if (ptrace (PTRACE_GETREGS, tid, 0, (int) &regs) < 0)
     perror_with_name (_("Couldn't get registers"));
 
-  fill_gregset (regcache, &regs, regno);
+  i386_fill_gregset (regcache, &regs, regno);
   
   if (ptrace (PTRACE_SETREGS, tid, 0, (int) &regs) < 0)
     perror_with_name (_("Couldn't write registers"));
diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index 3fc7ef0..d860d1a 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -636,6 +636,56 @@ i386_linux_core_read_description (struct gdbarch *gdbarch,
     return tdesc_i386_linux;
 }
 
+/* Supply register REGNUM from the buffer specified by GREGS and LEN
+   in the general-purpose register set REGSET to register cache
+   REGCACHE.  If REGNUM is -1, do this for all registers in REGSET.  */
+
+static void
+i386_linux_supply_gregset (const struct regset *regset,
+			   struct regcache *regcache,
+			   int regnum, const void *gregs, size_t len)
+{
+  const struct gdbarch_tdep *tdep = gdbarch_tdep (regset->arch);
+  i386_supply_gregset (regset, regcache, regnum, gregs,
+		       tdep->sizeof_gregset);
+}
+
+/* Collect register REGNUM from the register cache REGCACHE and store
+   it in the buffer specified by GREGS and LEN as described by the
+   general-purpose register set REGSET.  If REGNUM is -1, do this for
+   all registers in REGSET.  */
+
+static void
+i386_linux_collect_gregset (const struct regset *regset,
+			    const struct regcache *regcache,
+			    int regnum, void *gregs, size_t len)
+{
+  const struct gdbarch_tdep *tdep = gdbarch_tdep (regset->arch);
+  i386_collect_gregset (regset, regcache, regnum, gregs,
+			tdep->sizeof_gregset);
+}
+
+/* Return the appropriate register set for the core section identified
+   by SECT_NAME and SECT_SIZE.  */
+
+const struct regset *
+i386_linux_regset_from_core_section (struct gdbarch *gdbarch,
+				     const char *sect_name,
+				     size_t sect_size)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  if (strcmp (sect_name, ".reg") == 0)
+    {
+      if (tdep->gregset == NULL)
+	tdep->gregset = regset_alloc (gdbarch, i386_linux_supply_gregset,
+				      i386_linux_collect_gregset);
+      return tdep->gregset;
+    }
+
+  return i386_regset_from_core_section (gdbarch, sect_name, sect_size);
+}
+
 static void
 i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -879,6 +929,9 @@ i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_core_read_description (gdbarch,
 				     i386_linux_core_read_description);
 
+  set_gdbarch_regset_from_core_section (gdbarch,
+					i386_linux_regset_from_core_section);
+
   /* Displaced stepping.  */
   set_gdbarch_displaced_step_copy_insn (gdbarch,
                                         simple_displaced_step_copy_insn);
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 22854bd..8fe3a4a 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -2641,26 +2641,62 @@ i386_value_to_register (struct frame_info *frame, int regnum,
     }
 }
 \f
+/* Fetch register REGNUM from the buffer specified by GREGS and store
+   it to register cache REGCACHE.  If REGNUM is -1, do this for all
+   general-purpose registers.  */
+
+void
+i386_fetch_gregset (struct regcache *regcache, const void *gregs,
+		    int regnum)
+{
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  const gdb_byte *regs = gregs;
+  int i;
+
+  for (i = 0; i < tdep->gregset_num_regs; i++)
+    if (regnum == i || regnum == -1)
+      {
+	int offset = tdep->gregset_reg_offset[i];
+	if (offset != -1)
+	  regcache_raw_supply (regcache, i, regs + offset);
+      }
+}
+
 /* Supply register REGNUM from the buffer specified by GREGS and LEN
    in the general-purpose register set REGSET to register cache
    REGCACHE.  If REGNUM is -1, do this for all registers in REGSET.  */
 
 void
-i386_supply_gregset (const struct regset *regset, struct regcache *regcache,
-		     int regnum, const void *gregs, size_t len)
+i386_supply_gregset (const struct regset *regset,
+		     struct regcache *regcache, int regnum,
+		     const void *gregs, size_t len)
 {
   const struct gdbarch_tdep *tdep = gdbarch_tdep (regset->arch);
-  const gdb_byte *regs = gregs;
-  int i;
-
   gdb_assert (len == tdep->sizeof_gregset);
+  i386_fetch_gregset (regcache, gregs, regnum);
+}
+
+/* Retrieve register REGNUM from the register cache REGCACHE and store
+   it in the buffer specified by GREGS.  If REGNUM is -1, do this for
+   all general-purpose registers.  */
+
+void
+i386_fill_gregset (const struct regcache *regcache, void *gregs,
+		   int regnum)
+{
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  gdb_byte *regs = gregs;
+  int i;
 
   for (i = 0; i < tdep->gregset_num_regs; i++)
-    {
-      if ((regnum == i || regnum == -1)
-	  && tdep->gregset_reg_offset[i] != -1)
-	regcache_raw_supply (regcache, i, regs + tdep->gregset_reg_offset[i]);
-    }
+    if (regnum == i || regnum == -1)
+      {
+	int offset = tdep->gregset_reg_offset[i];
+	if (offset != -1)
+	  regcache_raw_collect (regcache, i, regs + offset);
+      }
 }
 
 /* Collect register REGNUM from the register cache REGCACHE and store
@@ -2674,17 +2710,8 @@ i386_collect_gregset (const struct regset *regset,
 		      int regnum, void *gregs, size_t len)
 {
   const struct gdbarch_tdep *tdep = gdbarch_tdep (regset->arch);
-  gdb_byte *regs = gregs;
-  int i;
-
   gdb_assert (len == tdep->sizeof_gregset);
-
-  for (i = 0; i < tdep->gregset_num_regs; i++)
-    {
-      if ((regnum == i || regnum == -1)
-	  && tdep->gregset_reg_offset[i] != -1)
-	regcache_raw_collect (regcache, i, regs + tdep->gregset_reg_offset[i]);
-    }
+  i386_fill_gregset (regcache, gregs, regnum);
 }
 
 /* Supply register REGNUM from the buffer specified by FPREGS and LEN
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index 6520d67..3eeb932 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -340,6 +340,21 @@ extern int i386_sigtramp_p (struct frame_info *this_frame);
 extern int i386_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
 				     struct reggroup *group);
 
+
+/* Fetch register REGNUM from the buffer specified by GREGS and store
+   it to register cache REGCACHE.  If REGNUM is -1, do this for all
+   general-purpose registers.  */
+
+extern void i386_fetch_gregset (struct regcache *regcache,
+				const void *gregs, int regnum);
+
+/* Retrieve register REGNUM from the register cache REGCACHE and store
+   it in the buffer specified by GREGS.  If REGNUM is -1, do this for
+   all general-purpose registers.  */
+
+extern void i386_fill_gregset (const struct regcache *regcache,
+			       void *gregs, int regnum);
+
 /* Supply register REGNUM from the general-purpose register set REGSET
    to register cache REGCACHE.  If REGNUM is -1, do this for all
    registers in REGSET.  */
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;
+}


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit  core file
  2010-04-13 17:18         ` H.J. Lu
@ 2010-04-13 17:27           ` Mark Kettenis
  2010-04-13 17:39             ` H.J. Lu
  2010-04-13 17:48           ` H.J. Lu
  1 sibling, 1 reply; 19+ messages in thread
From: Mark Kettenis @ 2010-04-13 17:27 UTC (permalink / raw)
  To: hjl.tools; +Cc: gdb-patches, mark.kettenis, jan.kratochvil

> Date: Tue, 13 Apr 2010 10:17:57 -0700
> From: "H.J. Lu" <hongjiu.lu@intel.com>
> 
> On Mon, Apr 12, 2010 at 06:22:25AM -0700, H.J. Lu wrote:
> > On Sun, Apr 11, 2010 at 01:52:50PM -0700, H.J. Lu wrote:
> > > Hi,
> > > 
> > > Thanks for Mark's pointer. Solution is very simple. We just need to
> > > make sure that we call the right fill_gregset for 32bit executable
> > > on both Linux/x86-64 and Linux/i386.  OK to install?
> > > 
> > > Thanks.
> > > 
> > > 
> > 
> > Small update to use tdep->gregset_reg_offset instead of
> > i386_linux_gregset_reg_offset.  OK to install?
> > 
> 
> 
> Here is the updated patch. It calls set_gdbarch_regset_from_core_section
> with i386_linux_regset_from_core_section.  OK to install?


Sorry, no.  You're adding far too much bloat.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit   core file
  2010-04-13 17:27           ` Mark Kettenis
@ 2010-04-13 17:39             ` H.J. Lu
  2010-04-13 18:43               ` Mark Kettenis
  0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2010-04-13 17:39 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches, jan.kratochvil

On Tue, Apr 13, 2010 at 10:26 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> Date: Tue, 13 Apr 2010 10:17:57 -0700
>> From: "H.J. Lu" <hongjiu.lu@intel.com>
>>
>> On Mon, Apr 12, 2010 at 06:22:25AM -0700, H.J. Lu wrote:
>> > On Sun, Apr 11, 2010 at 01:52:50PM -0700, H.J. Lu wrote:
>> > > Hi,
>> > >
>> > > Thanks for Mark's pointer. Solution is very simple. We just need to
>> > > make sure that we call the right fill_gregset for 32bit executable
>> > > on both Linux/x86-64 and Linux/i386.  OK to install?
>> > >
>> > > Thanks.
>> > >
>> > >
>> >
>> > Small update to use tdep->gregset_reg_offset instead of
>> > i386_linux_gregset_reg_offset.  OK to install?
>> >
>>
>>
>> Here is the updated patch. It calls set_gdbarch_regset_from_core_section
>> with i386_linux_regset_from_core_section.  OK to install?
>
>
> Sorry, no.  You're adding far too much bloat.
>

Can you be more specific?

I added i386_linux_supply_gregset, i386_linux_collect_gregset
and i386_linux_regset_from_core_section because the the
size check on general purpose registers:

if (strcmp (sect_name, ".reg") == 0 && sect_size == tdep->sizeof_gregset)

and

gdb_assert (len == tdep->sizeof_gregset);

They are always false on Linux. I suspect that they are also false
on most other x86 OSes. But I can't verify it.

If it can be removed, many changes in my patch wont' be needed.

Thanks.

-- 
H.J.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit  core file
  2010-04-13 17:18         ` H.J. Lu
  2010-04-13 17:27           ` Mark Kettenis
@ 2010-04-13 17:48           ` H.J. Lu
  2010-04-13 20:38             ` H.J. Lu
  1 sibling, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2010-04-13 17:48 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GDB, mark.kettenis, jan.kratochvil

On Tue, Apr 13, 2010 at 10:17:57AM -0700, H.J. Lu wrote:
> On Mon, Apr 12, 2010 at 06:22:25AM -0700, H.J. Lu wrote:
> > On Sun, Apr 11, 2010 at 01:52:50PM -0700, H.J. Lu wrote:
> > > Hi,
> > > 
> > > Thanks for Mark's pointer. Solution is very simple. We just need to
> > > make sure that we call the right fill_gregset for 32bit executable
> > > on both Linux/x86-64 and Linux/i386.  OK to install?
> > > 
> > > Thanks.
> > > 
> > > 
> > 
> > Small update to use tdep->gregset_reg_offset instead of
> > i386_linux_gregset_reg_offset.  OK to install?
> > 
> 
> 
> Here is the updated patch. It calls set_gdbarch_regset_from_core_section
> with i386_linux_regset_from_core_section.  OK to install?
> 


This patch doesn't add i386_linux_regset_from_core_section. Instead
it removes the core section size check:

gdb_assert (len == tdep->sizeof_gregset);

which seems always false for most, if not all, x86 OSes.  OK to install?

Thanks.


H.J.
---
gdb/

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

	PR corefiles/11467
	* i386-linux-nat.c (regmap): Removed.
	(fetch_register): Replace regmap with tdep->gregset_reg_offset.
	(store_register): Likewise.
	(supply_gregset): Call i386_fetch_gregset.
	(fetch_regs): Likewise.
	(fill_gregset): Call i386_fill_gregset.
	(store_regs): Likewise.

	* i386-tdep.c (i386_fetch_gregset): New.
	(i386_supply_gregset): Use it.
	(i386_fill_gregset): New.
	(i386_collect_gregset): Use it.

	* i386-tdep.h (i386_fetch_gregset): New.
	(i386_fill_gregset): Likewise..

gdb/testsuite/

2010-04-13  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/i386-linux-nat.c b/gdb/i386-linux-nat.c
index d559811..023c471 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -99,26 +99,6 @@ static int have_ptrace_getregset = -1;
    those names are now used for the register sets used in the
    `mcontext_t' type, and have a different size and layout.  */
 
-/* Mapping between the general-purpose registers in `struct user'
-   format and GDB's register array layout.  */
-static int regmap[] = 
-{
-  EAX, ECX, EDX, EBX,
-  UESP, EBP, ESI, EDI,
-  EIP, EFL, CS, SS,
-  DS, ES, FS, GS,
-  -1, -1, -1, -1,		/* st0, st1, st2, st3 */
-  -1, -1, -1, -1,		/* st4, st5, st6, st7 */
-  -1, -1, -1, -1,		/* fctrl, fstat, ftag, fiseg */
-  -1, -1, -1, -1,		/* fioff, foseg, fooff, fop */
-  -1, -1, -1, -1,		/* xmm0, xmm1, xmm2, xmm3 */
-  -1, -1, -1, -1,		/* xmm4, xmm5, xmm6, xmm6 */
-  -1,				/* mxcsr */
-  -1, -1, -1, -1,		/* ymm0h, ymm1h, ymm2h, ymm3h */
-  -1, -1, -1, -1,		/* ymm4h, ymm5h, ymm6h, ymm6h */
-  ORIG_EAX
-};
-
 /* Which ptrace request retrieves which registers?
    These apply to the corresponding SET requests as well.  */
 
@@ -166,9 +146,12 @@ fetch_register (struct regcache *regcache, int regno)
 {
   int tid;
   int val;
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  int *reg_offset = tdep->gregset_reg_offset;
 
   gdb_assert (!have_ptrace_getregs);
-  if (regmap[regno] == -1)
+  if (reg_offset[regno] == -1)
     {
       regcache_raw_supply (regcache, regno, NULL);
       return;
@@ -180,7 +163,7 @@ fetch_register (struct regcache *regcache, int regno)
     tid = PIDGET (inferior_ptid); /* Not a threaded program.  */
 
   errno = 0;
-  val = ptrace (PTRACE_PEEKUSER, tid, 4 * regmap[regno], 0);
+  val = ptrace (PTRACE_PEEKUSER, tid, reg_offset[regno], 0);
   if (errno != 0)
     error (_("Couldn't read register %s (#%d): %s."), 
 	   gdbarch_register_name (get_regcache_arch (regcache), regno),
@@ -196,9 +179,12 @@ store_register (const struct regcache *regcache, int regno)
 {
   int tid;
   int val;
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  int *reg_offset = tdep->gregset_reg_offset;
 
   gdb_assert (!have_ptrace_getregs);
-  if (regmap[regno] == -1)
+  if (reg_offset[regno] == -1)
     return;
 
   /* GNU/Linux LWP ID's are process ID's.  */
@@ -208,7 +194,7 @@ store_register (const struct regcache *regcache, int regno)
 
   errno = 0;
   regcache_raw_collect (regcache, regno, &val);
-  ptrace (PTRACE_POKEUSER, tid, 4 * regmap[regno], val);
+  ptrace (PTRACE_POKEUSER, tid, reg_offset[regno], val);
   if (errno != 0)
     error (_("Couldn't write register %s (#%d): %s."),
 	   gdbarch_register_name (get_regcache_arch (regcache), regno),
@@ -225,16 +211,7 @@ store_register (const struct regcache *regcache, int regno)
 void
 supply_gregset (struct regcache *regcache, const elf_gregset_t *gregsetp)
 {
-  const elf_greg_t *regp = (const elf_greg_t *) gregsetp;
-  int i;
-
-  for (i = 0; i < I386_NUM_GREGS; i++)
-    regcache_raw_supply (regcache, i, regp + regmap[i]);
-
-  if (I386_LINUX_ORIG_EAX_REGNUM
-	< gdbarch_num_regs (get_regcache_arch (regcache)))
-    regcache_raw_supply (regcache, I386_LINUX_ORIG_EAX_REGNUM,
-			 regp + ORIG_EAX);
+  i386_fetch_gregset (regcache, gregsetp, -1);
 }
 
 /* Fill register REGNO (if it is a general-purpose register) in
@@ -245,18 +222,7 @@ void
 fill_gregset (const struct regcache *regcache,
 	      elf_gregset_t *gregsetp, int regno)
 {
-  elf_greg_t *regp = (elf_greg_t *) gregsetp;
-  int i;
-
-  for (i = 0; i < I386_NUM_GREGS; i++)
-    if (regno == -1 || regno == i)
-      regcache_raw_collect (regcache, i, regp + regmap[i]);
-
-  if ((regno == -1 || regno == I386_LINUX_ORIG_EAX_REGNUM)
-      && I386_LINUX_ORIG_EAX_REGNUM
-	   < gdbarch_num_regs (get_regcache_arch (regcache)))
-    regcache_raw_collect (regcache, I386_LINUX_ORIG_EAX_REGNUM,
-			  regp + ORIG_EAX);
+  i386_fill_gregset (regcache, gregsetp, -1);
 }
 
 #ifdef HAVE_PTRACE_GETREGS
@@ -283,7 +249,7 @@ fetch_regs (struct regcache *regcache, int tid)
       perror_with_name (_("Couldn't get registers"));
     }
 
-  supply_gregset (regcache, (const elf_gregset_t *) regs_p);
+  i386_fetch_gregset (regcache, (const void *) regs_p, -1);
 }
 
 /* Store all valid general-purpose registers in GDB's register array
@@ -297,7 +263,7 @@ store_regs (const struct regcache *regcache, int tid, int regno)
   if (ptrace (PTRACE_GETREGS, tid, 0, (int) &regs) < 0)
     perror_with_name (_("Couldn't get registers"));
 
-  fill_gregset (regcache, &regs, regno);
+  i386_fill_gregset (regcache, &regs, regno);
   
   if (ptrace (PTRACE_SETREGS, tid, 0, (int) &regs) < 0)
     perror_with_name (_("Couldn't write registers"));
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 22854bd..09b8289 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -2641,26 +2641,60 @@ i386_value_to_register (struct frame_info *frame, int regnum,
     }
 }
 \f
+/* Fetch register REGNUM from the buffer specified by GREGS and store
+   it to register cache REGCACHE.  If REGNUM is -1, do this for all
+   general-purpose registers.  */
+
+void
+i386_fetch_gregset (struct regcache *regcache, const void *gregs,
+		    int regnum)
+{
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  const gdb_byte *regs = gregs;
+  int i;
+
+  for (i = 0; i < tdep->gregset_num_regs; i++)
+    if (regnum == i || regnum == -1)
+      {
+	int offset = tdep->gregset_reg_offset[i];
+	if (offset != -1)
+	  regcache_raw_supply (regcache, i, regs + offset);
+      }
+}
+
 /* Supply register REGNUM from the buffer specified by GREGS and LEN
    in the general-purpose register set REGSET to register cache
    REGCACHE.  If REGNUM is -1, do this for all registers in REGSET.  */
 
 void
-i386_supply_gregset (const struct regset *regset, struct regcache *regcache,
-		     int regnum, const void *gregs, size_t len)
+i386_supply_gregset (const struct regset *regset,
+		     struct regcache *regcache, int regnum,
+		     const void *gregs, size_t len)
 {
-  const struct gdbarch_tdep *tdep = gdbarch_tdep (regset->arch);
-  const gdb_byte *regs = gregs;
-  int i;
+  i386_fetch_gregset (regcache, gregs, regnum);
+}
+
+/* Retrieve register REGNUM from the register cache REGCACHE and store
+   it in the buffer specified by GREGS.  If REGNUM is -1, do this for
+   all general-purpose registers.  */
 
-  gdb_assert (len == tdep->sizeof_gregset);
+void
+i386_fill_gregset (const struct regcache *regcache, void *gregs,
+		   int regnum)
+{
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  gdb_byte *regs = gregs;
+  int i;
 
   for (i = 0; i < tdep->gregset_num_regs; i++)
-    {
-      if ((regnum == i || regnum == -1)
-	  && tdep->gregset_reg_offset[i] != -1)
-	regcache_raw_supply (regcache, i, regs + tdep->gregset_reg_offset[i]);
-    }
+    if (regnum == i || regnum == -1)
+      {
+	int offset = tdep->gregset_reg_offset[i];
+	if (offset != -1)
+	  regcache_raw_collect (regcache, i, regs + offset);
+      }
 }
 
 /* Collect register REGNUM from the register cache REGCACHE and store
@@ -2673,18 +2707,7 @@ i386_collect_gregset (const struct regset *regset,
 		      const struct regcache *regcache,
 		      int regnum, void *gregs, size_t len)
 {
-  const struct gdbarch_tdep *tdep = gdbarch_tdep (regset->arch);
-  gdb_byte *regs = gregs;
-  int i;
-
-  gdb_assert (len == tdep->sizeof_gregset);
-
-  for (i = 0; i < tdep->gregset_num_regs; i++)
-    {
-      if ((regnum == i || regnum == -1)
-	  && tdep->gregset_reg_offset[i] != -1)
-	regcache_raw_collect (regcache, i, regs + tdep->gregset_reg_offset[i]);
-    }
+  i386_fill_gregset (regcache, gregs, regnum);
 }
 
 /* Supply register REGNUM from the buffer specified by FPREGS and LEN
@@ -2760,7 +2783,7 @@ i386_regset_from_core_section (struct gdbarch *gdbarch,
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
-  if (strcmp (sect_name, ".reg") == 0 && sect_size == tdep->sizeof_gregset)
+  if (strcmp (sect_name, ".reg") == 0)
     {
       if (tdep->gregset == NULL)
 	tdep->gregset = regset_alloc (gdbarch, i386_supply_gregset,
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index 6520d67..3eeb932 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -340,6 +340,21 @@ extern int i386_sigtramp_p (struct frame_info *this_frame);
 extern int i386_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
 				     struct reggroup *group);
 
+
+/* Fetch register REGNUM from the buffer specified by GREGS and store
+   it to register cache REGCACHE.  If REGNUM is -1, do this for all
+   general-purpose registers.  */
+
+extern void i386_fetch_gregset (struct regcache *regcache,
+				const void *gregs, int regnum);
+
+/* Retrieve register REGNUM from the register cache REGCACHE and store
+   it in the buffer specified by GREGS.  If REGNUM is -1, do this for
+   all general-purpose registers.  */
+
+extern void i386_fill_gregset (const struct regcache *regcache,
+			       void *gregs, int regnum);
+
 /* Supply register REGNUM from the general-purpose register set REGSET
    to register cache REGCACHE.  If REGNUM is -1, do this for all
    registers in REGSET.  */
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;
+}


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit   core file
  2010-04-12 18:50           ` H.J. Lu
@ 2010-04-13 18:40             ` Mark Kettenis
  2010-04-13 19:19               ` H.J. Lu
  2010-04-13 20:03               ` H.J. Lu
  0 siblings, 2 replies; 19+ messages in thread
From: Mark Kettenis @ 2010-04-13 18:40 UTC (permalink / raw)
  To: hjl.tools; +Cc: gdb-patches, jan.kratochvil

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2964 bytes --]

> Date: Mon, 12 Apr 2010 11:50:47 -0700
> From: "H.J. Lu" <hjl.tools@gmail.com>
> 
> On Mon, Apr 12, 2010 at 11:23 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >> Date: Mon, 12 Apr 2010 06:22:25 -0700
> >> From: "H.J. Lu" <hongjiu.lu@intel.com>
> >>
> >> On Sun, Apr 11, 2010 at 01:52:50PM -0700, H.J. Lu wrote:
> >> > Hi,
> >> >
> >> > Thanks for Mark's pointer. Solution is very simple. We just need to
> >> > make sure that we call the right fill_gregset for 32bit executable
> >> > on both Linux/x86-64 and Linux/i386.  OK to install?
> >> >
> >> > Thanks.
> >> >
> >> >
> >>
> >> Small update to use tdep->gregset_reg_offset instead of
> >> i386_linux_gregset_reg_offset.  OK to install?
> >
> > No.  Like I explained in an earlier mail, we're not supposed to end up
> > in fetch_gregset() in the first place.
> >
> 
> fetch_gregset is always defined and used to fill .reg section in
> coredump on Linux/x86. i386-tdep.c has

No, that's not how it's supposed to be.  And unless there is a bug
somewhere, this is not what happens for the 64x64 and 32x32 cases.

> const struct regset *
> i386_regset_from_core_section (struct gdbarch *gdbarch,
>                                const char *sect_name, size_t sect_size)
> {
>   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> 
>   if (strcmp (sect_name, ".reg") == 0 && sect_size == tdep->sizeof_gregset)
>     {
>       if (tdep->gregset == NULL)
>         tdep->gregset = regset_alloc (gdbarch, i386_supply_gregset,
>                                       i386_collect_gregset);
>       return tdep->gregset;
>     }
> 
> For Linux,  tdep->sizeof_gregset != the size of .reg section.

Then there is your bug.

> In fact, they have nothing to do with each other. sizeof_gregset
> includes SSE and AVX registers, which have offset == -1 since they
> aren't general-purpose registers. I don't believe we should set
> tdep->gregset since general-purpose registers is a special case for
> Linux/x86. They are handled differently. I don't want to change it.

You're wrong here.  This code used to work just fine.  And I believe
it still works fine for the 32x32 and 64x64 cases.

I see that in i386-linux-tdep.c:i386_linux_regset_sections[] specifies
the size of the regset as 144 and sets tdep->sizeof_gregset to 17 * 4
= 68.  That can't be right.  Given that
amd64-linux-tdep.c:amd64_linux_regset_sections[] specifies the size as
144 as well, I'm betting 68 is the right valaue for i386.  I think
somebody got confused here, since the size of the NT_PRSTATUS note in
32-bit core dumps happens to be 144, but the actual size of the space
reserved for storing the registers in there is smaller.

Anyway, there is a somewhat fundamental flaw in
linux-nat.c:linux_nat_do_thread_registers() in that it always passes
the size of 64-bit version of gregset_t in the
gdbarch_regset_from_core_section() call.  That's wrong, and probably
the ultimate reason why the 64x32 gcore case isn't working properly.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit   core file
  2010-04-13 17:39             ` H.J. Lu
@ 2010-04-13 18:43               ` Mark Kettenis
  0 siblings, 0 replies; 19+ messages in thread
From: Mark Kettenis @ 2010-04-13 18:43 UTC (permalink / raw)
  To: hjl.tools; +Cc: gdb-patches, jan.kratochvil

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]

> Date: Tue, 13 Apr 2010 10:39:24 -0700
> From: "H.J. Lu" <hjl.tools@gmail.com>
> 
> On Tue, Apr 13, 2010 at 10:26 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >> Date: Tue, 13 Apr 2010 10:17:57 -0700
> >> From: "H.J. Lu" <hongjiu.lu@intel.com>
> >>
> >> On Mon, Apr 12, 2010 at 06:22:25AM -0700, H.J. Lu wrote:
> >> > On Sun, Apr 11, 2010 at 01:52:50PM -0700, H.J. Lu wrote:
> >> > > Hi,
> >> > >
> >> > > Thanks for Mark's pointer. Solution is very simple. We just need to
> >> > > make sure that we call the right fill_gregset for 32bit executable
> >> > > on both Linux/x86-64 and Linux/i386.  OK to install?
> >> > >
> >> > > Thanks.
> >> > >
> >> > >
> >> >
> >> > Small update to use tdep->gregset_reg_offset instead of
> >> > i386_linux_gregset_reg_offset.  OK to install?
> >> >
> >>
> >>
> >> Here is the updated patch. It calls set_gdbarch_regset_from_core_section
> >> with i386_linux_regset_from_core_section.  OK to install?
> >
> >
> > Sorry, no.  You're adding far too much bloat.
> >
> 
> Can you be more specific?

Yup; sent an explanation as a reply to one of your earlier mail.  Just
wanted to make sure you didn't dig yourself an even bigger hole while
I was looking at some of the details here.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit   core file
  2010-04-13 18:40             ` Mark Kettenis
@ 2010-04-13 19:19               ` H.J. Lu
  2010-04-13 20:03               ` H.J. Lu
  1 sibling, 0 replies; 19+ messages in thread
From: H.J. Lu @ 2010-04-13 19:19 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches, jan.kratochvil

On Tue, Apr 13, 2010 at 11:40 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> Date: Mon, 12 Apr 2010 11:50:47 -0700
>> From: "H.J. Lu" <hjl.tools@gmail.com>
>>
>> On Mon, Apr 12, 2010 at 11:23 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> >> Date: Mon, 12 Apr 2010 06:22:25 -0700
>> >> From: "H.J. Lu" <hongjiu.lu@intel.com>
>> >>
>> >> On Sun, Apr 11, 2010 at 01:52:50PM -0700, H.J. Lu wrote:
>> >> > Hi,
>> >> >
>> >> > Thanks for Mark's pointer. Solution is very simple. We just need to
>> >> > make sure that we call the right fill_gregset for 32bit executable
>> >> > on both Linux/x86-64 and Linux/i386.  OK to install?
>> >> >
>> >> > Thanks.
>> >> >
>> >> >
>> >>
>> >> Small update to use tdep->gregset_reg_offset instead of
>> >> i386_linux_gregset_reg_offset.  OK to install?
>> >
>> > No.  Like I explained in an earlier mail, we're not supposed to end up
>> > in fetch_gregset() in the first place.
>> >
>>
>> fetch_gregset is always defined and used to fill .reg section in
>> coredump on Linux/x86. i386-tdep.c has
>
> No, that's not how it's supposed to be.  And unless there is a bug
> somewhere, this is not what happens for the 64x64 and 32x32 cases.
>
>> const struct regset *
>> i386_regset_from_core_section (struct gdbarch *gdbarch,
>>                                const char *sect_name, size_t sect_size)
>> {
>>   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>>
>>   if (strcmp (sect_name, ".reg") == 0 && sect_size == tdep->sizeof_gregset)
>>     {
>>       if (tdep->gregset == NULL)
>>         tdep->gregset = regset_alloc (gdbarch, i386_supply_gregset,
>>                                       i386_collect_gregset);
>>       return tdep->gregset;
>>     }
>>
>> For Linux,  tdep->sizeof_gregset != the size of .reg section.
>
> Then there is your bug.
>
>> In fact, they have nothing to do with each other. sizeof_gregset
>> includes SSE and AVX registers, which have offset == -1 since they
>> aren't general-purpose registers. I don't believe we should set
>> tdep->gregset since general-purpose registers is a special case for
>> Linux/x86. They are handled differently. I don't want to change it.
>
> You're wrong here.  This code used to work just fine.  And I believe
> it still works fine for the 32x32 and 64x64 cases.

It works in the 32x32 and 64x64 cases because sect_size != tdep->sizeof_gregset
on Linux.

> I see that in i386-linux-tdep.c:i386_linux_regset_sections[] specifies
> the size of the regset as 144 and sets tdep->sizeof_gregset to 17 * 4
> = 68.  That can't be right.  Given that
> amd64-linux-tdep.c:amd64_linux_regset_sections[] specifies the size as
> 144 as well, I'm betting 68 is the right valaue for i386.  I think

I tried to set it to 68 and gcore generated the incorrect coredump.

> somebody got confused here, since the size of the NT_PRSTATUS note in
> 32-bit core dumps happens to be 144, but the actual size of the space
> reserved for storing the registers in there is smaller.
>

There are

lfcore_write_prstatus (bfd *abfd,
                        char *buf,
                        int *bufsiz,
                        long pid,
                        int cursig,
                        const void *gregs)
{
  const char *note_name = "CORE";
  const struct elf_backend_data *bed = get_elf_backend_data (abfd);

  if (bed->elf_backend_write_core_note != NULL)
    {
      char *ret;
      ret = (*bed->elf_backend_write_core_note) (abfd, buf, bufsiz,
                                                 NT_PRSTATUS,
                                                 pid, cursig, gregs);
      if (ret != NULL)
        return ret;
    }

#if defined (HAVE_PRSTATUS32_T)
  if (bed->s->elfclass == ELFCLASS32)
    {
      prstatus32_t prstat;

      memset (&prstat, 0, sizeof (prstat));
      prstat.pr_pid = pid;
      prstat.pr_cursig = cursig;
      memcpy (&prstat.pr_reg, gregs, sizeof (prstat.pr_reg));
      return elfcore_write_note (abfd, buf, bufsiz, note_name,
                                 NT_PRSTATUS, &prstat, sizeof (prstat));
    }
  else
#endif
    {
      prstatus_t prstat;

      memset (&prstat, 0, sizeof (prstat));
      prstat.pr_pid = pid;
      prstat.pr_cursig = cursig;
      memcpy (&prstat.pr_reg, gregs, sizeof (prstat.pr_reg));
      return elfcore_write_note (abfd, buf, bufsiz, note_name,
                                 NT_PRSTATUS, &prstat, sizeof (prstat));
    }
}

tdep->sizeof_gregset ==  sizeof (prstat.pr_reg) and *bufize is the
section size:

(gdb) gcore good

Breakpoint 3, elfcore_write_prstatus (abfd=0xc49230, buf=0xc727e0 "\005",
    bufsiz=0x7fffffffdb2c, pid=15819, cursig=5, gregs=0x7fffffffd6b0)
    at /export/gnu/import/git/gdb/bfd/elf.c:8644
8644	{
(top-gdb) p *bufsiz
$1 = 144
(top-gdb)

Checking sect_size == tdep->sizeof_gregset  doesn't  make
any senses on Linux. It doesn't make any senses to any OSes
with

struct elf_prstatus32
  {
    struct elf_siginfo pr_info;         /* Info associated with signal.  */
    short int pr_cursig;                /* Current signal.  */
    unsigned int pr_sigpend;            /* Set of pending signals.  */
    unsigned int pr_sighold;            /* Set of held signals.  */
    __pid_t pr_pid;
    __pid_t pr_ppid;
    __pid_t pr_pgrp;
    __pid_t pr_sid;
    struct prstatus32_timeval pr_utime;         /* User time.  */
    struct prstatus32_timeval pr_stime;         /* System time.  */
    struct prstatus32_timeval pr_cutime;        /* Cumulative user time.  */
    struct prstatus32_timeval pr_cstime;        /* Cumulative system time.  */
    elf_gregset32_t pr_reg;             /* GP registers.  */
    int pr_fpvalid;                     /* True if math copro being used.  */
  };

where

elf_gregset32_t pr_reg;             /* GP registers.  */

is in the middle of NT_PRSTATUS section.


-- 
H.J.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit   core file
  2010-04-13 18:40             ` Mark Kettenis
  2010-04-13 19:19               ` H.J. Lu
@ 2010-04-13 20:03               ` H.J. Lu
  1 sibling, 0 replies; 19+ messages in thread
From: H.J. Lu @ 2010-04-13 20:03 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches, jan.kratochvil

On Tue, Apr 13, 2010 at 11:40 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
> Anyway, there is a somewhat fundamental flaw in
> linux-nat.c:linux_nat_do_thread_registers() in that it always passes
> the size of 64-bit version of gregset_t in the
> gdbarch_regset_from_core_section() call.  That's wrong, and probably
> the ultimate reason why the 64x32 gcore case isn't working properly.
>

That is problematic. For GP registers, total size of GP registers
may not be the size of the note section of GP registers. So we
can't use that section size. For x86, we can use

size >= tdep->sizeof_gregset

with a comment saying something like "size of 64bit version
of gregset_t may be passed down."

-- 
H.J.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit  core file
  2010-04-13 17:48           ` H.J. Lu
@ 2010-04-13 20:38             ` H.J. Lu
  2010-04-13 20:48               ` Mark Kettenis
  0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2010-04-13 20:38 UTC (permalink / raw)
  To: GDB, mark.kettenis, jan.kratochvil

Here is the new patch to check of gregset >= sizeof_gregset. OK to
install?

Thanks.


H.J.
---
gdb/

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

	PR corefiles/11467
	* i386-tdep.c (i386_supply_gregset): Check size of gregset >=
	sizeof_gregset.
	(i386_collect_gregset): Likewise.
	(i386_regset_from_core_section): Likewwise.

gdb/testsuite/

2010-04-13  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/i386-tdep.c b/gdb/i386-tdep.c
index 22854bd..8ac1892 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -2653,7 +2653,9 @@ i386_supply_gregset (const struct regset *regset, struct regcache *regcache,
   const gdb_byte *regs = gregs;
   int i;
 
-  gdb_assert (len == tdep->sizeof_gregset);
+  /* The size of 64-bit version of gdb_gregset_t may be passed down
+     here when reading 32-bit coredump from 64-bit gdb.  */
+  gdb_assert (len >= tdep->sizeof_gregset);
 
   for (i = 0; i < tdep->gregset_num_regs; i++)
     {
@@ -2677,7 +2679,9 @@ i386_collect_gregset (const struct regset *regset,
   gdb_byte *regs = gregs;
   int i;
 
-  gdb_assert (len == tdep->sizeof_gregset);
+  /* The size of 64-bit version of gdb_gregset_t may be passed down
+     here when writing 32-bit coredump from 64-bit gdb.  */
+  gdb_assert (len >= tdep->sizeof_gregset);
 
   for (i = 0; i < tdep->gregset_num_regs; i++)
     {
@@ -2760,7 +2764,9 @@ i386_regset_from_core_section (struct gdbarch *gdbarch,
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
-  if (strcmp (sect_name, ".reg") == 0 && sect_size == tdep->sizeof_gregset)
+  /* The size of 64-bit version of gdb_gregset_t may be passed down
+     here when reading/writing 32-bit coredump from 64-bit gdb.  */
+  if (strcmp (sect_name, ".reg") == 0 && sect_size >= tdep->sizeof_gregset)
     {
       if (tdep->gregset == NULL)
 	tdep->gregset = regset_alloc (gdbarch, i386_supply_gregset,
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;
+}


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit  core file
  2010-04-13 20:38             ` H.J. Lu
@ 2010-04-13 20:48               ` Mark Kettenis
  0 siblings, 0 replies; 19+ messages in thread
From: Mark Kettenis @ 2010-04-13 20:48 UTC (permalink / raw)
  To: hjl.tools; +Cc: gdb-patches, jan.kratochvil

> Date: Tue, 13 Apr 2010 13:37:51 -0700
> From: "H.J. Lu" <hongjiu.lu@intel.com>
> 
> Here is the new patch to check of gregset >= sizeof_gregset. OK to
> install?

Please stop sending diffs until you understand how the code is
supposed to work.


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2010-04-13 20:48 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-10 22:19 PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit core file H.J. Lu
2010-04-10 22:27 ` H.J. Lu
2010-04-11  0:01   ` H.J. Lu
2010-04-11 20:53     ` H.J. Lu
2010-04-12 13:22       ` H.J. Lu
2010-04-12 18:24         ` Mark Kettenis
2010-04-12 18:50           ` H.J. Lu
2010-04-13 18:40             ` Mark Kettenis
2010-04-13 19:19               ` H.J. Lu
2010-04-13 20:03               ` H.J. Lu
2010-04-13 17:18         ` H.J. Lu
2010-04-13 17:27           ` Mark Kettenis
2010-04-13 17:39             ` H.J. Lu
2010-04-13 18:43               ` Mark Kettenis
2010-04-13 17:48           ` H.J. Lu
2010-04-13 20:38             ` H.J. Lu
2010-04-13 20:48               ` Mark Kettenis
2010-04-11 16:50   ` Mark Kettenis
2010-04-11 17:33     ` H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox