program crashes at movaps, unaligned stack address, -O3

[llvmbot]

Bugzilla Link	34038
Resolution	FIXED
Resolved on	Aug 07, 2017 13:07
Version	4.0
OS	Linux
Blocks	#33196
Attachments	C program for repro
Reporter	LLVM Bugzilla Contributor
CC	@efriedma-quic,@zmodem,@hfinkel,@RKSimon,@rnk

Extended Description

ericy@mrpink:~$ /usr/bin/clang --version
clang version 4.0.0-1ubuntu1 (tags/RELEASE_400/rc1)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

ericy@mrpink:~$ /usr/bin/clang -O3 movaps_unaligned_crash.c && ./a.out
Segmentation fault

It does not repro with -O2.

You can see that it used the movaps instruction with an unaligned address.

(gdb) disassemble
Dump of assembler code for function main:
0x0000000000400490 <+0>: sub $0x28,%rsp
0x0000000000400494 <+4>: movq $0x0,0x20(%rsp)
0x000000000040049d <+13>: movq $0x0,0x18(%rsp)
0x00000000004004a6 <+22>: mov 0x200b7b(%rip),%rax # 0x601028
=> 0x00000000004004ad <+29>: movaps 0x18(%rsp),%xmm0
0x00000000004004b2 <+34>: movaps %xmm0,(%rsp)
0x00000000004004b6 <+38>: mov $0x601028,%edi
0x00000000004004bb <+43>: xor %esi,%esi
0x00000000004004bd <+45>: xor %edx,%edx
0x00000000004004bf <+47>: xor %ecx,%ecx
0x00000000004004c1 <+49>: xor %r8d,%r8d
0x00000000004004c4 <+52>: callq *(%rax)
0x00000000004004c6 <+54>: xor %eax,%eax
0x00000000004004c8 <+56>: add $0x28,%rsp
0x00000000004004cc <+60>: retq
End of assembler dump.
(gdb) p $rsp
$1 = (void *) 0x7fffffffdf50

[llvmbot]

Strangely, when I tried compiling the provided code as a .cpp file (instead of a c file,) it doesn't crash and produces slightly different assembly:

$ clang -O3 crash.cpp

(gdb) disassemble main
Dump of assembler code for function main:
0x0000000000400490 <+0>: sub $0x28,%rsp
0x0000000000400494 <+4>: mov 0x200b8d(%rip),%rax # 0x601028 <_ZL3obj>
0x000000000040049b <+11>: mov (%rax),%rax
0x000000000040049e <+14>: movq $0x0,0x18(%rsp)
0x00000000004004a7 <+23>: movq $0x0,0x10(%rsp)
0x00000000004004b0 <+32>: movaps 0x10(%rsp),%xmm0
0x00000000004004b5 <+37>: movaps %xmm0,(%rsp)
0x00000000004004b9 <+41>: mov $0x601028,%edi
0x00000000004004be <+46>: xor %esi,%esi
0x00000000004004c0 <+48>: xor %edx,%edx
0x00000000004004c2 <+50>: xor %ecx,%ecx
0x00000000004004c4 <+52>: xor %r8d,%r8d
0x00000000004004c7 <+55>: callq *%rax
0x00000000004004c9 <+57>: xor %eax,%eax
0x00000000004004cb <+59>: add $0x28,%rsp
0x00000000004004cf <+63>: retq
End of assembler dump.
(gdb)

[llvmbot]

The code which crashes is setting up the argument on the stack in the wrong location 0x18(%rsp). Possibly a bug in the X86-64 code in clang/lib/CodeGen/TargetInfo.cpp?

[llvmbot]

Looks like the bug even exists as far back as clang-3.0 if you pass '-O3 -fomit-frame-pointer'

[llvmbot]

I just tried on the current version of clang-6.0 and the bug is still there.

[rnk]

Reduced LLVM IR showing the bug:

target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64--linux-gnu"
declare void @​real_foo(i32, i32, i32, i32, i32, i128* byval nocapture readnone align 16 %end)
define i32 @​main() {
entry:
%end.i = alloca i128, align 8
store i128 0, i128* %end.i, align 8
call void @​real_foo(i32 0, i32 0, i32 0, i32 0, i32 0, i128* byval nonnull align 16 %end.i) #​3
ret i32 0
}

Asm looks like this:

    subq    $40, %rsp

.Lcfi0:
.cfi_def_cfa_offset 48
movq $0, 32(%rsp)
movq $0, 24(%rsp)
movaps 24(%rsp), %xmm0 # <-- not aligned
movaps %xmm0, (%rsp)
xorl %edi, %edi
xorl %esi, %esi
xorl %edx, %edx
xorl %ecx, %ecx
xorl %r8d, %r8d
callq real_foo
xorl %eax, %eax
addq $40, %rsp
retq

I think the source of the bug is the "align 8" on the alloca. Clang seems to think that alignof(struct key) is 8, not 16.

[efriedma-quic]

That IR has undefined behavior. From http://llvm.org/docs/LangRef.html#parameter-attributes: "The byval attribute [...] indicates [...] the known alignment of the pointer specified to the call site."

[rnk]

I always assumed the 'align' attribute was to control the alignment of the memory used within the call, not to indicate the alignment of the source of the byval copy. Am I just wrong, the align is always supposed to apply to the source?

[efriedma-quic]

It's both the alignment of the source ptr and the alignment of the allocated memory.

It's possible this could be improved to allow specifying the two forms of alignment separately, but it usually isn't important.

[rnk]

I think argument promotion is the source of the under-aligned alloca:

*** IR Dump After Deduce function attributes ***
; Function Attrs: inlinehint nounwind uwtable
define internal fastcc void @​foo(%struct.key* byval align 16 %end) unnamed_addr #​2 {
entry:
%0 = load i8* (%struct.obj*, i64, i32, i64, i64, %struct.key*), i8* (%struct.obj*, i64, i32, i64, i64, %struct.key*)* bitcast (%struct.obj* @​obj to i8* (%struc
t.obj*, i64, i32, i64, i64, %struct.key*)), align 8, !tbaa !​2
%1 = load i8 (%struct.obj, i64, i32, i64, i64, %struct.key), i8 (%struct.obj*, i64, i32, i64, i64, %struct.key*)** %0, align 8, !tbaa !​7
%call = call i8* %1(%struct.obj* nonnull @​obj, i64 0, i32 0, i64 0, i64 0, %struct.key* byval nonnull align 16 %end) #​3
ret void
}
*** IR Dump After Promote 'by reference' arguments to scalars ***
; Function Attrs: inlinehint nounwind uwtable
define internal fastcc void @​foo(i128 %end.0) unnamed_addr #​2 {
entry:
%end = alloca %struct.key
%.0 = getelementptr %struct.key, %struct.key* %end, i32 0, i32 0
store i128 %end.0, i128* %.0
%0 = load i8* (%struct.obj*, i64, i32, i64, i64, %struct.key*), i8* (%struct.obj*, i64, i32, i64, i64, %struct.key*)* bitcast (%struct.obj* @​obj to i8* (%struct.obj*, i64, i32, i64, i64, %struct.key*)), align 8, !tbaa !​2
%1 = load i8 (%struct.obj, i64, i32, i64, i64, %struct.key), i8 (%struct.obj*, i64, i32, i64, i64, %struct.key*)** %0, align 8, !tbaa !​7
%call = call i8* %1(%struct.obj* nonnull @​obj, i64 0, i32 0, i64 0, i64 0, %struct.key* byval nonnull align 16 %end) #​3
ret void
}

It should use the 'align' attribute to set the alignment of the alloca it creates, rather than relying on DataLayout to figure it out.

[rnk]

After r310071 we get this:

    subq    $40, %rsp

.Lcfi0:
.cfi_def_cfa_offset 48
movq $0, 24(%rsp)
movq $0, 16(%rsp)
movq obj(%rip), %rax
movaps 16(%rsp), %xmm0 # aligned
movaps %xmm0, (%rsp)
movl $obj, %edi
xorl %esi, %esi
xorl %edx, %edx
xorl %ecx, %ecx
xorl %r8d, %r8d
callq *(%rax)
xorl %eax, %eax
addq $40, %rsp

Should be fixed. We should merge this to 5.0.

[zmodem]

After r310071 we get this:

    subq    $40, %rsp

.Lcfi0:
.cfi_def_cfa_offset 48
movq $0, 24(%rsp)
movq $0, 16(%rsp)
movq obj(%rip), %rax
movaps 16(%rsp), %xmm0 # aligned
movaps %xmm0, (%rsp)
movl $obj, %edi
xorl %esi, %esi
xorl %edx, %edx
xorl %ecx, %ecx
xorl %r8d, %r8d
callq *(%rax)
xorl %eax, %eax
addq $40, %rsp

Should be fixed. We should merge this to 5.0.

Sounds good to me. Let's have it bake in the tree for a bit, and then I'll merge it.

[llvmbot]

Thanks, Reid!

[zmodem]

After r310071 we get this:

    subq    $40, %rsp

.Lcfi0:
.cfi_def_cfa_offset 48
movq $0, 24(%rsp)
movq $0, 16(%rsp)
movq obj(%rip), %rax
movaps 16(%rsp), %xmm0 # aligned
movaps %xmm0, (%rsp)
movl $obj, %edi
xorl %esi, %esi
xorl %edx, %edx
xorl %ecx, %ecx
xorl %r8d, %r8d
callq *(%rax)
xorl %eax, %eax
addq $40, %rsp

Should be fixed. We should merge this to 5.0.

Sounds good to me. Let's have it bake in the tree for a bit, and then I'll
merge it.

Merged to 5.0 in r310292.