Fix PR breakpoints/2080

Fix PR breakpoints/2080

[Mark Kettenis]

This fixes the testcase I checked in yesterday, so I committed it.

Fred, can you check whether this also fixes your problem?

Mark

Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	Fix PR breakpoints/2080.
	* i386-tdep.c (struct i386_frame_cache): Add stack_align member.
	(i386_analyze_stack_align): New function.
	(i386_analyze_prologue): Use i386_analyze_stack_align.
	(i386_frame_cache): Deal with stack realignment.

Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.222
diff -u -p -r1.222 i386-tdep.c
--- i386-tdep.c 21 Jan 2006 20:59:50 -0000 1.222
+++ i386-tdep.c 14 Feb 2006 15:40:43 -0000
@@ -308,6 +308,7 @@ struct i386_frame_cache
   /* Saved registers.  */
   CORE_ADDR saved_regs[I386_NUM_SAVED_REGS];
   CORE_ADDR saved_sp;
+  int stack_align;
   int pc_in_eax;
 
   /* Stack space reserved for local variables.  */
@@ -334,6 +335,7 @@ i386_alloc_frame_cache (void)
   for (i = 0; i < I386_NUM_SAVED_REGS; i++)
     cache->saved_regs[i] = -1;
   cache->saved_sp = 0;
+  cache->stack_align = 0;
   cache->pc_in_eax = 0;
 
   /* Frameless until proven otherwise.  */
@@ -485,6 +487,33 @@ i386_skip_probe (CORE_ADDR pc)
   return pc;
 }
 
+/* GCC 4.1 and later, can put code in the prologue to realign the
+   stack pointer.  Check whether PC points to such code, and update
+   CACHE accordingly.  Return the first instruction after the code
+   sequence or CURRENT_PC, whichever is smaller.  If we don't
+   recognize the code, return PC.  */
+
+static CORE_ADDR
+i386_analyze_stack_align (CORE_ADDR pc, CORE_ADDR current_pc,
+			  struct i386_frame_cache *cache)
+{
+  static const gdb_byte insns[10] = { 
+    0x8d, 0x4c, 0x24, 0x04,	/* leal  4(%esp), %ecx */
+    0x83, 0xe4, 0xf0,		/* andl  $-16, %esp */
+    0xff, 0x71, 0xfc		/* pushl -4(%ecx) */
+  };
+  gdb_byte buf[10];
+
+  if (target_read_memory (pc, buf, sizeof buf)
+      || memcmp (buf, insns, sizeof buf) != 0)
+    return pc;
+
+  if (current_pc > pc + 4)
+    cache->stack_align = 1;
+
+  return min (pc + 10, current_pc);
+}
+
 /* Maximum instruction length we need to handle.  */
 #define I386_MAX_INSN_LEN	6
 
@@ -777,6 +806,7 @@ i386_analyze_prologue (CORE_ADDR pc, COR
   pc = i386_follow_jump (pc);
   pc = i386_analyze_struct_return (pc, current_pc, cache);
   pc = i386_skip_probe (pc);
+  pc = i386_analyze_stack_align (pc, current_pc, cache);
   pc = i386_analyze_frame_setup (pc, current_pc, cache);
   return i386_analyze_register_saves (pc, current_pc, cache);
 }
@@ -907,6 +937,13 @@ i386_frame_cache (struct frame_info *nex
   if (cache->pc != 0)
     i386_analyze_prologue (cache->pc, frame_pc_unwind (next_frame), cache);
 
+  if (cache->stack_align)
+    {
+      /* Saved stack pointer has been saved in %ecx.  */
+      frame_unwind_register (next_frame, I386_ECX_REGNUM, buf);
+      cache->saved_sp = extract_unsigned_integer(buf, 4);
+    }
+
   if (cache->locals < 0)
     {
       /* We didn't find a valid frame, which means that CACHE->base
@@ -917,13 +954,26 @@ i386_frame_cache (struct frame_info *nex
 	 frame by looking at the stack pointer.  For truly "frameless"
 	 functions this might work too.  */
 
-      frame_unwind_register (next_frame, I386_ESP_REGNUM, buf);
-      cache->base = extract_unsigned_integer (buf, 4) + cache->sp_offset;
+      if (cache->stack_align)
+	{
+	  /* We're halfway aligning the stack.  */
+	  cache->base = ((cache->saved_sp - 4) & 0xfffffff0) - 4;
+	  cache->saved_regs[I386_EIP_REGNUM] = cache->saved_sp - 4;
+
+	  /* This will be added back below.  */
+	  cache->saved_regs[I386_EIP_REGNUM] -= cache->base;
+	}
+      else
+	{
+	  frame_unwind_register (next_frame, I386_ESP_REGNUM, buf);
+	  cache->base = extract_unsigned_integer (buf, 4) + cache->sp_offset;
+	}
     }
 
   /* Now that we have the base address for the stack frame we can
      calculate the value of %esp in the calling frame.  */
-  cache->saved_sp = cache->base + 8;
+  if (cache->saved_sp == 0)
+    cache->saved_sp = cache->base + 8;
 
   /* Adjust all the saved registers such that they contain addresses
      instead of offsets.  */

Re: Fix PR breakpoints/2080

[Daniel Jacobowitz]

On Tue, Feb 14, 2006 at 04:45:40PM +0100, Mark Kettenis wrote:
> This fixes the testcase I checked in yesterday, so I committed it.

FYI, GCC generates this sequence as RTL rather than text, which means
it can be scheduled and optimized.  Of course in optimized code we
don't expect the prologue skipper to work very well, and we use dwarf2
to bypass the prologue analyzer, so this may not be a big deal.  But
I'm somewhat curious to see if we could make this extremely
sophisticated i386 prologue analyzer use Jim's prologue-value.[ch]
interface...


-- 
Daniel Jacobowitz
CodeSourcery

Re: Fix PR breakpoints/2080

[Mark Kettenis]

> X-Spam-Checker-Version: SpamAssassin 3.1.0 (2005-09-13) on 
> 	elgar.sibelius.xs4all.nl
> X-Spam-Level: 
> X-Spam-Status: No, score=0.0 required=5.0 tests=none autolearn=no 
> 	version=3.1.0
> Date: Tue, 14 Feb 2006 10:52:32 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: gdb-patches@sourceware.org, fnf@specifix.com
> Mail-Followup-To: Mark Kettenis <mark.kettenis@xs4all.nl>,
> 	gdb-patches@sourceware.org, fnf@specifix.com
> Content-Disposition: inline
> X-XS4ALL-DNSBL-Checked: mxdrop9.xs4all.nl checked 66.93.172.17 against DNS blacklists
> X-Virus-Scanned: by XS4ALL Virus Scanner
> X-XS4ALL-Spam-Score: 0 () 
> X-XS4ALL-Spam: NO
> Envelope-To: mark.kettenis@xs4all.nl
> X-UIDL: 1139932355._smtp.mxdrop9.54361,S=1797
> 
> On Tue, Feb 14, 2006 at 04:45:40PM +0100, Mark Kettenis wrote:
> > This fixes the testcase I checked in yesterday, so I committed it.
> 
> FYI, GCC generates this sequence as RTL rather than text, which means
> it can be scheduled and optimized.

Fortunately the scheduler doesn't have a lot of room to do this.  I'm
pretty sure the first three instructions (which are the most important
ones) are pretty much fixed with the current GCC implementation of the
stack alignment.

> Of course in optimized code we don't expect the prologue skipper to
> work very well, and we use dwarf2 to bypass the prologue analyzer,
> so this may not be a big deal.  But I'm somewhat curious to see if
> we could make this extremely sophisticated i386 prologue analyzer
> use Jim's prologue-value.[ch] interface...

Well, writing that prologue analyzer will be hell of a lot more
difficult for CISC than it is for RISC; even with Jim's
prologue-value.[ch].

Mark

Re: Fix PR breakpoints/2080

[Daniel Jacobowitz]

On Tue, Feb 14, 2006 at 05:38:46PM +0100, Mark Kettenis wrote:
> > X-Spam-Checker-Version: SpamAssassin 3.1.0 (2005-09-13) on 

Hmm, your mailer likes inserting blank lines in headers sometimes. 
Odd.

> > Of course in optimized code we don't expect the prologue skipper to
> > work very well, and we use dwarf2 to bypass the prologue analyzer,
> > so this may not be a big deal.  But I'm somewhat curious to see if
> > we could make this extremely sophisticated i386 prologue analyzer
> > use Jim's prologue-value.[ch] interface...
> 
> Well, writing that prologue analyzer will be hell of a lot more
> difficult for CISC than it is for RISC; even with Jim's
> prologue-value.[ch].

Totally true.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Fix PR breakpoints/2080

[Jim Blandy]

> > Of course in optimized code we don't expect the prologue skipper to
> > work very well, and we use dwarf2 to bypass the prologue analyzer,
> > so this may not be a big deal.  But I'm somewhat curious to see if
> > we could make this extremely sophisticated i386 prologue analyzer
> > use Jim's prologue-value.[ch] interface...
>
> Well, writing that prologue analyzer will be hell of a lot more
> difficult for CISC than it is for RISC; even with Jim's
> prologue-value.[ch].

Looking at the prologue you put in gdb.arch/i386-prologue.c, I think
it'd be okay.  The 'and' would set esp to 'unknown', but that should
be okay, since the original stack pointer has been saved in ebp.

In fact, there's already a pv_logical_and operation in
prologue-value.c.  If I remember right, s390 prologues can use logical
and from time to time.

Re: Fix PR breakpoints/2080

[Fred Fish]

On Tuesday 14 February 2006 10:45, Mark Kettenis wrote:
> This fixes the testcase I checked in yesterday, so I committed it.
> 
> Fred, can you check whether this also fixes your problem?

Sorry this took so long.  I updated my source tree, rebuilt the tools,
and reran the gdb tests with the native Fedora Core 4 compiler and the
latest development gcc.  Both got 72 unexpected failures, with only
minor differences in the results due to the fact that I didn't build
and install an objective C compiler.  Prior to your gdb changes, I was
getting somewhere around 150 unexpected failures if I recall
correctly.

-Fred