[04/25] Centralize i386 linux target descriptions
Commit Message
This patch moves all the tdesc_i386*_linux target descriptions to a
function i386_linux_read_description, which returns the right target
description according to xcr0. This also remove the duplication in
getting target descriptions in corefile and native target.
gdb:
2017-04-27 Yao Qi <yao.qi@linaro.org>
* i386-linux-tdep.c (i386_linux_read_description): New function.
(i386_linux_core_read_description): Call
i386_linux_read_description.
* i386-linux-tdep.h (i386_linux_read_description): Declare.
(tdesc_i386_linux, tdesc_i386_mmx_linux): Remove declarations.
(tdesc_i386_avx_linux, tdesc_i386_mpx_linux): Likewise
(tdesc_i386_avx_mpx_linux, tdesc_i386_avx_avx512_linux): Likewise.
(tdesc_i386_avx_mpx_avx512_pku_linux): Likewise.
* x86-linux-nat.c (x86_linux_read_description): Call
i386_linux_read_description.
---
gdb/i386-linux-tdep.c | 34 ++++++++++++++++++++++------------
gdb/i386-linux-tdep.h | 10 ++--------
gdb/x86-linux-nat.c | 24 ++++++++----------------
3 files changed, 32 insertions(+), 36 deletions(-)
Comments
On 2017-06-12 10:41, Yao Qi wrote:
> This patch moves all the tdesc_i386*_linux target descriptions to a
> function i386_linux_read_description, which returns the right target
> description according to xcr0. This also remove the duplication in
> getting target descriptions in corefile and native target.
>
> gdb:
>
> 2017-04-27 Yao Qi <yao.qi@linaro.org>
>
> * i386-linux-tdep.c (i386_linux_read_description): New function.
> (i386_linux_core_read_description): Call
> i386_linux_read_description.
> * i386-linux-tdep.h (i386_linux_read_description): Declare.
> (tdesc_i386_linux, tdesc_i386_mmx_linux): Remove declarations.
> (tdesc_i386_avx_linux, tdesc_i386_mpx_linux): Likewise
> (tdesc_i386_avx_mpx_linux, tdesc_i386_avx_avx512_linux): Likewise.
> (tdesc_i386_avx_mpx_avx512_pku_linux): Likewise.
> * x86-linux-nat.c (x86_linux_read_description): Call
> i386_linux_read_description.
> ---
> gdb/i386-linux-tdep.c | 34 ++++++++++++++++++++++------------
> gdb/i386-linux-tdep.h | 10 ++--------
> gdb/x86-linux-nat.c | 24 ++++++++----------------
> 3 files changed, 32 insertions(+), 36 deletions(-)
>
> diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
> index 1909d61..1bc1a6f 100644
> --- a/gdb/i386-linux-tdep.c
> +++ b/gdb/i386-linux-tdep.c
> @@ -678,16 +678,9 @@ i386_linux_core_read_xcr0 (bfd *abfd)
> return xcr0;
> }
>
> -/* Get Linux/x86 target description from core dump. */
> -
> -static const struct target_desc *
> -i386_linux_core_read_description (struct gdbarch *gdbarch,
> - struct target_ops *target,
> - bfd *abfd)
> +const struct target_desc *
> +i386_linux_read_description (uint64_t xcr0)
This would need a comment
/* See i386-linux-tdep.h. */
> {
> - /* Linux/i386. */
> - uint64_t xcr0 = i386_linux_core_read_xcr0 (abfd);
> -
> switch ((xcr0 & X86_XSTATE_ALL_MASK))
> {
> case X86_XSTATE_AVX_MPX_AVX512_PKU_MASK:
> @@ -708,10 +701,27 @@ i386_linux_core_read_description (struct gdbarch
> *gdbarch,
> break;
> }
>
> + return NULL;
> +}
I was going to say: I think it would make more sense if
i386_linux_read_description always returned something non-NULL, but then
I realized you need to handle the special case in
i386_linux_core_read_description...
> +
> +/* Get Linux/x86 target description from core dump. */
> +
> +static const struct target_desc *
> +i386_linux_core_read_description (struct gdbarch *gdbarch,
> + struct target_ops *target,
> + bfd *abfd)
> +{
> + /* Linux/i386. */
> + uint64_t xcr0 = i386_linux_core_read_xcr0 (abfd);
> + const struct target_desc * tdesc = i386_linux_read_description
> (xcr0);
Extra space after *.
The logic seems OK to me. It's not related to your patch, but I find a
bit confusing that i386-linux has SSE, and in i386-mmx-linux, the mmx is
there to denote the lack of sse. I looks like Mark agrees with me :)
https://sourceware.org/ml/gdb-patches/2010-04/msg00300.html
Simon
@@ -678,16 +678,9 @@ i386_linux_core_read_xcr0 (bfd *abfd)
return xcr0;
}
-/* Get Linux/x86 target description from core dump. */
-
-static const struct target_desc *
-i386_linux_core_read_description (struct gdbarch *gdbarch,
- struct target_ops *target,
- bfd *abfd)
+const struct target_desc *
+i386_linux_read_description (uint64_t xcr0)
{
- /* Linux/i386. */
- uint64_t xcr0 = i386_linux_core_read_xcr0 (abfd);
-
switch ((xcr0 & X86_XSTATE_ALL_MASK))
{
case X86_XSTATE_AVX_MPX_AVX512_PKU_MASK:
@@ -708,10 +701,27 @@ i386_linux_core_read_description (struct gdbarch *gdbarch,
break;
}
+ return NULL;
+}
+
+/* Get Linux/x86 target description from core dump. */
+
+static const struct target_desc *
+i386_linux_core_read_description (struct gdbarch *gdbarch,
+ struct target_ops *target,
+ bfd *abfd)
+{
+ /* Linux/i386. */
+ uint64_t xcr0 = i386_linux_core_read_xcr0 (abfd);
+ const struct target_desc * tdesc = i386_linux_read_description (xcr0);
+
+ if (tdesc != NULL)
+ return tdesc;
+
if (bfd_get_section_by_name (abfd, ".reg-xfp") != NULL)
- return tdesc_i386_linux;
+ return i386_linux_read_description (X86_XSTATE_SSE_MASK);
else
- return tdesc_i386_mmx_linux;
+ return i386_linux_read_description (X86_XSTATE_X87_MASK);
}
/* Similar to i386_supply_fpregset, but use XSAVE extended state. */
@@ -835,7 +845,7 @@ i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
set_gdbarch_num_regs (gdbarch, I386_LINUX_NUM_REGS);
if (! tdesc_has_registers (tdesc))
- tdesc = tdesc_i386_linux;
+ tdesc = i386_linux_read_description (X86_XSTATE_SSE_MASK);
tdep->tdesc = tdesc;
feature = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.linux");
@@ -42,14 +42,8 @@ extern uint64_t i386_linux_core_read_xcr0 (bfd *abfd);
extern void i386_linux_handle_segmentation_fault (struct gdbarch *gdbarch,
struct ui_out *uiout);
-/* Linux target description. */
-extern struct target_desc *tdesc_i386_linux;
-extern struct target_desc *tdesc_i386_mmx_linux;
-extern struct target_desc *tdesc_i386_avx_linux;
-extern struct target_desc *tdesc_i386_mpx_linux;
-extern struct target_desc *tdesc_i386_avx_mpx_linux;
-extern struct target_desc *tdesc_i386_avx_avx512_linux;
-extern struct target_desc *tdesc_i386_avx_mpx_avx512_pku_linux;
+/* Return the target description according to XCR0. */
+extern const struct target_desc *i386_linux_read_description (uint64_t xcr0);
/* Format of XSAVE extended state is:
struct
@@ -153,7 +153,7 @@ x86_linux_read_description (struct target_ops *ops)
{
have_ptrace_getfpxregs = 0;
have_ptrace_getregset = TRIBOOL_FALSE;
- return tdesc_i386_mmx_linux;
+ return i386_linux_read_description (X86_XSTATE_X87_MASK);
}
}
#endif
@@ -230,21 +230,13 @@ x86_linux_read_description (struct target_ops *ops)
}
else
{
- switch (xcr0_features_bits)
- {
- case X86_XSTATE_AVX_MPX_AVX512_PKU_MASK:
- return tdesc_i386_avx_mpx_avx512_pku_linux;
- case X86_XSTATE_AVX_AVX512_MASK:
- return tdesc_i386_avx_avx512_linux;
- case X86_XSTATE_MPX_MASK:
- return tdesc_i386_mpx_linux;
- case X86_XSTATE_AVX_MPX_MASK:
- return tdesc_i386_avx_mpx_linux;
- case X86_XSTATE_AVX_MASK:
- return tdesc_i386_avx_linux;
- default:
- return tdesc_i386_linux;
- }
+ const struct target_desc * tdesc
+ = i386_linux_read_description (xcr0_features_bits);
+
+ if (tdesc == NULL)
+ tdesc = i386_linux_read_description (X86_XSTATE_SSE_MASK);
+
+ return tdesc;
}
gdb_assert_not_reached ("failed to return tdesc");