These are chat archives for dropbox/pyston

29th
Feb 2016
Marius Wachtler
@undingen
Feb 29 2016 20:36
oh yeah I think I finally tracked down why on travis ci all ctype test crash
It looks like clang introduces a miscompile (or the c code breaks alignment rule)
I could not reproduce the issue because even though I am compiling pyston with clang the modules get compiled by gcc because I don't overwrite CC like the travis build dose
by using clang I can now reproduce it locally
Marius Wachtler
@undingen
Feb 29 2016 20:42
looks like clang thinks that the PyCArgObject::valuefield is 16byte aligned and generates a movaps while it's only 8byte aligned
from from_cpython/Modules/_ctypes/callbacks.c (but other access of this field are also wrong):
PyCArgObject *
PyCArgObject_new(void)
{
    PyCArgObject *p;
    p = PyObject_New(PyCArgObject, &PyCArg_Type);
    if (p == NULL)
        return NULL;
    p->pffi_type = NULL;
    p->tag = '\0';
    p->obj = NULL;
    memset(&p->value, 0, sizeof(p->value));
    return p;
}
clang 3-5: (crash)
Dump of assembler code for function PyCArgObject_new:
   0x00007ffff36ca110 <+0>:    push   %rax
   0x00007ffff36ca111 <+1>:    mov    0x20ee80(%rip),%rdi        # 0x7ffff38d8f98
   0x00007ffff36ca118 <+8>:    callq  0x7ffff36c18c0 <_PyObject_New@plt>
   0x00007ffff36ca11d <+13>:    xor    %ecx,%ecx
   0x00007ffff36ca11f <+15>:    test   %rax,%rax
   0x00007ffff36ca122 <+18>:    je     0x7ffff36ca142 <PyCArgObject_new+50>
   0x00007ffff36ca124 <+20>:    movq   $0x0,0x8(%rax)   ; p->pffi_type = NULL;
   0x00007ffff36ca12c <+28>:    movb   $0x0,0x10(%rax)  ; p->tag = '\0';
   0x00007ffff36ca130 <+32>:    xorps  %xmm0,%xmm0      ; memset(&p->value, 0, sizeof(p->value));
=> 0x00007ffff36ca133 <+35>:    movaps %xmm0,0x20(%rax) ; rax = 0x1270190d28
   0x00007ffff36ca137 <+39>:    movq   $0x0,0x30(%rax)  ; p->obj = NULL;
   0x00007ffff36ca13f <+47>:    mov    %rax,%rcx
   0x00007ffff36ca142 <+50>:    mov    %rcx,%rax
   0x00007ffff36ca145 <+53>:    pop    %rdx
   0x00007ffff36ca146 <+54>:    retq
Marius Wachtler
@undingen
Feb 29 2016 20:47
gcc 4.8 (works)
Dump of assembler code for function PyCArgObject_new:
=> 0x00007ffff36cb040 <+0>:    sub    $0x8,%rsp
   0x00007ffff36cb044 <+4>:    mov    0x20df4d(%rip),%rdi        # 0x7ffff38d8f98
   0x00007ffff36cb04b <+11>:    callq  0x7ffff36c1880 <_PyObject_New@plt>
   0x00007ffff36cb050 <+16>:    test   %rax,%rax
   0x00007ffff36cb053 <+19>:    je     0x7ffff36cb079 <PyCArgObject_new+57>
   0x00007ffff36cb055 <+21>:    movq   $0x0,0x8(%rax)
   0x00007ffff36cb05d <+29>:    movb   $0x0,0x10(%rax)
   0x00007ffff36cb061 <+33>:    movq   $0x0,0x30(%rax)
   0x00007ffff36cb069 <+41>:    movq   $0x0,0x20(%rax)
   0x00007ffff36cb071 <+49>:    movq   $0x0,0x28(%rax)
   0x00007ffff36cb079 <+57>:    add    $0x8,%rsp
   0x00007ffff36cb07d <+61>:    retq
Kevin Modzelewski
@kmod
Feb 29 2016 21:21
oh crazy
is it our fault for not aligning the object to 16 bytes?
Kevin Modzelewski
@kmod
Feb 29 2016 21:30
it looks like alignof(PyCArgObject) is 16
and malloc (and other allocators presumably) are supposed to return data that is 16-byte aligned because of this kind of issue
I think this might fall into the category of "things that will be fixed with refcounting" :P
since we no longer have the 8-byte gc header
can we comment out the long double field for now? :)
Marius Wachtler
@undingen
Feb 29 2016 22:02
I will fix it tomorrow
this should allow us to enable two cpython tests and finally allow me to merge in the cffi change :-)