Fix PR78444.

Message ID C8A84E78-F327-4CF5-9BB5-928DDE35FFEC@sandoe.co.uk
State New
Headers show
Series
  • Fix PR78444.
Related show

Commit Message

Iain Sandoe Nov. 27, 2018, 4:17 p.m.
Hi,

As described in comment #4/5 in the PR, there are [corner] cases where the the compiler incorrectly concludes that only 64b alignment is needed at a call site.  This is caught by Darwin’s dynamic linker which enforces the ABI requirement for 128b alignment, causing the exe be aborted.

With this fix, we no longer need the work-around that was applied when profiling was on, so we can remove the special-casing there.

The attached patch has been tested on x86_64-darwin and x86_64-linux-gnu.

The problem exists on all open branches,

OK for trunk?

Backports?

thanks
Iain

gcc/
	* config/i386/darwin.h (STACK_BOUNDARY): Remove macro.
	* config/i386/i386.c (ix86_compute_frame_layout): Ensure at least 128b
	stack alignment in non-leaf functions.

From e199bce63f952ae1c3f6bffcdc20360aaf4deae8 Mon Sep 17 00:00:00 2001
From: Iain Sandoe <iain@codesourcery.com>

Date: Fri, 16 Nov 2018 16:25:47 +0000
Subject: [PATCH] [patch] Fix PR78444, update stack alignment for non-leaf
 functions.

---
 gcc/config/i386/darwin.h |  3 ---
 gcc/config/i386/i386.c   | 10 ++++++++--
 2 files changed, 8 insertions(+), 5 deletions(-)

-- 
2.17.1

Comments

Uros Bizjak Nov. 27, 2018, 4:44 p.m. | #1
On Tue, Nov 27, 2018 at 5:17 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>

> Hi,

>

> As described in comment #4/5 in the PR, there are [corner] cases where the the compiler incorrectly concludes that only 64b alignment is needed at a call site.  This is caught by Darwin’s dynamic linker which enforces the ABI requirement for 128b alignment, causing the exe be aborted.

>

> With this fix, we no longer need the work-around that was applied when profiling was on, so we can remove the special-casing there.

>

> The attached patch has been tested on x86_64-darwin and x86_64-linux-gnu.

>

> The problem exists on all open branches,

>

> OK for trunk?

>

> Backports?

>

> thanks

> Iain

>

> gcc/

>         * config/i386/darwin.h (STACK_BOUNDARY): Remove macro.

>         * config/i386/i386.c (ix86_compute_frame_layout): Ensure at least 128b

>         stack alignment in non-leaf functions.


OK for mainline and branches after a week or so without problems in mainline.

Thanks,
Uros.

> From e199bce63f952ae1c3f6bffcdc20360aaf4deae8 Mon Sep 17 00:00:00 2001

> From: Iain Sandoe <iain@codesourcery.com>

> Date: Fri, 16 Nov 2018 16:25:47 +0000

> Subject: [PATCH] [patch] Fix PR78444, update stack alignment for non-leaf

>  functions.

>

> ---

>  gcc/config/i386/darwin.h |  3 ---

>  gcc/config/i386/i386.c   | 10 ++++++++--

>  2 files changed, 8 insertions(+), 5 deletions(-)

>

> diff --git a/gcc/config/i386/darwin.h b/gcc/config/i386/darwin.h

> index 735abeddb2..64133aa8dc 100644

> --- a/gcc/config/i386/darwin.h

> +++ b/gcc/config/i386/darwin.h

> @@ -85,9 +85,6 @@ extern int darwin_emit_branch_islands;

>  /* On Darwin, the stack is 128-bit aligned at the point of every call.

>     Failure to ensure this will lead to a crash in the system libraries

>     or dynamic loader.  */

> -#undef STACK_BOUNDARY

> -#define STACK_BOUNDARY \

> -  ((profile_flag || TARGET_64BIT_MS_ABI) ? 128 : BITS_PER_WORD)

>

>  #undef MAIN_STACK_BOUNDARY

>  #define MAIN_STACK_BOUNDARY 128

> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c

> index ef58533344..c88617e3c1 100644

> --- a/gcc/config/i386/i386.c

> +++ b/gcc/config/i386/i386.c

> @@ -11161,10 +11161,16 @@ ix86_compute_frame_layout (void)

>    /* 64-bit MS ABI seem to require stack alignment to be always 16,

>       except for function prologues, leaf functions and when the defult

>       incoming stack boundary is overriden at command line or via

> -     force_align_arg_pointer attribute.  */

> -  if ((TARGET_64BIT_MS_ABI && crtl->preferred_stack_boundary < 128)

> +     force_align_arg_pointer attribute.

> +

> +     Darwin's ABI specifies 128b alignment for both 32 and  64 bit variants

> +     at call sites, including profile function calls.

> + */

> +  if (((TARGET_64BIT_MS_ABI || TARGET_MACHO)

> +        && crtl->preferred_stack_boundary < 128)

>        && (!crtl->is_leaf || cfun->calls_alloca != 0

>           || ix86_current_function_calls_tls_descriptor

> +         || (TARGET_MACHO && crtl->profile)

>           || ix86_incoming_stack_boundary < 128))

>      {

>        crtl->preferred_stack_boundary = 128;

> --

> 2.17.1

>

>

Patch

diff --git a/gcc/config/i386/darwin.h b/gcc/config/i386/darwin.h
index 735abeddb2..64133aa8dc 100644
--- a/gcc/config/i386/darwin.h
+++ b/gcc/config/i386/darwin.h
@@ -85,9 +85,6 @@  extern int darwin_emit_branch_islands;
 /* On Darwin, the stack is 128-bit aligned at the point of every call.
    Failure to ensure this will lead to a crash in the system libraries
    or dynamic loader.  */
-#undef STACK_BOUNDARY
-#define STACK_BOUNDARY \
-  ((profile_flag || TARGET_64BIT_MS_ABI) ? 128 : BITS_PER_WORD)
 
 #undef MAIN_STACK_BOUNDARY
 #define MAIN_STACK_BOUNDARY 128
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ef58533344..c88617e3c1 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -11161,10 +11161,16 @@  ix86_compute_frame_layout (void)
   /* 64-bit MS ABI seem to require stack alignment to be always 16,
      except for function prologues, leaf functions and when the defult
      incoming stack boundary is overriden at command line or via
-     force_align_arg_pointer attribute.  */
-  if ((TARGET_64BIT_MS_ABI && crtl->preferred_stack_boundary < 128)
+     force_align_arg_pointer attribute.
+
+     Darwin's ABI specifies 128b alignment for both 32 and  64 bit variants
+     at call sites, including profile function calls.
+ */
+  if (((TARGET_64BIT_MS_ABI || TARGET_MACHO)
+        && crtl->preferred_stack_boundary < 128)
       && (!crtl->is_leaf || cfun->calls_alloca != 0
 	  || ix86_current_function_calls_tls_descriptor
+	  || (TARGET_MACHO && crtl->profile)
 	  || ix86_incoming_stack_boundary < 128))
     {
       crtl->preferred_stack_boundary = 128;