Re: [PATCH, PowerPC] Fix PR57949 (ABI alignment issue)

[Bill Schmidt <wschmidt at linux dot vnet dot ibm dot com>]

On Wed, 2013-09-11 at 21:08 +0930, Alan Modra wrote:
> On Wed, Aug 14, 2013 at 10:32:01AM -0500, Bill Schmidt wrote:
> > This fixes a long-standing problem with GCC's implementation of the
> > PPC64 ELF ABI.  If a structure contains a member requiring 128-bit
> > alignment, and that structure is passed as a parameter, the parameter
> > currently receives only 64-bit alignment.  This is an error, and is
> > incompatible with correct code generated by the IBM XL compilers.
> 
> This caused multiple failures in the libffi testsuite:
> libffi.call/cls_align_longdouble.c
> libffi.call/cls_align_longdouble_split.c
> libffi.call/cls_align_longdouble_split2.c
> libffi.call/nested_struct5.c
> 
> Fixed by making the same alignment adjustment in libffi to structures
> passed by value.  Bill, I think your patch needs to go on all active
> gcc branches as otherwise we'll need different versions of libffi for
> the next gcc releases.

Hm, the libffi case is unfortunate. :(

The alternative is to leave libffi alone, and require code that calls
these interfaces with "bad" structs passed by value to be built using
-mcompat-align-parm, which was provided for such compatibility issues.
Hopefully there is a small number of cases where this can happen, and
this could be documented with libffi and gcc.  What do you think?

Thanks,
Bill

> 
> The following was bootstrapped and regression checked powerpc64-linux.
> OK for mainline, and the 4.7 and 4.8 branches when/if Bill's patch
> goes in there?
> 
> 	* src/powerpc/ffi.c (ffi_prep_args64): Align FFI_TYPE_STRUCT.
> 	(ffi_closure_helper_LINUX64): Likewise.
> 
> Index: libffi/src/powerpc/ffi.c
> ===================================================================
> --- libffi/src/powerpc/ffi.c	(revision 202428)
> +++ libffi/src/powerpc/ffi.c	(working copy)
> @@ -462,6 +462,7 @@ ffi_prep_args64 (extended_cif *ecif, unsigned long
>      double **d;
>    } p_argv;
>    unsigned long gprvalue;
> +  unsigned long align;
> 
>    stacktop.c = (char *) stack + bytes;
>    gpr_base.ul = stacktop.ul - ASM_NEEDS_REGISTERS64 - NUM_GPR_ARG_REGISTERS64;
> @@ -532,6 +533,10 @@ ffi_prep_args64 (extended_cif *ecif, unsigned long
>  #endif
> 
>  	case FFI_TYPE_STRUCT:
> +	  align = (*ptr)->alignment;
> +	  if (align > 16)
> +	    align = 16;
> +	  next_arg.ul = ALIGN (next_arg.ul, align);
>  	  words = ((*ptr)->size + 7) / 8;
>  	  if (next_arg.ul >= gpr_base.ul && next_arg.ul + words > gpr_end.ul)
>  	    {
> @@ -1349,6 +1354,7 @@ ffi_closure_helper_LINUX64 (ffi_closure *closure,
>    long i, avn;
>    ffi_cif *cif;
>    ffi_dblfl *end_pfr = pfr + NUM_FPR_ARG_REGISTERS64;
> +  unsigned long align;
> 
>    cif = closure->cif;
>    avalue = alloca (cif->nargs * sizeof (void *));
> @@ -1399,6 +1405,10 @@ ffi_closure_helper_LINUX64 (ffi_closure *closure,
>  	  break;
> 
>  	case FFI_TYPE_STRUCT:
> +	  align = arg_types[i]->alignment;
> +	  if (align > 16)
> +	    align = 16;
> +	  pst = ALIGN (pst, align);
>  #ifndef __LITTLE_ENDIAN__
>  	  /* Structures with size less than eight bytes are passed
>  	     left-padded.  */
> 
>