bpo-22273: Update ctypes to correctly handle arrays in small structur… by vsajip · Pull Request #15839 · python/cpython (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation19 Commits2 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

@eryksun Would appreciate your comments on this PR. Here, I only added support for arrays in small structures - I will address the union and bitfield issues in bpo-16575 and bpo-16576 in a separate PR.

StgDictObject *dict;
int bitsize = 0;

if (!pair \|

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to PEP 7, lines should be limited to 79 chars.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO we shouldn't apply this rule too strictly - it's intended to avoid too-deep nesting in code. In just about all cases in this file, the longer lines are due to verbose error messages or long variable names (neither of which are a bad thing).

Thanks @vsajip for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

Thanks @vsajip for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Sep 25, 2019

pythonGH-15839)

(cherry picked from commit 12f209e)

Co-authored-by: Vinay Sajip vinay_sajip@yahoo.co.uk

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot ARMv7 Debian buster 3.x has failed when building commit 12f209e.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/176/builds/1331) and take a look at the build logs.
Check if the failure is related to this commit (12f209e) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/176/builds/1331

Failed tests:

test_ctypes

Failed subtests:

test_array_in_struct - ctypes.test.test_structures.StructureTestCase

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

405 tests OK.

10 slowest tests:

test_tokenize: 6 min 16 sec
test_tools: 5 min 27 sec
test_gdb: 4 min 43 sec
test_lib2to3: 4 min 33 sec
test_concurrent_futures: 4 min 14 sec
test_multiprocessing_spawn: 4 min 8 sec
test_smtpnet: 3 min 2 sec
test_asyncio: 2 min 51 sec
test_unicodedata: 2 min 42 sec
test_multiprocessing_forkserver: 2 min 25 sec

1 test failed:
test_ctypes

13 tests skipped:
test_devpoll test_ioctl test_kqueue test_msilib test_ossaudiodev
test_startfile test_tix test_tk test_ttk_guionly test_winconsoleio
test_winreg test_winsound test_zipfile64

1 re-run test:
test_ctypes

Total duration: 14 min 45 sec

Click to see traceback logs

Traceback (most recent call last): File "/ssd/buildbot/buildarea/3.x.gps-ubuntu-exynos5-armv7l/build/Lib/ctypes/test/test_structures.py", line 517, in test_array_in_struct self.assertEqual(result, expected) AssertionError: 2.71828 != 5.85987

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot PPC64LE Fedora 3.x has failed when building commit 12f209e.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/85/builds/3594) and take a look at the build logs.
Check if the failure is related to this commit (12f209e) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/85/builds/3594

Failed tests:

test_ctypes

Failed subtests:

test_array_in_struct - ctypes.test.test_structures.StructureTestCase

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

405 tests OK.

10 slowest tests:

test_multiprocessing_spawn: 3 min 3 sec
test_tools: 3 min 1 sec
test_tokenize: 3 min 456 ms
test_concurrent_futures: 2 min 48 sec
test_lib2to3: 2 min 6 sec
test_unicodedata: 1 min 44 sec
test_multiprocessing_forkserver: 1 min 36 sec
test_asyncio: 1 min 23 sec
test_gdb: 1 min 13 sec
test_multiprocessing_fork: 1 min 12 sec

1 test failed:
test_ctypes

13 tests skipped:
test_devpoll test_ioctl test_kqueue test_msilib test_ossaudiodev
test_startfile test_tix test_tk test_ttk_guionly test_winconsoleio
test_winreg test_winsound test_zipfile64

1 re-run test:
test_ctypes

Total duration: 23 min 52 sec

Click to see traceback logs

Traceback (most recent call last): File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64le/build/Lib/ctypes/test/test_structures.py", line 517, in test_array_in_struct self.assertEqual(result, expected) AssertionError: 2.71828 != 5.85987

vsajip pushed a commit that referenced this pull request

Sep 25, 2019

vsajip pushed a commit that referenced this pull request

Sep 25, 2019

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot ARMv7 Debian buster 3.8 has failed when building commit ce62dcc.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/207/builds/427) and take a look at the build logs.
Check if the failure is related to this commit (ce62dcc) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/207/builds/427

Failed tests:

test_ctypes

Failed subtests:

test_array_in_struct - ctypes.test.test_structures.StructureTestCase

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

409 tests OK.

10 slowest tests:

test_tokenize: 5 min 56 sec
test_concurrent_futures: 4 min 11 sec
test_lib2to3: 3 min 57 sec
test_gdb: 3 min 50 sec
test_multiprocessing_spawn: 3 min 49 sec
test_tools: 3 min 45 sec
test_asyncio: 3 min 8 sec
test_smtpnet: 3 min 2 sec
test_multiprocessing_forkserver: 2 min 34 sec
test_capi: 2 min 10 sec

1 test failed:
test_ctypes

13 tests skipped:
test_devpoll test_ioctl test_kqueue test_msilib test_ossaudiodev
test_startfile test_tix test_tk test_ttk_guionly test_winconsoleio
test_winreg test_winsound test_zipfile64

1 re-run test:
test_ctypes

Total duration: 13 min 43 sec

Click to see traceback logs

Traceback (most recent call last): File "/ssd/buildbot/buildarea/3.8.gps-ubuntu-exynos5-armv7l/build/Lib/ctypes/test/test_structures.py", line 517, in test_array_in_struct self.assertEqual(result, expected) AssertionError: 2.71828 != 5.85987

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot PPC64LE Fedora 3.8 has failed when building commit ce62dcc.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/228/builds/393) and take a look at the build logs.
Check if the failure is related to this commit (ce62dcc) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/228/builds/393

Failed tests:

test_ctypes

Failed subtests:

test_array_in_struct - ctypes.test.test_structures.StructureTestCase

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

409 tests OK.

10 slowest tests:

test_concurrent_futures: 2 min 41 sec
test_multiprocessing_spawn: 2 min 38 sec
test_tokenize: 2 min 26 sec
test_tools: 2 min 19 sec
test_lib2to3: 1 min 55 sec
test_multiprocessing_forkserver: 1 min 28 sec
test_asyncio: 1 min 16 sec
test_multiprocessing_fork: 1 min 12 sec
test_io: 56 sec 700 ms
test_capi: 54 sec 346 ms

1 test failed:
test_ctypes

13 tests skipped:
test_devpoll test_ioctl test_kqueue test_msilib test_ossaudiodev
test_startfile test_tix test_tk test_ttk_guionly test_winconsoleio
test_winreg test_winsound test_zipfile64

1 re-run test:
test_ctypes

Total duration: 20 min 35 sec

Click to see traceback logs

Traceback (most recent call last): File "/home/shager/cpython-buildarea/3.8.edelsohn-fedora-ppc64le/build/Lib/ctypes/test/test_structures.py", line 517, in test_array_in_struct self.assertEqual(result, expected) AssertionError: 2.71828 != 5.85987

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot PPC64LE Fedora 3.7 has failed when building commit 16c0f6d.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/120/builds/1315) and take a look at the build logs.
Check if the failure is related to this commit (16c0f6d) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/120/builds/1315

Failed tests:

test_ctypes

Failed subtests:

test_array_in_struct - ctypes.test.test_structures.StructureTestCase

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

402 tests OK.

10 slowest tests:

test_concurrent_futures: 2 min 37 sec
test_tokenize: 2 min 23 sec
test_multiprocessing_spawn: 2 min 21 sec
test_tools: 2 min 19 sec
test_lib2to3: 1 min 42 sec
test_multiprocessing_forkserver: 1 min 28 sec
test_multiprocessing_fork: 1 min 11 sec
test_io: 1 min 7 sec
test_asyncio: 1 min 2 sec
test_weakref: 41 sec 8 ms

1 test failed:
test_ctypes

13 tests skipped:
test_devpoll test_ioctl test_kqueue test_msilib test_ossaudiodev
test_startfile test_tix test_tk test_ttk_guionly test_winconsoleio
test_winreg test_winsound test_zipfile64

1 re-run test:
test_ctypes

Total duration: 19 min 23 sec

Click to see traceback logs

Traceback (most recent call last): File "/home/shager/cpython-buildarea/3.7.edelsohn-fedora-ppc64le/build/Lib/ctypes/test/test_structures.py", line 478, in test_array_in_struct self.assertEqual(result, expected) AssertionError: 2.71828 != 5.85987


#define MAX_ELEMENTS 16

if (arrays_seen && (size <= 16)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vsajip Is it intentional that 16 is used here instead of MAX_ELEMENTS?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, no. That's a slip-up. Looks like I'll need to find a fix for the buildbot failures, anyway ... so I'll deal with this soon.

/* Copy over the element's type, length times. */
while (length > 0) {
actual_types[actual_type_index++] = &edict->ffi_type_pointer;
assert(actual_type_index <= MAX_ELEMENTS);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assertion can be checked just once before the while loop.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But actual_type_index is incremented in the loop, and in theory the increment could take it past the end of the array, which is what the assertion is guarding against.

jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request

Dec 5, 2019

This was referenced

Apr 10, 2022