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

vsajip

@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 \|\| !PyArg_ParseTuple(pair, "UO\|i", &name, &desc, &bitsize)) {

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