Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

GCC and Clang disagree about alignment of global structs containing arrays #26778

Open Quuxplusone opened 8 years ago

Quuxplusone commented 8 years ago
Bugzilla Link PR26779
Status NEW
Importance P normal
Reported by Matthew (f14_tomcat1@hotmail.com)
Reported on 2016-02-29 19:05:37 -0800
Last modified on 2016-03-01 16:03:04 -0800
Version 3.5
Hardware PC Linux
CC dgregor@apple.com, hjl.tools@gmail.com, llvm-bugs@lists.llvm.org, rnk@google.com
Fixed by commit(s)
Attachments ClangCrashRepro.tgz (856 bytes, application/x-compressed-tar)
Blocks
Blocked by
See also
Created attachment 15961
Crash repro

I have a library compiled with gcc 4.8.2 which statically initialises an array
using movaps. The movaps instruction is generated by gcc at optimization level
3.

When the library is loaded from a clang compiled application, I get a seg fault
on the movaps instruction. The pointer movaps is trying to write to is not
aligned to 16 bytes.

If I recompile the clang application with -mstackrealign, it works. It looks
like clang does not ensure the stack is aligned before calling __global_ctx_aux.

A simple repro is attached. Here's the relevant call stack for the repro:

#0  0x00007ffff7dfc6de in _GLOBAL__sub_I_Mat4.cpp () from
./libClangCrashRepro.so
#1  0x00007ffff7dfc736 in __do_global_ctors_aux () from ./libClangCrashRepro.so
#2  0x00007ffff7dfc54b in _init () from ./libClangCrashRepro.so
#3  0x00007ffff78bb000 in ?? ()
#4  0x0000003e7260e605 in _dl_init_internal () from /lib64/ld-linux-x86-64.so.2
#5  0x0000003e72600b3a in _dl_start_user () from /lib64/ld-linux-x86-64.so.2
#6  0x0000000000000001 in ?? ()
#7  0x00007fffffffca93 in ?? ()
#8  0x0000000000000000 in ?? ()

Dump of assembler code for function _GLOBAL__sub_I_Mat4.cpp:
   0x00007ffff7dfc6c0 <+0>: mov    0x200319(%rip),%rax        # 0x7ffff7ffc9e0
   0x00007ffff7dfc6c7 <+7>: cmpb   $0x0,(%rax)
   0x00007ffff7dfc6ca <+10>:    jne    0x7ffff7dfc702 <_GLOBAL__sub_I_Mat4.cpp+66>
   0x00007ffff7dfc6cc <+12>:    movb   $0x1,(%rax)
   0x00007ffff7dfc6cf <+15>:    mov    0x20033a(%rip),%rax        # 0x7ffff7ffca10
   0x00007ffff7dfc6d6 <+22>:    movss  0x82(%rip),%xmm0        # 0x7ffff7dfc760
=> 0x00007ffff7dfc6de <+30>:    movaps %xmm0,(%rax)
   0x00007ffff7dfc6e1 <+33>:    movaps 0x88(%rip),%xmm0        # 0x7ffff7dfc770
   0x00007ffff7dfc6e8 <+40>:    movaps %xmm0,0x10(%rax)
   0x00007ffff7dfc6ec <+44>:    movaps 0x8d(%rip),%xmm0        # 0x7ffff7dfc780
   0x00007ffff7dfc6f3 <+51>:    movaps %xmm0,0x20(%rax)
   0x00007ffff7dfc6f7 <+55>:    movaps 0x92(%rip),%xmm0        # 0x7ffff7dfc790
   0x00007ffff7dfc6fe <+62>:    movaps %xmm0,0x30(%rax)
   0x00007ffff7dfc702 <+66>:    repz retq

The highlighted movaps line corresponds to Mat4.h:13. The value of xmm0 is
{1,0,0,0} as expected, and rax is 6294696 which is not on a 16 byte boundary.
Quuxplusone commented 8 years ago

Attached ClangCrashRepro.tgz (856 bytes, application/x-compressed-tar): Crash repro

Quuxplusone commented 8 years ago
This is not stack misalignment, this is a disagreement between clang and GCC
about the alignment of the static data member in your example.

Reduced:
struct S { float myarray[16]; } s;
float useit() { return s.myarray[0]; }

If you emit LLVM IR you can see the low alignment:
$ clang -S -emit-llvm t.cpp -O3 -o - | grep '@s = '
@s = global %struct.S zeroinitializer, align 4

If you remove the wrapper struct around myarray, the alignment goes up to 16.
So, this boils down to two interpretations of the Sys V psABI doc, which has a
special case for global arrays larger than 16 bytes.
Quuxplusone commented 8 years ago
ABI doc quote:

http://www.x86-64.org/documentation/abi.pdf
Aggregates and Unions
An array uses the same alignment as its elements, except that a local or global
array variable of length at least 16 bytes or a C99 variable-length array
variable always has alignment of at least 16 bytes.

So, we need clarification about whether this counts as a global array or not.
Quuxplusone commented 8 years ago
(In reply to comment #2)
>
> So, we need clarification about whether this counts as a global array or not.

struct S { float myarray[16]; } s;
extern void useit(float []);

void
bar (void)
{
  return useit (s.myarray);
}

useit should be passed an array aligned to 16 bytes,
Quuxplusone commented 8 years ago
(In reply to comment #3)
> useit should be passed an array aligned to 16 bytes,

Why? Consider this case:

struct S {
  int x;
  float myarray[16];
} s;
extern void useit(float []);
void bar (void) { return useit (s.myarray); }

Surely we will not change the layout of 'S' to ensure that the second field is
16 byte aligned.

It sounds like you are saying that the alignment of any global should be
increased to 16 if it has a subobject which is an array of floating point
values larger than 16 bytes.
Quuxplusone commented 8 years ago
(In reply to comment #4)
> (In reply to comment #3)
> > useit should be passed an array aligned to 16 bytes,
>
> Why? Consider this case:
>
> struct S {
>   int x;
>   float myarray[16];
> } s;
> extern void useit(float []);
> void bar (void) { return useit (s.myarray); }
>
> Surely we will not change the layout of 'S' to ensure that the second field
> is 16 byte aligned.

Will you do it for

struct S {
  int x;
  float myarray[16];
} s;
extern void useit(float [16]);
void bar (void) { return useit (s.myarray); }

> It sounds like you are saying that the alignment of any global should be
> increased to 16 if it has a subobject which is an array of floating point
> values larger than 16 bytes.

Yes.