From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28473 invoked by alias); 13 Apr 2010 17:48:17 -0000 Received: (qmail 28451 invoked by uid 22791); 13 Apr 2010 17:48:14 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=BAYES_00,NO_DNS_FOR_FROM,TW_CP,TW_EG,TW_MX,TW_XC,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mga10.intel.com (HELO fmsmga102.fm.intel.com) (192.55.52.92) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 13 Apr 2010 17:48:08 +0000 Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP; 13 Apr 2010 10:47:26 -0700 X-ExtLoop1: 1 Received: from gnu-6.sc.intel.com ([10.3.194.107]) by fmsmga002.fm.intel.com with ESMTP; 13 Apr 2010 10:47:52 -0700 Received: by gnu-6.sc.intel.com (Postfix, from userid 500) id DBA55812386; Tue, 13 Apr 2010 10:48:06 -0700 (PDT) Date: Tue, 13 Apr 2010 17:48:00 -0000 From: "H.J. Lu" To: "H.J. Lu" Cc: GDB , mark.kettenis@xs4all.nl, jan.kratochvil@redhat.com Subject: Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit core file Message-ID: <20100413174806.GA30532@intel.com> Reply-To: "H.J. Lu" References: <20100410221943.GA9675@intel.com> <20100410222742.GA9901@intel.com> <20100411000053.GA12675@intel.com> <20100411205250.GA24158@intel.com> <20100412132225.GA5932@intel.com> <20100413171757.GA6387@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100413171757.GA6387@intel.com> User-Agent: Mutt/1.5.20 (2009-08-17) Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-04/txt/msg00418.txt.bz2 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 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 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 . + +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 . */ + +/* + * Test GDB's ability to save and reload a corefile. + */ + +#include +#include + +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; +}