[patch] Add support for single register window model on SPARC

[patch] Add support for single register window model on SPARC

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 2563 bytes --]

Hi,

support for the single register window (aka flat) model on SPARC was just 
re-introduced in GCC: http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00820.html
The -mflat option had been present in the 3.x series of compilers but was 
removed when the 4.x series debuted.  Due to renewed interest, most notably 
from the LEON folks, it will be supported again in future GCC releases.

The implementation has been almost entirely overhauled and, in particular, the 
weird register usage of the old flavor (e.g. %i7 as frame pointer) has been 
dropped.  Instead the new flavor preserves the canonical register usage and 
frame layout; the only visible change is that 'save' & 'restore' instructions 
are replaced with their "manual" equivalents in the generated code.

While CFIs are adjusted automatically by GCC, GDB has hardcoded assumptions 
about how frames are established on SPARC, which makes it unable to unwind 
the "flat" frames; in particular backtraces don't work.

The attached patch is aimed at fixing that.  It extends the frame sniffer to 
recognize the "flat" frames and decode the locations of call-saved registers.
Regtested (in normal mode) on SPARC/Solaris and SPARC64/Solaris.  It was also 
used to debug the new -mflat implementation of GCC, both 32-bit and 64-bit, so 
it works reasonably well in this mode.

OK for the mainline?


2011-06-16  Eric Botcazou  <ebotcazou@adacore.com>

	* sparc-tdep.h (struct sparc_frame_cache): Add frame_offset,
	saved_regs_mask and copied_regs_mask fields.
	(sparc_record_save_insn): New prototype.
	* sparc-tdep.c (sparc_alloc_frame_cache): Initialize the new fields.
	(sparc_record_save_insn): New function.
	(sparc_analyze_prologue): Add head comment.  Recognize store insns
	of call-saved registers.  Use OFFSET consistently.  Recognize flat
	frames and cache their settings.
	(sparc32_skip_prologue): Handle flat frames.
	(sparc_frame_cache): Add frame_offset to the base address.
	(sparc32_frame_cache): Adjust to new frame description.
	(sparc32_frame_prev_register): Likewise.
	* sparc64-tdep.c (sparc64_frame_prev_register): Likewise.
	* sparc-sol2-tdep.c (sparc32_sol2_sigtramp_frame_cache): Likewise.
	* sparc64-sol2-tdep.c (sparc64_sol2_sigtramp_frame_cache): Likewise.
	* sparcnbsd-tdep.c (sparc32nbsd_sigcontext_frame_cache): Force the
	frame by calling sparc_record_save_insn.
	* sparc64nbsd-tdep.c (sparc64nbsd_sigcontext_frame_cache): Likewise.
	* sparcobsd-tdep.c (sparc32obsd_sigtramp_frame_cache): Likewise.
	* sparc64obsd-tdep.c (sparc64obsd_frame_cache): Likewise.


-- 
Eric Botcazou

[-- Attachment #2: gdb-mflat.diff --]
[-- Type: text/x-diff, Size: 17233 bytes --]

Index: sparc-sol2-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc-sol2-tdep.c,v
retrieving revision 1.25
diff -u -p -r1.25 sparc-sol2-tdep.c
--- sparc-sol2-tdep.c	18 Mar 2011 18:52:32 -0000	1.25
+++ sparc-sol2-tdep.c	16 Jun 2011 13:13:24 -0000
@@ -93,7 +93,7 @@ sparc32_sol2_sigtramp_frame_cache (struc
   /* The third argument is a pointer to an instance of `ucontext_t',
      which has a member `uc_mcontext' that contains the saved
      registers.  */
-  regnum = (cache->frameless_p ? SPARC_O2_REGNUM : SPARC_I2_REGNUM);
+  regnum = (cache->copied_regs_mask & 4) ? SPARC_I2_REGNUM : SPARC_O2_REGNUM;
   mcontext_addr = get_frame_register_unsigned (this_frame, regnum) + 40;
 
   cache->saved_regs[SPARC32_PSR_REGNUM].addr = mcontext_addr + 0 * 4;
Index: sparc-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc-tdep.c,v
retrieving revision 1.221
diff -u -p -r1.221 sparc-tdep.c
--- sparc-tdep.c	23 May 2011 16:38:05 -0000	1.221
+++ sparc-tdep.c	16 Jun 2011 13:13:24 -0000
@@ -592,7 +592,9 @@ sparc_alloc_frame_cache (void)
 
   /* Frameless until proven otherwise.  */
   cache->frameless_p = 1;
-
+  cache->frame_offset = 0;
+  cache->saved_regs_mask = 0;
+  cache->copied_regs_mask = 0;
   cache->struct_return_p = 0;
 
   return cache;
@@ -784,6 +786,31 @@ sparc_skip_stack_check (const CORE_ADDR 
   return start_pc;
 }
 
+/* Record the effect of a SAVE instruction on CACHE.  */
+
+void
+sparc_record_save_insn (struct sparc_frame_cache *cache)
+{
+  /* The frame is set up.  */
+  cache->frameless_p = 0;
+
+  /* The frame pointer contains the CFA.  */
+  cache->frame_offset = 0;
+
+  /* The `local' and `in' registers are all saved.  */
+  cache->saved_regs_mask = 0xffff;
+
+  /* The `out' registers are all renamed.  */
+  cache->copied_regs_mask = 0xff;
+}
+
+/* Do a full analysis of the prologue at PC and update CACHE accordingly.
+   Bail out early if CURRENT_PC is reached.  Return the address where
+   the analysis stopped.
+
+   We handle both the traditional register window model and the single
+   register window (aka flat) model.  */
+
 CORE_ADDR
 sparc_analyze_prologue (struct gdbarch *gdbarch, CORE_ADDR pc,
 			CORE_ADDR current_pc, struct sparc_frame_cache *cache)
@@ -813,13 +840,40 @@ sparc_analyze_prologue (struct gdbarch *
 
   insn = sparc_fetch_instruction (pc);
 
+  /* Recognize store insns and record their sources.  */
+  while (X_OP (insn) == 3
+	 && (X_OP3 (insn) == 0x4     /* stw */
+	     || X_OP3 (insn) == 0x7  /* std */
+	     || X_OP3 (insn) == 0xe) /* stx */
+	 && X_RS1 (insn) == SPARC_SP_REGNUM)
+    {
+      int regnum = X_RD (insn);
+
+      /* Recognize stores into the corresponding stack slots.  */
+      if (regnum >= SPARC_L0_REGNUM && regnum <= SPARC_I7_REGNUM
+	  && ((X_I (insn)
+	       && X_SIMM13 (insn) == (X_OP3 (insn) == 0xe
+				      ? (regnum - SPARC_L0_REGNUM) * 8 + BIAS
+				      : (regnum - SPARC_L0_REGNUM) * 4))
+	      || (!X_I (insn) && regnum == SPARC_L0_REGNUM)))
+	{
+	  cache->saved_regs_mask |= (1 << (regnum - SPARC_L0_REGNUM));
+	  if (X_OP3 (insn) == 0x7)
+	    cache->saved_regs_mask |= (1 << (regnum + 1 - SPARC_L0_REGNUM));
+	}
+
+      offset += 4;
+
+      insn = sparc_fetch_instruction (pc + offset);
+    }
+
   /* Recognize a SETHI insn and record its destination.  */
   if (X_OP (insn) == 0 && X_OP2 (insn) == 0x04)
     {
       dest = X_RD (insn);
       offset += 4;
 
-      insn = sparc_fetch_instruction (pc + 4);
+      insn = sparc_fetch_instruction (pc + offset);
     }
 
   /* Allow for an arithmetic operation on DEST or %g1.  */
@@ -828,14 +882,62 @@ sparc_analyze_prologue (struct gdbarch *
     {
       offset += 4;
 
-      insn = sparc_fetch_instruction (pc + 8);
+      insn = sparc_fetch_instruction (pc + offset);
     }
 
   /* Check for the SAVE instruction that sets up the frame.  */
   if (X_OP (insn) == 2 && X_OP3 (insn) == 0x3c)
     {
-      cache->frameless_p = 0;
-      return pc + offset + 4;
+      sparc_record_save_insn (cache);
+      offset += 4;
+      return pc + offset;
+    }
+
+  /* Check for an arithmetic operation on %sp.  */
+  if (X_OP (insn) == 2
+      && (X_OP3 (insn) == 0 || X_OP3 (insn) == 0x4)
+      && X_RS1 (insn) == SPARC_SP_REGNUM
+      && X_RD (insn) == SPARC_SP_REGNUM)
+    {
+      if (X_I (insn))
+	{
+	  cache->frame_offset = X_SIMM13 (insn);
+	  if (X_OP3 (insn) == 0)
+	    cache->frame_offset = -cache->frame_offset;
+	}
+      offset += 4;
+
+      insn = sparc_fetch_instruction (pc + offset);
+
+      /* Check for an arithmetic operation that sets up the frame.  */
+      if (X_OP (insn) == 2
+	  && (X_OP3 (insn) == 0 || X_OP3 (insn) == 0x4)
+	  && X_RS1 (insn) == SPARC_SP_REGNUM
+	  && X_RD (insn) == SPARC_FP_REGNUM)
+	{
+	  cache->frameless_p = 0;
+	  cache->frame_offset = 0;
+	  /* We could check that the amount subtracted to %sp above is the
+	     same as the one added here, but this seems superfluous.  */
+	  cache->copied_regs_mask |= 0x40;
+	  offset += 4;
+
+	  insn = sparc_fetch_instruction (pc + offset);
+	}
+
+      /* Check for a move (or) operation that copies the return register.  */
+      if (X_OP (insn) == 2
+	  && X_OP3 (insn) == 0x2
+	  && !X_I (insn)
+	  && X_RS1 (insn) == SPARC_G0_REGNUM
+	  && X_RS2 (insn) == SPARC_O7_REGNUM
+	  && X_RD (insn) == SPARC_I7_REGNUM)
+	{
+	   cache->copied_regs_mask |= 0x80;
+	   offset += 4;
+	}
+
+      return pc + offset;
     }
 
   return pc;
@@ -878,21 +980,36 @@ sparc32_skip_prologue (struct gdbarch *g
      indeed what GCC seems to be doing.  In that case GCC will
      generate debug information that points to the stack slots instead
      of the registers, so we should consider the instructions that
-     write out these incoming arguments onto the stack.  Of course we
-     only need to do this if we have a stack frame.  */
+     write out these incoming arguments onto the stack.  */
 
-  while (!cache.frameless_p)
+  while (1)
     {
       unsigned long insn = sparc_fetch_instruction (start_pc);
 
-      /* Recognize instructions that store incoming arguments in
-         %i0...%i5 into the corresponding stack slot.  */
-      if (X_OP (insn) == 3 && (X_OP3 (insn) & 0x3c) == 0x04 && X_I (insn)
-	  && (X_RD (insn) >= 24 && X_RD (insn) <= 29) && X_RS1 (insn) == 30
-	  && X_SIMM13 (insn) == 68 + (X_RD (insn) - 24) * 4)
+      /* Recognize instructions that store incoming arguments into the
+	 corresponding stack slots.  */
+      if (X_OP (insn) == 3 && (X_OP3 (insn) & 0x3c) == 0x04
+	  && X_I (insn) && X_RS1 (insn) == SPARC_FP_REGNUM)
 	{
-	  start_pc += 4;
-	  continue;
+	  int regnum = X_RD (insn);
+
+	  /* Case of arguments still in %o[0..5].  */
+	  if (regnum >= SPARC_O0_REGNUM && regnum <= SPARC_O5_REGNUM
+	      && !(cache.copied_regs_mask & (1 << (regnum - SPARC_O0_REGNUM)))
+	      && X_SIMM13 (insn) == 68 + (regnum - SPARC_O0_REGNUM) * 4)
+	    {
+	      start_pc += 4;
+	      continue;
+	    }
+
+	  /* Case of arguments copied into %i[0..5].  */
+	  if (regnum >= SPARC_I0_REGNUM && regnum <= SPARC_I5_REGNUM
+	      && (cache.copied_regs_mask & (1 << (regnum - SPARC_I0_REGNUM)))
+	      && X_SIMM13 (insn) == 68 + (regnum - SPARC_I0_REGNUM) * 4)
+	    {
+	      start_pc += 4;
+	      continue;
+	    }
 	}
 
       break;
@@ -935,6 +1052,8 @@ sparc_frame_cache (struct frame_info *th
 	get_frame_register_unsigned (this_frame, SPARC_FP_REGNUM);
     }
 
+  cache->base += cache->frame_offset;
+
   if (cache->base & 1)
     cache->base += BIAS;
 
@@ -983,7 +1102,8 @@ sparc32_frame_cache (struct frame_info *
          an "unimp" instruction.  If it is, then it is a struct-return
          function.  */
       CORE_ADDR pc;
-      int regnum = cache->frameless_p ? SPARC_O7_REGNUM : SPARC_I7_REGNUM;
+      int regnum
+	= (cache->copied_regs_mask & 0x80) ? SPARC_I7_REGNUM : SPARC_O7_REGNUM;
 
       pc = get_frame_register_unsigned (this_frame, regnum) + 8;
       if (sparc_is_unimp_insn (pc))
@@ -1025,7 +1145,8 @@ sparc32_frame_prev_register (struct fram
       if (cache->struct_return_p)
 	pc += 4;
 
-      regnum = cache->frameless_p ? SPARC_O7_REGNUM : SPARC_I7_REGNUM;
+      regnum
+	= (cache->copied_regs_mask & 0x80)? SPARC_I7_REGNUM : SPARC_O7_REGNUM;
       pc += get_frame_register_unsigned (this_frame, regnum) + 8;
       return frame_unwind_got_constant (this_frame, regnum, pc);
     }
@@ -1034,7 +1155,9 @@ sparc32_frame_prev_register (struct fram
   {
     ULONGEST wcookie = sparc_fetch_wcookie (gdbarch);
 
-    if (wcookie != 0 && !cache->frameless_p && regnum == SPARC_I7_REGNUM)
+    if (wcookie != 0
+	&& (cache->copied_regs_mask & 0x80)
+	&& regnum == SPARC_I7_REGNUM)
       {
         CORE_ADDR addr = cache->base + (regnum - SPARC_L0_REGNUM) * 4;
         ULONGEST i7;
@@ -1045,20 +1168,20 @@ sparc32_frame_prev_register (struct fram
       }
   }
 
-  /* The previous frame's `local' and `in' registers have been saved
+  /* The previous frame's `local' and `in' registers may have been saved
      in the register save area.  */
-  if (!cache->frameless_p
-      && regnum >= SPARC_L0_REGNUM && regnum <= SPARC_I7_REGNUM)
+  if (regnum >= SPARC_L0_REGNUM && regnum <= SPARC_I7_REGNUM
+      && (cache->saved_regs_mask & (1 << (regnum - SPARC_L0_REGNUM))))
     {
       CORE_ADDR addr = cache->base + (regnum - SPARC_L0_REGNUM) * 4;
 
       return frame_unwind_got_memory (this_frame, regnum, addr);
     }
 
-  /* The previous frame's `out' registers are accessible as the
-     current frame's `in' registers.  */
-  if (!cache->frameless_p
-      && regnum >= SPARC_O0_REGNUM && regnum <= SPARC_O7_REGNUM)
+  /* The previous frame's `out' registers may be accessible as the current
+     frame's `in' registers.  */
+  if (regnum >= SPARC_O0_REGNUM && regnum <= SPARC_O7_REGNUM
+      && (cache->copied_regs_mask & (1 << (regnum - SPARC_O0_REGNUM))))
     regnum += (SPARC_I0_REGNUM - SPARC_O0_REGNUM);
 
   return frame_unwind_got_register (this_frame, regnum, regnum);
Index: sparc-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/sparc-tdep.h,v
retrieving revision 1.26
diff -u -p -r1.26 sparc-tdep.h
--- sparc-tdep.h	11 Jan 2011 21:53:24 -0000	1.26
+++ sparc-tdep.h	16 Jun 2011 13:13:24 -0000
@@ -146,6 +146,15 @@ struct sparc_frame_cache
   /* Do we have a frame?  */
   int frameless_p;
 
+  /* The offset from the base register to the CFA.  */
+  int frame_offset;
+
+  /* Mask of `local' and `in' registers saved in the register save area.  */
+  unsigned short int saved_regs_mask;
+
+  /* Mask of `out' registers copied or renamed to their `in' sibling.  */
+  unsigned char copied_regs_mask;
+
   /* Do we have a Structure, Union or Quad-Precision return value?  */
   int struct_return_p;
 
@@ -159,6 +168,10 @@ extern unsigned long sparc_fetch_instruc
 /* Fetch StackGhost Per-Process XOR cookie.  */
 extern ULONGEST sparc_fetch_wcookie (struct gdbarch *gdbarch);
 
+/* Record the effect of a SAVE instruction on CACHE.  */
+extern void sparc_record_save_insn (struct sparc_frame_cache *cache);
+
+/* Do a full analysis of the prologue at PC and update CACHE accordingly.  */
 extern CORE_ADDR sparc_analyze_prologue (struct gdbarch *gdbarch,
 					 CORE_ADDR pc, CORE_ADDR current_pc,
 					 struct sparc_frame_cache *cache);
Index: sparc64-sol2-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc64-sol2-tdep.c,v
retrieving revision 1.23
diff -u -p -r1.23 sparc64-sol2-tdep.c
--- sparc64-sol2-tdep.c	18 Mar 2011 18:52:32 -0000	1.23
+++ sparc64-sol2-tdep.c	16 Jun 2011 13:13:24 -0000
@@ -67,7 +67,7 @@ sparc64_sol2_sigtramp_frame_cache (struc
   /* The third argument is a pointer to an instance of `ucontext_t',
      which has a member `uc_mcontext' that contains the saved
      registers.  */
-  regnum = (cache->frameless_p ? SPARC_O2_REGNUM : SPARC_I2_REGNUM);
+  regnum = (cache->copied_regs_mask & 4) ? SPARC_I2_REGNUM : SPARC_O2_REGNUM;
   mcontext_addr = get_frame_register_unsigned (this_frame, regnum) + 64;
 
   cache->saved_regs[SPARC64_CCR_REGNUM].addr = mcontext_addr + 0 * 8;
Index: sparc64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc64-tdep.c,v
retrieving revision 1.54
diff -u -p -r1.54 sparc64-tdep.c
--- sparc64-tdep.c	21 May 2011 19:19:45 -0000	1.54
+++ sparc64-tdep.c	16 Jun 2011 13:13:27 -0000
@@ -520,7 +520,8 @@ sparc64_frame_prev_register (struct fram
     {
       CORE_ADDR pc = (regnum == SPARC64_NPC_REGNUM) ? 4 : 0;
 
-      regnum = cache->frameless_p ? SPARC_O7_REGNUM : SPARC_I7_REGNUM;
+      regnum
+	= (cache->copied_regs_mask) & 0x80? SPARC_I7_REGNUM : SPARC_O7_REGNUM;
       pc += get_frame_register_unsigned (this_frame, regnum) + 8;
       return frame_unwind_got_constant (this_frame, regnum, pc);
     }
@@ -529,7 +530,9 @@ sparc64_frame_prev_register (struct fram
   {
     ULONGEST wcookie = sparc_fetch_wcookie (gdbarch);
 
-    if (wcookie != 0 && !cache->frameless_p && regnum == SPARC_I7_REGNUM)
+    if (wcookie != 0
+	&& (cache->copied_regs_mask & 0x80)
+	&& regnum == SPARC_I7_REGNUM)
       {
         CORE_ADDR addr = cache->base + (regnum - SPARC_L0_REGNUM) * 8;
         ULONGEST i7;
@@ -540,20 +543,20 @@ sparc64_frame_prev_register (struct fram
       }
   }
 
-  /* The previous frame's `local' and `in' registers have been saved
+  /* The previous frame's `local' and `in' registers may have been saved
      in the register save area.  */
-  if (!cache->frameless_p
-      && regnum >= SPARC_L0_REGNUM && regnum <= SPARC_I7_REGNUM)
+  if (regnum >= SPARC_L0_REGNUM && regnum <= SPARC_I7_REGNUM
+      && (cache->saved_regs_mask & (1 << (regnum - SPARC_L0_REGNUM))))
     {
       CORE_ADDR addr = cache->base + (regnum - SPARC_L0_REGNUM) * 8;
 
       return frame_unwind_got_memory (this_frame, regnum, addr);
     }
 
-  /* The previous frame's `out' registers are accessable as the
-     current frame's `in' registers.  */
-  if (!cache->frameless_p
-      && regnum >= SPARC_O0_REGNUM && regnum <= SPARC_O7_REGNUM)
+  /* The previous frame's `out' registers may be accessible as the current
+     frame's `in' registers.  */
+  if (regnum >= SPARC_O0_REGNUM && regnum <= SPARC_O7_REGNUM
+      && (cache->copied_regs_mask & (1 << (regnum - SPARC_O0_REGNUM))))
     regnum += (SPARC_I0_REGNUM - SPARC_O0_REGNUM);
 
   return frame_unwind_got_register (this_frame, regnum, regnum);
Index: sparc64nbsd-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc64nbsd-tdep.c,v
retrieving revision 1.27
diff -u -p -r1.27 sparc64nbsd-tdep.c
--- sparc64nbsd-tdep.c	18 Mar 2011 18:52:32 -0000	1.27
+++ sparc64nbsd-tdep.c	16 Jun 2011 13:13:27 -0000
@@ -173,7 +173,7 @@ sparc64nbsd_sigcontext_frame_cache (stru
 
       /* Since we couldn't find the frame's function, the cache was
          initialized under the assumption that we're frameless.  */
-      cache->frameless_p = 0;
+      sparc_record_save_insn (cache);
       addr = get_frame_register_unsigned (this_frame, SPARC_FP_REGNUM);
       if (addr & 1)
 	addr += BIAS;
Index: sparc64obsd-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc64obsd-tdep.c,v
retrieving revision 1.30
diff -u -p -r1.30 sparc64obsd-tdep.c
--- sparc64obsd-tdep.c	18 Mar 2011 18:52:32 -0000	1.30
+++ sparc64obsd-tdep.c	16 Jun 2011 13:13:27 -0000
@@ -142,7 +142,7 @@ sparc64obsd_frame_cache (struct frame_in
 
       /* Since we couldn't find the frame's function, the cache was
          initialized under the assumption that we're frameless.  */
-      cache->frameless_p = 0;
+      sparc_record_save_insn (cache);
       addr = get_frame_register_unsigned (this_frame, SPARC_FP_REGNUM);
       if (addr & 1)
 	addr += BIAS;
Index: sparcnbsd-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparcnbsd-tdep.c,v
retrieving revision 1.41
diff -u -p -r1.41 sparcnbsd-tdep.c
--- sparcnbsd-tdep.c	18 Mar 2011 18:52:32 -0000	1.41
+++ sparcnbsd-tdep.c	16 Jun 2011 13:13:27 -0000
@@ -202,7 +202,7 @@ sparc32nbsd_sigcontext_frame_cache (stru
 
       /* Since we couldn't find the frame's function, the cache was
          initialized under the assumption that we're frameless.  */
-      cache->frameless_p = 0;
+      sparc_record_save_insn (cache);
       addr = get_frame_register_unsigned (this_frame, SPARC_FP_REGNUM);
       cache->base = addr;
     }
Index: sparcobsd-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparcobsd-tdep.c,v
retrieving revision 1.20
diff -u -p -r1.20 sparcobsd-tdep.c
--- sparcobsd-tdep.c	18 Mar 2011 18:52:32 -0000	1.20
+++ sparcobsd-tdep.c	16 Jun 2011 13:13:27 -0000
@@ -91,7 +91,7 @@ sparc32obsd_sigtramp_frame_cache (struct
 
       /* Since we couldn't find the frame's function, the cache was
          initialized under the assumption that we're frameless.  */
-      cache->frameless_p = 0;
+      sparc_record_save_insn (cache);
       addr = get_frame_register_unsigned (this_frame, SPARC_FP_REGNUM);
       cache->base = addr;
     }

Re: [patch] Add support for single register window model on SPARC

[Mark Kettenis]

> From: Eric Botcazou <ebotcazou@adacore.com>
> Date: Thu, 16 Jun 2011 15:36:08 +0200
> 
> Hi,
> 
> support for the single register window (aka flat) model on SPARC was just 
> re-introduced in GCC: http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00820.html
> The -mflat option had been present in the 3.x series of compilers but was 
> removed when the 4.x series debuted.  Due to renewed interest, most notably 
> from the LEON folks, it will be supported again in future GCC releases.

Ugh, that makes me feel dirty.  But I guess it makes some sense to
want this for a RT environment.

> The implementation has been almost entirely overhauled and, in
> particular, the weird register usage of the old flavor (e.g. %i7 as
> frame pointer) has been dropped.  Instead the new flavor preserves
> the canonical register usage and frame layout; the only visible
> change is that 'save' & 'restore' instructions are replaced with
> their "manual" equivalents in the generated code.
> 
> While CFIs are adjusted automatically by GCC, GDB has hardcoded assumptions 
> about how frames are established on SPARC, which makes it unable to unwind 
> the "flat" frames; in particular backtraces don't work.

Hmm, if full CFI for the explicit register saving code is emitted, I'd
expect backtraces to work.

> The attached patch is aimed at fixing that.  It extends the frame
> sniffer to recognize the "flat" frames and decode the locations of
> call-saved registers.  Regtested (in normal mode) on SPARC/Solaris
> and SPARC64/Solaris.  It was also used to debug the new -mflat
> implementation of GCC, both 32-bit and 64-bit, so it works
> reasonably well in this mode.
> 
> OK for the mainline?

I'd like to get a chance to run a regression test on OpenBSD/sparc64.
Can you give me a couple of days and perhaps ping me towards the end
of this week to remind me if I haven't responded by then?

The code itself makes sense to me, a few minor nits inline below.

Cheers,

Mark

> 2011-06-16  Eric Botcazou  <ebotcazou@adacore.com>
> 
> 	* sparc-tdep.h (struct sparc_frame_cache): Add frame_offset,
> 	saved_regs_mask and copied_regs_mask fields.
> 	(sparc_record_save_insn): New prototype.
> 	* sparc-tdep.c (sparc_alloc_frame_cache): Initialize the new fields.
> 	(sparc_record_save_insn): New function.
> 	(sparc_analyze_prologue): Add head comment.  Recognize store insns
> 	of call-saved registers.  Use OFFSET consistently.  Recognize flat
> 	frames and cache their settings.
> 	(sparc32_skip_prologue): Handle flat frames.
> 	(sparc_frame_cache): Add frame_offset to the base address.
> 	(sparc32_frame_cache): Adjust to new frame description.
> 	(sparc32_frame_prev_register): Likewise.
> 	* sparc64-tdep.c (sparc64_frame_prev_register): Likewise.
> 	* sparc-sol2-tdep.c (sparc32_sol2_sigtramp_frame_cache): Likewise.
> 	* sparc64-sol2-tdep.c (sparc64_sol2_sigtramp_frame_cache): Likewise.
> 	* sparcnbsd-tdep.c (sparc32nbsd_sigcontext_frame_cache): Force the
> 	frame by calling sparc_record_save_insn.
> 	* sparc64nbsd-tdep.c (sparc64nbsd_sigcontext_frame_cache): Likewise.
> 	* sparcobsd-tdep.c (sparc32obsd_sigtramp_frame_cache): Likewise.
> 	* sparc64obsd-tdep.c (sparc64obsd_frame_cache): Likewise.
> 
> Index: sparc-sol2-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/sparc-sol2-tdep.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 sparc-sol2-tdep.c
> --- sparc-sol2-tdep.c	18 Mar 2011 18:52:32 -0000	1.25
> +++ sparc-sol2-tdep.c	16 Jun 2011 13:13:24 -0000
> @@ -93,7 +93,7 @@ sparc32_sol2_sigtramp_frame_cache (struc
>    /* The third argument is a pointer to an instance of `ucontext_t',
>       which has a member `uc_mcontext' that contains the saved
>       registers.  */
> -  regnum = (cache->frameless_p ? SPARC_O2_REGNUM : SPARC_I2_REGNUM);
> +  regnum = (cache->copied_regs_mask & 4) ? SPARC_I2_REGNUM : SPARC_O2_REGNUM;

For consistency that probably should be (cache->copied_regs_mask & 0x04).

> Index: sparc-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/sparc-tdep.c,v
> retrieving revision 1.221
> diff -u -p -r1.221 sparc-tdep.c
> --- sparc-tdep.c	23 May 2011 16:38:05 -0000	1.221
> +++ sparc-tdep.c	16 Jun 2011 13:13:24 -0000
> @@ -983,7 +1102,8 @@ sparc32_frame_cache (struct frame_info *
>           an "unimp" instruction.  If it is, then it is a struct-return
>           function.  */
>        CORE_ADDR pc;
> -      int regnum = cache->frameless_p ? SPARC_O7_REGNUM : SPARC_I7_REGNUM;
> +      int regnum
> +	= (cache->copied_regs_mask & 0x80) ? SPARC_I7_REGNUM : SPARC_O7_REGNUM;

Elsewhere in GDB, or at least in the code written by me, like most of
the existing sparc/sparc64 code places the '=' sign on the line
before.  I can see how you can interpret the GNU coding style to say
that you should put the '=' sign on the next line, but for now it'd be
better to be consistent with the rest of the code.  Can you change
this?

>        pc = get_frame_register_unsigned (this_frame, regnum) + 8;
>        if (sparc_is_unimp_insn (pc))
> @@ -1025,7 +1145,8 @@ sparc32_frame_prev_register (struct fram
>        if (cache->struct_return_p)
>  	pc += 4;
>  
> -      regnum = cache->frameless_p ? SPARC_O7_REGNUM : SPARC_I7_REGNUM;
> +      regnum
> +	= (cache->copied_regs_mask & 0x80)? SPARC_I7_REGNUM : SPARC_O7_REGNUM;

Same here.

> @@ -1034,7 +1155,9 @@ sparc32_frame_prev_register (struct fram
>    {
>      ULONGEST wcookie = sparc_fetch_wcookie (gdbarch);
>  
> -    if (wcookie != 0 && !cache->frameless_p && regnum == SPARC_I7_REGNUM)
> +    if (wcookie != 0
> +	&& (cache->copied_regs_mask & 0x80)
> +	&& regnum == SPARC_I7_REGNUM)

This change isn't quite right.  The wcookie stuff is there to support
StackGhost[1], which is unsupportable for the "flat" model.  So what
needs to be checked here is whether we executed a save instruction or
not.  As far as I know OpenBSD is the only OS that does StackGhost.
On everything else wcookie will be 0.  So you can probably leave this
as is right now, since the existing frameless_p check is more correct
than what you have in your diff.

> Index: sparc64-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/sparc64-tdep.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 sparc64-tdep.c
> --- sparc64-tdep.c	21 May 2011 19:19:45 -0000	1.54
> +++ sparc64-tdep.c	16 Jun 2011 13:13:27 -0000
> @@ -520,7 +520,8 @@ sparc64_frame_prev_register (struct fram
>      {
>        CORE_ADDR pc = (regnum == SPARC64_NPC_REGNUM) ? 4 : 0;
>  
> -      regnum = cache->frameless_p ? SPARC_O7_REGNUM : SPARC_I7_REGNUM;
> +      regnum
> +	= (cache->copied_regs_mask) & 0x80? SPARC_I7_REGNUM : SPARC_O7_REGNUM;

Missing space after between 0x80 and ?; same comment about '=' as above.

>        pc += get_frame_register_unsigned (this_frame, regnum) + 8;
>        return frame_unwind_got_constant (this_frame, regnum, pc);
>      }
> @@ -529,7 +530,9 @@ sparc64_frame_prev_register (struct fram
>    {
>      ULONGEST wcookie = sparc_fetch_wcookie (gdbarch);
>  
> -    if (wcookie != 0 && !cache->frameless_p && regnum == SPARC_I7_REGNUM)
> +    if (wcookie != 0
> +	&& (cache->copied_regs_mask & 0x80)
> +	&& regnum == SPARC_I7_REGNUM)

StackGhost again.  Better leave this alone as well.

[1] http://projects.cerias.purdue.edu/stackghost/stackghost/index.html

Re: [patch] Add support for single register window model on SPARC

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 1850 bytes --]

> Hmm, if full CFI for the explicit register saving code is emitted, I'd
> expect backtraces to work.

Exceptions work well so the CFIs are very likely correct, at least in eh_frame.
A quick look didn't reveal differences between eh_frame and debug_frame so this 
is a little strange then.  Is GDB supposed to fall back to "manual" unwinding 
only when the DWARF unwinder gives up?

> I'd like to get a chance to run a regression test on OpenBSD/sparc64.
> Can you give me a couple of days and perhaps ping me towards the end
> of this week to remind me if I haven't responded by then?

Sure.

> For consistency that probably should be (cache->copied_regs_mask & 0x04).

Fixed.

> Elsewhere in GDB, or at least in the code written by me, like most of
> the existing sparc/sparc64 code places the '=' sign on the line
> before.  I can see how you can interpret the GNU coding style to say
> that you should put the '=' sign on the next line, but for now it'd be
> better to be consistent with the rest of the code.  Can you change
> this?

Sure, done.  This was the standard GCC style.

> This change isn't quite right.  The wcookie stuff is there to support
> StackGhost[1], which is unsupportable for the "flat" model.  So what
> needs to be checked here is whether we executed a save instruction or
> not.  As far as I know OpenBSD is the only OS that does StackGhost.
> On everything else wcookie will be 0.  So you can probably leave this
> as is right now, since the existing frameless_p check is more correct
> than what you have in your diff.

OK, reverted.

> StackGhost again.  Better leave this alone as well.

Yes, there is a bit of code duplication in these files. ;-)

> [1] http://projects.cerias.purdue.edu/stackghost/stackghost/index.html

Thanks for the pointer.


Revised patch attached, will retest if approved.

-- 
Eric Botcazou

[-- Attachment #2: gdb-mflat-2.diff --]
[-- Type: text/x-diff, Size: 16463 bytes --]

Index: sparc-sol2-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc-sol2-tdep.c,v
retrieving revision 1.25
diff -u -p -r1.25 sparc-sol2-tdep.c
--- sparc-sol2-tdep.c	18 Mar 2011 18:52:32 -0000	1.25
+++ sparc-sol2-tdep.c	21 Jun 2011 12:56:00 -0000
@@ -93,7 +93,8 @@ sparc32_sol2_sigtramp_frame_cache (struc
   /* The third argument is a pointer to an instance of `ucontext_t',
      which has a member `uc_mcontext' that contains the saved
      registers.  */
-  regnum = (cache->frameless_p ? SPARC_O2_REGNUM : SPARC_I2_REGNUM);
+  regnum =
+    (cache->copied_regs_mask & 0x04) ? SPARC_I2_REGNUM : SPARC_O2_REGNUM;
   mcontext_addr = get_frame_register_unsigned (this_frame, regnum) + 40;
 
   cache->saved_regs[SPARC32_PSR_REGNUM].addr = mcontext_addr + 0 * 4;
Index: sparc-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc-tdep.c,v
retrieving revision 1.221
diff -u -p -r1.221 sparc-tdep.c
--- sparc-tdep.c	23 May 2011 16:38:05 -0000	1.221
+++ sparc-tdep.c	21 Jun 2011 12:56:00 -0000
@@ -592,7 +592,9 @@ sparc_alloc_frame_cache (void)
 
   /* Frameless until proven otherwise.  */
   cache->frameless_p = 1;
-
+  cache->frame_offset = 0;
+  cache->saved_regs_mask = 0;
+  cache->copied_regs_mask = 0;
   cache->struct_return_p = 0;
 
   return cache;
@@ -784,6 +786,31 @@ sparc_skip_stack_check (const CORE_ADDR 
   return start_pc;
 }
 
+/* Record the effect of a SAVE instruction on CACHE.  */
+
+void
+sparc_record_save_insn (struct sparc_frame_cache *cache)
+{
+  /* The frame is set up.  */
+  cache->frameless_p = 0;
+
+  /* The frame pointer contains the CFA.  */
+  cache->frame_offset = 0;
+
+  /* The `local' and `in' registers are all saved.  */
+  cache->saved_regs_mask = 0xffff;
+
+  /* The `out' registers are all renamed.  */
+  cache->copied_regs_mask = 0xff;
+}
+
+/* Do a full analysis of the prologue at PC and update CACHE accordingly.
+   Bail out early if CURRENT_PC is reached.  Return the address where
+   the analysis stopped.
+
+   We handle both the traditional register window model and the single
+   register window (aka flat) model.  */
+
 CORE_ADDR
 sparc_analyze_prologue (struct gdbarch *gdbarch, CORE_ADDR pc,
 			CORE_ADDR current_pc, struct sparc_frame_cache *cache)
@@ -813,13 +840,40 @@ sparc_analyze_prologue (struct gdbarch *
 
   insn = sparc_fetch_instruction (pc);
 
+  /* Recognize store insns and record their sources.  */
+  while (X_OP (insn) == 3
+	 && (X_OP3 (insn) == 0x4     /* stw */
+	     || X_OP3 (insn) == 0x7  /* std */
+	     || X_OP3 (insn) == 0xe) /* stx */
+	 && X_RS1 (insn) == SPARC_SP_REGNUM)
+    {
+      int regnum = X_RD (insn);
+
+      /* Recognize stores into the corresponding stack slots.  */
+      if (regnum >= SPARC_L0_REGNUM && regnum <= SPARC_I7_REGNUM
+	  && ((X_I (insn)
+	       && X_SIMM13 (insn) == (X_OP3 (insn) == 0xe
+				      ? (regnum - SPARC_L0_REGNUM) * 8 + BIAS
+				      : (regnum - SPARC_L0_REGNUM) * 4))
+	      || (!X_I (insn) && regnum == SPARC_L0_REGNUM)))
+	{
+	  cache->saved_regs_mask |= (1 << (regnum - SPARC_L0_REGNUM));
+	  if (X_OP3 (insn) == 0x7)
+	    cache->saved_regs_mask |= (1 << (regnum + 1 - SPARC_L0_REGNUM));
+	}
+
+      offset += 4;
+
+      insn = sparc_fetch_instruction (pc + offset);
+    }
+
   /* Recognize a SETHI insn and record its destination.  */
   if (X_OP (insn) == 0 && X_OP2 (insn) == 0x04)
     {
       dest = X_RD (insn);
       offset += 4;
 
-      insn = sparc_fetch_instruction (pc + 4);
+      insn = sparc_fetch_instruction (pc + offset);
     }
 
   /* Allow for an arithmetic operation on DEST or %g1.  */
@@ -828,14 +882,62 @@ sparc_analyze_prologue (struct gdbarch *
     {
       offset += 4;
 
-      insn = sparc_fetch_instruction (pc + 8);
+      insn = sparc_fetch_instruction (pc + offset);
     }
 
   /* Check for the SAVE instruction that sets up the frame.  */
   if (X_OP (insn) == 2 && X_OP3 (insn) == 0x3c)
     {
-      cache->frameless_p = 0;
-      return pc + offset + 4;
+      sparc_record_save_insn (cache);
+      offset += 4;
+      return pc + offset;
+    }
+
+  /* Check for an arithmetic operation on %sp.  */
+  if (X_OP (insn) == 2
+      && (X_OP3 (insn) == 0 || X_OP3 (insn) == 0x4)
+      && X_RS1 (insn) == SPARC_SP_REGNUM
+      && X_RD (insn) == SPARC_SP_REGNUM)
+    {
+      if (X_I (insn))
+	{
+	  cache->frame_offset = X_SIMM13 (insn);
+	  if (X_OP3 (insn) == 0)
+	    cache->frame_offset = -cache->frame_offset;
+	}
+      offset += 4;
+
+      insn = sparc_fetch_instruction (pc + offset);
+
+      /* Check for an arithmetic operation that sets up the frame.  */
+      if (X_OP (insn) == 2
+	  && (X_OP3 (insn) == 0 || X_OP3 (insn) == 0x4)
+	  && X_RS1 (insn) == SPARC_SP_REGNUM
+	  && X_RD (insn) == SPARC_FP_REGNUM)
+	{
+	  cache->frameless_p = 0;
+	  cache->frame_offset = 0;
+	  /* We could check that the amount subtracted to %sp above is the
+	     same as the one added here, but this seems superfluous.  */
+	  cache->copied_regs_mask |= 0x40;
+	  offset += 4;
+
+	  insn = sparc_fetch_instruction (pc + offset);
+	}
+
+      /* Check for a move (or) operation that copies the return register.  */
+      if (X_OP (insn) == 2
+	  && X_OP3 (insn) == 0x2
+	  && !X_I (insn)
+	  && X_RS1 (insn) == SPARC_G0_REGNUM
+	  && X_RS2 (insn) == SPARC_O7_REGNUM
+	  && X_RD (insn) == SPARC_I7_REGNUM)
+	{
+	   cache->copied_regs_mask |= 0x80;
+	   offset += 4;
+	}
+
+      return pc + offset;
     }
 
   return pc;
@@ -878,21 +980,36 @@ sparc32_skip_prologue (struct gdbarch *g
      indeed what GCC seems to be doing.  In that case GCC will
      generate debug information that points to the stack slots instead
      of the registers, so we should consider the instructions that
-     write out these incoming arguments onto the stack.  Of course we
-     only need to do this if we have a stack frame.  */
+     write out these incoming arguments onto the stack.  */
 
-  while (!cache.frameless_p)
+  while (1)
     {
       unsigned long insn = sparc_fetch_instruction (start_pc);
 
-      /* Recognize instructions that store incoming arguments in
-         %i0...%i5 into the corresponding stack slot.  */
-      if (X_OP (insn) == 3 && (X_OP3 (insn) & 0x3c) == 0x04 && X_I (insn)
-	  && (X_RD (insn) >= 24 && X_RD (insn) <= 29) && X_RS1 (insn) == 30
-	  && X_SIMM13 (insn) == 68 + (X_RD (insn) - 24) * 4)
+      /* Recognize instructions that store incoming arguments into the
+	 corresponding stack slots.  */
+      if (X_OP (insn) == 3 && (X_OP3 (insn) & 0x3c) == 0x04
+	  && X_I (insn) && X_RS1 (insn) == SPARC_FP_REGNUM)
 	{
-	  start_pc += 4;
-	  continue;
+	  int regnum = X_RD (insn);
+
+	  /* Case of arguments still in %o[0..5].  */
+	  if (regnum >= SPARC_O0_REGNUM && regnum <= SPARC_O5_REGNUM
+	      && !(cache.copied_regs_mask & (1 << (regnum - SPARC_O0_REGNUM)))
+	      && X_SIMM13 (insn) == 68 + (regnum - SPARC_O0_REGNUM) * 4)
+	    {
+	      start_pc += 4;
+	      continue;
+	    }
+
+	  /* Case of arguments copied into %i[0..5].  */
+	  if (regnum >= SPARC_I0_REGNUM && regnum <= SPARC_I5_REGNUM
+	      && (cache.copied_regs_mask & (1 << (regnum - SPARC_I0_REGNUM)))
+	      && X_SIMM13 (insn) == 68 + (regnum - SPARC_I0_REGNUM) * 4)
+	    {
+	      start_pc += 4;
+	      continue;
+	    }
 	}
 
       break;
@@ -935,6 +1052,8 @@ sparc_frame_cache (struct frame_info *th
 	get_frame_register_unsigned (this_frame, SPARC_FP_REGNUM);
     }
 
+  cache->base += cache->frame_offset;
+
   if (cache->base & 1)
     cache->base += BIAS;
 
@@ -983,7 +1102,8 @@ sparc32_frame_cache (struct frame_info *
          an "unimp" instruction.  If it is, then it is a struct-return
          function.  */
       CORE_ADDR pc;
-      int regnum = cache->frameless_p ? SPARC_O7_REGNUM : SPARC_I7_REGNUM;
+      int regnum =
+	(cache->copied_regs_mask & 0x80) ? SPARC_I7_REGNUM : SPARC_O7_REGNUM;
 
       pc = get_frame_register_unsigned (this_frame, regnum) + 8;
       if (sparc_is_unimp_insn (pc))
@@ -1025,7 +1145,8 @@ sparc32_frame_prev_register (struct fram
       if (cache->struct_return_p)
 	pc += 4;
 
-      regnum = cache->frameless_p ? SPARC_O7_REGNUM : SPARC_I7_REGNUM;
+      regnum =
+	(cache->copied_regs_mask & 0x80) ? SPARC_I7_REGNUM : SPARC_O7_REGNUM;
       pc += get_frame_register_unsigned (this_frame, regnum) + 8;
       return frame_unwind_got_constant (this_frame, regnum, pc);
     }
@@ -1045,20 +1166,20 @@ sparc32_frame_prev_register (struct fram
       }
   }
 
-  /* The previous frame's `local' and `in' registers have been saved
+  /* The previous frame's `local' and `in' registers may have been saved
      in the register save area.  */
-  if (!cache->frameless_p
-      && regnum >= SPARC_L0_REGNUM && regnum <= SPARC_I7_REGNUM)
+  if (regnum >= SPARC_L0_REGNUM && regnum <= SPARC_I7_REGNUM
+      && (cache->saved_regs_mask & (1 << (regnum - SPARC_L0_REGNUM))))
     {
       CORE_ADDR addr = cache->base + (regnum - SPARC_L0_REGNUM) * 4;
 
       return frame_unwind_got_memory (this_frame, regnum, addr);
     }
 
-  /* The previous frame's `out' registers are accessible as the
-     current frame's `in' registers.  */
-  if (!cache->frameless_p
-      && regnum >= SPARC_O0_REGNUM && regnum <= SPARC_O7_REGNUM)
+  /* The previous frame's `out' registers may be accessible as the current
+     frame's `in' registers.  */
+  if (regnum >= SPARC_O0_REGNUM && regnum <= SPARC_O7_REGNUM
+      && (cache->copied_regs_mask & (1 << (regnum - SPARC_O0_REGNUM))))
     regnum += (SPARC_I0_REGNUM - SPARC_O0_REGNUM);
 
   return frame_unwind_got_register (this_frame, regnum, regnum);
Index: sparc-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/sparc-tdep.h,v
retrieving revision 1.26
diff -u -p -r1.26 sparc-tdep.h
--- sparc-tdep.h	11 Jan 2011 21:53:24 -0000	1.26
+++ sparc-tdep.h	21 Jun 2011 12:56:00 -0000
@@ -146,6 +146,15 @@ struct sparc_frame_cache
   /* Do we have a frame?  */
   int frameless_p;
 
+  /* The offset from the base register to the CFA.  */
+  int frame_offset;
+
+  /* Mask of `local' and `in' registers saved in the register save area.  */
+  unsigned short int saved_regs_mask;
+
+  /* Mask of `out' registers copied or renamed to their `in' sibling.  */
+  unsigned char copied_regs_mask;
+
   /* Do we have a Structure, Union or Quad-Precision return value?  */
   int struct_return_p;
 
@@ -159,6 +168,10 @@ extern unsigned long sparc_fetch_instruc
 /* Fetch StackGhost Per-Process XOR cookie.  */
 extern ULONGEST sparc_fetch_wcookie (struct gdbarch *gdbarch);
 
+/* Record the effect of a SAVE instruction on CACHE.  */
+extern void sparc_record_save_insn (struct sparc_frame_cache *cache);
+
+/* Do a full analysis of the prologue at PC and update CACHE accordingly.  */
 extern CORE_ADDR sparc_analyze_prologue (struct gdbarch *gdbarch,
 					 CORE_ADDR pc, CORE_ADDR current_pc,
 					 struct sparc_frame_cache *cache);
Index: sparc64-sol2-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc64-sol2-tdep.c,v
retrieving revision 1.23
diff -u -p -r1.23 sparc64-sol2-tdep.c
--- sparc64-sol2-tdep.c	18 Mar 2011 18:52:32 -0000	1.23
+++ sparc64-sol2-tdep.c	21 Jun 2011 12:56:00 -0000
@@ -67,7 +67,8 @@ sparc64_sol2_sigtramp_frame_cache (struc
   /* The third argument is a pointer to an instance of `ucontext_t',
      which has a member `uc_mcontext' that contains the saved
      registers.  */
-  regnum = (cache->frameless_p ? SPARC_O2_REGNUM : SPARC_I2_REGNUM);
+  regnum =
+    (cache->copied_regs_mask & 0x04) ? SPARC_I2_REGNUM : SPARC_O2_REGNUM;
   mcontext_addr = get_frame_register_unsigned (this_frame, regnum) + 64;
 
   cache->saved_regs[SPARC64_CCR_REGNUM].addr = mcontext_addr + 0 * 8;
Index: sparc64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc64-tdep.c,v
retrieving revision 1.54
diff -u -p -r1.54 sparc64-tdep.c
--- sparc64-tdep.c	21 May 2011 19:19:45 -0000	1.54
+++ sparc64-tdep.c	21 Jun 2011 12:56:00 -0000
@@ -520,7 +520,8 @@ sparc64_frame_prev_register (struct fram
     {
       CORE_ADDR pc = (regnum == SPARC64_NPC_REGNUM) ? 4 : 0;
 
-      regnum = cache->frameless_p ? SPARC_O7_REGNUM : SPARC_I7_REGNUM;
+      regnum =
+	(cache->copied_regs_mask & 0x80) ? SPARC_I7_REGNUM : SPARC_O7_REGNUM;
       pc += get_frame_register_unsigned (this_frame, regnum) + 8;
       return frame_unwind_got_constant (this_frame, regnum, pc);
     }
@@ -540,20 +541,20 @@ sparc64_frame_prev_register (struct fram
       }
   }
 
-  /* The previous frame's `local' and `in' registers have been saved
+  /* The previous frame's `local' and `in' registers may have been saved
      in the register save area.  */
-  if (!cache->frameless_p
-      && regnum >= SPARC_L0_REGNUM && regnum <= SPARC_I7_REGNUM)
+  if (regnum >= SPARC_L0_REGNUM && regnum <= SPARC_I7_REGNUM
+      && (cache->saved_regs_mask & (1 << (regnum - SPARC_L0_REGNUM))))
     {
       CORE_ADDR addr = cache->base + (regnum - SPARC_L0_REGNUM) * 8;
 
       return frame_unwind_got_memory (this_frame, regnum, addr);
     }
 
-  /* The previous frame's `out' registers are accessable as the
-     current frame's `in' registers.  */
-  if (!cache->frameless_p
-      && regnum >= SPARC_O0_REGNUM && regnum <= SPARC_O7_REGNUM)
+  /* The previous frame's `out' registers may be accessible as the current
+     frame's `in' registers.  */
+  if (regnum >= SPARC_O0_REGNUM && regnum <= SPARC_O7_REGNUM
+      && (cache->copied_regs_mask & (1 << (regnum - SPARC_O0_REGNUM))))
     regnum += (SPARC_I0_REGNUM - SPARC_O0_REGNUM);
 
   return frame_unwind_got_register (this_frame, regnum, regnum);
Index: sparc64nbsd-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc64nbsd-tdep.c,v
retrieving revision 1.27
diff -u -p -r1.27 sparc64nbsd-tdep.c
--- sparc64nbsd-tdep.c	18 Mar 2011 18:52:32 -0000	1.27
+++ sparc64nbsd-tdep.c	21 Jun 2011 12:56:00 -0000
@@ -173,7 +173,7 @@ sparc64nbsd_sigcontext_frame_cache (stru
 
       /* Since we couldn't find the frame's function, the cache was
          initialized under the assumption that we're frameless.  */
-      cache->frameless_p = 0;
+      sparc_record_save_insn (cache);
       addr = get_frame_register_unsigned (this_frame, SPARC_FP_REGNUM);
       if (addr & 1)
 	addr += BIAS;
Index: sparc64obsd-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc64obsd-tdep.c,v
retrieving revision 1.30
diff -u -p -r1.30 sparc64obsd-tdep.c
--- sparc64obsd-tdep.c	18 Mar 2011 18:52:32 -0000	1.30
+++ sparc64obsd-tdep.c	21 Jun 2011 12:56:01 -0000
@@ -142,7 +142,7 @@ sparc64obsd_frame_cache (struct frame_in
 
       /* Since we couldn't find the frame's function, the cache was
          initialized under the assumption that we're frameless.  */
-      cache->frameless_p = 0;
+      sparc_record_save_insn (cache);
       addr = get_frame_register_unsigned (this_frame, SPARC_FP_REGNUM);
       if (addr & 1)
 	addr += BIAS;
Index: sparcnbsd-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparcnbsd-tdep.c,v
retrieving revision 1.41
diff -u -p -r1.41 sparcnbsd-tdep.c
--- sparcnbsd-tdep.c	18 Mar 2011 18:52:32 -0000	1.41
+++ sparcnbsd-tdep.c	21 Jun 2011 12:56:01 -0000
@@ -202,7 +202,7 @@ sparc32nbsd_sigcontext_frame_cache (stru
 
       /* Since we couldn't find the frame's function, the cache was
          initialized under the assumption that we're frameless.  */
-      cache->frameless_p = 0;
+      sparc_record_save_insn (cache);
       addr = get_frame_register_unsigned (this_frame, SPARC_FP_REGNUM);
       cache->base = addr;
     }
Index: sparcobsd-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparcobsd-tdep.c,v
retrieving revision 1.20
diff -u -p -r1.20 sparcobsd-tdep.c
--- sparcobsd-tdep.c	18 Mar 2011 18:52:32 -0000	1.20
+++ sparcobsd-tdep.c	21 Jun 2011 12:56:01 -0000
@@ -91,7 +91,7 @@ sparc32obsd_sigtramp_frame_cache (struct
 
       /* Since we couldn't find the frame's function, the cache was
          initialized under the assumption that we're frameless.  */
-      cache->frameless_p = 0;
+      sparc_record_save_insn (cache);
       addr = get_frame_register_unsigned (this_frame, SPARC_FP_REGNUM);
       cache->base = addr;
     }

Re: [patch] Add support for single register window model on SPARC

[Mark Kettenis]

> From: Eric Botcazou <ebotcazou@adacore.com>
> Date: Tue, 21 Jun 2011 20:08:10 +0200
> 
> > Hmm, if full CFI for the explicit register saving code is emitted, I'd
> > expect backtraces to work.
> 
> Exceptions work well so the CFIs are very likely correct, at least
> in eh_frame.  A quick look didn't reveal differences between
> eh_frame and debug_frame so this is a little strange then.  Is GDB
> supposed to fall back to "manual" unwinding only when the DWARF
> unwinder gives up?

Typically, yes, but because of:

  /* FIXME: kettenis/20050423: Don't enable the unwinder until the
     StackGhost issues have been resolved.  */

we only install the DWARF unwinder for Linux, and I guess you tested
on some other platform.

> > StackGhost again.  Better leave this alone as well.
> 
> Yes, there is a bit of code duplication in these files. ;-)

Somewhat difficult to avoid though.

> Revised patch attached, will retest if approved.

No regression on OpenBSD/sparc64, so all fine with me.

Re: [patch] Add support for single register window model on SPARC

[Eric Botcazou]

> Typically, yes, but because of:
>
>   /* FIXME: kettenis/20050423: Don't enable the unwinder until the
>      StackGhost issues have been resolved.  */
>
> we only install the DWARF unwinder for Linux, and I guess you tested
> on some other platform.

Yes, on Solaris.  I'll give it a whirl on Linux to see how the DWARF unwinder 
fares then.

> No regression on OpenBSD/sparc64, so all fine with me.

Thanks for the review.


Joel, I have commit rights to src but I'm not listed in MAINTAINERS so would 
you mind applying the revised patch (gdb-mflat-2.diff) attached to my previous 
message in the thread?  TIA.


2011-06-27  Eric Botcazou  <ebotcazou@adacore.com>

        * sparc-tdep.h (struct sparc_frame_cache): Add frame_offset,
        saved_regs_mask and copied_regs_mask fields.
        (sparc_record_save_insn): New prototype.
        * sparc-tdep.c (sparc_alloc_frame_cache): Initialize the new fields.
        (sparc_record_save_insn): New function.
        (sparc_analyze_prologue): Add head comment.  Recognize store insns
        of call-saved registers.  Use OFFSET consistently.  Recognize flat
        frames and cache their settings.
        (sparc32_skip_prologue): Handle flat frames.
        (sparc_frame_cache): Add frame_offset to the base address.
        (sparc32_frame_cache): Adjust to new frame description.
        (sparc32_frame_prev_register): Likewise.
        * sparc64-tdep.c (sparc64_frame_prev_register): Likewise.
        * sparc-sol2-tdep.c (sparc32_sol2_sigtramp_frame_cache): Likewise.
        * sparc64-sol2-tdep.c (sparc64_sol2_sigtramp_frame_cache): Likewise.
        * sparcnbsd-tdep.c (sparc32nbsd_sigcontext_frame_cache): Force the
        frame by calling sparc_record_save_insn.
        * sparc64nbsd-tdep.c (sparc64nbsd_sigcontext_frame_cache): Likewise.
        * sparcobsd-tdep.c (sparc32obsd_sigtramp_frame_cache): Likewise.
        * sparc64obsd-tdep.c (sparc64obsd_frame_cache): Likewise.


-- 
Eric Botcazou

Re: [patch] Add support for single register window model on SPARC

[Joel Brobecker]

> Joel, I have commit rights to src but I'm not listed in MAINTAINERS so
> would you mind applying the revised patch (gdb-mflat-2.diff) attached
> to my previous message in the thread?  TIA.

Sure! Now done.  And I've also taken the liberty to add you to
the MAINTAINERS file, so you don't have to wait on me (although
I'll be happy to commit for you if more convenient).

> 2011-06-27  Eric Botcazou  <ebotcazou@adacore.com>
> 
>         * sparc-tdep.h (struct sparc_frame_cache): Add frame_offset,
>         saved_regs_mask and copied_regs_mask fields.
>         (sparc_record_save_insn): New prototype.
>         * sparc-tdep.c (sparc_alloc_frame_cache): Initialize the new fields.
>         (sparc_record_save_insn): New function.
>         (sparc_analyze_prologue): Add head comment.  Recognize store insns
>         of call-saved registers.  Use OFFSET consistently.  Recognize flat
>         frames and cache their settings.
>         (sparc32_skip_prologue): Handle flat frames.
>         (sparc_frame_cache): Add frame_offset to the base address.
>         (sparc32_frame_cache): Adjust to new frame description.
>         (sparc32_frame_prev_register): Likewise.
>         * sparc64-tdep.c (sparc64_frame_prev_register): Likewise.
>         * sparc-sol2-tdep.c (sparc32_sol2_sigtramp_frame_cache): Likewise.
>         * sparc64-sol2-tdep.c (sparc64_sol2_sigtramp_frame_cache): Likewise.
>         * sparcnbsd-tdep.c (sparc32nbsd_sigcontext_frame_cache): Force the
>         frame by calling sparc_record_save_insn.
>         * sparc64nbsd-tdep.c (sparc64nbsd_sigcontext_frame_cache): Likewise.
>         * sparcobsd-tdep.c (sparc32obsd_sigtramp_frame_cache): Likewise.
>         * sparc64obsd-tdep.c (sparc64obsd_frame_cache): Likewise.

-- 
Joel

Re: [patch] Add support for single register window model on SPARC

[Eric Botcazou]

> Sure! Now done.  And I've also taken the liberty to add you to
> the MAINTAINERS file, so you don't have to wait on me (although
> I'll be happy to commit for you if more convenient).

Great, thanks.  I've changed the email address to ebotcazou@libertysurf.fr (the 
one mentioned in my personal FSF copyright assignment) to match the GCC setup.

-- 
Eric Botcazou

Re: [patch] Add support for single register window model on SPARC

[Eric Botcazou]

> Typically, yes, but because of:
>
>   /* FIXME: kettenis/20050423: Don't enable the unwinder until the
>      StackGhost issues have been resolved.  */
>
> we only install the DWARF unwinder for Linux, and I guess you tested
> on some other platform.

Backtraces also work fine on Linux, both 32-bit and 64-bit.  Very nice.

-- 
Eric Botcazou