* [PATCH v2 10/11] s390: Add comments to uncommented functions in s390-tdep.c
2017-12-05 12:29 [PATCH v2 00/11] Split up s390-linux-tdep.c Philipp Rudo
@ 2017-12-05 12:29 ` Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 11/11] s390: Add comments to uncommented functions in s390-linux-tdep.c Philipp Rudo
` (8 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Philipp Rudo @ 2017-12-05 12:29 UTC (permalink / raw)
To: gdb-patches; +Cc: Andreas Arnez, Ulrich Weigand
Moving common functions from s390-linux-tdep to s390-tdep showed that some
of them are lacking a description. Fix that now.
gdb/ChangeLog:
* s390-tdep.c: Add comments to uncommented functions
---
gdb/s390-tdep.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 56 insertions(+), 2 deletions(-)
diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c
index 0961c5d8c7..8b05dfc130 100644
--- a/gdb/s390-tdep.c
+++ b/gdb/s390-tdep.c
@@ -79,7 +79,9 @@ s390_readinstruction (bfd_byte instr[], CORE_ADDR at)
come first, even though they're sometimes scattered around the
instructions. And displacements appear before base and extension
registers, as they do in the assembly syntax, not at the end, as
- they do in the machine language. */
+ they do in the machine language.
+
+ Test for RI instruction format. */
static int
is_ri (bfd_byte *insn, int op1, int op2, unsigned int *r1, int *i2)
@@ -95,6 +97,8 @@ is_ri (bfd_byte *insn, int op1, int op2, unsigned int *r1, int *i2)
return 0;
}
+/* Test for RIL instruction format. See comment on is_ri for details. */
+
static int
is_ril (bfd_byte *insn, int op1, int op2,
unsigned int *r1, int *i2)
@@ -115,6 +119,8 @@ is_ril (bfd_byte *insn, int op1, int op2,
return 0;
}
+/* Test for RR instruction format. See comment on is_ri for details. */
+
static int
is_rr (bfd_byte *insn, int op, unsigned int *r1, unsigned int *r2)
{
@@ -128,6 +134,8 @@ is_rr (bfd_byte *insn, int op, unsigned int *r1, unsigned int *r2)
return 0;
}
+/* Test for RRE instruction format. See comment on is_ri for details. */
+
static int
is_rre (bfd_byte *insn, int op, unsigned int *r1, unsigned int *r2)
{
@@ -142,6 +150,8 @@ is_rre (bfd_byte *insn, int op, unsigned int *r1, unsigned int *r2)
return 0;
}
+/* Test for RS instruction format. See comment on is_ri for details. */
+
static int
is_rs (bfd_byte *insn, int op,
unsigned int *r1, unsigned int *r3, int *d2, unsigned int *b2)
@@ -158,6 +168,8 @@ is_rs (bfd_byte *insn, int op,
return 0;
}
+/* Test for RSY instruction format. See comment on is_ri for details. */
+
static int
is_rsy (bfd_byte *insn, int op1, int op2,
unsigned int *r1, unsigned int *r3, int *d2, unsigned int *b2)
@@ -177,6 +189,8 @@ is_rsy (bfd_byte *insn, int op1, int op2,
return 0;
}
+/* Test for RX instruction format. See comment on is_ri for details. */
+
static int
is_rx (bfd_byte *insn, int op,
unsigned int *r1, int *d2, unsigned int *x2, unsigned int *b2)
@@ -193,6 +207,8 @@ is_rx (bfd_byte *insn, int op,
return 0;
}
+/* Test for RXY instruction format. See comment on is_ri for details. */
+
static int
is_rxy (bfd_byte *insn, int op1, int op2,
unsigned int *r1, int *d2, unsigned int *x2, unsigned int *b2)
@@ -526,6 +542,8 @@ s390_displaced_step_fixup (struct gdbarch *gdbarch,
paddress (gdbarch, regcache_read_pc (regs)));
}
+/* Implement displaced_step_hw_singlestep gdbarch method. */
+
static int
s390_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
struct displaced_step_closure *closure)
@@ -1088,6 +1106,9 @@ s390_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
/* Pseudo registers. */
+/* Check whether REGNUM indicates a coupled general purpose register.
+ These pseudo-registers are composed of two adjacent gprs. */
+
static int
regnum_is_gpr_full (struct gdbarch_tdep *tdep, int regnum)
{
@@ -1129,6 +1150,8 @@ s390_value_from_register (struct gdbarch *gdbarch, struct type *type,
return value;
}
+/* Implement pseudo_register_name tdesc method. */
+
static const char *
s390_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
{
@@ -1161,6 +1184,8 @@ s390_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
internal_error (__FILE__, __LINE__, _("invalid regnum"));
}
+/* Implement pseudo_register_type tdesc method. */
+
static struct type *
s390_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
{
@@ -1181,6 +1206,8 @@ s390_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
internal_error (__FILE__, __LINE__, _("invalid regnum"));
}
+/* Implement pseudo_register_read gdbarch method. */
+
static enum register_status
s390_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
int regnum, gdb_byte *buf)
@@ -1255,6 +1282,8 @@ s390_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
internal_error (__FILE__, __LINE__, _("invalid regnum"));
}
+/* Implement pseudo_register_write gdbarch method. */
+
static void
s390_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
int regnum, const gdb_byte *buf)
@@ -1312,6 +1341,8 @@ s390_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
/* Register groups. */
+/* Implement pseudo_register_reggroup_p tdesc method. */
+
static int
s390_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
struct reggroup *group)
@@ -1435,12 +1466,18 @@ s390_gen_return_address (struct gdbarch *gdbarch,
/* Address handling. */
+/* Implement addr_bits_remove gdbarch method.
+ Only used for ABI_LINUX_S390. */
+
static CORE_ADDR
s390_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR addr)
{
return addr & 0x7fffffff;
}
+/* Implement addr_class_type_flags gdbarch method.
+ Only used for ABI_LINUX_ZSERIES. */
+
static int
s390_address_class_type_flags (int byte_size, int dwarf2_addr_class)
{
@@ -1450,6 +1487,9 @@ s390_address_class_type_flags (int byte_size, int dwarf2_addr_class)
return 0;
}
+/* Implement addr_class_type_flags_to_name gdbarch method.
+ Only used for ABI_LINUX_ZSERIES. */
+
static const char *
s390_address_class_type_flags_to_name (struct gdbarch *gdbarch, int type_flags)
{
@@ -1459,6 +1499,9 @@ s390_address_class_type_flags_to_name (struct gdbarch *gdbarch, int type_flags)
return NULL;
}
+/* Implement addr_class_name_to_type_flags gdbarch method.
+ Only used for ABI_LINUX_ZSERIES. */
+
static int
s390_address_class_name_to_type_flags (struct gdbarch *gdbarch,
const char *name,
@@ -1842,6 +1885,8 @@ s390_dummy_id (struct gdbarch *gdbarch, struct frame_info *this_frame)
get_frame_pc (this_frame));
}
+/* Implement frame_align gdbarch method. */
+
static CORE_ADDR
s390_frame_align (struct gdbarch *gdbarch, CORE_ADDR addr)
{
@@ -1970,7 +2015,7 @@ s390_stack_frame_destroyed_p (struct gdbarch *gdbarch, CORE_ADDR pc)
{
int word_size = gdbarch_ptr_bit (gdbarch) / 8;
- /* In frameless functions, there's not frame to destroy and thus
+ /* In frameless functions, there's no frame to destroy and thus
we don't care about the epilogue.
In functions with frame, the epilogue sequence is a pair of
@@ -2014,6 +2059,8 @@ s390_stack_frame_destroyed_p (struct gdbarch *gdbarch, CORE_ADDR pc)
return 0;
}
+/* Implement unwind_pc gdbarch method. */
+
static CORE_ADDR
s390_unwind_pc (struct gdbarch *gdbarch, struct frame_info *next_frame)
{
@@ -2023,6 +2070,8 @@ s390_unwind_pc (struct gdbarch *gdbarch, struct frame_info *next_frame)
return gdbarch_addr_bits_remove (gdbarch, pc);
}
+/* Implement unwind_sp gdbarch method. */
+
static CORE_ADDR
s390_unwind_sp (struct gdbarch *gdbarch, struct frame_info *next_frame)
{
@@ -2101,6 +2150,9 @@ s390_adjust_frame_regnum (struct gdbarch *gdbarch, int num, int eh_frame_p)
/* DWARF-2 frame unwinding. */
+/* Function to unwind a pseudo-register in dwarf2_frame unwinder. Used by
+ s390_dwarf2_frame_init_reg. */
+
static struct value *
s390_dwarf2_prev_register (struct frame_info *this_frame, void **this_cache,
int regnum)
@@ -2108,6 +2160,8 @@ s390_dwarf2_prev_register (struct frame_info *this_frame, void **this_cache,
return s390_unwind_pseudo_register (this_frame, regnum);
}
+/* Implement init_reg dwarf2_frame method. */
+
static void
s390_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
struct dwarf2_frame_state_reg *reg,
--
2.13.5
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 11/11] s390: Add comments to uncommented functions in s390-linux-tdep.c
2017-12-05 12:29 [PATCH v2 00/11] Split up s390-linux-tdep.c Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 10/11] s390: Add comments to uncommented functions in s390-tdep.c Philipp Rudo
@ 2017-12-05 12:29 ` Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 04/11] s390: gdbarch_tdep add field tdesc Philipp Rudo
` (7 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Philipp Rudo @ 2017-12-05 12:29 UTC (permalink / raw)
To: gdb-patches; +Cc: Andreas Arnez, Ulrich Weigand
Moving common functions from s390-linux-tdep to s390-tdep showed that some of
the left functions are lacking a description. Fix that now.
gdb/ChangeLog:
* s390-linux-tdep.c: Add comments to uncommented functions
---
gdb/s390-linux-tdep.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index 497f02303b..a815724e4d 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -63,6 +63,11 @@
#define XML_SYSCALL_FILENAME_S390 "syscalls/s390-linux.xml"
#define XML_SYSCALL_FILENAME_S390X "syscalls/s390x-linux.xml"
+
+/* Register handling. */
+
+/* Implement cannot_store_register gdbarch method. */
+
static int
s390_cannot_store_register (struct gdbarch *gdbarch, int regnum)
{
@@ -70,6 +75,8 @@ s390_cannot_store_register (struct gdbarch *gdbarch, int regnum)
return regnum == S390_LAST_BREAK_REGNUM;
}
+/* Implement write_pc gdbarch method. */
+
static void
s390_write_pc (struct regcache *regcache, CORE_ADDR pc)
{
@@ -356,6 +363,8 @@ s390_iterate_over_regset_sections (struct gdbarch *gdbarch,
}
}
+/* Implement core_read_description gdbarch method. */
+
static const struct target_desc *
s390_core_read_description (struct gdbarch *gdbarch,
struct target_ops *target, bfd *abfd)
@@ -403,6 +412,11 @@ s390_core_read_description (struct gdbarch *gdbarch,
}
}
+/* Frame unwinding. */
+
+/* Wrapper for trad_frame_get_prev_register to allow for s390 pseudo
+ register translation. */
+
static struct value *
s390_trad_frame_prev_register (struct frame_info *this_frame,
struct trad_frame_saved_reg saved_regs[],
@@ -425,6 +439,9 @@ struct s390_unwind_cache {
struct trad_frame_saved_reg *saved_regs;
};
+/* Unwind THIS_FRAME and write the information into unwind cache INFO using
+ prologue analysis. Helper for s390_frame_unwind_cache. */
+
static int
s390_prologue_frame_unwind_cache (struct frame_info *this_frame,
struct s390_unwind_cache *info)
@@ -609,6 +626,9 @@ s390_prologue_frame_unwind_cache (struct frame_info *this_frame,
return 1;
}
+/* Unwind THIS_FRAME and write the information into unwind cache INFO using
+ back chain unwinding. Helper for s390_frame_unwind_cache. */
+
static void
s390_backchain_frame_unwind_cache (struct frame_info *this_frame,
struct s390_unwind_cache *info)
@@ -663,6 +683,9 @@ s390_backchain_frame_unwind_cache (struct frame_info *this_frame,
info->func = get_frame_pc (this_frame);
}
+/* Unwind THIS_FRAME and return the corresponding unwind cache for
+ s390_frame_unwind and s390_frame_base. */
+
static struct s390_unwind_cache *
s390_frame_unwind_cache (struct frame_info *this_frame,
void **this_prologue_cache)
@@ -696,6 +719,8 @@ s390_frame_unwind_cache (struct frame_info *this_frame,
return info;
}
+/* Implement this_id frame_unwind method for s390_frame_unwind. */
+
static void
s390_frame_this_id (struct frame_info *this_frame,
void **this_prologue_cache,
@@ -714,6 +739,8 @@ s390_frame_this_id (struct frame_info *this_frame,
*this_id = frame_id_build (info->frame_base, info->func);
}
+/* Implement prev_register frame_unwind method for s390_frame_unwind. */
+
static struct value *
s390_frame_prev_register (struct frame_info *this_frame,
void **this_prologue_cache, int regnum)
@@ -724,6 +751,8 @@ s390_frame_prev_register (struct frame_info *this_frame,
return s390_trad_frame_prev_register (this_frame, info->saved_regs, regnum);
}
+/* Default S390 frame unwinder. */
+
static const struct frame_unwind s390_frame_unwind = {
NORMAL_FRAME,
default_frame_unwind_stop_reason,
@@ -743,6 +772,9 @@ struct s390_stub_unwind_cache
struct trad_frame_saved_reg *saved_regs;
};
+/* Unwind THIS_FRAME and return the corresponding unwind cache for
+ s390_stub_frame_unwind. */
+
static struct s390_stub_unwind_cache *
s390_stub_frame_unwind_cache (struct frame_info *this_frame,
void **this_prologue_cache)
@@ -769,6 +801,8 @@ s390_stub_frame_unwind_cache (struct frame_info *this_frame,
return info;
}
+/* Implement this_id frame_unwind method for s390_stub_frame_unwind. */
+
static void
s390_stub_frame_this_id (struct frame_info *this_frame,
void **this_prologue_cache,
@@ -779,6 +813,8 @@ s390_stub_frame_this_id (struct frame_info *this_frame,
*this_id = frame_id_build (info->frame_base, get_frame_pc (this_frame));
}
+/* Implement prev_register frame_unwind method for s390_stub_frame_unwind. */
+
static struct value *
s390_stub_frame_prev_register (struct frame_info *this_frame,
void **this_prologue_cache, int regnum)
@@ -788,6 +824,8 @@ s390_stub_frame_prev_register (struct frame_info *this_frame,
return s390_trad_frame_prev_register (this_frame, info->saved_regs, regnum);
}
+/* Implement sniffer frame_unwind method for s390_stub_frame_unwind. */
+
static int
s390_stub_frame_sniffer (const struct frame_unwind *self,
struct frame_info *this_frame,
@@ -806,6 +844,8 @@ s390_stub_frame_sniffer (const struct frame_unwind *self,
return 0;
}
+/* S390 stub frame unwinder. */
+
static const struct frame_unwind s390_stub_frame_unwind = {
NORMAL_FRAME,
default_frame_unwind_stop_reason,
@@ -822,6 +862,9 @@ struct s390_sigtramp_unwind_cache {
struct trad_frame_saved_reg *saved_regs;
};
+/* Unwind THIS_FRAME and return the corresponding unwind cache for
+ s390_sigtramp_frame_unwind. */
+
static struct s390_sigtramp_unwind_cache *
s390_sigtramp_frame_unwind_cache (struct frame_info *this_frame,
void **this_prologue_cache)
@@ -930,6 +973,8 @@ s390_sigtramp_frame_unwind_cache (struct frame_info *this_frame,
return info;
}
+/* Implement this_id frame_unwind method for s390_sigtramp_frame_unwind. */
+
static void
s390_sigtramp_frame_this_id (struct frame_info *this_frame,
void **this_prologue_cache,
@@ -940,6 +985,8 @@ s390_sigtramp_frame_this_id (struct frame_info *this_frame,
*this_id = frame_id_build (info->frame_base, get_frame_pc (this_frame));
}
+/* Implement prev_register frame_unwind method for sigtramp frames. */
+
static struct value *
s390_sigtramp_frame_prev_register (struct frame_info *this_frame,
void **this_prologue_cache, int regnum)
@@ -949,6 +996,8 @@ s390_sigtramp_frame_prev_register (struct frame_info *this_frame,
return s390_trad_frame_prev_register (this_frame, info->saved_regs, regnum);
}
+/* Implement sniffer frame_unwind method for sigtramp frames. */
+
static int
s390_sigtramp_frame_sniffer (const struct frame_unwind *self,
struct frame_info *this_frame,
@@ -970,6 +1019,8 @@ s390_sigtramp_frame_sniffer (const struct frame_unwind *self,
return 1;
}
+/* S390 sigtramp frame unwinder. */
+
static const struct frame_unwind s390_sigtramp_frame_unwind = {
SIGTRAMP_FRAME,
default_frame_unwind_stop_reason,
@@ -1077,6 +1128,9 @@ s390_all_but_pc_registers_record (struct regcache *regcache)
return 0;
}
+/* Canonicalize system call SYSCALL belonging to ABI. Helper for
+ s390_linux_syscall_record. */
+
static enum gdb_syscall
s390_canonicalize_syscall (int syscall, enum s390_abi_kind abi)
{
@@ -1245,6 +1299,9 @@ s390_canonicalize_syscall (int syscall, enum s390_abi_kind abi)
}
}
+/* Record a system call. Returns 0 on success, -1 otherwise.
+ Helper function for s390_process_record. */
+
static int
s390_linux_syscall_record (struct regcache *regcache, LONGEST syscall_native)
{
@@ -1293,6 +1350,8 @@ s390_linux_syscall_record (struct regcache *regcache, LONGEST syscall_native)
return 0;
}
+/* Implement process_record_signal gdbarch method. */
+
static int
s390_linux_record_signal (struct gdbarch *gdbarch, struct regcache *regcache,
enum gdb_signal signal)
@@ -1496,6 +1555,8 @@ s390_record_vr (struct gdbarch *gdbarch, struct regcache *regcache, int i)
return 0;
}
+/* Implement process_record gdbarch method. */
+
static int
s390_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
CORE_ADDR addr)
--
2.13.5
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 04/11] s390: gdbarch_tdep add field tdesc
2017-12-05 12:29 [PATCH v2 00/11] Split up s390-linux-tdep.c Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 10/11] s390: Add comments to uncommented functions in s390-tdep.c Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 11/11] s390: Add comments to uncommented functions in s390-linux-tdep.c Philipp Rudo
@ 2017-12-05 12:29 ` Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 02/11] s390: Allocate gdbarch & tdep at start of gdbarch init Philipp Rudo
` (6 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Philipp Rudo @ 2017-12-05 12:29 UTC (permalink / raw)
To: gdb-patches; +Cc: Andreas Arnez, Ulrich Weigand
Add a field for the target description to gdbarch_tdep. This will later be
needed to pass the 'correct' target description from osabi_init to
gdbarch_init. Unfortunately this cannot be done using gdbarch_info as it
is only passed by copy, not reference.
gdb/ChangeLog:
* s390-linux-tdep.c (gdbarch_tdep) <tdesc>: New field.
(s390_gdbarch_tdep_alloc): Adjust.
(s390_gdbarch_init): Adjust.
---
gdb/s390-linux-tdep.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index afee82304c..caef47a2a4 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -100,6 +100,9 @@ enum s390_vector_abi_kind
struct gdbarch_tdep
{
+ /* Target description. */
+ const struct target_desc *tdesc;
+
/* ABI version. */
enum s390_abi_kind abi;
@@ -7816,6 +7819,8 @@ s390_gdbarch_tdep_alloc ()
{
struct gdbarch_tdep *tdep = XCNEW (struct gdbarch_tdep);
+ tdep->tdesc = NULL;
+
tdep->abi = ABI_NONE;
tdep->vector_abi = S390_VECTOR_ABI_NONE;
@@ -7882,6 +7887,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
else
tdesc = tdesc_s390x_linux64;
}
+ tdep->tdesc = tdesc;
/* Check any target description for validity. */
if (tdesc_has_registers (tdesc))
@@ -8124,7 +8130,7 @@ 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, tdesc, tdesc_data);
+ tdesc_use_registers (gdbarch, tdep->tdesc, tdesc_data);
set_gdbarch_register_name (gdbarch, s390_register_name);
/* Assign pseudo register numbers. */
--
2.13.5
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 02/11] s390: Allocate gdbarch & tdep at start of gdbarch init
2017-12-05 12:29 [PATCH v2 00/11] Split up s390-linux-tdep.c Philipp Rudo
` (2 preceding siblings ...)
2017-12-05 12:29 ` [PATCH v2 04/11] s390: gdbarch_tdep add field tdesc Philipp Rudo
@ 2017-12-05 12:29 ` Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 09/11] s390: Clean up s390-linux-tdep.c Philipp Rudo
` (5 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Philipp Rudo @ 2017-12-05 12:29 UTC (permalink / raw)
To: gdb-patches; +Cc: Andreas Arnez, Ulrich Weigand
Moving the allocation of gdbarch_tdep to the start of s390_gdbarch_init
allows to use its fields to track the different features instead of using
separate variables. To make the code a little nicer move actual allocation
and initialization in a separate function. Also move the allocation of
gdbarch to keep the two together.
gdb/ChangeLog:
* s390-linux-tdep (s390_abi_kind) <ABI_NONE>: New default field.
(gdbarch_tdep) <have_upper, have_vx>: New fields.
(s390_gdbarch_tdep_alloc): New function.
(s390_gdbarch_init): Allocate tdep at start and use its fields
instead of separate variables.
---
gdb/s390-linux-tdep.c | 100 ++++++++++++++++++++++++++++++--------------------
1 file changed, 61 insertions(+), 39 deletions(-)
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index e3e036a70d..89e5ec55ba 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -85,6 +85,7 @@ static char *s390_disassembler_options;
enum s390_abi_kind
{
+ ABI_NONE,
ABI_LINUX_S390,
ABI_LINUX_ZSERIES
};
@@ -111,9 +112,11 @@ struct gdbarch_tdep
int cc_regnum;
int v0_full_regnum;
+ bool have_upper;
int have_linux_v1;
int have_linux_v2;
int have_tdb;
+ bool have_vx;
bool have_gs;
};
@@ -7805,6 +7808,32 @@ s390_init_linux_record_tdep (struct linux_record_tdep *record_tdep,
record_tdep->ioctl_FIOQSIZE = 0x545e;
}
+/* Allocate and initialize new gdbarch_tdep. Caller is responsible to free
+ memory after use. */
+
+static struct gdbarch_tdep *
+s390_gdbarch_tdep_alloc ()
+{
+ struct gdbarch_tdep *tdep = XCNEW (struct gdbarch_tdep);
+
+ tdep->abi = ABI_NONE;
+ tdep->vector_abi = S390_VECTOR_ABI_NONE;
+
+ tdep->gpr_full_regnum = -1;
+ tdep->v0_full_regnum = -1;
+ tdep->pc_regnum = -1;
+ tdep->cc_regnum = -1;
+
+ tdep->have_upper = false;
+ tdep->have_linux_v1 = 0;
+ tdep->have_linux_v2 = 0;
+ tdep->have_tdb = 0;
+ tdep->have_vx = false;
+ tdep->have_gs = false;
+
+ return tdep;
+}
+
/* Set up gdbarch struct. */
static struct gdbarch *
@@ -7812,16 +7841,6 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
{
const struct target_desc *tdesc = info.target_desc;
struct tdesc_arch_data *tdesc_data = NULL;
- struct gdbarch *gdbarch;
- struct gdbarch_tdep *tdep;
- enum s390_abi_kind tdep_abi;
- enum s390_vector_abi_kind vector_abi;
- int have_upper = 0;
- int have_linux_v1 = 0;
- int have_linux_v2 = 0;
- int have_tdb = 0;
- int have_vx = 0;
- int have_gs = 0;
int first_pseudo_reg, last_pseudo_reg;
static const char *const stap_register_prefixes[] = { "%", NULL };
static const char *const stap_register_indirection_prefixes[] = { "(",
@@ -7834,25 +7853,31 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
if (arches != NULL)
return arches->gdbarch;
+ /* 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);
+
/* Default ABI and register size. */
switch (info.bfd_arch_info->mach)
{
case bfd_mach_s390_31:
- tdep_abi = ABI_LINUX_S390;
+ tdep->abi = ABI_LINUX_S390;
break;
case bfd_mach_s390_64:
- tdep_abi = ABI_LINUX_ZSERIES;
+ tdep->abi = ABI_LINUX_ZSERIES;
break;
default:
+ xfree (tdep);
+ gdbarch_free (gdbarch);
return NULL;
}
/* Use default target description if none provided by the target. */
if (!tdesc_has_registers (tdesc))
{
- if (tdep_abi == ABI_LINUX_S390)
+ if (tdep->abi == ABI_LINUX_S390)
tdesc = tdesc_s390_linux32;
else
tdesc = tdesc_s390x_linux64;
@@ -7905,7 +7930,11 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.core");
if (feature == NULL)
- return NULL;
+ {
+ xfree (tdep);
+ gdbarch_free (gdbarch);
+ return NULL;
+ }
tdesc_data = tdesc_data_alloc ();
@@ -7922,7 +7951,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
}
else
{
- have_upper = 1;
+ tdep->have_upper = true;
for (i = 0; i < 16; i++)
valid_p &= tdesc_numbered_register (feature, tdesc_data,
@@ -7938,6 +7967,8 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
if (feature == NULL)
{
tdesc_data_cleanup (tdesc_data);
+ xfree (tdep);
+ gdbarch_free (gdbarch);
return NULL;
}
@@ -7951,6 +7982,8 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
if (feature == NULL)
{
tdesc_data_cleanup (tdesc_data);
+ xfree (tdep);
+ gdbarch_free (gdbarch);
return NULL;
}
@@ -7967,13 +8000,13 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
if (tdesc_numbered_register (feature, tdesc_data,
S390_LAST_BREAK_REGNUM, "last_break"))
- have_linux_v1 = 1;
+ tdep->have_linux_v1 = 1;
if (tdesc_numbered_register (feature, tdesc_data,
S390_SYSTEM_CALL_REGNUM, "system_call"))
- have_linux_v2 = 1;
+ tdep->have_linux_v2 = 1;
- if (have_linux_v2 > have_linux_v1)
+ if (tdep->have_linux_v2 > tdep->have_linux_v1)
valid_p = 0;
}
@@ -7985,7 +8018,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
valid_p &= tdesc_numbered_register (feature, tdesc_data,
S390_TDB_DWORD0_REGNUM + i,
tdb_regs[i]);
- have_tdb = 1;
+ tdep->have_tdb = 1;
}
/* Vector registers. */
@@ -8000,7 +8033,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
valid_p &= tdesc_numbered_register (feature, tdesc_data,
S390_V16_REGNUM + i,
vxrs_high[i]);
- have_vx = 1;
+ tdep->have_vx = true;
}
/* Guarded-storage registers. */
@@ -8011,14 +8044,14 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
valid_p &= tdesc_numbered_register (feature, tdesc_data,
S390_GSD_REGNUM + i,
gs_cb[i]);
- have_gs = 1;
+ tdep->have_gs = true;
}
/* Guarded-storage broadcast control. */
feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.gsbc");
if (feature)
{
- valid_p &= have_gs;
+ valid_p &= tdep->have_gs;
for (i = 0; i < 3; i++)
valid_p &= tdesc_numbered_register (feature, tdesc_data,
@@ -8029,32 +8062,23 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
if (!valid_p)
{
tdesc_data_cleanup (tdesc_data);
+ xfree (tdep);
+ gdbarch_free (gdbarch);
return NULL;
}
}
/* Determine vector ABI. */
- vector_abi = S390_VECTOR_ABI_NONE;
#ifdef HAVE_ELF
- if (have_vx
+ 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)
- vector_abi = S390_VECTOR_ABI_128;
+ tdep->vector_abi = S390_VECTOR_ABI_128;
#endif
- /* Otherwise create a new gdbarch for the specified machine type. */
- tdep = XCNEW (struct gdbarch_tdep);
- tdep->abi = tdep_abi;
- tdep->vector_abi = vector_abi;
- tdep->have_linux_v1 = have_linux_v1;
- tdep->have_linux_v2 = have_linux_v2;
- tdep->have_tdb = have_tdb;
- tdep->have_gs = have_gs;
- gdbarch = gdbarch_alloc (&info, tdep);
-
set_gdbarch_believe_pcc_promotion (gdbarch, 0);
set_gdbarch_char_signed (gdbarch, 0);
@@ -8106,14 +8130,12 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
/* Assign pseudo register numbers. */
first_pseudo_reg = gdbarch_num_regs (gdbarch);
last_pseudo_reg = first_pseudo_reg;
- tdep->gpr_full_regnum = -1;
- if (have_upper)
+ if (tdep->have_upper)
{
tdep->gpr_full_regnum = last_pseudo_reg;
last_pseudo_reg += 16;
}
- tdep->v0_full_regnum = -1;
- if (have_vx)
+ if (tdep->have_vx)
{
tdep->v0_full_regnum = last_pseudo_reg;
last_pseudo_reg += 16;
--
2.13.5
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 09/11] s390: Clean up s390-linux-tdep.c
2017-12-05 12:29 [PATCH v2 00/11] Split up s390-linux-tdep.c Philipp Rudo
` (3 preceding siblings ...)
2017-12-05 12:29 ` [PATCH v2 02/11] s390: Allocate gdbarch & tdep at start of gdbarch init Philipp Rudo
@ 2017-12-05 12:29 ` Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 05/11] s390: Move tdesc validation to separate function Philipp Rudo
` (4 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Philipp Rudo @ 2017-12-05 12:29 UTC (permalink / raw)
To: gdb-patches; +Cc: Andreas Arnez, Ulrich Weigand
After moving big parts of the code to the new s390-tdep.c file
s390-linux-tdep.c now contains many includes it doesn't need any more.
Further more the code for record-replay is disrupted by some code that does
not belong to the feature.
So clean up the include list and move the code disrupting record-replay to
places where it makes more sense. Also remove some superfluous newlines.
gdb/ChangeLog
* s390-linux-tdep.c: Reorder code.
Remove unneeded includes and order alphabetically.
---
gdb/s390-linux-tdep.c | 167 ++++++++++++++++++++++----------------------------
1 file changed, 73 insertions(+), 94 deletions(-)
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index c0283db3b4..497f02303b 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -21,46 +21,27 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include "defs.h"
-#include "arch-utils.h"
-#include "frame.h"
-#include "inferior.h"
-#include "infrun.h"
-#include "symtab.h"
-#include "target.h"
+
+#include "auxv.h"
+#include "elf/common.h"
+#include "frame-base.h"
+#include "frame-unwind.h"
+#include "gdbarch.h"
#include "gdbcore.h"
-#include "gdbcmd.h"
+#include "linux-record.h"
+#include "linux-tdep.h"
#include "objfiles.h"
#include "osabi.h"
#include "regcache.h"
-#include "trad-frame.h"
-#include "frame-base.h"
-#include "frame-unwind.h"
-#include "dwarf2-frame.h"
-#include "reggroups.h"
+#include "record-full.h"
#include "regset.h"
-#include "value.h"
-#include "dis-asm.h"
-#include "solib-svr4.h"
-#include "prologue-value.h"
-#include "linux-tdep.h"
#include "s390-tdep.h"
#include "s390-linux-tdep.h"
-#include "linux-record.h"
-#include "record-full.h"
-#include "auxv.h"
+#include "solib-svr4.h"
+#include "target.h"
+#include "trad-frame.h"
#include "xml-syscall.h"
-#include "stap-probe.h"
-#include "ax.h"
-#include "ax-gdb.h"
-#include "user-regs.h"
-#include "cli/cli-utils.h"
-#include <ctype.h>
-#include "elf/common.h"
-#include "elf/s390.h"
-#include "elf-bfd.h"
-#include <algorithm>
-
#include "features/s390-linux32.c"
#include "features/s390-linux32v1.c"
#include "features/s390-linux32v2.c"
@@ -433,7 +414,6 @@ s390_trad_frame_prev_register (struct frame_info *this_frame,
return s390_unwind_pseudo_register (this_frame, regnum);
}
-
/* Normal stack frames. */
struct s390_unwind_cache {
@@ -529,7 +509,6 @@ s390_prologue_frame_unwind_cache (struct frame_info *this_frame,
}
}
-
/* OK, we've found valid prologue data. */
size = -sp->k;
@@ -754,7 +733,6 @@ static const struct frame_unwind s390_frame_unwind = {
default_frame_sniffer
};
-
/* Code stubs and their stack frames. For things like PLTs and NULL
function calls (where there is no true frame and the return address
is in the RETADDR register). */
@@ -837,7 +815,6 @@ static const struct frame_unwind s390_stub_frame_unwind = {
s390_stub_frame_sniffer
};
-
/* Signal trampoline stack frames. */
struct s390_sigtramp_unwind_cache {
@@ -1002,6 +979,33 @@ static const struct frame_unwind s390_sigtramp_frame_unwind = {
s390_sigtramp_frame_sniffer
};
+/* Frame base handling. */
+
+static CORE_ADDR
+s390_frame_base_address (struct frame_info *this_frame, void **this_cache)
+{
+ struct s390_unwind_cache *info
+ = s390_frame_unwind_cache (this_frame, this_cache);
+ return info->frame_base;
+}
+
+static CORE_ADDR
+s390_local_base_address (struct frame_info *this_frame, void **this_cache)
+{
+ struct s390_unwind_cache *info
+ = s390_frame_unwind_cache (this_frame, this_cache);
+ return info->local_base;
+}
+
+static const struct frame_base s390_frame_base = {
+ &s390_frame_unwind,
+ s390_frame_base_address,
+ s390_local_base_address,
+ s390_local_base_address
+};
+
+/* Syscall handling. */
+
/* Retrieve the syscall number at a ptrace syscall-stop. Return -1
upon error. */
@@ -1328,65 +1332,6 @@ s390_linux_record_signal (struct gdbarch *gdbarch, struct regcache *regcache,
return 0;
}
-/* Frame base handling. */
-
-static CORE_ADDR
-s390_frame_base_address (struct frame_info *this_frame, void **this_cache)
-{
- struct s390_unwind_cache *info
- = s390_frame_unwind_cache (this_frame, this_cache);
- return info->frame_base;
-}
-
-static CORE_ADDR
-s390_local_base_address (struct frame_info *this_frame, void **this_cache)
-{
- struct s390_unwind_cache *info
- = s390_frame_unwind_cache (this_frame, this_cache);
- return info->local_base;
-}
-
-static const struct frame_base s390_frame_base = {
- &s390_frame_unwind,
- s390_frame_base_address,
- s390_local_base_address,
- s390_local_base_address
-};
-
-/* Implement gdbarch_gcc_target_options. GCC does not know "-m32" or
- "-mcmodel=large". */
-
-static char *
-s390_gcc_target_options (struct gdbarch *gdbarch)
-{
- return xstrdup (gdbarch_ptr_bit (gdbarch) == 64 ? "-m64" : "-m31");
-}
-
-/* Implement gdbarch_gnu_triplet_regexp. Target triplets are "s390-*"
- for 31-bit and "s390x-*" for 64-bit, while the BFD arch name is
- always "s390". Note that an s390x compiler supports "-m31" as
- well. */
-
-static const char *
-s390_gnu_triplet_regexp (struct gdbarch *gdbarch)
-{
- return "s390x?";
-}
-
-/* Implementation of `gdbarch_stap_is_single_operand', as defined in
- gdbarch.h. */
-
-static int
-s390_stap_is_single_operand (struct gdbarch *gdbarch, const char *s)
-{
- return ((isdigit (*s) && s[1] == '(' && s[2] == '%') /* Displacement
- or indirection. */
- || *s == '%' /* Register access. */
- || isdigit (*s)); /* Literal number. */
-}
-
-/* Process record and replay helpers. */
-
/* Takes the intermediate sum of address calculations and masks off upper
bits according to current addressing mode. */
@@ -5558,6 +5503,40 @@ s390_init_linux_record_tdep (struct linux_record_tdep *record_tdep,
record_tdep->ioctl_FIOQSIZE = 0x545e;
}
+/* Miscellaneous. */
+
+/* Implement gdbarch_gcc_target_options. GCC does not know "-m32" or
+ "-mcmodel=large". */
+
+static char *
+s390_gcc_target_options (struct gdbarch *gdbarch)
+{
+ return xstrdup (gdbarch_ptr_bit (gdbarch) == 64 ? "-m64" : "-m31");
+}
+
+/* Implement gdbarch_gnu_triplet_regexp. Target triplets are "s390-*"
+ for 31-bit and "s390x-*" for 64-bit, while the BFD arch name is
+ always "s390". Note that an s390x compiler supports "-m31" as
+ well. */
+
+static const char *
+s390_gnu_triplet_regexp (struct gdbarch *gdbarch)
+{
+ return "s390x?";
+}
+
+/* Implementation of `gdbarch_stap_is_single_operand', as defined in
+ gdbarch.h. */
+
+static int
+s390_stap_is_single_operand (struct gdbarch *gdbarch, const char *s)
+{
+ return ((isdigit (*s) && s[1] == '(' && s[2] == '%') /* Displacement
+ or indirection. */
+ || *s == '%' /* Register access. */
+ || isdigit (*s)); /* Literal number. */
+}
+
/* Initialize OSABI common for GNU/Linux on 31- and 64-bit systems. */
static void
--
2.13.5
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 05/11] s390: Move tdesc validation to separate function
2017-12-05 12:29 [PATCH v2 00/11] Split up s390-linux-tdep.c Philipp Rudo
` (4 preceding siblings ...)
2017-12-05 12:29 ` [PATCH v2 09/11] s390: Clean up s390-linux-tdep.c Philipp Rudo
@ 2017-12-05 12:29 ` Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 06/11] s390: if -> gdb_assert for tdesc_has_registers check Philipp Rudo
` (3 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Philipp Rudo @ 2017-12-05 12:29 UTC (permalink / raw)
To: gdb-patches; +Cc: Andreas Arnez, Ulrich Weigand
Simplify s390_gdbarch by moving the target description validation to a
separate function.
gdb/ChangeLog:
* s390-linux-tdep.c (s390_tdesc_valid): New Function.
(s390_validate_reg_range): New macro.
(s390_gdbarch_init): Adjust.
---
gdb/s390-linux-tdep.c | 339 ++++++++++++++++++++++++--------------------------
1 file changed, 164 insertions(+), 175 deletions(-)
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index caef47a2a4..f631c60f09 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -7811,6 +7811,167 @@ s390_init_linux_record_tdep (struct linux_record_tdep *record_tdep,
record_tdep->ioctl_FIOQSIZE = 0x545e;
}
+/* Validate the range of registers. NAMES must be known at compile time. */
+
+#define s390_validate_reg_range(feature, tdesc_data, start, names) \
+do \
+{ \
+ for (int i = 0; i < ARRAY_SIZE (names); i++) \
+ if (!tdesc_numbered_register (feature, tdesc_data, start + i, names[i])) \
+ return false; \
+} \
+while (0)
+
+/* Validate the target description. Also numbers registers contained in
+ tdesc. */
+
+static bool
+s390_tdesc_valid (struct gdbarch_tdep *tdep,
+ struct tdesc_arch_data *tdesc_data)
+{
+ static const char *const psw[] = {
+ "pswm", "pswa"
+ };
+ static const char *const gprs[] = {
+ "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
+ "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15"
+ };
+ static const char *const fprs[] = {
+ "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",
+ "f8", "f9", "f10", "f11", "f12", "f13", "f14", "f15"
+ };
+ static const char *const acrs[] = {
+ "acr0", "acr1", "acr2", "acr3", "acr4", "acr5", "acr6", "acr7",
+ "acr8", "acr9", "acr10", "acr11", "acr12", "acr13", "acr14", "acr15"
+ };
+ static const char *const gprs_lower[] = {
+ "r0l", "r1l", "r2l", "r3l", "r4l", "r5l", "r6l", "r7l",
+ "r8l", "r9l", "r10l", "r11l", "r12l", "r13l", "r14l", "r15l"
+ };
+ static const char *const gprs_upper[] = {
+ "r0h", "r1h", "r2h", "r3h", "r4h", "r5h", "r6h", "r7h",
+ "r8h", "r9h", "r10h", "r11h", "r12h", "r13h", "r14h", "r15h"
+ };
+ static const char *const tdb_regs[] = {
+ "tdb0", "tac", "tct", "atia",
+ "tr0", "tr1", "tr2", "tr3", "tr4", "tr5", "tr6", "tr7",
+ "tr8", "tr9", "tr10", "tr11", "tr12", "tr13", "tr14", "tr15"
+ };
+ static const char *const vxrs_low[] = {
+ "v0l", "v1l", "v2l", "v3l", "v4l", "v5l", "v6l", "v7l", "v8l",
+ "v9l", "v10l", "v11l", "v12l", "v13l", "v14l", "v15l",
+ };
+ static const char *const vxrs_high[] = {
+ "v16", "v17", "v18", "v19", "v20", "v21", "v22", "v23", "v24",
+ "v25", "v26", "v27", "v28", "v29", "v30", "v31",
+ };
+ static const char *const gs_cb[] = {
+ "gsd", "gssm", "gsepla",
+ };
+ static const char *const gs_bc[] = {
+ "bc_gsd", "bc_gssm", "bc_gsepla",
+ };
+
+ const struct target_desc *tdesc = tdep->tdesc;
+ const struct tdesc_feature *feature;
+
+ /* Core registers, i.e. general purpose and PSW. */
+ feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.core");
+ if (feature == NULL)
+ return false;
+
+ s390_validate_reg_range (feature, tdesc_data, S390_PSWM_REGNUM, psw);
+
+ if (tdesc_unnumbered_register (feature, "r0"))
+ {
+ s390_validate_reg_range (feature, tdesc_data, S390_R0_REGNUM, gprs);
+ }
+ else
+ {
+ tdep->have_upper = true;
+ s390_validate_reg_range (feature, tdesc_data, S390_R0_REGNUM,
+ gprs_lower);
+ s390_validate_reg_range (feature, tdesc_data, S390_R0_UPPER_REGNUM,
+ gprs_upper);
+ }
+
+ /* Floating point registers. */
+ feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.fpr");
+ if (feature == NULL)
+ return false;
+
+ if (!tdesc_numbered_register (feature, tdesc_data, S390_FPC_REGNUM, "fpc"))
+ return false;
+
+ s390_validate_reg_range (feature, tdesc_data, S390_F0_REGNUM, fprs);
+
+ /* Access control registers. */
+ feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.acr");
+ if (feature == NULL)
+ return false;
+
+ s390_validate_reg_range (feature, tdesc_data, S390_A0_REGNUM, acrs);
+
+ /* Optional GNU/Linux-specific "registers". */
+ feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.linux");
+ if (feature)
+ {
+ tdesc_numbered_register (feature, tdesc_data,
+ S390_ORIG_R2_REGNUM, "orig_r2");
+
+ if (tdesc_numbered_register (feature, tdesc_data,
+ S390_LAST_BREAK_REGNUM, "last_break"))
+ tdep->have_linux_v1 = true;
+
+ if (tdesc_numbered_register (feature, tdesc_data,
+ S390_SYSTEM_CALL_REGNUM, "system_call"))
+ tdep->have_linux_v2 = true;
+
+ if (tdep->have_linux_v2 && !tdep->have_linux_v1)
+ return false;
+ }
+
+ /* Transaction diagnostic block. */
+ feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.tdb");
+ if (feature)
+ {
+ s390_validate_reg_range (feature, tdesc_data, S390_TDB_DWORD0_REGNUM,
+ tdb_regs);
+ tdep->have_tdb = true;
+ }
+
+ /* Vector registers. */
+ feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.vx");
+ if (feature)
+ {
+ s390_validate_reg_range (feature, tdesc_data, S390_V0_LOWER_REGNUM,
+ vxrs_low);
+ s390_validate_reg_range (feature, tdesc_data, S390_V16_REGNUM,
+ vxrs_high);
+ tdep->have_vx = true;
+ }
+
+ /* Guarded-storage registers. */
+ feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.gs");
+ if (feature)
+ {
+ s390_validate_reg_range (feature, tdesc_data, S390_GSD_REGNUM, gs_cb);
+ tdep->have_gs = true;
+ }
+
+ /* Guarded-storage broadcast control. */
+ feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.gsbc");
+ if (feature)
+ {
+ if (!tdep->have_gs)
+ return false;
+ s390_validate_reg_range (feature, tdesc_data, S390_BC_GSD_REGNUM,
+ gs_bc);
+ }
+
+ return true;
+}
+
/* Allocate and initialize new gdbarch_tdep. Caller is responsible to free
memory after use. */
@@ -7845,7 +8006,6 @@ static struct gdbarch *
s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
{
const struct target_desc *tdesc = info.target_desc;
- struct tdesc_arch_data *tdesc_data = NULL;
int first_pseudo_reg, last_pseudo_reg;
static const char *const stap_register_prefixes[] = { "%", NULL };
static const char *const stap_register_indirection_prefixes[] = { "(",
@@ -7861,6 +8021,8 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
/* 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 ();
+ info.tdesc_data = tdesc_data;
/* Default ABI and register size. */
switch (info.bfd_arch_info->mach)
@@ -7892,180 +8054,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
/* Check any target description for validity. */
if (tdesc_has_registers (tdesc))
{
- static const char *const gprs[] = {
- "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
- "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15"
- };
- static const char *const fprs[] = {
- "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",
- "f8", "f9", "f10", "f11", "f12", "f13", "f14", "f15"
- };
- static const char *const acrs[] = {
- "acr0", "acr1", "acr2", "acr3", "acr4", "acr5", "acr6", "acr7",
- "acr8", "acr9", "acr10", "acr11", "acr12", "acr13", "acr14", "acr15"
- };
- static const char *const gprs_lower[] = {
- "r0l", "r1l", "r2l", "r3l", "r4l", "r5l", "r6l", "r7l",
- "r8l", "r9l", "r10l", "r11l", "r12l", "r13l", "r14l", "r15l"
- };
- static const char *const gprs_upper[] = {
- "r0h", "r1h", "r2h", "r3h", "r4h", "r5h", "r6h", "r7h",
- "r8h", "r9h", "r10h", "r11h", "r12h", "r13h", "r14h", "r15h"
- };
- static const char *const tdb_regs[] = {
- "tdb0", "tac", "tct", "atia",
- "tr0", "tr1", "tr2", "tr3", "tr4", "tr5", "tr6", "tr7",
- "tr8", "tr9", "tr10", "tr11", "tr12", "tr13", "tr14", "tr15"
- };
- static const char *const vxrs_low[] = {
- "v0l", "v1l", "v2l", "v3l", "v4l", "v5l", "v6l", "v7l", "v8l",
- "v9l", "v10l", "v11l", "v12l", "v13l", "v14l", "v15l",
- };
- static const char *const vxrs_high[] = {
- "v16", "v17", "v18", "v19", "v20", "v21", "v22", "v23", "v24",
- "v25", "v26", "v27", "v28", "v29", "v30", "v31",
- };
- static const char *const gs_cb[] = {
- "gsd", "gssm", "gsepla",
- };
- static const char *const gs_bc[] = {
- "bc_gsd", "bc_gssm", "bc_gsepla",
- };
- const struct tdesc_feature *feature;
- int i, valid_p = 1;
-
- feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.core");
- if (feature == NULL)
- {
- xfree (tdep);
- gdbarch_free (gdbarch);
- return NULL;
- }
-
- tdesc_data = tdesc_data_alloc ();
-
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- S390_PSWM_REGNUM, "pswm");
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- S390_PSWA_REGNUM, "pswa");
-
- if (tdesc_unnumbered_register (feature, "r0"))
- {
- for (i = 0; i < 16; i++)
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- S390_R0_REGNUM + i, gprs[i]);
- }
- else
- {
- tdep->have_upper = true;
-
- for (i = 0; i < 16; i++)
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- S390_R0_REGNUM + i,
- gprs_lower[i]);
- for (i = 0; i < 16; i++)
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- S390_R0_UPPER_REGNUM + i,
- gprs_upper[i]);
- }
-
- feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.fpr");
- if (feature == NULL)
- {
- tdesc_data_cleanup (tdesc_data);
- xfree (tdep);
- gdbarch_free (gdbarch);
- return NULL;
- }
-
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- S390_FPC_REGNUM, "fpc");
- for (i = 0; i < 16; i++)
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- S390_F0_REGNUM + i, fprs[i]);
-
- feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.acr");
- if (feature == NULL)
- {
- tdesc_data_cleanup (tdesc_data);
- xfree (tdep);
- gdbarch_free (gdbarch);
- return NULL;
- }
-
- for (i = 0; i < 16; i++)
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- S390_A0_REGNUM + i, acrs[i]);
-
- /* Optional GNU/Linux-specific "registers". */
- feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.linux");
- if (feature)
- {
- tdesc_numbered_register (feature, tdesc_data,
- S390_ORIG_R2_REGNUM, "orig_r2");
-
- if (tdesc_numbered_register (feature, tdesc_data,
- S390_LAST_BREAK_REGNUM, "last_break"))
- tdep->have_linux_v1 = true;
-
- if (tdesc_numbered_register (feature, tdesc_data,
- S390_SYSTEM_CALL_REGNUM, "system_call"))
- tdep->have_linux_v2 = true;
-
- if (tdep->have_linux_v2 && !tdep->have_linux_v1)
- valid_p = 0;
- }
-
- /* Transaction diagnostic block. */
- feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.tdb");
- if (feature)
- {
- for (i = 0; i < ARRAY_SIZE (tdb_regs); i++)
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- S390_TDB_DWORD0_REGNUM + i,
- tdb_regs[i]);
- tdep->have_tdb = true;
- }
-
- /* Vector registers. */
- feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.vx");
- if (feature)
- {
- for (i = 0; i < 16; i++)
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- S390_V0_LOWER_REGNUM + i,
- vxrs_low[i]);
- for (i = 0; i < 16; i++)
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- S390_V16_REGNUM + i,
- vxrs_high[i]);
- tdep->have_vx = true;
- }
-
- /* Guarded-storage registers. */
- feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.gs");
- if (feature)
- {
- for (i = 0; i < 3; i++)
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- S390_GSD_REGNUM + i,
- gs_cb[i]);
- tdep->have_gs = true;
- }
-
- /* Guarded-storage broadcast control. */
- feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.gsbc");
- if (feature)
- {
- valid_p &= tdep->have_gs;
-
- for (i = 0; i < 3; i++)
- valid_p &= tdesc_numbered_register (feature, tdesc_data,
- S390_BC_GSD_REGNUM + i,
- gs_bc[i]);
- }
-
- if (!valid_p)
+ if (!s390_tdesc_valid (tdep, tdesc_data))
{
tdesc_data_cleanup (tdesc_data);
xfree (tdep);
--
2.13.5
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 06/11] s390: if -> gdb_assert for tdesc_has_registers check
2017-12-05 12:29 [PATCH v2 00/11] Split up s390-linux-tdep.c Philipp Rudo
` (5 preceding siblings ...)
2017-12-05 12:29 ` [PATCH v2 05/11] s390: Move tdesc validation to separate function Philipp Rudo
@ 2017-12-05 12:29 ` Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 07/11] s390: Hook s390 into OSABI mechanism Philipp Rudo
` (2 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Philipp Rudo @ 2017-12-05 12:29 UTC (permalink / raw)
To: gdb-patches; +Cc: Andreas Arnez, Ulrich Weigand
Before doing the tdesc validation there is a check whether the tdesc has
registers or not. This check is not only unnecessary but wrong.
First the check is done after a default tdesc is assigned if the original
tdesc has no registers. These default tdescs always have registers so the
check alway returns true.
Second if the default tdesc would not have registers the check only skips
the tdesc validation instead of returning an error. This would trigger a
gdb_assert later on in tdesc_use_registers.
gdb/ChangeLog:
* s390-linux-tdep.c (s390_gdbarch_init): Use gdb_assert for
tdesc_has_registers check
---
gdb/s390-linux-tdep.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index f631c60f09..f6a39868be 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -8052,15 +8052,13 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
tdep->tdesc = tdesc;
/* Check any target description for validity. */
- if (tdesc_has_registers (tdesc))
+ gdb_assert (tdesc_has_registers (tdep->tdesc));
+ if (!s390_tdesc_valid (tdep, tdesc_data))
{
- if (!s390_tdesc_valid (tdep, tdesc_data))
- {
- tdesc_data_cleanup (tdesc_data);
- xfree (tdep);
- gdbarch_free (gdbarch);
- return NULL;
- }
+ tdesc_data_cleanup (tdesc_data);
+ xfree (tdep);
+ gdbarch_free (gdbarch);
+ return NULL;
}
/* Determine vector ABI. */
--
2.13.5
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 07/11] s390: Hook s390 into OSABI mechanism
2017-12-05 12:29 [PATCH v2 00/11] Split up s390-linux-tdep.c Philipp Rudo
` (6 preceding siblings ...)
2017-12-05 12:29 ` [PATCH v2 06/11] s390: if -> gdb_assert for tdesc_has_registers check Philipp Rudo
@ 2017-12-05 12:29 ` Philipp Rudo
2017-12-05 13:21 ` Ulrich Weigand
2017-12-05 12:29 ` [PATCH v2 03/11] s390: gdbarch_tdep.have_* int -> bool Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 01/11] s390: Remove duplicate checks for cached gdbarch at init Philipp Rudo
9 siblings, 1 reply; 20+ messages in thread
From: Philipp Rudo @ 2017-12-05 12:29 UTC (permalink / raw)
To: gdb-patches; +Cc: Andreas Arnez, Ulrich Weigand
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(-)
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 ();
--
2.13.5
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 07/11] s390: Hook s390 into OSABI mechanism
2017-12-05 12:29 ` [PATCH v2 07/11] s390: Hook s390 into OSABI mechanism Philipp Rudo
@ 2017-12-05 13:21 ` Ulrich Weigand
2017-12-05 17:06 ` Philipp Rudo
0 siblings, 1 reply; 20+ messages in thread
From: Ulrich Weigand @ 2017-12-05 13:21 UTC (permalink / raw)
To: Philipp Rudo; +Cc: gdb-patches, Andreas Arnez
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.
> + /* 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
> 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!).
> + /* 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?
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 07/11] s390: Hook s390 into OSABI mechanism
2017-12-05 13:21 ` Ulrich Weigand
@ 2017-12-05 17:06 ` Philipp Rudo
2017-12-05 17:44 ` Ulrich Weigand
0 siblings, 1 reply; 20+ messages in thread
From: Philipp Rudo @ 2017-12-05 17:06 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches, Andreas Arnez
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 07/11] s390: Hook s390 into OSABI mechanism
2017-12-05 17:06 ` Philipp Rudo
@ 2017-12-05 17:44 ` Ulrich Weigand
2017-12-06 11:39 ` Philipp Rudo
0 siblings, 1 reply; 20+ messages in thread
From: Ulrich Weigand @ 2017-12-05 17:44 UTC (permalink / raw)
To: Philipp Rudo; +Cc: gdb-patches, Andreas Arnez
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 ...).
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.
- 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.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 07/11] s390: Hook s390 into OSABI mechanism
2017-12-05 17:44 ` Ulrich Weigand
@ 2017-12-06 11:39 ` Philipp Rudo
0 siblings, 0 replies; 20+ messages in thread
From: Philipp Rudo @ 2017-12-06 11:39 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches, Andreas Arnez
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 03/11] s390: gdbarch_tdep.have_* int -> bool
2017-12-05 12:29 [PATCH v2 00/11] Split up s390-linux-tdep.c Philipp Rudo
` (7 preceding siblings ...)
2017-12-05 12:29 ` [PATCH v2 07/11] s390: Hook s390 into OSABI mechanism Philipp Rudo
@ 2017-12-05 12:29 ` Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 01/11] s390: Remove duplicate checks for cached gdbarch at init Philipp Rudo
9 siblings, 0 replies; 20+ messages in thread
From: Philipp Rudo @ 2017-12-05 12:29 UTC (permalink / raw)
To: gdb-patches; +Cc: Andreas Arnez, Ulrich Weigand
Currently the gdbarch_tdep.have_* flags are a mix of int and bool. Clean
this up by making them all bool.
gdb/ChangeLog:
* s390-linux-tdep.c (gdbarch_tdep) <have_linux_v1, have_linux_v2>
<have_tdb>: Change type to bool.
(s390_gdbarch_tdep_alloc): Adjust.
(s390_gdbarch_init): Adjust.
---
gdb/s390-linux-tdep.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index 89e5ec55ba..afee82304c 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -113,9 +113,9 @@ struct gdbarch_tdep
int v0_full_regnum;
bool have_upper;
- int have_linux_v1;
- int have_linux_v2;
- int have_tdb;
+ bool have_linux_v1;
+ bool have_linux_v2;
+ bool have_tdb;
bool have_vx;
bool have_gs;
};
@@ -7825,9 +7825,9 @@ s390_gdbarch_tdep_alloc ()
tdep->cc_regnum = -1;
tdep->have_upper = false;
- tdep->have_linux_v1 = 0;
- tdep->have_linux_v2 = 0;
- tdep->have_tdb = 0;
+ tdep->have_linux_v1 = false;
+ tdep->have_linux_v2 = false;
+ tdep->have_tdb = false;
tdep->have_vx = false;
tdep->have_gs = false;
@@ -8000,13 +8000,13 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
if (tdesc_numbered_register (feature, tdesc_data,
S390_LAST_BREAK_REGNUM, "last_break"))
- tdep->have_linux_v1 = 1;
+ tdep->have_linux_v1 = true;
if (tdesc_numbered_register (feature, tdesc_data,
S390_SYSTEM_CALL_REGNUM, "system_call"))
- tdep->have_linux_v2 = 1;
+ tdep->have_linux_v2 = true;
- if (tdep->have_linux_v2 > tdep->have_linux_v1)
+ if (tdep->have_linux_v2 && !tdep->have_linux_v1)
valid_p = 0;
}
@@ -8018,7 +8018,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
valid_p &= tdesc_numbered_register (feature, tdesc_data,
S390_TDB_DWORD0_REGNUM + i,
tdb_regs[i]);
- tdep->have_tdb = 1;
+ tdep->have_tdb = true;
}
/* Vector registers. */
--
2.13.5
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 01/11] s390: Remove duplicate checks for cached gdbarch at init
2017-12-05 12:29 [PATCH v2 00/11] Split up s390-linux-tdep.c Philipp Rudo
` (8 preceding siblings ...)
2017-12-05 12:29 ` [PATCH v2 03/11] s390: gdbarch_tdep.have_* int -> bool Philipp Rudo
@ 2017-12-05 12:29 ` Philipp Rudo
2017-12-05 16:16 ` Yao Qi
9 siblings, 1 reply; 20+ messages in thread
From: Philipp Rudo @ 2017-12-05 12:29 UTC (permalink / raw)
To: gdb-patches; +Cc: Andreas Arnez, Ulrich Weigand
When initializing the gdbarch there is a check whether an appropriate
gdbarch already exists in the gdbarch_list. If any of the checks fails
this would lead to a different target description. However
gdbarch_list_lookup_by_info already checks for
if (info->target_desc != arches->gdbarch->target_desc)
continue;
So it never returns a gdbarch where any of the checks can fail.
Remove the duplicate check. This also allows to move the lookup at the
start of the function.
gdb/ChangeLog:
* s390-linux-tdep.c (s390_gdbarch_init): Remove douplicate checks when
looking for cached gdbarch and move to start of function
---
gdb/s390-linux-tdep.c | 26 +++++---------------------
1 file changed, 5 insertions(+), 21 deletions(-)
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index a0d4cdd740..e3e036a70d 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -7829,6 +7829,11 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
static const char *const stap_register_indirection_suffixes[] = { ")",
NULL };
+ /* Find a candidate among extant architectures. */
+ arches = gdbarch_list_lookup_by_info (arches, &info);
+ if (arches != NULL)
+ return arches->gdbarch;
+
/* Default ABI and register size. */
switch (info.bfd_arch_info->mach)
{
@@ -8040,27 +8045,6 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
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))
- {
- tdep = gdbarch_tdep (arches->gdbarch);
- if (!tdep)
- continue;
- if (tdep->abi != tdep_abi)
- continue;
- if (tdep->vector_abi != vector_abi)
- continue;
- if ((tdep->gpr_full_regnum != -1) != have_upper)
- continue;
- if (tdep->have_gs != have_gs)
- continue;
- if (tdesc_data != NULL)
- tdesc_data_cleanup (tdesc_data);
- return arches->gdbarch;
- }
-
/* Otherwise create a new gdbarch for the specified machine type. */
tdep = XCNEW (struct gdbarch_tdep);
tdep->abi = tdep_abi;
--
2.13.5
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 01/11] s390: Remove duplicate checks for cached gdbarch at init
2017-12-05 12:29 ` [PATCH v2 01/11] s390: Remove duplicate checks for cached gdbarch at init Philipp Rudo
@ 2017-12-05 16:16 ` Yao Qi
2017-12-06 9:56 ` Philipp Rudo
0 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2017-12-05 16:16 UTC (permalink / raw)
To: Philipp Rudo; +Cc: gdb-patches, Andreas Arnez, Ulrich Weigand
Philipp Rudo <prudo@linux.vnet.ibm.com> writes:
> - /* 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))
> - {
> - tdep = gdbarch_tdep (arches->gdbarch);
> - if (!tdep)
> - continue;
> - if (tdep->abi != tdep_abi)
> - continue;
> - if (tdep->vector_abi != vector_abi)
> - continue;
Is it possible that we have two instances of gdbarch, with the same
target description, but different vector_abi? Two different executables
compiled with different vector abis, and GDB can debug them together
(multi-process debugging. If we consider multi-target debugging in the
future, these two executable can from different targets).
> - if ((tdep->gpr_full_regnum != -1) != have_upper)
> - continue;
> - if (tdep->have_gs != have_gs)
> - continue;
> - if (tdesc_data != NULL)
> - tdesc_data_cleanup (tdesc_data);
> - return arches->gdbarch;
> - }
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 01/11] s390: Remove duplicate checks for cached gdbarch at init
2017-12-05 16:16 ` Yao Qi
@ 2017-12-06 9:56 ` Philipp Rudo
2017-12-06 10:28 ` [PATCH v2 01/11] s390: Remove duplicate checks for cached gdbarch@init Ulrich Weigand
0 siblings, 1 reply; 20+ messages in thread
From: Philipp Rudo @ 2017-12-06 9:56 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches, Andreas Arnez, Ulrich Weigand
Hi Yao,
On Tue, 05 Dec 2017 16:16:07 +0000
Yao Qi <qiyaoltc@gmail.com> wrote:
> Philipp Rudo <prudo@linux.vnet.ibm.com> writes:
>
> > - /* 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))
> > - {
> > - tdep = gdbarch_tdep (arches->gdbarch);
> > - if (!tdep)
> > - continue;
> > - if (tdep->abi != tdep_abi)
> > - continue;
> > - if (tdep->vector_abi != vector_abi)
> > - continue;
>
> Is it possible that we have two instances of gdbarch, with the same
> target description, but different vector_abi? Two different executables
> compiled with different vector abis, and GDB can debug them together
> (multi-process debugging. If we consider multi-target debugging in the
> future, these two executable can from different targets).
For s390 there only is one vector abi (or non at all) at the time. If you were
debugging two different executables at the same time you would have two
inferiors each with its own gdbarch (same would be for multi-target debugging).
So I don't think those are the reasons.
For me it more looks like a mix of copy&paste code combined with an improper
clean up when the gdbarch_info->tdesc field was introduced. This pattern was
introduced in 2000 (7a78ae4e6b0) for ppc/rs6k (in rs6000-tdep.c) and copy&pasted
to s390 in 2010 (7803799a0fb). Between the two commits in 2006 (424163ea152)
the tdesc field (with the corresponding check) was added to gdbarch_info
without cleaning up the uses of gdbarch_list_lookup_by_info. Unfortunately
none of the commits have a commit message and the ChangeLog isn't very
helpful...
Thanks
Philipp
> > - if ((tdep->gpr_full_regnum != -1) != have_upper)
> > - continue;
> > - if (tdep->have_gs != have_gs)
> > - continue;
> > - if (tdesc_data != NULL)
> > - tdesc_data_cleanup (tdesc_data);
> > - return arches->gdbarch;
> > - }
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 01/11] s390: Remove duplicate checks for cached gdbarch@init
2017-12-06 9:56 ` Philipp Rudo
@ 2017-12-06 10:28 ` Ulrich Weigand
2017-12-07 9:18 ` Philipp Rudo
0 siblings, 1 reply; 20+ messages in thread
From: Ulrich Weigand @ 2017-12-06 10:28 UTC (permalink / raw)
To: Philipp Rudo; +Cc: Yao Qi, gdb-patches, Andreas Arnez
Philipp Rudo wrote:
> On Tue, 05 Dec 2017 16:16:07 +0000
> Yao Qi <qiyaoltc@gmail.com> wrote:
> > Is it possible that we have two instances of gdbarch, with the same
> > target description, but different vector_abi? Two different executables
> > compiled with different vector abis, and GDB can debug them together
> > (multi-process debugging. If we consider multi-target debugging in the
> > future, these two executable can from different targets).
>
> For s390 there only is one vector abi (or non at all) at the time. If you were
> debugging two different executables at the same time you would have two
> inferiors each with its own gdbarch (same would be for multi-target debugging).
> So I don't think those are the reasons.
Actually, I think Yao is right here. As you say, we can have two executables,
one using the vector ABI and one not. These will require two different gdbarch
structures. But with the patch you propose, when trying to allocate the second
of those two, GDB would see the first one that was already created earlier,
and incorrectly assume that it can simply be reused.
Basically, the problem is that there *can* be different gdbarchs that share
the *same* tdesc, but differ in vector ABI. Therefore *only* checking for
tdesc does not suffice to correctly identify cached gdbarch structures.
I agree that it is redundant to again check differences (e.g. in register set)
that would already have led to a different tdesc; but the vector ABI at least
is not one of those.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 01/11] s390: Remove duplicate checks for cached gdbarch@init
2017-12-06 10:28 ` [PATCH v2 01/11] s390: Remove duplicate checks for cached gdbarch@init Ulrich Weigand
@ 2017-12-07 9:18 ` Philipp Rudo
2017-12-07 9:59 ` Yao Qi
0 siblings, 1 reply; 20+ messages in thread
From: Philipp Rudo @ 2017-12-07 9:18 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: Yao Qi, gdb-patches, Andreas Arnez
Hi Yao,
I quickly talked to Uli yesterday about this and you and Uli are right. There
is a possibility that a program chooses not to use the vector registers for
their abi even when they are present. I fixed the patch locally.
Thanks for catching this!
Philipp
On Wed, 6 Dec 2017 11:28:23 +0100 (CET)
"Ulrich Weigand" <uweigand@de.ibm.com> wrote:
> Philipp Rudo wrote:
>
> > On Tue, 05 Dec 2017 16:16:07 +0000
> > Yao Qi <qiyaoltc@gmail.com> wrote:
> > > Is it possible that we have two instances of gdbarch, with the same
> > > target description, but different vector_abi? Two different executables
> > > compiled with different vector abis, and GDB can debug them together
> > > (multi-process debugging. If we consider multi-target debugging in the
> > > future, these two executable can from different targets).
> >
> > For s390 there only is one vector abi (or non at all) at the time. If you were
> > debugging two different executables at the same time you would have two
> > inferiors each with its own gdbarch (same would be for multi-target debugging).
> > So I don't think those are the reasons.
>
> Actually, I think Yao is right here. As you say, we can have two executables,
> one using the vector ABI and one not. These will require two different gdbarch
> structures. But with the patch you propose, when trying to allocate the second
> of those two, GDB would see the first one that was already created earlier,
> and incorrectly assume that it can simply be reused.
>
> Basically, the problem is that there *can* be different gdbarchs that share
> the *same* tdesc, but differ in vector ABI. Therefore *only* checking for
> tdesc does not suffice to correctly identify cached gdbarch structures.
>
> I agree that it is redundant to again check differences (e.g. in register set)
> that would already have led to a different tdesc; but the vector ABI at least
> is not one of those.
>
> Bye,
> Ulrich
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 01/11] s390: Remove duplicate checks for cached gdbarch@init
2017-12-07 9:18 ` Philipp Rudo
@ 2017-12-07 9:59 ` Yao Qi
0 siblings, 0 replies; 20+ messages in thread
From: Yao Qi @ 2017-12-07 9:59 UTC (permalink / raw)
To: Philipp Rudo; +Cc: Ulrich Weigand, GDB Patches, Andreas Arnez
On Thu, Dec 7, 2017 at 9:17 AM, Philipp Rudo <prudo@linux.vnet.ibm.com> wrote:
> Hi Yao,
>
> I quickly talked to Uli yesterday about this and you and Uli are right. There
> is a possibility that a program chooses not to use the vector registers for
> their abi even when they are present. I fixed the patch locally.
>
> Thanks for catching this!
Great, happy to help :)
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 20+ messages in thread