Re: [Patch, AArch64, ILP32] 3/5 Minor change in function.c:assign_parm_find_data_types()

[Yufeng Zhang <Yufeng dot Zhang at arm dot com>]

On 06/27/13 00:04, Andrew Pinski wrote:
> On Wed, Jun 26, 2013 at 3:39 PM, Yufeng Zhang<Yufeng.Zhang@arm.com>  wrote:
> > This patch updates assign_parm_find_data_types to assign passed_mode and
> > nominal_mode with the mode of the built pointer type instead of the
> > hard-coded Pmode in the case of pass-by-reference.  This is in line with the
> > assignment to passed_mode and nominal_mode in other cases inside the
> > function.
> >
> > assign_parm_find_data_types generally uses TYPE_MODE to calculate
> > passed_mode and nominal_mode:
> >
> >    /* Find mode of arg as it is passed, and mode of arg as it should be
> >       during execution of this function.  */
> >    passed_mode = TYPE_MODE (passed_type);
> >    nominal_mode = TYPE_MODE (nominal_type);
> >
> > this includes the case when the passed argument is a pointer by itself.
> >
> > However there is a discrepancy when it deals with argument passed by
> > invisible reference; it builds the argument's corresponding pointer type,
> > but sets passed_mode and nominal_mode with Pmode directly.
> >
> > This is OK for targets where Pmode == ptr_mode, but on AArch64 with ILP32
> > they are different with Pmode as DImode and ptr_mode as SImode. When such a
> > reference is passed on stack, the reference is prepared by the caller in the
> > lower 4 bytes of an 8-byte slot but is fetched by the callee as an 8-byte
> > datum, of which the higher 4 bytes may contain junk.  It is probably the
> > combination of Pmode != ptr_mode and the particular ABI specification that
> > make the AArch64 ILP32 the first target on which the issue manifests itself.
> >
> > Bootstrapped on x86_64-none-linux-gnu.
> >
> > OK for the trunk?
>
>
> IA64-hpux also uses Pmode != ptr_mode, can you provide the testcase
> which fails without this change?
> I used a powerpc64 target where Pmode != ptr_mode which did not hit
> this bug either.

The issue was firstly observed in one of the compat tests which passes a 
large number of non-small structures.  The following is a trimmed-down 
reproducible code snippet (although not runnable but shall be easy to be 
make runnable):

struct s5
{
  double a;
  double b;
  double c;
  double d;
  double e;
} gS;

double foo (struct s5 p1, struct s5 p2,struct s5 p3,struct s5 p4,struct 
s5 p5,struct s5 p6,struct s5 p7,struct s5 p8, struct s5 p9)
{
  return p9.c;
}
--------------- CUT ---------------

The code-gen (-O2) without the patch is:

        .text
        .align  2
        .global foo
        .type   foo, %function
foo:
        ldr     x0, [sp]	<<=== here!
        ldr     d0, [x0,16]
        ret
        .size   foo, .-foo

Where the arrow points is the load of the pointer to 'p9' that is passed 
on stack.  The instruction really should be ldr w0, [sp], i.e. the 
pointer mode is SImode rather than DImode.

It needs a number of conditions for the issue to manifest:

1. pass-by-reference; on aarch64 one example is a struct that is larger 
than 16 bytes.
2. the reference is passed on stack; on aarch64, this usually only 
happens after registers x0 - x7 are used.
3. the size of stack slot for passing pointer is larger than the pointer 
size; on aarch64, it is 8-byte vs. 4-byte
4. the unused part of the stack slot is not zeroed out, i.e. undefined 
by the ABI
5. in the runtime, the unused part of such a stack slot contains junk.

The runtime segmentation fault may only be generated when all the above 
conditions are met.  I'm not familiar with IA64-hpux or powerpc64 
procedure call ABIs, but I guess those targets are just being lucky?

Thanks,
Yufeng