[commit, s390] Proper handling of PSW address/mask everywhere

[commit, s390] Proper handling of PSW address/mask everywhere

[Ulrich Weigand]

Hello,

I'm seeing the following failures on 31-bit s390:
FAIL: gdb.base/step-resume-infcall.exp: next
FAIL: gdb.base/step-resume-infcall.exp: p cond_hit

It turns out these are due to a long-standing shortcut where we didn't
always correctly distinguish between the PSW address (which includes the
addressing mode bit) and the PC value (which doesn't).

To fix this, I've had to remove that shortcut and actually handle PSW
addresses vs. PC values as appropriate.  This turned out to necessitate
a number of follow-on changes, but the end result should be more robust
anyway.  The changes involved are:

- In all unwinders, unwind PSW address and mask instead of PC and CC
  (note that the "return address" includes the addressing mode bit and
  therefore needs to be considered a PSW address, not a PC value).

- In every unwind step, recalculate the PC and CC pseudos from the
  actual PSW address and mask registers that are now being unwound.
  (This simplifies some of the unwinders as well.)

- We now need to add PSW address/mask to the save/restore register groups,
  instead of (or rather, in addition to) PC and CC.  (This is preferable
  anyway, e.g. if a function called via inferior call modifies some other
  bits in the PSW mask.)

- This in turn requires removal of another shortcut: when running as a
  64-bit process, GDB will receive the PSW in 16-byte form from the
  kernel; if the inferior is a 31-bit process, this needs to be properly
  converted to 8-byte form (old code used an approximation of that
  conversion step; the patch below implementes the fully-correct form).

- That latter change also needs to be done in gdbserver.

Tested with no regression on s390x-ibm-linux (with -m31 and -m64) and
on s390-ibm-linux, all both native and gdbserver.  Fixes the above
step-resume-infcall.exp regressions.

Committed to mainline.

Bye,
Ulrich
 

ChangeLog:

	* s390-nat.c (SUBOFF): Remove.
	(s390_native_supply, s390_native_collect): New functions.
	(supply_gregset, supply_fpregset): Use s390_native_supply.
	(fill_gregset, fill_fpregset): Use s390_native_collect.

	* s390-tdep.c (s390_pseudo_register_reggroup_p): Update comment.
	(s390_unwind_pseudo_register): New function.
	(s390_prologue_frame_unwind_cache): Unwind PSW address and mask
	registers instead of PC and CC.
	(s390_backchain_frame_unwind_cache): Likewise.
	(s390_sigtramp_frame_unwind_cache): Do not unwind PC, CC, or
	full GPR pseudos.
	(s390_trad_frame_prev_register): New function.
	(s390_frame_prev_register): Use it.
	(s390_sigtramp_frame_prev_register): Likewise.
	(s390_dwarf2_prev_register): Use s390_unwind_pseudo_register.
	(s390_dwarf2_frame_init_reg): Unwind PSW address and mask.  Use
	special callback to unwind any pseudo.

	* features/s390-core32.xml: Add pswm/pswa to save/restore group.
	* features/s390-core64.xml: Likewise.
	* features/s390x-core64.xml: Likewise.
	* features/s390-linux32.c: Regenerate.
	* features/s390-linux64.c: Likewise.
	* features/s390x-linux64.c: Likewise.

gdbserver/ChangeLog:

	* linux-s390-low.c (s390_collect_ptrace_register): Fully convert
	PSW address/mask between 8-byte and 16-byte formats.
	(s390_supply_ptrace_register): Likewise.
	(s390_get_pc, s390_set_pc): 4-byte PSW address always includes
	basic addressing mode bit.


Index: gdb-head/gdb/s390-nat.c
===================================================================
--- gdb-head.orig/gdb/s390-nat.c
+++ gdb-head/gdb/s390-nat.c
@@ -55,28 +55,122 @@
 /* When debugging a 32-bit executable running under a 64-bit kernel,
    we have to fix up the 64-bit registers we get from the kernel
    to make them look like 32-bit registers.  */
+
+static void
+s390_native_supply (struct regcache *regcache, int regno,
+		    const gdb_byte *regp, int *regmap)
+{
+  int offset = regmap[regno];
+
+#ifdef __s390x__
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  if (offset != -1 && gdbarch_ptr_bit (gdbarch) == 32)
+    {
+      enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+
+      if (regno == S390_PSWM_REGNUM)
+	{
+	  ULONGEST pswm;
+	  gdb_byte buf[4];
+
+	  pswm = extract_unsigned_integer (regp + regmap[S390_PSWM_REGNUM],
+					   8, byte_order);
+
+	  store_unsigned_integer (buf, 4, byte_order, (pswm >> 32) | 0x80000);
+	  regcache_raw_supply (regcache, regno, buf);
+	  return;
+	}
+
+      if (regno == S390_PSWA_REGNUM)
+	{
+	  ULONGEST pswm, pswa;
+	  gdb_byte buf[4];
+
+	  pswa = extract_unsigned_integer (regp + regmap[S390_PSWA_REGNUM],
+					   8, byte_order);
+	  pswm = extract_unsigned_integer (regp + regmap[S390_PSWM_REGNUM],
+					   8, byte_order);
+
+	  store_unsigned_integer (buf, 4, byte_order,
+				  (pswa & 0x7fffffff) | (pswm & 0x80000000));
+	  regcache_raw_supply (regcache, regno, buf);
+	  return;
+	}
+
+      if (regno >= S390_R0_REGNUM && regno <= S390_R15_REGNUM)
+	offset += 4;
+    }
+#endif
+
+  if (offset != -1)
+    regcache_raw_supply (regcache, regno, regp + offset);
+}
+
+static void
+s390_native_collect (const struct regcache *regcache, int regno,
+		     gdb_byte *regp, int *regmap)
+{
+  int offset = regmap[regno];
+
 #ifdef __s390x__
-#define SUBOFF(gdbarch, i) \
-	((gdbarch_ptr_bit (gdbarch) == 32 \
-	  && ((i) == S390_PSWA_REGNUM \
-	      || ((i) >= S390_R0_REGNUM && (i) <= S390_R15_REGNUM)))? 4 : 0)
-#else
-#define SUBOFF(gdbarch, i) 0
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  if (offset != -1 && gdbarch_ptr_bit (gdbarch) == 32)
+    {
+      enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+
+      if (regno == S390_PSWM_REGNUM)
+	{
+	  ULONGEST pswm;
+	  gdb_byte buf[4];
+
+	  regcache_raw_collect (regcache, regno, buf);
+	  pswm = extract_unsigned_integer (buf, 4, byte_order);
+
+	  /* We don't know the final addressing mode until the PSW address
+	     is known, so leave it as-is.  When the PSW address is collected
+	     (below), the addressing mode will be updated.  */
+	  store_unsigned_integer (regp + regmap[S390_PSWM_REGNUM],
+				  4, byte_order, pswm & 0xfff7ffff);
+	  return;
+	}
+
+      if (regno == S390_PSWA_REGNUM)
+	{
+	  ULONGEST pswa;
+	  gdb_byte buf[4];
+
+	  regcache_raw_collect (regcache, regno, buf);
+	  pswa = extract_unsigned_integer (buf, 4, byte_order);
+
+	  store_unsigned_integer (regp + regmap[S390_PSWA_REGNUM],
+				  8, byte_order, pswa & 0x7fffffff);
+
+	  /* Update basic addressing mode bit in PSW mask, see above.  */
+	  store_unsigned_integer (regp + regmap[S390_PSWM_REGNUM] + 4,
+				  4, byte_order, pswa & 0x80000000);
+	  return;
+	}
+
+      if (regno >= S390_R0_REGNUM && regno <= S390_R15_REGNUM)
+	{
+	  memset (regp + offset, 0, 4);
+	  offset += 4;
+	}
+    }
 #endif
 
+  if (offset != -1)
+    regcache_raw_collect (regcache, regno, regp + offset);
+}
 
 /* Fill GDB's register array with the general-purpose register values
    in *REGP.  */
 void
 supply_gregset (struct regcache *regcache, const gregset_t *regp)
 {
-  struct gdbarch *gdbarch = get_regcache_arch (regcache);
   int i;
   for (i = 0; i < S390_NUM_REGS; i++)
-    if (regmap_gregset[i] != -1)
-      regcache_raw_supply (regcache, i, 
-			   (const char *)regp + regmap_gregset[i]
-			     + SUBOFF (gdbarch, i));
+    s390_native_supply (regcache, i, (const gdb_byte *) regp, regmap_gregset);
 }
 
 /* Fill register REGNO (if it is a general-purpose register) in
@@ -85,14 +179,10 @@ supply_gregset (struct regcache *regcach
 void
 fill_gregset (const struct regcache *regcache, gregset_t *regp, int regno)
 {
-  struct gdbarch *gdbarch = get_regcache_arch (regcache);
   int i;
   for (i = 0; i < S390_NUM_REGS; i++)
-    if (regmap_gregset[i] != -1)
-      if (regno == -1 || regno == i)
-	regcache_raw_collect (regcache, i, 
-			      (char *)regp + regmap_gregset[i]
-				+ SUBOFF (gdbarch, i));
+    if (regno == -1 || regno == i)
+      s390_native_collect (regcache, i, (gdb_byte *) regp, regmap_gregset);
 }
 
 /* Fill GDB's register array with the floating-point register values
@@ -102,9 +192,7 @@ supply_fpregset (struct regcache *regcac
 {
   int i;
   for (i = 0; i < S390_NUM_REGS; i++)
-    if (regmap_fpregset[i] != -1)
-      regcache_raw_supply (regcache, i,
-			   (const char *)regp + regmap_fpregset[i]);
+    s390_native_supply (regcache, i, (const gdb_byte *) regp, regmap_fpregset);
 }
 
 /* Fill register REGNO (if it is a general-purpose register) in
@@ -115,10 +203,8 @@ fill_fpregset (const struct regcache *re
 {
   int i;
   for (i = 0; i < S390_NUM_REGS; i++)
-    if (regmap_fpregset[i] != -1)
-      if (regno == -1 || regno == i)
-        regcache_raw_collect (regcache, i, 
-			      (char *)regp + regmap_fpregset[i]);
+    if (regno == -1 || regno == i)
+      s390_native_collect (regcache, i, (gdb_byte *) regp, regmap_fpregset);
 }
 
 /* Find the TID for the current inferior thread to use with ptrace.  */
Index: gdb-head/gdb/s390-tdep.c
===================================================================
--- gdb-head.orig/gdb/s390-tdep.c
+++ gdb-head/gdb/s390-tdep.c
@@ -352,8 +352,14 @@ s390_pseudo_register_reggroup_p (struct 
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
-  /* PC and CC pseudo registers need to be saved/restored in order to
-     push or pop frames.  */
+  /* We usually save/restore the whole PSW, which includes PC and CC.
+     However, some older gdbservers may not support saving/restoring
+     the whole PSW yet, and will return an XML register description
+     excluding those from the save/restore register groups.  In those
+     cases, we still need to explicitly save/restore PC and CC in order
+     to push or pop frames.  Since this doesn't hurt anything if we
+     already save/restore the whole PSW (it's just redundant), we add
+     PC and CC at this point unconditionally.  */
   if (group == save_reggroup || group == restore_reggroup)
     return regnum == tdep->pc_regnum || regnum == tdep->cc_regnum;
 
@@ -1449,6 +1455,79 @@ s390_displaced_step_fixup (struct gdbarc
 			paddress (gdbarch, regcache_read_pc (regs)));
 }
 
+
+/* Helper routine to unwind pseudo registers.  */
+
+static struct value *
+s390_unwind_pseudo_register (struct frame_info *this_frame, int regnum)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  struct type *type = register_type (gdbarch, regnum);
+
+  /* Unwind PC via PSW address.  */
+  if (regnum == tdep->pc_regnum)
+    {
+      struct value *val;
+
+      val = frame_unwind_register_value (this_frame, S390_PSWA_REGNUM);
+      if (!value_optimized_out (val))
+	{
+	  LONGEST pswa = value_as_long (val);
+
+	  if (TYPE_LENGTH (type) == 4)
+	    return value_from_pointer (type, pswa & 0x7fffffff);
+	  else
+	    return value_from_pointer (type, pswa);
+	}
+    }
+
+  /* Unwind CC via PSW mask.  */
+  if (regnum == tdep->cc_regnum)
+    {
+      struct value *val;
+
+      val = frame_unwind_register_value (this_frame, S390_PSWM_REGNUM);
+      if (!value_optimized_out (val))
+	{
+	  LONGEST pswm = value_as_long (val);
+
+	  if (TYPE_LENGTH (type) == 4)
+	    return value_from_longest (type, (pswm >> 12) & 3);
+	  else
+	    return value_from_longest (type, (pswm >> 44) & 3);
+	}
+    }
+
+  /* Unwind full GPRs to show at least the lower halves (as the
+     upper halves are undefined).  */
+  if (tdep->gpr_full_regnum != -1
+      && regnum >= tdep->gpr_full_regnum
+      && regnum < tdep->gpr_full_regnum + 16)
+    {
+      int reg = regnum - tdep->gpr_full_regnum;
+      struct value *val;
+
+      val = frame_unwind_register_value (this_frame, S390_R0_REGNUM + reg);
+      if (!value_optimized_out (val))
+	return value_cast (type, val);
+    }
+
+  return allocate_optimized_out_value (type);
+}
+
+static struct value *
+s390_trad_frame_prev_register (struct frame_info *this_frame,
+			       struct trad_frame_saved_reg saved_regs[],
+			       int regnum)
+{
+  if (regnum < S390_NUM_REGS)
+    return trad_frame_get_prev_register (this_frame, saved_regs, regnum);
+  else
+    return s390_unwind_pseudo_register (this_frame, regnum);
+}
+
+
 /* Normal stack frames.  */
 
 struct s390_unwind_cache {
@@ -1465,7 +1544,6 @@ s390_prologue_frame_unwind_cache (struct
 				  struct s390_unwind_cache *info)
 {
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   int word_size = gdbarch_ptr_bit (gdbarch) / 8;
   struct s390_prologue_data data;
   pv_t *fp = &data.gpr[S390_FRAME_REGNUM - S390_R0_REGNUM];
@@ -1591,7 +1669,7 @@ s390_prologue_frame_unwind_cache (struct
       trad_frame_set_unknown (info->saved_regs, i);
 
   /* CC is always call-clobbered.  */
-  trad_frame_set_unknown (info->saved_regs, tdep->cc_regnum);
+  trad_frame_set_unknown (info->saved_regs, S390_PSWM_REGNUM);
 
   /* Record the addresses of all register spill slots the prologue parser
      has recognized.  Consider only registers defined as call-saved by the
@@ -1609,16 +1687,16 @@ s390_prologue_frame_unwind_cache (struct
       info->saved_regs[S390_F0_REGNUM + i].addr = cfa - data.fpr_slot[i];
 
   /* Function return will set PC to %r14.  */
-  info->saved_regs[tdep->pc_regnum] = info->saved_regs[S390_RETADDR_REGNUM];
+  info->saved_regs[S390_PSWA_REGNUM] = info->saved_regs[S390_RETADDR_REGNUM];
 
   /* In frameless functions, we unwind simply by moving the return
      address to the PC.  However, if we actually stored to the
      save area, use that -- we might only think the function frameless
      because we're in the middle of the prologue ...  */
   if (size == 0
-      && !trad_frame_addr_p (info->saved_regs, tdep->pc_regnum))
+      && !trad_frame_addr_p (info->saved_regs, S390_PSWA_REGNUM))
     {
-      info->saved_regs[tdep->pc_regnum].realreg = S390_RETADDR_REGNUM;
+      info->saved_regs[S390_PSWA_REGNUM].realreg = S390_RETADDR_REGNUM;
     }
 
   /* Another sanity check: unless this is a frameless function,
@@ -1628,7 +1706,7 @@ s390_prologue_frame_unwind_cache (struct
   if (size > 0)
     {
       if (!trad_frame_addr_p (info->saved_regs, S390_SP_REGNUM)
-	  || !trad_frame_addr_p (info->saved_regs, tdep->pc_regnum))
+	  || !trad_frame_addr_p (info->saved_regs, S390_PSWA_REGNUM))
 	prev_sp = -1;
     }
 
@@ -1649,7 +1727,6 @@ s390_backchain_frame_unwind_cache (struc
 				   struct s390_unwind_cache *info)
 {
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   int word_size = gdbarch_ptr_bit (gdbarch) / 8;
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   CORE_ADDR backchain;
@@ -1663,7 +1740,7 @@ s390_backchain_frame_unwind_cache (struc
       trad_frame_set_unknown (info->saved_regs, i);
 
   /* CC is always call-clobbered.  */
-  trad_frame_set_unknown (info->saved_regs, tdep->cc_regnum);
+  trad_frame_set_unknown (info->saved_regs, S390_PSWM_REGNUM);
 
   /* Get the backchain.  */
   reg = get_frame_register_unsigned (this_frame, S390_SP_REGNUM);
@@ -1685,7 +1762,7 @@ s390_backchain_frame_unwind_cache (struc
       info->saved_regs[S390_RETADDR_REGNUM].addr = backchain + 14*word_size;
 
       /* Function return will set PC to %r14.  */
-      info->saved_regs[tdep->pc_regnum]
+      info->saved_regs[S390_PSWA_REGNUM]
 	= info->saved_regs[S390_RETADDR_REGNUM];
 
       /* We use the current value of the frame register as local_base,
@@ -1739,28 +1816,10 @@ s390_frame_prev_register (struct frame_i
 			  void **this_prologue_cache, int regnum)
 {
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   struct s390_unwind_cache *info
     = s390_frame_unwind_cache (this_frame, this_prologue_cache);
 
-  /* Unwind full GPRs to show at least the lower halves (as the
-     upper halves are undefined).  */
-  if (tdep->gpr_full_regnum != -1
-      && regnum >= tdep->gpr_full_regnum
-      && regnum < tdep->gpr_full_regnum + 16)
-    {
-      int reg = regnum - tdep->gpr_full_regnum + S390_R0_REGNUM;
-      struct value *val, *newval;
-
-      val = trad_frame_get_prev_register (this_frame, info->saved_regs, reg);
-      newval = value_cast (register_type (gdbarch, regnum), val);
-      if (value_optimized_out (val))
-	set_value_optimized_out (newval, 1);
-
-      return newval;
-    }
-
-  return trad_frame_get_prev_register (this_frame, info->saved_regs, regnum);
+  return s390_trad_frame_prev_register (this_frame, info->saved_regs, regnum);
 }
 
 static const struct frame_unwind s390_frame_unwind = {
@@ -1788,7 +1847,6 @@ s390_stub_frame_unwind_cache (struct fra
 			      void **this_prologue_cache)
 {
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   int word_size = gdbarch_ptr_bit (gdbarch) / 8;
   struct s390_stub_unwind_cache *info;
   ULONGEST reg;
@@ -1801,7 +1859,7 @@ s390_stub_frame_unwind_cache (struct fra
   info->saved_regs = trad_frame_alloc_saved_regs (this_frame);
 
   /* The return address is in register %r14.  */
-  info->saved_regs[tdep->pc_regnum].realreg = S390_RETADDR_REGNUM;
+  info->saved_regs[S390_PSWA_REGNUM].realreg = S390_RETADDR_REGNUM;
 
   /* Retrieve stack pointer and determine our frame base.  */
   reg = get_frame_register_unsigned (this_frame, S390_SP_REGNUM);
@@ -1826,7 +1884,7 @@ s390_stub_frame_prev_register (struct fr
 {
   struct s390_stub_unwind_cache *info
     = s390_stub_frame_unwind_cache (this_frame, this_prologue_cache);
-  return trad_frame_get_prev_register (this_frame, info->saved_regs, regnum);
+  return s390_trad_frame_prev_register (this_frame, info->saved_regs, regnum);
 }
 
 static int
@@ -1875,7 +1933,6 @@ s390_sigtramp_frame_unwind_cache (struct
   struct s390_sigtramp_unwind_cache *info;
   ULONGEST this_sp, prev_sp;
   CORE_ADDR next_ra, next_cfa, sigreg_ptr, sigreg_high_off;
-  ULONGEST pswm;
   int i;
 
   if (*this_prologue_cache)
@@ -1928,16 +1985,6 @@ s390_sigtramp_frame_unwind_cache (struct
   info->saved_regs[S390_PSWA_REGNUM].addr = sigreg_ptr;
   sigreg_ptr += word_size;
 
-  /* Point PC to PSWA as well.  */
-  info->saved_regs[tdep->pc_regnum] = info->saved_regs[S390_PSWA_REGNUM];
-
-  /* Extract CC from PSWM.  */
-  pswm = read_memory_unsigned_integer (
-			info->saved_regs[S390_PSWM_REGNUM].addr,
-			word_size, byte_order);
-  trad_frame_set_value (info->saved_regs, tdep->cc_regnum,
-			(pswm >> (8 * word_size - 20)) & 3);
-
   /* Then the GPRs.  */
   for (i = 0; i < 16; i++)
     {
@@ -1972,22 +2019,6 @@ s390_sigtramp_frame_unwind_cache (struct
 	sigreg_ptr += 4;
       }
 
-  /* Provide read-only copies of the full registers.  */
-  if (tdep->gpr_full_regnum != -1)
-    for (i = 0; i < 16; i++)
-      {
-	ULONGEST low, high;
-	low = read_memory_unsigned_integer (
-			info->saved_regs[S390_R0_REGNUM + i].addr,
-			4, byte_order);
-	high = read_memory_unsigned_integer (
-			info->saved_regs[S390_R0_UPPER_REGNUM + i].addr,
-			4, byte_order);
-	
-	trad_frame_set_value (info->saved_regs, tdep->gpr_full_regnum + i,
-			      (high << 32) | low);
-      }
-
   /* Restore the previous frame's SP.  */
   prev_sp = read_memory_unsigned_integer (
 			info->saved_regs[S390_SP_REGNUM].addr,
@@ -2015,7 +2046,7 @@ s390_sigtramp_frame_prev_register (struc
 {
   struct s390_sigtramp_unwind_cache *info
     = s390_sigtramp_frame_unwind_cache (this_frame, this_prologue_cache);
-  return trad_frame_get_prev_register (this_frame, info->saved_regs, regnum);
+  return s390_trad_frame_prev_register (this_frame, info->saved_regs, regnum);
 }
 
 static int
@@ -2098,17 +2129,7 @@ static struct value *
 s390_dwarf2_prev_register (struct frame_info *this_frame, void **this_cache,
 			   int regnum)
 {
-  struct gdbarch *gdbarch = get_frame_arch (this_frame);
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-  int reg = regnum - tdep->gpr_full_regnum;
-  struct value *val, *newval;
-
-  val = frame_unwind_register_value (this_frame, S390_R0_REGNUM + reg);
-  newval = value_cast (register_type (gdbarch, regnum), val);
-  if (value_optimized_out (val))
-    set_value_optimized_out (newval, 1);
-
-  return newval;
+  return s390_unwind_pseudo_register (this_frame, regnum);
 }
 
 static void
@@ -2118,9 +2139,17 @@ s390_dwarf2_frame_init_reg (struct gdbar
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
+  /* The condition code (and thus PSW mask) is call-clobbered.  */
+  if (regnum == S390_PSWM_REGNUM)
+    reg->how = DWARF2_FRAME_REG_UNDEFINED;
+
+  /* The PSW address unwinds to the return address.  */
+  else if (regnum == S390_PSWA_REGNUM)
+    reg->how = DWARF2_FRAME_REG_RA;
+
   /* Fixed registers are call-saved or call-clobbered
      depending on the ABI in use.  */
-  if (regnum >= 0 && regnum < S390_NUM_REGS)
+  else if (regnum < S390_NUM_REGS)
     {
       if (s390_register_call_saved (gdbarch, regnum))
 	reg->how = DWARF2_FRAME_REG_SAME_VALUE;
@@ -2128,19 +2157,8 @@ s390_dwarf2_frame_init_reg (struct gdbar
 	reg->how = DWARF2_FRAME_REG_UNDEFINED;
     }
 
-  /* The CC pseudo register is call-clobbered.  */
-  else if (regnum == tdep->cc_regnum)
-    reg->how = DWARF2_FRAME_REG_UNDEFINED;
-
-  /* The PC register unwinds to the return address.  */
-  else if (regnum == tdep->pc_regnum)
-    reg->how = DWARF2_FRAME_REG_RA;
-
-  /* We install a special function to unwind full GPRs to show at
-     least the lower halves (as the upper halves are undefined).  */
-  else if (tdep->gpr_full_regnum != -1
-	   && regnum >= tdep->gpr_full_regnum
-	   && regnum < tdep->gpr_full_regnum + 16)
+  /* We install a special function to unwind pseudos.  */
+  else
     {
       reg->how = DWARF2_FRAME_REG_FN;
       reg->loc.fn = s390_dwarf2_prev_register;
Index: gdb-head/gdb/features/s390-core32.xml
===================================================================
--- gdb-head.orig/gdb/features/s390-core32.xml
+++ gdb-head/gdb/features/s390-core32.xml
@@ -7,8 +7,8 @@
 
 <!DOCTYPE feature SYSTEM "gdb-target.dtd">
 <feature name="org.gnu.gdb.s390.core">
-  <reg name="pswm" bitsize="32" type="uint32" group="psw" save-restore="no"/>
-  <reg name="pswa" bitsize="32" type="uint32" group="psw" save-restore="no"/>
+  <reg name="pswm" bitsize="32" type="uint32" group="psw"/>
+  <reg name="pswa" bitsize="32" type="uint32" group="psw"/>
   <reg name="r0" bitsize="32" type="uint32" group="general"/>
   <reg name="r1" bitsize="32" type="uint32" group="general"/>
   <reg name="r2" bitsize="32" type="uint32" group="general"/>
Index: gdb-head/gdb/features/s390-core64.xml
===================================================================
--- gdb-head.orig/gdb/features/s390-core64.xml
+++ gdb-head/gdb/features/s390-core64.xml
@@ -7,8 +7,8 @@
 
 <!DOCTYPE feature SYSTEM "gdb-target.dtd">
 <feature name="org.gnu.gdb.s390.core">
-  <reg name="pswm" bitsize="32" type="uint32" group="psw" save-restore="no"/>
-  <reg name="pswa" bitsize="32" type="uint32" group="psw" save-restore="no"/>
+  <reg name="pswm" bitsize="32" type="uint32" group="psw"/>
+  <reg name="pswa" bitsize="32" type="uint32" group="psw"/>
   <reg name="r0h" bitsize="32" type="uint32" group="upper"/>
   <reg name="r0l" bitsize="32" type="uint32" group="lower"/>
   <reg name="r1h" bitsize="32" type="uint32" group="upper"/>
Index: gdb-head/gdb/features/s390-linux32.c
===================================================================
--- gdb-head.orig/gdb/features/s390-linux32.c
+++ gdb-head/gdb/features/s390-linux32.c
@@ -1,6 +1,7 @@
 /* THIS FILE IS GENERATED.  Original: s390-linux32.xml */
 
 #include "defs.h"
+#include "osabi.h"
 #include "target-descriptions.h"
 
 struct target_desc *tdesc_s390_linux32;
@@ -14,8 +15,8 @@ initialize_tdesc_s390_linux32 (void)
   set_tdesc_architecture (result, bfd_scan_arch ("s390:31-bit"));
 
   feature = tdesc_create_feature (result, "org.gnu.gdb.s390.core");
-  tdesc_create_reg (feature, "pswm", 0, 0, "psw", 32, "uint32");
-  tdesc_create_reg (feature, "pswa", 1, 0, "psw", 32, "uint32");
+  tdesc_create_reg (feature, "pswm", 0, 1, "psw", 32, "uint32");
+  tdesc_create_reg (feature, "pswa", 1, 1, "psw", 32, "uint32");
   tdesc_create_reg (feature, "r0", 2, 1, "general", 32, "uint32");
   tdesc_create_reg (feature, "r1", 3, 1, "general", 32, "uint32");
   tdesc_create_reg (feature, "r2", 4, 1, "general", 32, "uint32");
Index: gdb-head/gdb/features/s390-linux64.c
===================================================================
--- gdb-head.orig/gdb/features/s390-linux64.c
+++ gdb-head/gdb/features/s390-linux64.c
@@ -1,6 +1,7 @@
 /* THIS FILE IS GENERATED.  Original: s390-linux64.xml */
 
 #include "defs.h"
+#include "osabi.h"
 #include "target-descriptions.h"
 
 struct target_desc *tdesc_s390_linux64;
@@ -14,8 +15,8 @@ initialize_tdesc_s390_linux64 (void)
   set_tdesc_architecture (result, bfd_scan_arch ("s390:31-bit"));
 
   feature = tdesc_create_feature (result, "org.gnu.gdb.s390.core");
-  tdesc_create_reg (feature, "pswm", 0, 0, "psw", 32, "uint32");
-  tdesc_create_reg (feature, "pswa", 1, 0, "psw", 32, "uint32");
+  tdesc_create_reg (feature, "pswm", 0, 1, "psw", 32, "uint32");
+  tdesc_create_reg (feature, "pswa", 1, 1, "psw", 32, "uint32");
   tdesc_create_reg (feature, "r0h", 2, 1, "upper", 32, "uint32");
   tdesc_create_reg (feature, "r0l", 3, 1, "lower", 32, "uint32");
   tdesc_create_reg (feature, "r1h", 4, 1, "upper", 32, "uint32");
Index: gdb-head/gdb/features/s390x-core64.xml
===================================================================
--- gdb-head.orig/gdb/features/s390x-core64.xml
+++ gdb-head/gdb/features/s390x-core64.xml
@@ -7,8 +7,8 @@
 
 <!DOCTYPE feature SYSTEM "gdb-target.dtd">
 <feature name="org.gnu.gdb.s390.core">
-  <reg name="pswm" bitsize="64" type="uint64" group="psw" save-restore="no"/>
-  <reg name="pswa" bitsize="64" type="uint64" group="psw" save-restore="no"/>
+  <reg name="pswm" bitsize="64" type="uint64" group="psw"/>
+  <reg name="pswa" bitsize="64" type="uint64" group="psw"/>
   <reg name="r0" bitsize="64" type="uint64" group="general"/>
   <reg name="r1" bitsize="64" type="uint64" group="general"/>
   <reg name="r2" bitsize="64" type="uint64" group="general"/>
Index: gdb-head/gdb/features/s390x-linux64.c
===================================================================
--- gdb-head.orig/gdb/features/s390x-linux64.c
+++ gdb-head/gdb/features/s390x-linux64.c
@@ -1,6 +1,7 @@
 /* THIS FILE IS GENERATED.  Original: s390x-linux64.xml */
 
 #include "defs.h"
+#include "osabi.h"
 #include "target-descriptions.h"
 
 struct target_desc *tdesc_s390x_linux64;
@@ -14,8 +15,8 @@ initialize_tdesc_s390x_linux64 (void)
   set_tdesc_architecture (result, bfd_scan_arch ("s390:64-bit"));
 
   feature = tdesc_create_feature (result, "org.gnu.gdb.s390.core");
-  tdesc_create_reg (feature, "pswm", 0, 0, "psw", 64, "uint64");
-  tdesc_create_reg (feature, "pswa", 1, 0, "psw", 64, "uint64");
+  tdesc_create_reg (feature, "pswm", 0, 1, "psw", 64, "uint64");
+  tdesc_create_reg (feature, "pswa", 1, 1, "psw", 64, "uint64");
   tdesc_create_reg (feature, "r0", 2, 1, "general", 64, "uint64");
   tdesc_create_reg (feature, "r1", 3, 1, "general", 64, "uint64");
   tdesc_create_reg (feature, "r2", 4, 1, "general", 64, "uint64");
Index: gdb-head/gdb/gdbserver/linux-s390-low.c
===================================================================
--- gdb-head.orig/gdb/gdbserver/linux-s390-low.c
+++ gdb-head/gdb/gdbserver/linux-s390-low.c
@@ -126,16 +126,27 @@ s390_collect_ptrace_register (struct reg
 	  collect_register (regcache, (regno & ~1) + 1,
 			    buf + sizeof (long) - size);
 	}
-      else if (regaddr == PT_PSWADDR
-	       || (regaddr >= PT_GPR0 && regaddr <= PT_GPR15))
+      else if (regaddr == PT_PSWMASK)
+	{
+	  /* Convert 4-byte PSW mask to 8 bytes by clearing bit 12 and copying
+	     the basic addressing mode bit from the PSW address.  */
+	  char *addr = alloca (register_size (regno ^ 1));
+	  collect_register (regcache, regno, buf);
+	  collect_register (regcache, regno ^ 1, addr);
+	  buf[1] &= ~0x8;
+	  buf[size] |= (addr[0] & 0x80);
+	}
+      else if (regaddr == PT_PSWADDR)
+	{
+	  /* Convert 4-byte PSW address to 8 bytes by clearing the addressing
+	     mode bit (which gets copied to the PSW mask instead).  */
+	  collect_register (regcache, regno, buf + sizeof (long) - size);
+	  buf[sizeof (long) - size] &= ~0x80;
+	}
+      else if (regaddr >= PT_GPR0 && regaddr <= PT_GPR15)
 	collect_register (regcache, regno, buf + sizeof (long) - size);
       else
 	collect_register (regcache, regno, buf);
-
-      /* When debugging a 32-bit inferior on a 64-bit host, make sure
-	 the 31-bit addressing mode bit is set in the PSW mask.  */
-      if (regaddr == PT_PSWMASK)
-	buf[size] |= 0x80;
     }
   else
     collect_register (regcache, regno, buf);
@@ -157,8 +168,35 @@ s390_supply_ptrace_register (struct regc
 	  supply_register (regcache, (regno & ~1) + 1,
 			   buf + sizeof (long) - size);
 	}
-      else if (regaddr == PT_PSWADDR
-	       || (regaddr >= PT_GPR0 && regaddr <= PT_GPR15))
+      else if (regaddr == PT_PSWMASK)
+	{
+	  /* Convert 8-byte PSW mask to 4 bytes by setting bit 12 and copying
+	     the basic addressing mode into the PSW address.  */
+	  char *mask = alloca (size);
+	  char *addr = alloca (register_size (regno ^ 1));
+	  memcpy (mask, buf, size);
+	  mask[1] |= 0x8;
+	  supply_register (regcache, regno, mask);
+
+	  collect_register (regcache, regno ^ 1, addr);
+	  addr[0] &= ~0x80;
+	  addr[0] |= (buf[size] & 0x80);
+	  supply_register (regcache, regno ^ 1, addr);
+	}
+      else if (regaddr == PT_PSWADDR)
+	{
+	  /* Convert 8-byte PSW address to 4 bytes by truncating, but
+	     keeping the addressing mode bit (which was set from the mask).  */
+	  char *addr = alloca (size);
+	  char amode;
+	  collect_register (regcache, regno, addr);
+	  amode = addr[0] & 0x80;
+	  memcpy (addr, buf + sizeof (long) - size, size);
+	  addr[0] &= ~0x80;
+	  addr[0] |= amode;
+	  supply_register (regcache, regno, addr);
+	}
+      else if (regaddr >= PT_GPR0 && regaddr <= PT_GPR15)
 	supply_register (regcache, regno, buf + sizeof (long) - size);
       else
 	supply_register (regcache, regno, buf);
@@ -199,12 +237,9 @@ s390_get_pc (struct regcache *regcache)
 {
   if (register_size (0) == 4)
     {
-      unsigned int pc;
-      collect_register_by_name (regcache, "pswa", &pc);
-#ifndef __s390x__
-      pc &= 0x7fffffff;
-#endif
-      return pc;
+      unsigned int pswa;
+      collect_register_by_name (regcache, "pswa", &pswa);
+      return pswa & 0x7fffffff;
     }
   else
     {
@@ -219,11 +254,10 @@ s390_set_pc (struct regcache *regcache, 
 {
   if (register_size (0) == 4)
     {
-      unsigned int pc = newpc;
-#ifndef __s390x__
-      pc |= 0x80000000;
-#endif
-      supply_register_by_name (regcache, "pswa", &pc);
+      unsigned int pswa;
+      collect_register_by_name (regcache, "pswa", &pswa);
+      pswa = (pswa & 0x80000000) | (newpc & 0x7fffffff);
+      supply_register_by_name (regcache, "pswa", &pswa);
     }
   else
     {
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Regression gdb.base/callfuncs.exp for s390-on-s390x [Re: [commit, s390] Proper handling of PSW address/mask everywhere]

[Jan Kratochvil]

On Wed, 30 Nov 2011 17:09:25 +0100, Ulrich Weigand wrote:

commit 176997b00bd777fb63505a20e918118271600f38
Author: Ulrich Weigand <uweigand@de.ibm.com>
Date:   Wed Nov 30 16:06:46 2011 +0000
http://sourceware.org/ml/gdb-cvs/2011-11/msg00187.html

> Tested with no regression on s390x-ibm-linux (with -m31 and -m64) and
> on s390-ibm-linux, all both native and gdbserver.  Fixes the above
> step-resume-infcall.exp regressions.

I see a regression for s390x with -m31, RHEL-6.2, kernel-2.6.32-220.el6.s390x.

 return 7
 Make add return now? (y or n) y
-#0  main () at ./gdb.base/callfuncs.c:655
-655      t_structs_c(struct_val1);
-(gdb) PASS: gdb.base/callfuncs.exp: back at main after return from call dummy breakpoint
+Couldn't write registers: Invalid argument.
+(gdb) FAIL: gdb.base/callfuncs.exp: back at main after return from call dummy breakpoint

GDB reads PC as 0x8aaabbbb and triess to write it back as 0x0aaabbbb.

I rather did not try to fix it.

#0  error (string=0x80d7b3aa "%s.") at utils.c:810
#1  in perror_with_name (string=0x80daa600 "Couldn't write registers") at utils.c:1195
#2  in store_regs (regcache=0x8cb90eb0, tid=51221, regnum=1) at s390-nat.c:269
#3  in s390_linux_store_inferior_registers (ops=0x8c8ca340, regcache=0x8cb90eb0, regnum=1) at s390-nat.c:411
#4  in target_store_registers (regcache=0x8cb90eb0, regno=1) at target.c:3585
#5  in regcache_raw_write (regcache=0x8cb90eb0, regnum=1, buf=0x3ffff8d2b20 "") at regcache.c:877
#6  in regcache_cooked_write (regcache=0x8cb90eb0, regnum=1, buf=0x3ffff8d2b20 "") at regcache.c:889
#7  in regcache_restore (dst=0x8cb90eb0, cooked_read=0x8033a870 <do_cooked_read>, cooked_read_context=0x8cc55e80) at regcache.c:358
#8  in regcache_cpy (dst=0x8cb90eb0, src=0x8cc55e80) at regcache.c:382
#9  in frame_pop (this_frame=0x8c9350e8) at frame.c:835
#10 in return_command (retval_exp=0x8c709777 "7", from_tty=0) at stack.c:2402



Thanks,
Jan

[commit, s390] Fix regression in gdb.base/callfuncs.exp [Re: [commit, s390] Proper handling of PSW address/mask everywhere]

[Ulrich Weigand]

Jan Kratochvil wrote:

> On Wed, 30 Nov 2011 17:09:25 +0100, Ulrich Weigand wrote:
> > Tested with no regression on s390x-ibm-linux (with -m31 and -m64) and
> > on s390-ibm-linux, all both native and gdbserver.  Fixes the above
> > step-resume-infcall.exp regressions.
> 
> I see a regression for s390x with -m31, RHEL-6.2, kernel-2.6.32-220.el6.s390x.

Interesting, I had tested with a current kernel where I don't see that regression.
But I do see it with an older kernel as well ...

>  return 7
>  Make add return now? (y or n) y
> -#0  main () at ./gdb.base/callfuncs.c:655
> -655      t_structs_c(struct_val1);
> -(gdb) PASS: gdb.base/callfuncs.exp: back at main after return from call dummy breakpoint
> +Couldn't write registers: Invalid argument.
> +(gdb) FAIL: gdb.base/callfuncs.exp: back at main after return from call dummy breakpoint
> 
> GDB reads PC as 0x8aaabbbb and triess to write it back as 0x0aaabbbb.
> 
> I rather did not try to fix it.
> 
> #0  error (string=0x80d7b3aa "%s.") at utils.c:810
> #1  in perror_with_name (string=0x80daa600 "Couldn't write registers") at utils.c:1195
> #2  in store_regs (regcache=0x8cb90eb0, tid=51221, regnum=1) at s390-nat.c:269
> #3  in s390_linux_store_inferior_registers (ops=0x8c8ca340, regcache=0x8cb90eb0, regnum=1) at s390-nat.c:411
> #4  in target_store_registers (regcache=0x8cb90eb0, regno=1) at target.c:3585
> #5  in regcache_raw_write (regcache=0x8cb90eb0, regnum=1, buf=0x3ffff8d2b20 "") at regcache.c:877
> #6  in regcache_cooked_write (regcache=0x8cb90eb0, regnum=1, buf=0x3ffff8d2b20 "") at regcache.c:889
> #7  in regcache_restore (dst=0x8cb90eb0, cooked_read=0x8033a870 <do_cooked_read>, cooked_read_context=0x8cc55e80) at regcache.c:358
> #8  in regcache_cpy (dst=0x8cb90eb0, src=0x8cc55e80) at regcache.c:382
> #9  in frame_pop (this_frame=0x8c9350e8) at frame.c:835
> #10 in return_command (retval_exp=0x8c709777 "7", from_tty=0) at stack.c:2402

Looks like there is a place where I forgot to preserve the addressing mode bit.
Fixed by the following patch.

Tested with no regression (fixing the callfuncs.exp one) on s390x (-m31,-m64).
Committed to mainline and 7.4 branch.

Bye,
Ulrich


ChangeLog:

	* s390-tdep.c (s390_push_dummy_call): Set addressing mode bit
	in return PSWA.

Index: gdb/s390-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/s390-tdep.c,v
retrieving revision 1.201
diff -u -p -r1.201 s390-tdep.c
--- gdb/s390-tdep.c	7 Dec 2011 13:31:05 -0000	1.201
+++ gdb/s390-tdep.c	19 Dec 2011 20:59:28 -0000
@@ -2749,9 +2749,15 @@ s390_push_dummy_call (struct gdbarch *gd
       }
   }
 
-  /* Store return address.  */
+  /* Store return PSWA.  In 31-bit mode, keep addressing mode bit.  */
+  if (word_size == 4)
+    {
+      ULONGEST pswa;
+      regcache_cooked_read_unsigned (regcache, S390_PSWA_REGNUM, &pswa);
+      bp_addr = (bp_addr & 0x7fffffff) | (pswa & 0x80000000);
+    }
   regcache_cooked_write_unsigned (regcache, S390_RETADDR_REGNUM, bp_addr);
-  
+
   /* Store updated stack pointer.  */
   regcache_cooked_write_unsigned (regcache, S390_SP_REGNUM, sp);
 


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [commit, s390] Fix regression in gdb.base/callfuncs.exp [Re: [commit, s390] Proper handling of PSW address/mask everywhere]

[Jan Kratochvil]

On Mon, 19 Dec 2011 22:49:16 +0100, Ulrich Weigand wrote:
> Tested with no regression (fixing the callfuncs.exp one) on s390x (-m31,-m64).
> Committed to mainline and 7.4 branch.

Confirming it works, thanks.

There are yet some regressions of sigstep.exp, I will check them more:

 step
 
-Breakpoint 3, main () at ./gdb.base/sigstep.c:79
-79           done = 0;
+Program received signal SIGTRAP, Trace/breakpoint trap.
+handler (sig=14) at ./gdb.base/sigstep.c:30
+30     {
 1: x/i $pc
-=> 0x800008e8 <main+384>:      larl    %r1,0x80001da0 <done>
-(gdb) PASS: gdb.base/sigstep.exp: step on breakpoint, skip handler; performing step
+=> 0x80000734 <handler>:       stmg    %r11,%r15,88(%r15)
+(gdb) FAIL: gdb.base/sigstep.exp: step on breakpoint, skip handler; performing step


Regards,
Jan

Re: [commit, s390] Fix regression in gdb.base/callfuncs.exp [Re: [commit, s390] Proper handling of PSW address/mask everywhere]

[Ulrich Weigand]

Jan Kratochvil wrote:
> On Mon, 19 Dec 2011 22:49:16 +0100, Ulrich Weigand wrote:
> > Tested with no regression (fixing the callfuncs.exp one) on s390x (-m31,-m64).
> > Committed to mainline and 7.4 branch.
> 
> Confirming it works, thanks.
> 
> There are yet some regressions of sigstep.exp, I will check them more:

Hmm, I'm not seeing this particular symptom you mention below.

However, I did see failures on old kernels in 31-bit compat mode only;
it turned out these were caused by a kernel bug that is exposed by my
GDB change.

When the kernel sets up the registers for calling a signal handler,
it is supposed to place the return address including the addressing
mode bit into register %r14.  This does in fact happen in native
31-bit mode.  However, in 31-bit compat mode under a 64-bit kernel,
the addressing mode bit was not set.  This has been fixed at least
for regular signal handlers in current git (note that RT signals
are still broken).

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [commit, s390] Fix regression in gdb.base/callfuncs.exp [Re: [commit, s390] Proper handling of PSW address/mask everywhere]

[Jan Kratochvil]

On Wed, 21 Dec 2011 20:37:16 +0100, Ulrich Weigand wrote:
> Jan Kratochvil wrote:
> > There are yet some regressions of sigstep.exp, I will check them more:
> 
> Hmm, I'm not seeing this particular symptom you mention below.

I looked at it and I have seen it broken enough already in 7.3, due to some
kernel bug.  Nice to know it is therefore fixed in kernel GIT now.

This whole sigstep.exp issue is therefore a kernel one and not GDB one.


Thanks,
Jan