bpo-22273: Update ctypes to correctly handle arrays in small structur… #15839

vsajip · 2019-09-10T11:36:09Z

…es on x86_64.

https://bugs.python.org/issue22273

…es on x86_64.

vsajip · 2019-09-10T11:38:39Z

@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.

Modules/_ctypes/stgdict.c

ZackerySpytz · 2019-09-11T04:59:37Z

Modules/_ctypes/stgdict.c

+            StgDictObject *dict;
+            int bitsize = 0;
+
+            if (!pair || !PyArg_ParseTuple(pair, "UO|i", &name, &desc, &bitsize)) {


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

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).

Modules/_ctypes/stgdict.c

miss-islington · 2019-09-25T03:39:59Z

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

miss-islington · 2019-09-25T03:39:59Z

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

bedevere-bot · 2019-09-25T03:40:10Z

GH-16369 is a backport of this pull request to the 3.7 branch.

bedevere-bot · 2019-09-25T03:40:17Z

GH-16370 is a backport of this pull request to the 3.8 branch.

pythonGH-15839) (cherry picked from commit 12f209e) Co-authored-by: Vinay Sajip <vinay_sajip@yahoo.co.uk>

bedevere-bot · 2019-09-25T03:57:11Z

⚠️⚠️⚠️ 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

bedevere-bot · 2019-09-25T04:07:18Z

⚠️⚠️⚠️ 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

GH-15839) (GH-16370) (cherry picked from commit 12f209e)

GH-15839) (GH-16369) (cherry picked from commit 12f209e)

bedevere-bot · 2019-09-25T04:27:07Z

⚠️⚠️⚠️ 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

bedevere-bot · 2019-09-25T04:35:09Z

⚠️⚠️⚠️ 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

bedevere-bot · 2019-09-25T04:57:58Z

⚠️⚠️⚠️ 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

Kentzo · 2019-09-25T05:06:19Z

Modules/_ctypes/stgdict.c

+
+#define MAX_ELEMENTS 16
+
+    if (arrays_seen && (size <= 16)) {


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

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.

Kentzo · 2019-09-25T06:19:50Z

Modules/_ctypes/stgdict.c

+                /* 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);


Assertion can be checked just once before the while loop.

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.

pythonGH-15839)

bpo-22273: Update ctypes to correctly handle arrays in small structur…

eb13d50

…es on x86_64.

vsajip added type-bug An unexpected behavior, bug, or error skip news labels Sep 10, 2019

the-knights-who-say-ni added the CLA signed label Sep 10, 2019

bedevere-bot added the awaiting core review label Sep 10, 2019

vsajip requested a review from abalkin September 10, 2019 12:35

ZackerySpytz reviewed Sep 11, 2019

View reviewed changes

Addressed comments by Zackery Spytz.

91d7339

vsajip merged commit 12f209e into python:master Sep 25, 2019

vsajip deleted the fix-22273 branch September 25, 2019 03:38

bedevere-bot removed the awaiting core review label Sep 25, 2019

vsajip added needs backport to 3.7 labels Sep 25, 2019

bedevere-bot removed needs backport to 3.7 labels Sep 25, 2019

vsajip pushed a commit that referenced this pull request Sep 25, 2019

bpo-22273: Update ctypes to correctly handle arrays in small structur… (

ce62dcc

GH-15839) (GH-16370) (cherry picked from commit 12f209e)

vsajip pushed a commit that referenced this pull request Sep 25, 2019

bpo-22273: Update ctypes to correctly handle arrays in small structur… (

16c0f6d

GH-15839) (GH-16369) (cherry picked from commit 12f209e)

Kentzo reviewed Sep 25, 2019

View reviewed changes

jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019

bpo-22273: Update ctypes to correctly handle arrays in small structur… (

5fee2e4

pythonGH-15839)

This was referenced Apr 10, 2022

ctypes: unions as arguments #60779

Open

ctypes: structure with bitfields as argument #60780

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-22273: Update ctypes to correctly handle arrays in small structur… #15839

bpo-22273: Update ctypes to correctly handle arrays in small structur… #15839

vsajip commented Sep 10, 2019 •

edited by bedevere-bot

Loading

vsajip commented Sep 10, 2019 •

edited by bedevere-bot

Loading

ZackerySpytz Sep 11, 2019

vsajip Sep 11, 2019

miss-islington commented Sep 25, 2019

miss-islington commented Sep 25, 2019

bedevere-bot commented Sep 25, 2019

bedevere-bot commented Sep 25, 2019

bedevere-bot commented Sep 25, 2019

bedevere-bot commented Sep 25, 2019

bedevere-bot commented Sep 25, 2019

bedevere-bot commented Sep 25, 2019

bedevere-bot commented Sep 25, 2019

Kentzo Sep 25, 2019

vsajip Sep 25, 2019

Kentzo Sep 25, 2019

vsajip Sep 25, 2019 •

edited

Loading

bpo-22273: Update ctypes to correctly handle arrays in small structur… #15839

bpo-22273: Update ctypes to correctly handle arrays in small structur… #15839

Conversation

vsajip commented Sep 10, 2019 • edited by bedevere-bot Loading

vsajip commented Sep 10, 2019 • edited by bedevere-bot Loading

ZackerySpytz Sep 11, 2019

Choose a reason for hiding this comment

vsajip Sep 11, 2019

Choose a reason for hiding this comment

miss-islington commented Sep 25, 2019

miss-islington commented Sep 25, 2019

bedevere-bot commented Sep 25, 2019

bedevere-bot commented Sep 25, 2019

bedevere-bot commented Sep 25, 2019

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

bedevere-bot commented Sep 25, 2019

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

bedevere-bot commented Sep 25, 2019

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

bedevere-bot commented Sep 25, 2019

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

bedevere-bot commented Sep 25, 2019

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

Kentzo Sep 25, 2019

Choose a reason for hiding this comment

vsajip Sep 25, 2019

Choose a reason for hiding this comment

Kentzo Sep 25, 2019

Choose a reason for hiding this comment

vsajip Sep 25, 2019 • edited Loading

Choose a reason for hiding this comment

vsajip commented Sep 10, 2019 •

edited by bedevere-bot

Loading

vsajip commented Sep 10, 2019 •

edited by bedevere-bot

Loading

vsajip Sep 25, 2019 •

edited

Loading