-
-
Notifications
You must be signed in to change notification settings - Fork 32k
gh-97588: Fix ctypes structs #97702
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
gh-97588: Fix ctypes structs #97702
Changes from 88 commits
7922585
2307932
47f826c
5a32211
8d79d8f
2d07375
8bc8535
c3d162b
e5ed9ac
b0f9819
2dee0e3
79ef347
6170dad
871ca1a
c162144
a57cf2c
3f7c4cc
e39a271
359ed58
52ef8d2
e8102c4
600e144
9bad706
5121857
6cd27ea
6b6fa8a
ca9d580
1401ee4
235fa68
a534e18
53db061
9c249c3
3cf4747
8beddb9
9b64ff6
4d0dab5
bf98667
8223063
e80a09a
33a10fb
0f3c5a3
47eca3c
456afd0
bc799ac
212cf13
e7e5ae9
45e26ec
ab42187
640c062
f3e04af
bff34a1
5d6f0f7
d9c1fca
cc9ea8a
1554083
a86eadc
eefa1ad
28c7fe7
f0a92b0
a3a390b
e997b5f
5a7ea09
0f73919
cdfc3c6
6226e55
4d48eba
20b5582
eb502d7
0cf5f4e
3457558
fd3cd0b
2d8492e
a9f7f14
bee6c53
220e19e
60cebe2
3424a7d
2506eea
65654b4
98767da
3ca703f
c40ef7d
d840f01
8b9f0eb
4e8c220
0369d0d
d4dc2c0
f75d7d6
cdc5cdc
0da36ad
b6f7117
5e47d5f
de22b39
cdd1860
dd84ac3
8a6fb67
07ea42d
3fa7f55
eb2c4fb
22cd86c
637b961
5309b7e
489c5c8
3594473
f780496
6ade022
cfa6647
df162b0
836a5cc
b07ae33
6dd7a8d
0cf1049
bc91549
70bbc26
6e23b3d
1b90841
2c4874b
738323f
af7487c
5353352
fe76d45
c4714f1
c79725d
cace0c9
61e92bf
ba61051
8991444
bc1225b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,10 @@ | ||
import os | ||
import sys | ||
import unittest | ||
from ctypes import (CDLL, Structure, sizeof, POINTER, byref, alignment, | ||
LittleEndianStructure, BigEndianStructure, | ||
c_byte, c_ubyte, c_char, c_char_p, c_void_p, c_wchar, | ||
c_uint32, c_uint64, | ||
c_uint8, c_uint16, c_uint32, c_uint64, | ||
c_short, c_ushort, c_int, c_uint, c_long, c_ulong, c_longlong, c_ulonglong) | ||
from test import support | ||
from test.support import import_helper | ||
|
@@ -33,6 +34,30 @@ class BITS(Structure): | |
func.argtypes = POINTER(BITS), c_char | ||
|
||
|
||
class BITS_msvc(Structure): | ||
_ms_struct_ = 1 | ||
_fields_ = [("A", c_int, 1), | ||
("B", c_int, 2), | ||
("C", c_int, 3), | ||
("D", c_int, 4), | ||
("E", c_int, 5), | ||
("F", c_int, 6), | ||
("G", c_int, 7), | ||
("H", c_int, 8), | ||
("I", c_int, 9), | ||
|
||
("M", c_short, 1), | ||
("N", c_short, 2), | ||
("O", c_short, 3), | ||
("P", c_short, 4), | ||
("Q", c_short, 5), | ||
("R", c_short, 6), | ||
("S", c_short, 7)] | ||
|
||
func_msvc = CDLL(_ctypes_test.__file__).unpack_bitfields_msvc | ||
func_msvc.argtypes = POINTER(BITS_msvc), c_char | ||
|
||
|
||
class C_Test(unittest.TestCase): | ||
|
||
def test_ints(self): | ||
|
@@ -42,18 +67,43 @@ def test_ints(self): | |
setattr(b, name, i) | ||
self.assertEqual(getattr(b, name), func(byref(b), name.encode('ascii'))) | ||
|
||
# bpo-46913: _ctypes/cfield.c h_get() has an undefined behavior | ||
@support.skip_if_sanitizer(ub=True) | ||
def test_shorts(self): | ||
b = BITS() | ||
name = "M" | ||
# See Modules/_ctypes/_ctypes_test.c for where the magic 999 comes from. | ||
if func(byref(b), name.encode('ascii')) == 999: | ||
# unpack_bitfields and unpack_bitfields_msvc in | ||
# Modules/_ctypes/_ctypes_test.c return 999 to indicate | ||
# an invalid name. 'M' is only valid, if signed short bitfields | ||
# are supported by the C compiler. | ||
self.skipTest("Compiler does not support signed short bitfields") | ||
for i in range(256): | ||
for name in "MNOPQRS": | ||
b = BITS() | ||
setattr(b, name, i) | ||
self.assertEqual(getattr(b, name), func(byref(b), name.encode('ascii'))) | ||
self.assertEqual( | ||
getattr(b, name), | ||
func(byref(b), (name.encode('ascii'))), | ||
(name, i)) | ||
|
||
def test_shorts_msvc_mode(self): | ||
b = BITS_msvc() | ||
name = "M" | ||
# See Modules/_ctypes/_ctypes_test.c for where the magic 999 comes from. | ||
if func_msvc(byref(b), name.encode('ascii')) == 999: | ||
matthiasgoergens marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# unpack_bitfields and unpack_bitfields_msvc in | ||
# Modules/_ctypes/_ctypes_test.c return 999 to indicate | ||
# an invalid name. 'M' is only valid, if signed short bitfields | ||
# are supported by the C compiler. | ||
self.skipTest("Compiler does not support signed short bitfields") | ||
for i in range(256): | ||
for name in "MNOPQRS": | ||
b = BITS_msvc() | ||
setattr(b, name, i) | ||
self.assertEqual( | ||
getattr(b, name), | ||
func_msvc(byref(b), name.encode('ascii')), | ||
(name, i)) | ||
|
||
|
||
signed_int_types = (c_byte, c_short, c_int, c_long, c_longlong) | ||
|
@@ -236,6 +286,157 @@ class X(Structure): | |
else: | ||
self.assertEqual(sizeof(X), sizeof(c_int) * 2) | ||
|
||
def test_mixed_5(self): | ||
class X(Structure): | ||
_fields_ = [ | ||
('A', c_uint, 1), | ||
('B', c_ushort, 16)] | ||
a = X() | ||
a.A = 0 | ||
a.B = 1 | ||
self.assertEqual(1, a.B) | ||
|
||
def test_mixed_6(self): | ||
class X(Structure): | ||
_fields_ = [ | ||
('A', c_ulonglong, 1), | ||
('B', c_uint, 32)] | ||
a = X() | ||
a.A = 0 | ||
a.B = 1 | ||
self.assertEqual(1, a.B) | ||
|
||
def test_mixed_7(self): | ||
class X(Structure): | ||
_fields_ = [ | ||
("A", c_uint), | ||
('B', c_uint, 20), | ||
('C', c_ulonglong, 24)] | ||
self.assertEqual(16, sizeof(X)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assumes specific |
||
|
||
def test_mixed_8(self): | ||
class Foo(Structure): | ||
_fields_ = [ | ||
("A", c_uint), | ||
("B", c_uint, 32), | ||
("C", c_ulonglong, 1), | ||
] | ||
|
||
class Bar(Structure): | ||
_fields_ = [ | ||
("A", c_uint), | ||
("B", c_uint), | ||
("C", c_ulonglong, 1), | ||
] | ||
self.assertEqual(sizeof(Foo), sizeof(Bar)) | ||
|
||
def test_mixed_9(self): | ||
class X(Structure): | ||
_fields_ = [ | ||
("A", c_uint8), | ||
("B", c_uint, 1), | ||
] | ||
if sys.platform == 'win32': | ||
self.assertEqual(8, sizeof(X)) | ||
else: | ||
self.assertEqual(4, sizeof(X)) | ||
|
||
def test_mixed_10(self): | ||
class X(Structure): | ||
_fields_ = [ | ||
("A", c_uint32, 1), | ||
("B", c_uint64, 1), | ||
] | ||
if sys.platform == 'win32': | ||
self.assertEqual(8, alignment(X)) | ||
self.assertEqual(16, sizeof(X)) | ||
else: | ||
self.assertEqual(8, alignment(X)) | ||
self.assertEqual(8, sizeof(X)) | ||
|
||
def test_gh_95496(self): | ||
for field_width in range(1, 33): | ||
class TestStruct(Structure): | ||
_fields_ = [ | ||
("Field1", c_uint32, field_width), | ||
("Field2", c_uint8, 8) | ||
] | ||
|
||
cmd = TestStruct() | ||
cmd.Field2 = 1 | ||
self.assertEqual(1, cmd.Field2) | ||
|
||
def test_gh_84039(self): | ||
class Bad(Structure): | ||
_pack_ = 1 | ||
_fields_ = [ | ||
("a0", c_uint8, 1), | ||
("a1", c_uint8, 1), | ||
("a2", c_uint8, 1), | ||
("a3", c_uint8, 1), | ||
("a4", c_uint8, 1), | ||
("a5", c_uint8, 1), | ||
("a6", c_uint8, 1), | ||
("a7", c_uint8, 1), | ||
("b0", c_uint16, 4), | ||
("b1", c_uint16, 12), | ||
] | ||
|
||
|
||
class GoodA(Structure): | ||
_pack_ = 1 | ||
_fields_ = [ | ||
("a0", c_uint8, 1), | ||
("a1", c_uint8, 1), | ||
("a2", c_uint8, 1), | ||
("a3", c_uint8, 1), | ||
("a4", c_uint8, 1), | ||
("a5", c_uint8, 1), | ||
("a6", c_uint8, 1), | ||
("a7", c_uint8, 1), | ||
] | ||
|
||
|
||
class Good(Structure): | ||
_pack_ = 1 | ||
_fields_ = [ | ||
("a", GoodA), | ||
("b0", c_uint16, 4), | ||
("b1", c_uint16, 12), | ||
] | ||
|
||
self.assertEqual(3, sizeof(Bad)) | ||
self.assertEqual(3, sizeof(Good)) | ||
|
||
def test_gh_73939(self): | ||
class MyStructure(Structure): | ||
_pack_ = 1 | ||
_fields_ = [ | ||
("P", c_uint16), | ||
("L", c_uint16, 9), | ||
("Pro", c_uint16, 1), | ||
("G", c_uint16, 1), | ||
("IB", c_uint16, 1), | ||
("IR", c_uint16, 1), | ||
("R", c_uint16, 3), | ||
("T", c_uint32, 10), | ||
("C", c_uint32, 20), | ||
("R2", c_uint32, 2) | ||
] | ||
self.assertEqual(8, sizeof(MyStructure)) | ||
|
||
def test_gh_86098(self): | ||
class X(Structure): | ||
_fields_ = [ | ||
("a", c_uint8, 8), | ||
("b", c_uint8, 8), | ||
("c", c_uint32, 16) | ||
] | ||
if sys.platform == 'win32': | ||
self.assertEqual(8, sizeof(X)) | ||
else: | ||
self.assertEqual(4, sizeof(X)) | ||
|
||
def test_anon_bitfields(self): | ||
# anonymous bit-fields gave a strange error message | ||
class X(Structure): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Fix ctypes construction of structs from description. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you expand a bit here? This change could presumably impact anyone using packed bit fields through ctypes, so it would be good to mention at least some of those keywords here. A couple of sentences are okay (and feel free to add " There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In fact maybe a what's new entry would be appreciated too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe... "we're getting it right now" isn't the sort of thing we'd usually advertise as "new", but on the other hand, there's a good chance people have discovered it was buggy before and are now avoiding it. A note there may get noticed. We still have someone edit what's new before it goes out, right? It won't hurt to add it, and the editor can choose whether to keep it or rephrase/reframe it. For @matthiasgoergens, we're talking about |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider updating tests like this that loop over byte values into sub-tests via the https://docs.python.org/3/library/unittest.html#distinguishing-test-iterations-using-subtests API call so that you don't need to pass
msg=(name, i)
here to get a meaningful error message.