Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict conda-forge compilers for CI jobs #554

Merged
merged 3 commits into from
Sep 14, 2022
Merged

Conversation

ndevenish
Copy link
Collaborator

The generation on Clang 14 appears to be incorrect for this combination of boost, python. In particular, in

void init_module() {
using namespace boost::python;
def("read_uint8", read_uint8, (arg("file"), arg("count")));
, the generated assembly appears to early-call _Py_Dealloc near the top of the function without any kind of zero protections or checks:

    0x101c03be8 <+40>:   lea    rbx, [rip + 0x7d78]       ; "file"
    0x101c03bef <+47>:   mov    qword ptr [rbp - 0x88], rbx
    0x101c03bf6 <+54>:   mov    qword ptr [rbp - 0x30], 0x0
    0x101c03bfe <+62>:   lea    r12, [rip + 0x7d67]       ; "count"
    0x101c03c05 <+69>:   mov    qword ptr [rbp - 0x38], r12
    0x101c03c09 <+73>:   xorps  xmm0, xmm0
    0x101c03c0c <+76>:   movups xmmword ptr [rbp - 0x60], xmm0
    0x101c03c10 <+80>:   mov    qword ptr [rbp - 0x50], 0x0
    0x101c03c18 <+88>:   mov    qword ptr [rbp - 0x68], rbx
    0x101c03c1c <+92>:   call   0x101c0a63c               ; symbol stub for: _Py_Dealloc
    0x101c03c21 <+97>:   mov    qword ptr [rbp - 0x60], 0x0
    0x101c03c29 <+105>:  mov    qword ptr [rbp - 0x58], r12
    0x101c03c2d <+109>:  call   0x101c0a63c               ; symbol stub for: _Py_Dealloc

Whereas on clang 13 the calls to Dealloc are later, and include zero-protection around the calls (to _Py_Dealloc and the class destructors):

    0x101c03e85 <+197>:  mov    rdi, qword ptr [rbp - 0x50]
    0x101c03e89 <+201>:  test   rdi, rdi
    0x101c03e8c <+204>:  je     0x101c03e98               ; <+216> [inlined] boost::python::handle<_object>::~handle() + 19 at handle.hpp:183
    0x101c03e8e <+206>:  dec    qword ptr [rdi]
    0x101c03e91 <+209>:  jne    0x101c03e98               ; <+216> [inlined] boost::python::handle<_object>::~handle() + 19 at handle.hpp:183
    0x101c03e93 <+211>:  call   0x101c0a6bc               ; symbol stub for: _Py_Dealloc
    0x101c03e98 <+216>

I am unsure if this difference is coming from the compiler, or from the newer build of python associated with this compiler that conda-forge selects when constraining the compiler. In any case, constraining the compiler works for now and hopefully the problem will be fixed before this becomes a problem.

ndevenish and others added 3 commits September 14, 2022 11:47
The generation on Clang 14 appears to be incorrect for this
combination of boost. In particular, in dxtbx/src/boost_python/ext.cpp::init_module,
the code appears to early-call _Py_Dealloc near the top of the function without
any kind of zero protections or checks:

    0x101c03be8 <+40>:   lea    rbx, [rip + 0x7d78]       ; "file"
    0x101c03bef <+47>:   mov    qword ptr [rbp - 0x88], rbx
    0x101c03bf6 <+54>:   mov    qword ptr [rbp - 0x30], 0x0
    0x101c03bfe <+62>:   lea    r12, [rip + 0x7d67]       ; "count"
    0x101c03c05 <+69>:   mov    qword ptr [rbp - 0x38], r12
    0x101c03c09 <+73>:   xorps  xmm0, xmm0
    0x101c03c0c <+76>:   movups xmmword ptr [rbp - 0x60], xmm0
    0x101c03c10 <+80>:   mov    qword ptr [rbp - 0x50], 0x0
    0x101c03c18 <+88>:   mov    qword ptr [rbp - 0x68], rbx
    0x101c03c1c <+92>:   call   0x101c0a63c               ; symbol stub for: _Py_Dealloc
    0x101c03c21 <+97>:   mov    qword ptr [rbp - 0x60], 0x0
    0x101c03c29 <+105>:  mov    qword ptr [rbp - 0x58], r12
    0x101c03c2d <+109>:  call   0x101c0a63c               ; symbol stub for: _Py_Dealloc

Whereas on clang 13 the calls to Dealloc are later, and include zero-protection
around the calls (to _Py_Dealloc and the class destructors):

    0x101c03e85 <+197>:  mov    rdi, qword ptr [rbp - 0x50]
    0x101c03e89 <+201>:  test   rdi, rdi
    0x101c03e8c <+204>:  je     0x101c03e98               ; <+216> [inlined] boost::python::handle<_object>::~handle() + 19 at handle.hpp:183
    0x101c03e8e <+206>:  dec    qword ptr [rdi]
    0x101c03e91 <+209>:  jne    0x101c03e98               ; <+216> [inlined] boost::python::handle<_object>::~handle() + 19 at handle.hpp:183
    0x101c03e93 <+211>:  call   0x101c0a6bc               ; symbol stub for: _Py_Dealloc
    0x101c03e98 <+216>

I am unsure if this difference is coming from the compiler, or from the
newer build of python associated with this compiler that conda-forge selects
when constraining the compiler. In any case, constraining the compiler works
for now and hopefully the problem will be fixed before this becomes a problem.
@codecov
Copy link

codecov bot commented Sep 14, 2022

Codecov Report

Merging #554 (24dc7ca) into main (23f28ff) will not change coverage.
The diff coverage is n/a.

❗ Current head 24dc7ca differs from pull request most recent head b1b01c7. Consider uploading reports for the commit b1b01c7 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #554   +/-   ##
=======================================
  Coverage   40.85%   40.85%           
=======================================
  Files         176      176           
  Lines       15613    15613           
  Branches     2818     2818           
=======================================
  Hits         6378     6378           
  Misses       8666     8666           
  Partials      569      569           

@ndevenish ndevenish merged commit 947753e into cctbx:main Sep 14, 2022
@ndevenish ndevenish deleted the compilers branch September 14, 2022 13:33
@ndevenish
Copy link
Collaborator Author

This looks like it's a compiler-specific issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants