This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit core file
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) ®s) < 0)
perror_with_name (_("Couldn't get registers"));
- fill_gregset (regcache, ®s, regno);
+ i386_fill_gregset (regcache, ®s, regno);
if (ptrace (PTRACE_SETREGS, tid, 0, (int) ®s) < 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,
}
}
+/* 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;
+}