[v2,07/11] s390: Hook s390 into OSABI mechanism

Message ID 20171205122859.2919-8-prudo@linux.vnet.ibm.com
State New, archived
Headers

Commit Message

Philipp Rudo Dec. 5, 2017, 12:28 p.m. UTC
  Well do what the title says and distinguish between 31- and 64-bit systems.

There is one pitfall to take care of.  Appending the dwarf2 unwinder must
be moved _before_ the OSABI is initialized.  Otherwise a default unwinder
will take control before the dwarf unwinder even gets a chance.

gdb/ChangeLog:

	* s390-linux-tdep.c (osabi.h): New include
	(s390_linux_init_osabi_31, s390_linux_init_osabi_64)
	(s390_linux_init_osabi_any): New functions.
	(s390_gdbarch_init): Adjust.
---
 gdb/s390-linux-tdep.c | 155 +++++++++++++++++++++++++++++---------------------
 1 file changed, 91 insertions(+), 64 deletions(-)
  

Comments

Philipp Rudo Dec. 5, 2017, 5:06 p.m. UTC | #1
Hi Uli,

On Tue, 5 Dec 2017 14:20:55 +0100 (CET)
"Ulrich Weigand" <uweigand@de.ibm.com> wrote:

> Philipp Rudo wrote:
> 
> Just a couple of quick comments on the common vs. Linux split.
> 
> > +  set_gdbarch_guess_tracepoint_registers (gdbarch,
> > +					  s390_guess_tracepoint_registers);  
> 
> This is OS-independent as far as I can see.

Ok, I'll move it back.
 
> > +  /* Frame handling.  */
> > +  frame_unwind_append_unwinder (gdbarch, &s390_stub_frame_unwind);
> > +  frame_unwind_append_unwinder (gdbarch, &s390_sigtramp_frame_unwind);
> > +  frame_unwind_append_unwinder (gdbarch, &s390_frame_unwind);
> > +  frame_base_set_default (gdbarch, &s390_frame_base);  
> 
> All of that *except* the sigtramp unwinder is OS-independent.  In fact,
> if move the prolog-based sniffer to common code, you'll see that you no
> longer need to export various internal routines from s390-tdep.c to
> s390-linux-tdep.c.
> 
> This may require swapping the order of the stub and the sigtramp unwinder,
> but that should be harmless.  In the end you should have this sequence:
> 
> - first, in common code, announce all the DWARF-based unwinders
> - then, in Linux ABI code, announce the sigtramp unwinder
> - finally, back in common code, announce all the fallback unwinders

That's not true.  At least for the kernel the unwinders (except the DWARF-based)
failed and I had to write my own one.  Especially the fact that the kernel has 
multiple stack locations and no distinct 'End of Stack' caused problems.

However if I go with your approach and add my unwinder in the kernel ABI code
as default, i.e. that it always catches the call, it should work.  But that
won't be nice code.  That's why I moved them to the Linux ABI.

> >    set_gdbarch_process_record (gdbarch, s390_process_record);
> >    set_gdbarch_process_record_signal (gdbarch, s390_linux_record_signal);  
> 
> The first of these should be generic, only the second is Linux specific
> (that's why there are two different callbacks to begin with!).

Ok, I'll move the first one back.

> > +  /* Miscellaneous.  */
> > +  set_gdbarch_stap_is_single_operand (gdbarch, s390_stap_is_single_operand);
> > +  set_gdbarch_gcc_target_options (gdbarch, s390_gcc_target_options);
> > +  set_gdbarch_gnu_triplet_regexp (gdbarch, s390_gnu_triplet_regexp);  
> 
> Are these really Linux-specific?

No, not really.  They should be common for the architecture for all systems
with working GDB/GCC/BFD. I'll move them to the common file.

Thanks
Philipp
  
Philipp Rudo Dec. 6, 2017, 11:39 a.m. UTC | #2
Hi Uli,

On Tue, 5 Dec 2017 18:44:21 +0100 (CET)
"Ulrich Weigand" <uweigand@de.ibm.com> wrote:

> Philipp Rudo wrote:
> 
> > > All of that *except* the sigtramp unwinder is OS-independent.  In fact,
> > > if move the prolog-based sniffer to common code, you'll see that you no
> > > longer need to export various internal routines from s390-tdep.c to
> > > s390-linux-tdep.c.
> > > 
> > > This may require swapping the order of the stub and the sigtramp unwinder,
> > > but that should be harmless.  In the end you should have this sequence:
> > > 
> > > - first, in common code, announce all the DWARF-based unwinders
> > > - then, in Linux ABI code, announce the sigtramp unwinder
> > > - finally, back in common code, announce all the fallback unwinders  
> > 
> > That's not true.  At least for the kernel the unwinders (except the DWARF-based)
> > failed and I had to write my own one.  Especially the fact that the kernel has 
> > multiple stack locations and no distinct 'End of Stack' caused problems.
> > 
> > However if I go with your approach and add my unwinder in the kernel ABI code
> > as default, i.e. that it always catches the call, it should work.  But that
> > won't be nice code.  That's why I moved them to the Linux ABI.  
> 
> That's surprising, I would have thought the prolog parser generic enough
> to handle kernel code as well (and it really has to be anyway, since you
> install the s390_skip_prologue callback in generic code ...).

My best guess is that the prologue unwinder cannot handle the fact that there
is no classical branch to the interrupt handler.  So the old pc cannot be taken
from %r14 or the psw but has to be read from lowcore.  But I'd had to spend
more time to investigate it further.  However there are more pressing things
to do.  Thats why I'll stick with the solution I have for the time.  The
backchain unwinder I have also is a good fall-back, so implementing it wasn't a
waste of time.

Most likely I'll have to get back to the unwinders soon anyway.  At the moment
it looks like my next kernel item will be to port the ORC unwinder to s390.  So
support for it has to be added to GDB, too.

To be honest I don't know if the s390_skip_prologue works for kernel
debugging.  As I only support (and tested) debugging of dumps, the impact
should be minimal if it were broken.

> I agree that there may be some special cases.  About the issues you mention:
> 
> - By "multiple stack locations" I assume you refer to the interrupt stack
>   vs. the regular per-task kernel stacks?  I agree that we need kernel-
>   specific code to switch between the two.  But that should simply be
>   the equivalent to the user-space signal stack handling, so you'll install
>   a sniffer in the kernel-specific code that detects the frame where you
>   need to *switch* stacks.  All the normal frames should be handled fine
>   by the standard prolog-parser code.

Yes, "multiple stack locations" refers to interrupt vs. per-task kernel stack
(plus restart and panic stack).

The unwinder I have implemented mostly works like the user-space signal stack
handling.  However I needed a backchain unwinder to handle the assembler code
for two reasons.  First it doesn't contain debug info the dwarf unwinder could
use.  Second a backchain = 0 marks the End-of-this-Stack so the backchain
unwinder has to trigger the sigtramp unwinder to go to the next stack location
(or stop unwinding if its the end of the per-task kernel stack).
 
> - End-of-stack detection is always a bit hit-and-miss with the prolog
>   parser, and uses heuristics anyway.  If you need to tweak those a
>   bit so they work well for kernel code, I'd hope (without having seen
>   the code) that it should be possible to simply update the generic
>   code to handle both.

Currently I'm using brute-force to detect the End-of-Stack, i.e. simply check
if the SP points to any of the given stack locations or not.  Something similar
has to be added to the heuristics of the prologue unwinder.  The only other
chance would be to check if the SP is a kernel- or user-space address.  But you
had to do the heuristics for that by only using an unsigned long, so it will be
very error prone.

Thanks
Philipp
  

Patch

diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index f6a39868be..f8559eec49 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -30,6 +30,7 @@ 
 #include "gdbcore.h"
 #include "gdbcmd.h"
 #include "objfiles.h"
+#include "osabi.h"
 #include "regcache.h"
 #include "trad-frame.h"
 #include "frame-base.h"
@@ -8024,32 +8025,12 @@  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;
 
-  /* Default ABI and register size.  */
-  switch (info.bfd_arch_info->mach)
-    {
-    case bfd_mach_s390_31:
-      tdep->abi = ABI_LINUX_S390;
-      break;
-
-    case bfd_mach_s390_64:
-      tdep->abi = ABI_LINUX_ZSERIES;
-      break;
-
-    default:
-      xfree (tdep);
-      gdbarch_free (gdbarch);
-      return NULL;
-    }
+  /* 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);
 
-  /* Use default target description if none provided by the target.  */
-  if (!tdesc_has_registers (tdesc))
-    {
-      if (tdep->abi == ABI_LINUX_S390)
-	tdesc = tdesc_s390_linux32;
-      else
-	tdesc = tdesc_s390x_linux64;
-    }
-  tdep->tdesc = tdesc;
+  gdbarch_init_osabi (info, gdbarch);
 
   /* Check any target description for validity.  */
   gdb_assert (tdesc_has_registers (tdep->tdesc));
@@ -8100,12 +8081,7 @@  s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_stab_reg_to_regnum (gdbarch, s390_dwarf_reg_to_regnum);
   set_gdbarch_dwarf2_reg_to_regnum (gdbarch, s390_dwarf_reg_to_regnum);
   set_gdbarch_value_from_register (gdbarch, s390_value_from_register);
-  set_gdbarch_core_read_description (gdbarch, s390_core_read_description);
-  set_gdbarch_iterate_over_regset_sections (gdbarch,
-					    s390_iterate_over_regset_sections);
   set_gdbarch_cannot_store_register (gdbarch, s390_cannot_store_register);
-  set_gdbarch_write_pc (gdbarch, s390_write_pc);
-  set_gdbarch_guess_tracepoint_registers (gdbarch, s390_guess_tracepoint_registers);
   set_gdbarch_pseudo_register_read (gdbarch, s390_pseudo_register_read);
   set_gdbarch_pseudo_register_write (gdbarch, s390_pseudo_register_write);
   set_tdesc_pseudo_register_name (gdbarch, s390_pseudo_register_name);
@@ -8144,18 +8120,10 @@  s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_frame_align (gdbarch, s390_frame_align);
   set_gdbarch_return_value (gdbarch, s390_return_value);
 
-  /* Syscall handling.  */
-  set_gdbarch_get_syscall_number (gdbarch, s390_linux_get_syscall_number);
-
   /* Frame handling.  */
   dwarf2_frame_set_init_reg (gdbarch, s390_dwarf2_frame_init_reg);
   dwarf2_frame_set_adjust_regnum (gdbarch, s390_adjust_frame_regnum);
-  dwarf2_append_unwinders (gdbarch);
   frame_base_append_sniffer (gdbarch, dwarf2_frame_base_sniffer);
-  frame_unwind_append_unwinder (gdbarch, &s390_stub_frame_unwind);
-  frame_unwind_append_unwinder (gdbarch, &s390_sigtramp_frame_unwind);
-  frame_unwind_append_unwinder (gdbarch, &s390_frame_unwind);
-  frame_base_set_default (gdbarch, &s390_frame_base);
   set_gdbarch_unwind_pc (gdbarch, s390_unwind_pc);
   set_gdbarch_unwind_sp (gdbarch, s390_unwind_sp);
 
@@ -8166,65 +8134,118 @@  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);
 
-  /* Note that GNU/Linux is the only OS supported on this
-     platform.  */
-  linux_init_abi (info, gdbarch);
-
   switch (tdep->abi)
     {
     case ABI_LINUX_S390:
       set_gdbarch_addr_bits_remove (gdbarch, s390_addr_bits_remove);
-      set_solib_svr4_fetch_link_map_offsets
-	(gdbarch, svr4_ilp32_fetch_link_map_offsets);
-
-      set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_S390);
       break;
 
     case ABI_LINUX_ZSERIES:
       set_gdbarch_long_bit (gdbarch, 64);
       set_gdbarch_long_long_bit (gdbarch, 64);
       set_gdbarch_ptr_bit (gdbarch, 64);
-      set_solib_svr4_fetch_link_map_offsets
-	(gdbarch, svr4_lp64_fetch_link_map_offsets);
       set_gdbarch_address_class_type_flags (gdbarch,
 					    s390_address_class_type_flags);
       set_gdbarch_address_class_type_flags_to_name (gdbarch,
 						    s390_address_class_type_flags_to_name);
       set_gdbarch_address_class_name_to_type_flags (gdbarch,
 						    s390_address_class_name_to_type_flags);
-      set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_S390X);
       break;
     }
 
-  set_gdbarch_skip_trampoline_code (gdbarch, find_solib_trampoline_target);
-
-  /* Enable TLS support.  */
-  set_gdbarch_fetch_tls_load_module_address (gdbarch,
-					     svr4_fetch_objfile_link_map);
-
   /* SystemTap functions.  */
   set_gdbarch_stap_register_prefixes (gdbarch, stap_register_prefixes);
   set_gdbarch_stap_register_indirection_prefixes (gdbarch,
 					  stap_register_indirection_prefixes);
   set_gdbarch_stap_register_indirection_suffixes (gdbarch,
 					  stap_register_indirection_suffixes);
-  set_gdbarch_stap_is_single_operand (gdbarch, s390_stap_is_single_operand);
-  set_gdbarch_gcc_target_options (gdbarch, s390_gcc_target_options);
-  set_gdbarch_gnu_triplet_regexp (gdbarch, s390_gnu_triplet_regexp);
 
-  /* Support reverse debugging.  */
+  set_gdbarch_disassembler_options (gdbarch, &s390_disassembler_options);
+  set_gdbarch_valid_disassembler_options (gdbarch,
+					  disassembler_options_s390 ());
+
+  return gdbarch;
+}
+
+/* Initialize OSABI common for GNU/Linux on 31- and 64-bit systems.  */
+
+static void
+s390_linux_init_abi_any (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  linux_init_abi (info, gdbarch);
 
+  /* Register handling.  */
+  set_gdbarch_core_read_description (gdbarch, s390_core_read_description);
+  set_gdbarch_iterate_over_regset_sections (gdbarch,
+					    s390_iterate_over_regset_sections);
+  set_gdbarch_guess_tracepoint_registers (gdbarch,
+					  s390_guess_tracepoint_registers);
+  set_gdbarch_write_pc (gdbarch, s390_write_pc);
+
+  /* Syscall handling.  */
+  set_gdbarch_get_syscall_number (gdbarch, s390_linux_get_syscall_number);
+
+  /* Frame handling.  */
+  frame_unwind_append_unwinder (gdbarch, &s390_stub_frame_unwind);
+  frame_unwind_append_unwinder (gdbarch, &s390_sigtramp_frame_unwind);
+  frame_unwind_append_unwinder (gdbarch, &s390_frame_unwind);
+  frame_base_set_default (gdbarch, &s390_frame_base);
+  set_gdbarch_skip_trampoline_code (gdbarch, find_solib_trampoline_target);
+
+  /* Enable TLS support.  */
+  set_gdbarch_fetch_tls_load_module_address (gdbarch,
+					     svr4_fetch_objfile_link_map);
+
+  /* Support reverse debugging.  */
   set_gdbarch_process_record (gdbarch, s390_process_record);
   set_gdbarch_process_record_signal (gdbarch, s390_linux_record_signal);
-
   s390_init_linux_record_tdep (&s390_linux_record_tdep, ABI_LINUX_S390);
   s390_init_linux_record_tdep (&s390x_linux_record_tdep, ABI_LINUX_ZSERIES);
 
-  set_gdbarch_disassembler_options (gdbarch, &s390_disassembler_options);
-  set_gdbarch_valid_disassembler_options (gdbarch,
-					  disassembler_options_s390 ());
+  /* Miscellaneous.  */
+  set_gdbarch_stap_is_single_operand (gdbarch, s390_stap_is_single_operand);
+  set_gdbarch_gcc_target_options (gdbarch, s390_gcc_target_options);
+  set_gdbarch_gnu_triplet_regexp (gdbarch, s390_gnu_triplet_regexp);
+}
 
-  return gdbarch;
+/* Initialize OSABI for GNU/Linux on 31-bit systems.  */
+
+static void
+s390_linux_init_abi_31 (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  const struct target_desc *tdesc = info.target_desc;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  tdep->abi = ABI_LINUX_S390;
+  if (!tdesc_has_registers (tdesc))
+    tdesc = tdesc_s390_linux32;
+  tdep->tdesc = tdesc;
+
+  s390_linux_init_abi_any (info, gdbarch);
+
+  set_solib_svr4_fetch_link_map_offsets (gdbarch,
+					 svr4_ilp32_fetch_link_map_offsets);
+  set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_S390);
+}
+
+/* Initialize OSABI for GNU/Linux on 64-bit systems.  */
+
+static void
+s390_linux_init_abi_64 (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  const struct target_desc *tdesc = info.target_desc;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  tdep->abi = ABI_LINUX_ZSERIES;
+  if (!tdesc_has_registers (tdesc))
+    tdesc = tdesc_s390x_linux64;
+  tdep->tdesc = tdesc;
+
+  s390_linux_init_abi_any (info, gdbarch);
+
+  set_solib_svr4_fetch_link_map_offsets (gdbarch,
+					 svr4_lp64_fetch_link_map_offsets);
+  set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_S390X);
 }
 
 void
@@ -8233,6 +8254,12 @@  _initialize_s390_tdep (void)
   /* Hook us into the gdbarch mechanism.  */
   register_gdbarch_init (bfd_arch_s390, s390_gdbarch_init);
 
+  /* Hook us into the OSABI mechanism.  */
+  gdbarch_register_osabi (bfd_arch_s390, bfd_mach_s390_31, GDB_OSABI_LINUX,
+			  s390_linux_init_abi_31);
+  gdbarch_register_osabi (bfd_arch_s390, bfd_mach_s390_64, GDB_OSABI_LINUX,
+			  s390_linux_init_abi_64);
+
   /* Initialize the GNU/Linux target descriptions.  */
   initialize_tdesc_s390_linux32 ();
   initialize_tdesc_s390_linux32v1 ();