This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: PATCH: Fix 32bit coredump read on Linux/AVX
> Date: Tue, 20 Apr 2010 11:43:39 -0700
> From: "H.J. Lu" <hongjiu.lu@intel.com>
>
> On Mon, Apr 19, 2010 at 08:22:34PM -0700, H.J. Lu wrote:
> > Hi,
> >
> > This patch:
> >
> > http://sourceware.org/ml/gdb-patches/2010-04/msg00276.html
Sorry about that. I thought that you had tested that diff.
> > breaks 32bit coredump read on Linux/AVX since we only dump .reg and
> > .reg-xstate sections on AVX. But i386_linux_core_read_description
> > checks .reg2 and .reg-xfp sections first. Since there are no
> > .reg2 and .reg-xfp sections, NULL is returned and SSE target is used.
> > This patch changes the section check order to .reg-xstate, .reg-xfp,
> > .reg2. OK to install?
> >
> > Thanks.
> >
>
> .reg2 section has x87 regiters on i386 and SSE registers on amd64.
> Here is the updated patch to properly handle it. OK to install?
Not quite.
> 2010-04-20 H.J. Lu <hongjiu.lu@intel.com>
>
> PR corefiles/11523
> * amd64-linux-tdep.c (amd64_linux_core_read_description): Check
> XCR0 first.
>
> * i386-linux-tdep.c (i386_linux_core_read_xcr0): Return 0 if
> there is no .reg-xstate section.
> (i386_linux_core_read_description): Check XCR0 first.
>
> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
> index f249d5d..1f7a052 100644
> --- a/gdb/amd64-linux-tdep.c
> +++ b/gdb/amd64-linux-tdep.c
> @@ -1269,18 +1269,23 @@ amd64_linux_core_read_description (struct gdbarch *gdbarch,
> struct target_ops *target,
> bfd *abfd)
> {
> - asection *section = bfd_get_section_by_name (abfd, ".reg2");
> - uint64_t xcr0;
> -
> - if (section == NULL)
> - return NULL;
> -
> /* Linux/x86-64. */
> - xcr0 = i386_linux_core_read_xcr0 (gdbarch, target, abfd);
> - if ((xcr0 & I386_XSTATE_AVX_MASK) == I386_XSTATE_AVX_MASK)
> - return tdesc_amd64_avx_linux;
> - else
> + uint64_t xcr0 = i386_linux_core_read_xcr0 (gdbarch, target, abfd);
> + switch ((xcr0 & I386_XSTATE_AVX_MASK))
> + {
> + case I386_XSTATE_AVX_MASK:
> + return tdesc_amd64_avx_linux;
> + return tdesc_i386_avx_linux;
There's a duplicate return statement there.
> + case I386_XSTATE_SSE_MASK:
> + return tdesc_amd64_linux;
> + default:
> + break;
> + }
> +
> + if (bfd_get_section_by_name (abfd, ".reg2") != NULL)
> return tdesc_amd64_linux;
> + else
> + return NULL;
I think we should always return tdesc_amd64_linux here. That is the
minimal target description for amd64.
> diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
> index 5952153..ee7c23f 100644
> --- a/gdb/i386-linux-tdep.c
> +++ b/gdb/i386-linux-tdep.c
> @@ -611,7 +611,7 @@ i386_linux_core_read_xcr0 (struct gdbarch *gdbarch,
> }
> }
> else
> - xcr0 = I386_XSTATE_SSE_MASK;
> + xcr0 = 0;
>
> return xcr0;
> }
> @@ -623,22 +623,26 @@ i386_linux_core_read_description (struct gdbarch *gdbarch,
> struct target_ops *target,
> bfd *abfd)
> {
> - asection *section = bfd_get_section_by_name (abfd, ".reg2");
> - uint64_t xcr0;
> -
> - if (section == NULL)
> - return NULL;
> + /* Linux/i386. */
> + uint64_t xcr0 = i386_linux_core_read_xcr0 (gdbarch, target, abfd);
> + switch ((xcr0 & I386_XSTATE_AVX_MASK))
> + {
> + case I386_XSTATE_AVX_MASK:
> + return tdesc_i386_avx_linux;
> + case I386_XSTATE_SSE_MASK:
> + return tdesc_i386_linux;
> + case I386_XSTATE_X87_MASK:
> + return tdesc_i386_mmx_linux;
> + default:
> + break;
> + }
This is fine, however,
> - section = bfd_get_section_by_name (abfd, ".reg-xfp");
> - if (section == NULL)
> + if (bfd_get_section_by_name (abfd, ".reg-xfp") != NULL)
> + return tdesc_i386_linux;
> + else if (bfd_get_section_by_name (abfd, ".reg2") != NULL)
> return tdesc_i386_mmx_linux;
> -
> - /* Linux/i386. */
> - xcr0 = i386_linux_core_read_xcr0 (gdbarch, target, abfd);
> - if ((xcr0 & I386_XSTATE_AVX_MASK) == I386_XSTATE_AVX_MASK)
> - return tdesc_i386_avx_linux;
> else
> - return tdesc_i386_linux;
> + return NULL;
I think we should return tdesc_i386_linux if ".reg-xfp" is found,
otherwise return tdesc_i386_mmx_linux.