Vlad Riscutia <riscutiav...@gmail.com> added the comment:

Changing title and type to better reflect issue.

On Windows MSVC build, ctypes is not correctly setting bitfields backed by 64 
bit integers if specifying custom width. Simple repro:

from ctypes import *

class X(Structure):
    _fields_ = [("a", c_ulonglong, 16),
                ("b", c_ulonglong, 32),
                ("c", c_ulonglong, 16)]
s = X()
s.b = 1

print(s.b) # this prints 0

Whenever field width goes over 32 bits, setting or getting value of the field 
in cfield.c will only look at last (<actual size> - 32) bits of the field. So 
if we have a field of 34 bits, only least significant 2 bits will be operated 
on. The above repro results in an (<actual size> - 32) = 0 bits so nothing is 
getting set with the assignement.

This is not caught in unit test package because we have only this in 
test_bitfields.py:

    def test_ulonglong(self):
        class X(Structure):
            _fields_ = [("a", c_ulonglong, 1),
                        ("b", c_ulonglong, 62),
                        ("c", c_ulonglong, 1)]

        self.assertEqual(sizeof(X), sizeof(c_longlong))
        x = X()
        self.assertEqual((x.a, x.b, x.c), (0, 0, 0))
        x.a, x.b, x.c = 7, 7, 7
        self.assertEqual((x.a, x.b, x.c), (1, 7, 1))

For 62 bit width, we will actually operate on last 30 bits but this test passes 
as 7 fits in those bits. If we would actually try to set it to 
0x3FFFFFFFFFFFFFFF, result will be different than expected (0x3FFFFFFF).

I will look into extending UT package with some tests that set full range of 
bits and check if they are actually being set correctly.

This issue reproes with latest bits but only on Windows. Root cause seems to be 
BIT_MASK macro in cfield.c which is ifdef-ed to something different on Windows. 
I patched on my machine by removing the special treatment:

@@ -429,12 +429,7 @@
 #define LOW_BIT(x)  ((x) & 0xFFFF)
 #define NUM_BITS(x) ((x) >> 16)
 
-/* This seems nore a compiler issue than a Windows/non-Windows one */
-#ifdef MS_WIN32
-#  define BIT_MASK(size) ((1 << NUM_BITS(size))-1)
-#else
-#  define BIT_MASK(size) ((1LL << NUM_BITS(size))-1)
-#endif
+#define BIT_MASK(size) ((1LL << NUM_BITS(size))-1)
 
 /* This macro CHANGES the first parameter IN PLACE. For proper sign handling,
    we must first shift left, then right.



Unittests still pass with this in place and now fields are handled correctly 
though I don't know why this was put in in the first place. Looking at file 
history it seems it was there from the start (from 2006). Could it be that it 
was addressing some MSVC bug which got fixed in the meantime? Things seem to be 
building and working fine now when using 1LL for shift.

Also related to this I have doubts about the SWAP_8 macro which is similarly 
changed for MSVC, also in cfield.c.

I am only able to build 32 bit version so I can't say whether this was put in 
place due to some x64 issue, maybe someone can check behavior on x64 build. If 
that's the case, maybe #ifdef should take that into account.

----------
nosy: +vladris
title: support read/write c_ulonglong type bitfield structures -> ctypes is not 
correctly handling bitfields backed by 64 bit integers on Windows
type: feature request -> behavior
versions: +Python 3.3 -Python 3.2

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue6068>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to