Re: [patch 2/3] Align stack for SSE (PR tdep/14222)

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

> Date: Wed, 13 Jun 2012 17:50:20 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> Hi,
> 
> this is mostly independent patch.  But it does not make sense to discuss
> alignment in [patch 3/3] when it has been already broken.
> 
> 	http://groups.google.com/group/ia32-abi/browse_thread/thread/4f9b3e5069943bf1
> says that gcc-4.6 aligns stack to 32 bytes.  I do not see it anywhere:
> 	80482e8:       83 e4 f0                and    $0xfffffff0,%esp
> It is aligned just to 16 bytes.  Also checked that Intel CPU documentation
> does not require any more alignment than 16 bytes (sure it may be less
> effective but traps never occur).  Therefore I consider amd64-tdep.c to be
> correct and I have not changed its existing 16-bytes alignment.
> 
> gdb/
> 2012-06-13  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	PR tdep/14222
> 	* i386-tdep.c (i386_push_dummy_call): Align to 16 bytes unconditionally.

Committed a better diff (see below) that removes the now unused
have_16_byte_aligned_arg variable.

> gdb/testsuite/
> 2012-06-13  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	PR tdep/14222
> 	* gdb.arch/i386-sse-stack-align.S: New file.
> 	* gdb.arch/i386-sse-stack-align.c: New file.
> 	* gdb.arch/i386-sse-stack-align.exp: New file.

Can you commit this testsuite bit?


2012-06-13  Mark Kettenis  <kettenis@gnu.org>
	    Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR tdep/14222
	* i386-tdep.c (i386_push_dummy_call): Unconditionally align the
	stack on a 16-byte boundary.

Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.352
diff -u -p -r1.352 i386-tdep.c
--- i386-tdep.c	13 Jun 2012 20:29:15 -0000	1.352
+++ i386-tdep.c	13 Jun 2012 21:15:44 -0000
@@ -2351,7 +2351,6 @@ i386_push_dummy_call (struct gdbarch *gd
   for (write_pass = 0; write_pass < 2; write_pass++)
     {
       int args_space_used = 0;
-      int have_16_byte_aligned_arg = 0;
 
       if (struct_return)
 	{
@@ -2389,19 +2388,20 @@ i386_push_dummy_call (struct gdbarch *gd
 	  else
 	    {
 	      if (i386_16_byte_align_p (value_enclosing_type (args[i])))
-		{
-		  args_space = align_up (args_space, 16);
-		  have_16_byte_aligned_arg = 1;
-		}
+		args_space = align_up (args_space, 16);
 	      args_space += align_up (len, 4);
 	    }
 	}
 
       if (!write_pass)
 	{
-	  if (have_16_byte_aligned_arg)
-	    args_space = align_up (args_space, 16);
 	  sp -= args_space;
+
+	  /* The original System V ABI only requires word alignment,
+	     but modern incarnations need 16-byte alignment in order
+	     to support SSE.  Since wasting a few bytes here isn't
+	     harmful we unconditionally enforce 16-byte alignment.  */
+	  sp &= ~0xf;
 	}
     }