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

[Pedro Alves <palves@redhat.com>]

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