Re: [PATCH 1/2] Include the fs_base and gs_base registers in amd64 target descriptions.

Pedro Alves
2017-07-13T18:40:47Z

On 07/13/2017 05:55 PM, Yao Qi wrote:

> because each tdesc_reg is a singleton among the target description. The
> reason Simon observed that we have different "fs_base" tdesc_reg added
> and removed from the hash table is that they are from different target
> descriptions. GDB crashes in tdesc_use_registers. The arguments tdesc
> and tdesc_data are not consistent, tdesc is amd64 linux target
> description, but tdesc_data was got when tdesc is amd64 target
> description, so the tdesc_reg in tdesc_data are from amd64 target
> description as well. So, we push a "fs_base" from one target
> description and want to remove a "fs_base" from another target
> description.
>
> Does this answer your question? I think maybe there is some "better"
> fix that is to keep tdesc and tdes_data consistent. However, I don't
> think it further. Since current GDB trunk is unusable on x86_64-linux,
> it is better get a fix soon.

Yeah. It seems to me that the problem is that
amd64_linux_init_abi uses amd64_init_abi as helper, but
they don't coordinate on which fallback tdesc to use.

So amd64_init_abi does:

if (! tdesc_has_registers (tdesc))
tdesc = tdesc_amd64;

and creates a register in the "tdesc_amd64" architecture.

and then after, amd64_linux_init_abi does:

if (! tdesc_has_registers (tdesc))
tdesc = tdesc_amd64_linux;
tdep->tdesc = tdesc;

But when amd64_init_abi is being called as helper, it should
fallback to tdesc_amd64_linux instead.

Notice that amd64_linux_init_abi also calls tdesc_numbered_register,
for orig_rax. So if some other foo_init_abi routine would use
amd64_linux_init_abi as helper, then that tdesc_numbered_register call
would be problematic in the same way.

It seems to me that the proper fix is to make sure
that amd64_linux_init_abi and amd64_init_abi agree. Something like
this patch below. Fixes the crash for me.

From d5775b549291675d9bb5c095b837eca16733aa61 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 13 Jul 2017 19:09:13 +0100
Subject: [PATCH] fix

---
gdb/amd64-linux-tdep.c | 9 +++------
gdb/amd64-tdep.c | 16 ++++++++--------
gdb/amd64-tdep.h | 5 ++++-
3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index 4ef0f78..7863d58 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -1863,7 +1863,6 @@ static void
amd64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
{
struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
- const struct target_desc *tdesc = info.target_desc;
struct tdesc_arch_data *tdesc_data
= (struct tdesc_arch_data *) info.tdep_info;
const struct tdesc_feature *feature;
@@ -1875,15 +1874,13 @@ amd64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
tdep->gregset_num_regs = ARRAY_SIZE (amd64_linux_gregset_reg_offset);
tdep->sizeof_gregset = 27 * 8;

- amd64_init_abi (info, gdbarch);
+ amd64_init_abi (info, gdbarch, tdesc_amd64_linux);
+
+ const target_desc *tdesc = tdep->tdesc;

/* Reserve a number for orig_rax. */
set_gdbarch_num_regs (gdbarch, AMD64_LINUX_NUM_REGS);

- if (! tdesc_has_registers (tdesc))
- tdesc = tdesc_amd64_linux;
- tdep->tdesc = tdesc;
-
feature = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.linux");
if (feature == NULL)
return;
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 9ff7dfc..de89f95 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -3005,7 +3005,8 @@ static const int amd64_record_regmap[] =
};

void
-amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
+ target_desc *default_tdesc)
{
struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
const struct target_desc *tdesc = info.target_desc;
@@ -3022,7 +3023,11 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
tdep->fpregset = &amd64_fpregset;

if (! tdesc_has_registers (tdesc))
- tdesc = tdesc_amd64;
+ {
+ if (default_tdesc == NULL)
+ default_tdesc = tdesc_amd64;
+ tdesc = default_tdesc;
+ }
tdep->tdesc = tdesc;

tdep->num_core_regs = AMD64_NUM_GREGS + I387_NUM_REGS;
@@ -3199,13 +3204,8 @@ void
amd64_x32_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
{
struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
- const struct target_desc *tdesc = info.target_desc;

- amd64_init_abi (info, gdbarch);
-
- if (! tdesc_has_registers (tdesc))
- tdesc = tdesc_x32;
- tdep->tdesc = tdesc;
+ amd64_init_abi (info, gdbarch, tdesc_x32);

tdep->num_dword_regs = 17;
set_tdesc_pseudo_register_type (gdbarch, amd64_x32_pseudo_register_type);
diff --git a/gdb/amd64-tdep.h b/gdb/amd64-tdep.h
index 87f0ba3..9f946af 100644
--- a/gdb/amd64-tdep.h
+++ b/gdb/amd64-tdep.h
@@ -97,7 +97,10 @@ extern void amd64_displaced_step_fixup (struct gdbarch *gdbarch,
CORE_ADDR from, CORE_ADDR to,
struct regcache *regs);

-extern void amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch);
+extern void amd64_init_abi (struct gdbarch_info info,
+ struct gdbarch *gdbarch,
+ target_desc *default_tdesc = NULL);
+
extern void amd64_x32_init_abi (struct gdbarch_info info,
struct gdbarch *gdbarch);
extern const struct target_desc *amd64_target_description (uint64_t xcr0);
--
2.5.5