Re: ping: [patch 2/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> Date: Mon, 26 Mar 2012 21:04:14 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -2326,6 +2326,30 @@ i386_16_byte_align_p (struct type *type)
>    return 0;
>  }
>  
> +/* Implementation for set_gdbarch_push_dummy_code.  */
> +
> +static CORE_ADDR
> +i386_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp, CORE_ADDR funaddr,
> +		      struct value **args, int nargs, struct type *value_type,
> +		      CORE_ADDR *real_pc, CORE_ADDR *bp_addr,
> +		      struct regcache *regcache)
> +{
> +  int bplen;
> +  CORE_ADDR bppc = sp;
> +
> +  gdbarch_breakpoint_from_pc (gdbarch, &bppc, &bplen);
> +  sp -= bplen;
> +
> +  /* amd64_push_dummy_call does alignment on its own but i386_push_dummy_call
> +     does not.  ABI requires stack alignment for executables using SSE.  */
> +  if (gdbarch_frame_align_p (gdbarch))
> +    sp = gdbarch_frame_align (gdbarch, sp);
> +
> +  *bp_addr = sp;
> +  *real_pc = funaddr;
> +  return sp;
> +}

You're almost certainly right worrying about stack alignment.
However, the comment doesn't make a lot of sense.  For one thing,
amd64_push_dummy_call() doesn't explicitly align the stack.  It just
preserves alignment.  Also, if i386_push_dummy_call() doesn't preserve
alignment (and it sure looks like it doesn't), then aligning the stack
here doesn't help.  In any case, we don't need to to explicitly align
the stack since that's already done bu call_function_by_hand().  So we
only need to make sure the stack stays aligned, which can be easily
done by always allocating 16 bytes of stack space.

Calling gdbarch_breakpoint_from_pc() is also a bit overkill.  The
breakpoint instruction on i386 is pretty much fixed, we know it is
just a single byte and we know it can be placed just about anywhere.

So the simplified version below is perfectly adequate.  We have some
freedom on where to place the breakpoint in the 16-byte stack gap we
create.  I chose to put it up hight such that a small buffer overflow
isn't likely to overwrite the breakpoint instruction.

Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.346
diff -u -p -r1.346 i386-tdep.c
--- i386-tdep.c	29 Feb 2012 14:59:41 -0000	1.346
+++ i386-tdep.c	27 Mar 2012 18:30:24 -0000
@@ -2327,6 +2327,21 @@ i386_16_byte_align_p (struct type *type)
 }
 
 static CORE_ADDR
+i386_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp,
+		      CORE_ADDR funcaddr,
+		      struct value **args, int nargs,
+		      struct type *value_type,
+		      CORE_ADDR *real_pc, CORE_ADDR *bp_addr,
+		      struct regcache *regcache)
+{
+  *bp_addr = sp - 1;
+  *real_pc = funcaddr;
+
+  /* Keep the stack aligned.  */
+  return sp - 16;
+}
+
+static CORE_ADDR
 i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		      struct regcache *regcache, CORE_ADDR bp_addr, int nargs,
 		      struct value **args, CORE_ADDR sp, int struct_return,
@@ -7372,6 +7387,8 @@ i386_gdbarch_init (struct gdbarch_info i
   set_gdbarch_get_longjmp_target (gdbarch, i386_get_longjmp_target);
 
   /* Call dummy code.  */
+  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
+  set_gdbarch_push_dummy_code(gdbarch, i386_push_dummy_code);
   set_gdbarch_push_dummy_call (gdbarch, i386_push_dummy_call);
   set_gdbarch_frame_align (gdbarch, i386_frame_align);