[PATCH] s390: Fix gdb.base/all-architectures.exp with --enable-targets=all

[PATCH] s390: Fix gdb.base/all-architectures.exp with --enable-targets=all

[Philipp Rudo]

Hi everybody

my s390 split up patch set introduced a bug.  In particular GDB crashes with
an internal error when the architecture is set to s390 and the user sets the
osabi to something else than Linux.  The patch attached fixes this and
restores the behavior s390_gdbarch_init had before the split.

Please note that the fix is pretty ugly.  However i am currently working on
dynamically creating the target description for s390.  With this in place
the ugliness will be (hopefully soon) removed again.

Philipp

Philipp Rudo (1):
  s390: Fix gdb.base/all-architectures.exp with --enable-targets=all

 gdb/s390-linux-tdep.c | 10 ----------
 gdb/s390-linux-tdep.h |  2 --
 gdb/s390-tdep.c       | 23 +++++++++++++++++++++--
 gdb/s390-tdep.h       |  3 +++
 4 files changed, 24 insertions(+), 14 deletions(-)

-- 
2.13.5

[PATCH] s390: Fix gdb.base/all-architectures.exp with --enable-targets=all

[Philipp Rudo]

With 7042632bf79 (s390: Hook s390 into OSABI mechanism) assigning a default
target description was moved from s390_gdbarch_init to
s390_linux_init_abi_*.  This causes problems when GDB is build with
--enable-targets=all and the user sets a non supported osabi (e.g. set
osabi AIX).  In this case there is no valid tdesc and GDB crashes with an
internal error.  Fix this by reverting parts of 7042632bf79.

gdb/ChangeLog:

	* s390-linux-tdep.c: Move includes for features/s390-linux32.c and
	features/s390x-linux64.c to s390-tdep.c.
	(_initialize_s390_linux_tdep): Move initializaion of tdesc
	s390_linux32 and s390x_linux64 to s390-tdep.c.
	(s390_linux_init_abi_31, s390_linux_init_abi_64): Don't set default
	tdesc.
	* s390-tdep.c: Add include of features/s390-linux32.c and
	features/s390x-linux64.c.
	(s390_tdesc_valid): Add check for tdesc_has_registers.
	(s390_gdbarch_init): Make sure there is always an valid tdesc.
	(_initialize_s390_tdep): Initialize tdesc_s390_linux32 and
	tdesc_s390x_linux64.
	* s390-linux-tdep.h: Move export of tdesc_s390_linux32 and
	tdesc_s390x_linux64 to ...
	* s390-tdep.h: ... here.
---
 gdb/s390-linux-tdep.c | 10 ----------
 gdb/s390-linux-tdep.h |  2 --
 gdb/s390-tdep.c       | 23 +++++++++++++++++++++--
 gdb/s390-tdep.h       |  3 +++
 4 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index 9d1351946e..0e85e71089 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -42,7 +42,6 @@
 #include "trad-frame.h"
 #include "xml-syscall.h"
 
-#include "features/s390-linux32.c"
 #include "features/s390-linux32v1.c"
 #include "features/s390-linux32v2.c"
 #include "features/s390-linux64.c"
@@ -52,7 +51,6 @@
 #include "features/s390-vx-linux64.c"
 #include "features/s390-tevx-linux64.c"
 #include "features/s390-gs-linux64.c"
-#include "features/s390x-linux64.c"
 #include "features/s390x-linux64v1.c"
 #include "features/s390x-linux64v2.c"
 #include "features/s390x-te-linux64.c"
@@ -1158,9 +1156,6 @@ s390_linux_init_abi_31 (struct gdbarch_info info, struct gdbarch *gdbarch)
   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);
 
@@ -1178,9 +1173,6 @@ s390_linux_init_abi_64 (struct gdbarch_info info, struct gdbarch *gdbarch)
   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);
 
@@ -1199,7 +1191,6 @@ _initialize_s390_linux_tdep (void)
 			  s390_linux_init_abi_64);
 
   /* Initialize the GNU/Linux target descriptions.  */
-  initialize_tdesc_s390_linux32 ();
   initialize_tdesc_s390_linux32v1 ();
   initialize_tdesc_s390_linux32v2 ();
   initialize_tdesc_s390_linux64 ();
@@ -1209,7 +1200,6 @@ _initialize_s390_linux_tdep (void)
   initialize_tdesc_s390_vx_linux64 ();
   initialize_tdesc_s390_tevx_linux64 ();
   initialize_tdesc_s390_gs_linux64 ();
-  initialize_tdesc_s390x_linux64 ();
   initialize_tdesc_s390x_linux64v1 ();
   initialize_tdesc_s390x_linux64v2 ();
   initialize_tdesc_s390x_te_linux64 ();
diff --git a/gdb/s390-linux-tdep.h b/gdb/s390-linux-tdep.h
index 6542464951..a3288e3bb1 100644
--- a/gdb/s390-linux-tdep.h
+++ b/gdb/s390-linux-tdep.h
@@ -48,7 +48,6 @@ extern const struct regset s390_gs_regset;
 extern const struct regset s390_gsbc_regset;
 
 /* GNU/Linux target descriptions.  */
-extern struct target_desc *tdesc_s390_linux32;
 extern struct target_desc *tdesc_s390_linux32v1;
 extern struct target_desc *tdesc_s390_linux32v2;
 extern struct target_desc *tdesc_s390_linux64;
@@ -58,7 +57,6 @@ extern struct target_desc *tdesc_s390_te_linux64;
 extern struct target_desc *tdesc_s390_vx_linux64;
 extern struct target_desc *tdesc_s390_tevx_linux64;
 extern struct target_desc *tdesc_s390_gs_linux64;
-extern struct target_desc *tdesc_s390x_linux64;
 extern struct target_desc *tdesc_s390x_linux64v1;
 extern struct target_desc *tdesc_s390x_linux64v2;
 extern struct target_desc *tdesc_s390x_te_linux64;
diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c
index 1f2a536cd1..b60f4e2a04 100644
--- a/gdb/s390-tdep.c
+++ b/gdb/s390-tdep.c
@@ -40,6 +40,9 @@
 #include "trad-frame.h"
 #include "value.h"
 
+#include "features/s390-linux32.c"
+#include "features/s390x-linux64.c"
+
 /* Holds the current set of options to be passed to the disassembler.  */
 static char *s390_disassembler_options;
 
@@ -6789,6 +6792,9 @@ s390_tdesc_valid (struct gdbarch_tdep *tdep,
   const struct target_desc *tdesc = tdep->tdesc;
   const struct tdesc_feature *feature;
 
+  if (!tdesc_has_registers (tdesc))
+    return false;
+
   /* Core registers, i.e. general purpose and PSW.  */
   feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.core");
   if (feature == NULL)
@@ -6929,7 +6935,6 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   static const char *const stap_register_indirection_suffixes[] = { ")",
 								    NULL };
 
-  /* Otherwise create a new gdbarch for the specified machine type.  */
   struct gdbarch_tdep *tdep = s390_gdbarch_tdep_alloc ();
   struct gdbarch *gdbarch = gdbarch_alloc (&info, tdep);
   struct tdesc_arch_data *tdesc_data = tdesc_data_alloc ();
@@ -7044,8 +7049,19 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Initialize the OSABI.  */
   gdbarch_init_osabi (info, gdbarch);
 
+  /* Always create a default tdesc.  Otherwise commands like 'set osabi'
+     causes GDB to crash with an internal error when the user tries to set
+     a non supported osabi.  */
+  if (!tdesc_has_registers (tdesc))
+  {
+    if (info.bfd_arch_info->mach == bfd_mach_s390_31)
+      tdesc = tdesc_s390_linux32;
+    else
+      tdesc = tdesc_s390x_linux64;
+  }
+  tdep->tdesc = tdesc;
+
   /* 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);
@@ -7120,4 +7136,7 @@ _initialize_s390_tdep (void)
 {
   /* Hook us into the gdbarch mechanism.  */
   register_gdbarch_init (bfd_arch_s390, s390_gdbarch_init);
+
+  initialize_tdesc_s390_linux32 ();
+  initialize_tdesc_s390x_linux64 ();
 }
diff --git a/gdb/s390-tdep.h b/gdb/s390-tdep.h
index 4a44da0190..14530cef31 100644
--- a/gdb/s390-tdep.h
+++ b/gdb/s390-tdep.h
@@ -315,4 +315,7 @@ extern struct value *s390_trad_frame_prev_register
     (struct frame_info *this_frame, struct trad_frame_saved_reg saved_regs[],
      int regnum);
 
+extern struct target_desc *tdesc_s390_linux32;
+extern struct target_desc *tdesc_s390x_linux64;
+
 #endif /* S390_TDEP_H */
-- 
2.13.5

Re: [PATCH] s390: Fix gdb.base/all-architectures.exp with --enable-targets=all

[Andreas Arnez]

On Thu, Jan 25 2018, Philipp Rudo wrote:

> gdb/ChangeLog:
>
> 	* s390-linux-tdep.c: Move includes for features/s390-linux32.c and
> 	features/s390x-linux64.c to s390-tdep.c.
> 	(_initialize_s390_linux_tdep): Move initializaion of tdesc
> 	s390_linux32 and s390x_linux64 to s390-tdep.c.
> 	(s390_linux_init_abi_31, s390_linux_init_abi_64): Don't set default
> 	tdesc.
> 	* s390-tdep.c: Add include of features/s390-linux32.c and
> 	features/s390x-linux64.c.
> 	(s390_tdesc_valid): Add check for tdesc_has_registers.
> 	(s390_gdbarch_init): Make sure there is always an valid tdesc.
> 	(_initialize_s390_tdep): Initialize tdesc_s390_linux32 and
> 	tdesc_s390x_linux64.
> 	* s390-linux-tdep.h: Move export of tdesc_s390_linux32 and
> 	tdesc_s390x_linux64 to ...
> 	* s390-tdep.h: ... here.

Looks OK.  Pushed with minor spelling fixes as c81e88797907f.

--
Andreas