diff mbox

[v3,07/10] s390: Hook s390 into OSABI mechanism

Message ID 20171215171929.7bac6876@ThinkPad
State New, archived
Headers

Commit Message

Philipp Rudo Dec. 15, 2017, 4:19 p.m. UTC 
  Hi Uli,

On Thu, 14 Dec 2017 16:44:31 +0100 (CET)
"Ulrich Weigand" <uweigand@de.ibm.com> wrote:

> Philipp Rudo wrote:
> 
> > ... I moved init_osabi so early so only the functions which are overwritten by
> > the osabi (or needed for other reasons like the dwarf unwinder) are set before
> > it.  I hoped this would make the gdbarch_init clearer as the list before
> > init_osabi is shorter and you can say for sure that all hooks set after are
> > shared between all OSes. So you are not surprised with some special cases you
> > don't expect.  Of course this also means that if you want to overwrite a
> > function you first have to move it before osabi_init.  For me that is a small
> > price to pay.
> > 
> > However when you prefer the osabi_init to be done later I can move it to where
> > linux_init_abi is called today. In that case i can avoid moving
> > dwarf2_append_unwinders. Otherwise I will move the other
> > dwarf2 related routines up.  Just tell me what you prefer.  
> 
> Yes, please do move init_osabi to be done later.

I played around and got the fixup patch attached.  For some reason however now
some python testcases fail. In particular 

FAIL: gdb.python/py-breakpoint.exp: test_bkpt_qualified: qualified false
FAIL: gdb.python/py-breakpoint.exp: test_bkpt_qualified: qualified true
FAIL: gdb.python/py-breakpoint.exp: test_bkpt_qualified: qualified true and explicit
FAIL: gdb.python/py-breakpoint.exp: test_bkpt_qualified: qualified false and explicit
FAIL: gdb.python/py-breakpoint.exp: test_bkpt_qualified: -q in spec string and qualified false

each with the error message

Traceback (most recent call last):^M                                            
  File "<string>", line 1, in <module>^M                                        
TypeError: argument 10 (impossible<bad format char>)^M                          
Error while executing Python code.^M

I have absolutely no clue what causes this as the gdbarch'es (with and without
the patch) seem to be identical.  As this is my last day before the Christmas
vacation I cannot have a closer look at it right now.  So finalizing the split
up will have to wait until next year.

Merry Christmas and see you next year
Philipp
diff mbox

Patch

From e332d4fc2665707a125e63d17465b2aede5a554a Mon Sep 17 00:00:00 2001
From: Philipp Rudo <prudo@linux.vnet.ibm.com>
Date: Fri, 15 Dec 2017 12:51:45 +0100
Subject: [PATCH] fixup! s390: Hook s390 into OSABI mechanism

---
 gdb/s390-linux-tdep.c | 159 ++++++++++++++++++++++++++------------------------
 1 file changed, 84 insertions(+), 75 deletions(-)

diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index a4e27f03fc..f43c0d38f3 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -8020,54 +8020,6 @@  s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   struct tdesc_arch_data *tdesc_data = tdesc_data_alloc ();
   info.tdesc_data = tdesc_data;
 
-  /* The DWARF unwinders must be appended before the ABI is initialized.
-     Otherwise it is possible that a ABI default unwinder gets called before
-     the DWARF unwinder even gets the chance.  */
-  dwarf2_append_unwinders (gdbarch);
-
-  gdbarch_init_osabi (info, gdbarch);
-
-  /* Check any target description for validity.  */
-  gdb_assert (tdesc_has_registers (tdep->tdesc));
-  if (!s390_tdesc_valid (tdep, tdesc_data))
-    {
-      tdesc_data_cleanup (tdesc_data);
-      xfree (tdep);
-      gdbarch_free (gdbarch);
-      return NULL;
-    }
-
-  /* Determine vector ABI.  */
-#ifdef HAVE_ELF
-  if (tdep->have_vx
-      && info.abfd != NULL
-      && info.abfd->format == bfd_object
-      && bfd_get_flavour (info.abfd) == bfd_target_elf_flavour
-      && bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_GNU,
-				   Tag_GNU_S390_ABI_Vector) == 2)
-    tdep->vector_abi = S390_VECTOR_ABI_128;
-#endif
-
-  /* Find a candidate among extant architectures.  */
-  for (arches = gdbarch_list_lookup_by_info (arches, &info);
-       arches != NULL;
-       arches = gdbarch_list_lookup_by_info (arches->next, &info))
-    {
-      struct gdbarch_tdep *tmp = gdbarch_tdep (arches->gdbarch);
-      if (!tmp)
-	continue;
-      /* A program can 'choose' not to use the vector registers when they
-	 are present.  Leading to the same tdesc but different tdep and
-	 thereby a different gdbarch.  */
-      if (tmp->vector_abi != tdep->vector_abi)
-	continue;
-
-      tdesc_data_cleanup (tdesc_data);
-      xfree (tdep);
-      gdbarch_free (gdbarch);
-      return arches->gdbarch;
-    }
-
   set_gdbarch_believe_pcc_promotion (gdbarch, 0);
   set_gdbarch_char_signed (gdbarch, 0);
 
@@ -8108,26 +8060,6 @@  s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_ax_pseudo_register_push_stack
       (gdbarch, s390_ax_pseudo_register_push_stack);
   set_gdbarch_gen_return_address (gdbarch, s390_gen_return_address);
-  tdesc_use_registers (gdbarch, tdep->tdesc, tdesc_data);
-  set_gdbarch_register_name (gdbarch, s390_register_name);
-
-  /* Assign pseudo register numbers.  */
-  first_pseudo_reg = gdbarch_num_regs (gdbarch);
-  last_pseudo_reg = first_pseudo_reg;
-  if (tdep->have_upper)
-    {
-      tdep->gpr_full_regnum = last_pseudo_reg;
-      last_pseudo_reg += 16;
-    }
-  if (tdep->have_vx)
-    {
-      tdep->v0_full_regnum = last_pseudo_reg;
-      last_pseudo_reg += 16;
-    }
-  tdep->pc_regnum = last_pseudo_reg++;
-  tdep->cc_regnum = last_pseudo_reg++;
-  set_gdbarch_pc_regnum (gdbarch, tdep->pc_regnum);
-  set_gdbarch_num_pseudo_regs (gdbarch, last_pseudo_reg - first_pseudo_reg);
 
   /* Inferior function calls.  */
   set_gdbarch_push_dummy_call (gdbarch, s390_push_dummy_call);
@@ -8138,10 +8070,7 @@  s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Frame handling.  */
   dwarf2_frame_set_init_reg (gdbarch, s390_dwarf2_frame_init_reg);
   dwarf2_frame_set_adjust_regnum (gdbarch, s390_adjust_frame_regnum);
-  frame_base_append_sniffer (gdbarch, dwarf2_frame_base_sniffer);
-  frame_unwind_append_unwinder (gdbarch, &s390_stub_frame_unwind);
-  frame_unwind_append_unwinder (gdbarch, &s390_frame_unwind);
-  frame_base_set_default (gdbarch, &s390_frame_base);
+  dwarf2_append_unwinders (gdbarch);
   set_gdbarch_unwind_pc (gdbarch, s390_unwind_pc);
   set_gdbarch_unwind_sp (gdbarch, s390_unwind_sp);
 
@@ -8152,13 +8081,13 @@  s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_displaced_step_location (gdbarch, linux_displaced_step_location);
   set_gdbarch_max_insn_length (gdbarch, S390_MAX_INSTR_SIZE);
 
-  switch (tdep->abi)
+  switch (info.bfd_arch_info->mach)
     {
-    case ABI_LINUX_S390:
+    case bfd_mach_s390_31:
       set_gdbarch_addr_bits_remove (gdbarch, s390_addr_bits_remove);
       break;
 
-    case ABI_LINUX_ZSERIES:
+    case bfd_mach_s390_64:
       set_gdbarch_long_bit (gdbarch, 64);
       set_gdbarch_long_long_bit (gdbarch, 64);
       set_gdbarch_ptr_bit (gdbarch, 64);
@@ -8190,6 +8119,86 @@  s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_gcc_target_options (gdbarch, s390_gcc_target_options);
   set_gdbarch_gnu_triplet_regexp (gdbarch, s390_gnu_triplet_regexp);
 
+  /* Initialize the OSABI.  */
+  gdbarch_init_osabi (info, gdbarch);
+
+  /* Check any target description for validity.  */
+  gdb_assert (tdesc_has_registers (tdep->tdesc));
+  if (!s390_tdesc_valid (tdep, tdesc_data))
+    {
+      tdesc_data_cleanup (tdesc_data);
+      xfree (tdep);
+      gdbarch_free (gdbarch);
+      return NULL;
+    }
+
+  /* Determine vector ABI.  */
+#ifdef HAVE_ELF
+  if (tdep->have_vx
+      && info.abfd != NULL
+      && info.abfd->format == bfd_object
+      && bfd_get_flavour (info.abfd) == bfd_target_elf_flavour
+      && bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_GNU,
+				   Tag_GNU_S390_ABI_Vector) == 2)
+    tdep->vector_abi = S390_VECTOR_ABI_128;
+#endif
+
+  /* Find a candidate among extant architectures.  */
+  for (arches = gdbarch_list_lookup_by_info (arches, &info);
+       arches != NULL;
+       arches = gdbarch_list_lookup_by_info (arches->next, &info))
+    {
+      struct gdbarch_tdep *tmp = gdbarch_tdep (arches->gdbarch);
+      if (!tmp)
+	continue;
+      /* A program can 'choose' not to use the vector registers when they
+	 are present.  Leading to the same tdesc but different tdep and
+	 thereby a different gdbarch.  */
+      if (tmp->vector_abi != tdep->vector_abi)
+	continue;
+
+      tdesc_data_cleanup (tdesc_data);
+      xfree (tdep);
+      gdbarch_free (gdbarch);
+      return arches->gdbarch;
+    }
+
+  /* tdesc_use_registers sets:
+      * gdbarch_num_regs
+      * gdbarch_register_name
+      * gdbarch_register_type
+      * gdbarch_remote_register_num
+      * gdbarch_register_reggroup_p
+
+    Furthermore it deletes tdesc_data. No(!) call to tdesc_data_cleanup after
+    this point or GDB crashes with a double free.
+  */
+  tdesc_use_registers (gdbarch, tdep->tdesc, tdesc_data);
+  set_gdbarch_register_name (gdbarch, s390_register_name);
+
+  /* Assign pseudo register numbers.  */
+  first_pseudo_reg = gdbarch_num_regs (gdbarch);
+  last_pseudo_reg = first_pseudo_reg;
+  if (tdep->have_upper)
+    {
+      tdep->gpr_full_regnum = last_pseudo_reg;
+      last_pseudo_reg += 16;
+    }
+  if (tdep->have_vx)
+    {
+      tdep->v0_full_regnum = last_pseudo_reg;
+      last_pseudo_reg += 16;
+    }
+  tdep->pc_regnum = last_pseudo_reg++;
+  tdep->cc_regnum = last_pseudo_reg++;
+  set_gdbarch_pc_regnum (gdbarch, tdep->pc_regnum);
+  set_gdbarch_num_pseudo_regs (gdbarch, last_pseudo_reg - first_pseudo_reg);
+
+  frame_base_append_sniffer (gdbarch, dwarf2_frame_base_sniffer);
+  frame_unwind_append_unwinder (gdbarch, &s390_stub_frame_unwind);
+  frame_unwind_append_unwinder (gdbarch, &s390_frame_unwind);
+  frame_base_set_default (gdbarch, &s390_frame_base);
+
   return gdbarch;
 }
 
-- 
2.13.5