clang: x86: __force_align_arg_pointer__ assumes that stack arguments are aligned

[Quuxplusone]

Bugzilla Link	PR26662
Status	CONFIRMED
Importance	P normal
Reported by	Huw Davies (huw@codeweavers.com)
Reported on	2016-02-18 09:26:59 -0800
Last modified on	2017-08-02 17:14:52 -0700
Version	trunk
Hardware	Macintosh MacOS X
CC	eric@youngblut.net, huw@codeweavers.com, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments	0001-CodeGen-Don-t-assume-that-fixed-stack-objects-are-al.patch (6426 bytes, text/plain)
Blocks
Blocked by
See also

When accessing stack arguments in a function marked with the
__force_align_arg_pointer__ attibute, the generated code assumes that the stack
args are 16-byte aligned.  The point of re-aligning the stack with this
attribute is that the stack may not be so aligned.

This is of importance to Wine, where the caller is win32 code with 4-byte
alignment, yet the code is running on a OS/X / Linux host with 16-byte
alignment.

When the function below is compiled with:
clang -O1 -m32  -fno-stack-protector -c  ~/test.c
(stack protection disabled just to simplify disassembly)

struct sixteen
{
    char b[16];
};

void __attribute__((__force_align_arg_pointer__)) __attribute__((__stdcall__))
test_fn( struct sixteen s )
{
    volatile struct sixteen d = s;
}

We get:
_test_fn:
00000000        pushl   %ebp
00000001        movl    %esp, %ebp
00000003        andl    $-0x10, %esp
00000006        subl    $0x20, %esp
00000009        movaps  0x8(%ebp), %xmm0
0000000d        movaps  %xmm0, (%esp)
00000011        movl    %ebp, %esp
00000013        popl    %ebp
00000014        ret     $0x10

Note that while the stack is successfully re-aligned (andl $-0x10, %esp), the
movaps 0x8(%ebp), %xmm0 assumes that the stack was originally 16-byte aligned.
It should not make that assumption.

[Quuxplusone]

Attached 0001-CodeGen-Don-t-assume-that-fixed-stack-objects-are-al.patch (6426 bytes, text/plain): Patch to fix PR26662

[Quuxplusone]

This works great, thanks!

Note I also started a thread on llvm-dev with a somewhat different approach[1],
but yours seems like it has more chance of succeeding!

Thanks again.

[1] http://lists.llvm.org/pipermail/llvm-dev/2016-March/096762.html

[Quuxplusone]

This bug appears to be fixed.
https://reviews.llvm.org/D18471
I verified that the change is present in clang 4.0.0.