Skip to content
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

ctypes.Structure does not set value of fields #102844

Closed
DakshKK opened this issue Mar 20, 2023 · 11 comments
Closed

ctypes.Structure does not set value of fields #102844

DakshKK opened this issue Mar 20, 2023 · 11 comments
Labels
topic-ctypes type-bug An unexpected behavior, bug, or error

Comments

@DakshKK
Copy link

DakshKK commented Mar 20, 2023

Bug report

I have been using bit-fields with ctypes.Structure.

My issue is that no matter what I do, I am unable to set the values for my bitfield fields.

The structure has the following definition:

from ctypes import Structure, c_byte, c_int
class Demo (Structure):
 _fields_ = [
  ('f1', c_byte, 8),
  ('f2', c_int, 21),
  ('f3', c_byte, 2),
  ('f4', c_byte, 1)
 ]

I have set the above struct using from_buffer_copy with a value of b'\x01\x0f\x00\xe0' (Binary representation of C struct, with the underneath values).

This should set the following values

  • f1: 1
  • f2: 15
  • f3: -1 (3, overflow makes it -1)
  • f4: -1 (1, overflow makes it -1)

Manual setting, or from_buffer_copy, achieve nothing; f3, and f4 remain 0.

f3, and f4 are both supposed to be -1.

Please let me understand if there is supposed to be some special configuration or what the issue is.

Your environment

  • CPython versions tested on: 3.8.2, 3.9.9
  • Operating system and architecture: RHEL8, x86-64
@DakshKK DakshKK added the type-bug An unexpected behavior, bug, or error label Mar 20, 2023
@gaogaotiantian
Copy link
Member

According to ctypes documentation:

_fields_ must be a list of 2-tuples, containing a field name and a field type.

ctypes does not support passing unions or structures with bit-fields to functions by value. While this may work on 32-bit x86, it’s not guaranteed by the library to work in the general case. Unions and structures with bit-fields should always be passed to functions by pointer.

I don't think you are using it correctly.

@DakshKK
Copy link
Author

DakshKK commented Mar 20, 2023

_fields_ must be a list of 2-tuples, containing a field name and a field type.

As the documentation very clearly states for bit fields

It is possible to create structures and unions containing bit fields. Bit fields are only possible for integer fields, the bit width is specified as the third item in the _fields_ tuples

I have adhered to this. Also referring to your point:

ctypes does not support passing unions or structures with bit-fields to functions by value. While this may work on 32-bit x86, it’s not guaranteed by the library to work in the general case. Unions and structures with bit-fields should always be passed to functions by pointer.

I don't think you are using it correctly.

I have ensured to follow the guidelines laid out in the documentation.

There is also a SO thread regarding this same issue by me.

And according to this answer, it seems to be an issue with how the interpreter is working with the structure definition, and not an ill-formed program.

Also #59324, and #97588 highlight the same issue.

@gaogaotiantian
Copy link
Member

Sorry I was wrong about the bitfield support :(

I took a look at the bitfield code, and it's never gonna work. The code is fundamentally wrong.

For a quick example:

static PyObject *
b_get(void *ptr, Py_ssize_t size)
{
    signed char val = *(signed char *)ptr;
    GET_BITFIELD(val, size);
    return PyLong_FromLong(val);
}

This is the code trying to get a "byte" from a buffer with an offset. The way the current code does it is to cast the pointer to the value container first(which will truncate the actual data needed if size is large), then shift(which in this case, will simply set the value to 0). This is only correct, when the offset is very small and all the data bits are within the data range.

It seems like in #97588 @matthiasgoergens is trying on some new implementation? If it solves the issue, then let's just wait for it.

@DakshKK
Copy link
Author

DakshKK commented Mar 20, 2023

It took me some effort to come across these issues, initially first scouring https://bugs.python.org, which gave me the same revelation as it did you.

I hope @matthiasgoergens PR is accepted soon, but considering that the last version it will be backported to is 3.11, this is another area I would need some help.

Is there any way to compile a portable Python binary with it's dependencies (RHEL8)? Otherwise I don't think this can be utilised by me, and I will need to see how to serialize my various network protocol structs in C.

Thanks for guiding me towards the solution, and thank you @matthiasgoergens for your work on this.

@matthiasgoergens
Copy link
Contributor

I could probably backport it further. The main problem is that it breaks bug-for-but compatibility, and might thus break software that relies on the old broken behaviour.

However I guess we can sidestep this problem by hiding the backports behind a feature flag?

How early a version do you need?

@DakshKK
Copy link
Author

DakshKK commented Mar 20, 2023

Our systems are offline, and I know they work with 3.8.10. Due to the remote nature, we cannot access the internet.

I tried building 3.11 for these systems, but the issue was I could not package the dependencies along with, and every build listed out further dependencies to install.

If there is a method to package the dependencies with Python while building, then there would not be a need for backporting, and I could just build this and try installing it. Hence my question.

Otherwise if the backported 3.8 is utilised, can I just replace the binary, without adding any further files?

@CAM-Gerlach
Copy link
Member

You could backport to 3.10, but the releases before that (3.7-3.9) are in security-fix-only mode and so the backports won't be accepted past that. You could ask whomever packages and distributes your Python to backport the fix to their version (as, e.g., Linux distro maintainers sometimes do)—or if that's you, you could endevour to apply the patch to the 3.8 source and build that (at your own risk, of course).

@matthiasgoergens
Copy link
Contributor

@DakshKK Can you run docker containers or VM images or so?

If yes, you could build one of those with a modern Python while the internet is available, then seal it and ship it off to your offline system?

If you can't do docker, you might be able to rig something manually via chroot etc. That's how docker works internally anyway.

I can work with you in backporting the fixes to 3.8, but that wouldn't be an official release, of course.

I don't know whether it's just one binary, or whether you'd need to replace some shared libraries or so. We'd need to investigate.

@ferdnyc
Copy link
Contributor

ferdnyc commented Mar 21, 2023

Otherwise if the backported 3.8 is utilised, can I just replace the binary, without adding any further files?

Almost certainly not, because the ctypes code isn't in the Python interpreter binary to begin with.

On Linux, the ctypes module's C code is compiled to a loadable extension module named _ctypes, one of many in the standard library which are typically consolidated in a lib-dynload/ subdirectory of the Python standard library dir. For example, on my Fedora 37 system, for the system default Python 3.11, I have:

$ ls /usr/lib64/python3.11/lib-dynload/*ctypes*
/usr/lib64/python3.11/lib-dynload/_ctypes.cpython-311d-x86_64-linux-gnu.so
/usr/lib64/python3.11/lib-dynload/_ctypes.cpython-311-x86_64-linux-gnu.so
/usr/lib64/python3.11/lib-dynload/_ctypes_test.cpython-311d-x86_64-linux-gnu.so
/usr/lib64/python3.11/lib-dynload/_ctypes_test.cpython-311-x86_64-linux-gnu.so

(That's one shared object each for the debug and non-debug Python ABIs, for both _ctypes and _ctypes_test. The ctypes module loads symbols from _ctypes in its __init__.py.)

Since the fix may involve both C code changes and (possibly) Python module code changes, you'd likely need some combination of a new _ctypes.cpython-3*-$ARCH-linux-gnu.so, plus updated files from the ctypes/ subdirectory of the standard library dir, which is where the Python code for the ctypes module lives.

If you're lucky, only the C code changes and you only have to worry about _ctypes.cpython*.so, but unless you've backported the fix into exactly the same Python version you previously installed, and built it with absolutely no changes other than the backported fix, I'd be extremely reticent to try swapping out individual files within an installed Python build. Far safer to install the new build in its entirety, replacing the previous install.

@matthiasgoergens
Copy link
Contributor

For what it's worth, I ran your test against my branch in #97702 and it seems to pass:

from ctypes import Structure, c_byte, c_int, memmove, pointer, sizeof


class Demo (Structure):
    _fields_ = [
        ('f1', c_byte, 8),
        ('f2', c_int, 21),
        ('f3', c_byte, 2),
        ('f4', c_byte, 1)
    ]

def show(d):
    for (name, _, _) in d._fields_:
        print(f"{name}: {getattr(d, name)}")
    
if __name__=='__main__':
    s = Demo.from_buffer_copy(b'\x01\x0f\x00\xe0')
    show(s)

Result:

$ ./python demo.py 
f1: 1
f2: 15
f3: -1
f4: -1

@DakshKK
Copy link
Author

DakshKK commented Apr 14, 2023

I have been traveling and missed out on these updates, but I did build @matthiasgoergens PR on my home system and it helped me achieve what I require.

I can not run VM images but maybe docker is a possibility. I did try manual building but getting a hold of all dependencies has been a pain for me, so I am trying to look at a chroot system which I can overlay on to the current network of systems, else I am biting a bullet on having this easy fix for myself, and going to implement serialization across the Python-C/C++ interface, since I can't seem to find any other solution, or existing implementation of serialization with bitfield support.

And I thank everyone for their patience, and help in trying to get me a solution for this.

@DakshKK DakshKK closed this as completed Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-ctypes type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

5 participants