classification
Title: ctypes change made clang fail to build
Type: Stage: patch review
Components: ctypes Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Paul Monson, petr.viktorin, serge-sans-paille, steve.dower, vstinner
Priority: normal Keywords: patch

Created on 2019-06-03 09:21 by petr.viktorin, last changed 2019-06-24 23:40 by steve.dower.

Pull Requests
URL Status Linked Edit
PR 13796 open Paul Monson, 2019-06-04 01:14
Messages (11)
msg344394 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2019-06-03 09:21
Hello,
I haven't investigated this fully yet, and won't be able to find time today. I'm forwarding the report here in case someone familiar with the code wants to take a look.

On Fedora, "clang" fails to build with Python 3.8, probably due this change (which was supposed to be Windows-only):
https://github.com/python/cpython/commit/32119e10b792ad7ee4e5f951a2d89ddbaf111cc5#diff-998bfefaefe2ab83d5f523e18f158fa4R413

According to serge_sans_paille: if ``self->b_ptr`` contains pointer, the ``memcpy`` creates sharing, and this is dangerous: if a ``__del__`` happens to free the original pointer, we end up with a dangling reference in ``new_ptr``. As far as I can tell, this is what happens in the clang bindings code.

Fedora discussion with link to logs: https://bugzilla.redhat.com/show_bug.cgi?id=1715016
msg344461 - (view) Author: Paul Monson (Paul Monson) * Date: 2019-06-03 18:57
The change causing the build failure comes from this commit.
https://github.com/python/cpython/pull/3806/commits/ea8a0dcea68409d37f3ad9e60e42777c76ad903b

The commit comment says: "Fix copy of structures when passed by value."

Steve, do you recall what this change fixed?
msg344493 - (view) Author: Paul Monson (Paul Monson) * Date: 2019-06-03 23:42
If I undo the changes to StructUnionType_paramfunc then test_pass_by_value (ctypes.test.test_structures.StructureTestCase) fails on x64 on Windows.  Looking at the code I don't think this is specific to Windows.

This is a test for fixing this issue: https://bugs.python.org/issue29565
msg344498 - (view) Author: Paul Monson (Paul Monson) * Date: 2019-06-03 23:55
Reading the related bugs more carefully I think the struct/union passing conventions are different on Windows x64 and Linux.  I have a fix which works for Windows but preserves the prior code for Linux.
msg344579 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-04 14:43
> On Fedora, "clang" fails to build with Python 3.8

Would you mind to elaborate? It works for me on Fedora 30 with clang 8.0.0:

$ ./configure CC=clang
$ make
$ ./python -m test -v test_ctypes
...
Tests result: SUCCESS

I tested the master branch of Python.
msg344581 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2019-06-04 14:47
The issue is with building clang using Python 3.8; not building Python 3.8 using clang :)
msg344583 - (view) Author: (serge-sans-paille) * Date: 2019-06-04 14:50
@vstinner: to reproduce the issue

```
git clone https://github.com/llvm/llvm-project.git
cd llvm-project
mkdir _build
cd _build
cmake3 ../llvm -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra" -DBUILD_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=$PWD/install -DPYTHON_EXECUTABLE=$HOME/sources/cpython/_build/install/bin/python3
make -j4
```
msg344589 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-04 15:09
It would be great to be able to write a reproducer and then convert it into a proper unit test.
msg344625 - (view) Author: Paul Monson (Paul Monson) * Date: 2019-06-04 19:03
I added a unittest to the PR that illustrates the problem.  It doesn't pass yet.
msg345394 - (view) Author: Paul Monson (Paul Monson) * Date: 2019-06-12 17:57
I did some reading about parameter passing and it's still not clear to me whether https://bugs.python.org/issue37140 is a bug in CPython or whether the clang bindings were relying on incorrect parameter passing behavior to work.

The change in https://github.com/python/cpython/pull/13796 restores the previous behavior where Windows and non-Windows builds pass structs differently.
msg346451 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-06-24 23:40
> According to serge_sans_paille: if ``self->b_ptr`` contains pointer, the ``memcpy`` creates sharing, and this is dangerous: if a ``__del__`` happens to free the original pointer, we end up with a dangling reference in ``new_ptr``. As far as I can tell, this is what happens in the clang bindings code.

We probably need a second parg->obj to keep self alive for as long as copied_self. Or pack it into a tuple.

Having a repro test for this would be ideal, especially if we can make it happen (even crash) on all platforms. The double-free issue would seem to be real, and I don't want it to crash on Windows either.
History
Date User Action Args
2019-06-24 23:40:13steve.dowersetmessages: + msg346451
2019-06-12 17:57:26Paul Monsonsetmessages: + msg345394
2019-06-04 19:03:44Paul Monsonsetmessages: + msg344625
2019-06-04 15:09:47vstinnersetmessages: + msg344589
2019-06-04 14:50:54serge-sans-paillesetmessages: + msg344583
2019-06-04 14:47:34petr.viktorinsetmessages: + msg344581
2019-06-04 14:43:53vstinnersetnosy: + vstinner
messages: + msg344579
2019-06-04 01:14:05Paul Monsonsetkeywords: + patch
stage: patch review
pull_requests: + pull_request13681
2019-06-03 23:55:05Paul Monsonsetmessages: + msg344498
2019-06-03 23:42:22Paul Monsonsetmessages: + msg344493
2019-06-03 18:57:15Paul Monsonsetnosy: + steve.dower, Paul Monson
messages: + msg344461
2019-06-03 09:51:07serge-sans-paillesetnosy: + serge-sans-paille
2019-06-03 09:21:37petr.viktorincreate