Skip to content

ctypes: bit field data does not survive round trip #97588

Description

@matthiasgoergens

Bit-fields in structures don't seem to give you back the data you put in?

from ctypes import Structure, c_uint, c_ulonglong, c_ushort


class Foo(Structure):
    _fields_ = [("A", c_uint, 1), ("B", c_ushort, 16)]


class Bar(Structure):
    _fields_ = [("A", c_ulonglong, 1), ("B", c_uint, 32)]


if __name__ == "__main__":
    for a in [Foo(), Bar()]:
        a.A = 0
        a.B = 1
        print(a.A, a.B)

The above should print

0 1
0 1

But it actually prints

$ python3.10 mini.py 
0 0
0 0

For comparison and to test my understanding, I expect the following C code to be equivalent to the Python code above:

#include<stdio.h>

struct Foo {
  unsigned int A: 1;
  unsigned short B: 16;
};

struct Bar {
  unsigned long long int A: 1;
  unsigned int B: 32;
};

int main(int argc, char** argv) {
    struct Foo foo;
    foo.A = 0;
    foo.B = 1;
    printf("%d %d\n", foo.A, foo.B);

    struct Bar bar;
    bar.A = 0;
    bar.B = 1;
    printf("%d %d\n", bar.A, bar.B);
    return 0;
}

The C version prints what we expect:

$ gcc -fsanitize=undefined test.c && ./a.out
0 1
0 1

Your environment

I am on ArchLinux with Python 3.10.7. Python 3.11 and main are also affected. I also randomly tried Python 3.6 with the same result. (Python 3.6 is the oldest one that was easy to install.)

More comprehensive test case

Here's how I actually found the problem reported above. Using Hypothesis:

import ctypes
import string

from hypothesis import assume, example, given, note
from hypothesis import strategies as st

unsigned = [(ctypes.c_ushort, 16), (ctypes.c_uint, 32), (ctypes.c_ulonglong, 64)]
signed = [(ctypes.c_short, 16), (ctypes.c_int, 32), (ctypes.c_longlong, 64)]
types = unsigned + signed

unsigned_types = list(zip(*unsigned))[0]
signed_types = list(zip(*signed))[0]

names = st.lists(st.text(alphabet=string.ascii_letters, min_size=1), unique=True)


@st.composite
def fields_and_set(draw):
    names_ = draw(names)
    ops = []
    results = []
    for name in names_:
        t, l = draw(st.sampled_from(types))
        res = (name, t, draw(st.integers(min_value=1, max_value=l)))
        results.append(res)
        values = draw(st.lists(st.integers()))
        for value in values:
            ops.append((res, value))
    ops = draw(st.permutations(ops))
    return results, ops


def fit_in_bits(value, type_, size):
    expect = value % (2**size)
    if type_ not in unsigned_types:
        if expect >= 2 ** (size - 1):
            expect -= 2**size
    return expect


@given(fops=fields_and_set())
def test(fops):
    (fields, ops) = fops

    class BITS(ctypes.Structure):
        _fields_ = fields

    b = BITS()
    for (name, type_, size), value in ops:

        expect = fit_in_bits(value, type_, size)
        setattr(b, name, value)
        j = getattr(b, name)
        assert expect == j, f"{expect} != {j}"


if __name__ == "__main__":
    test()

Thanks to @mdickinson for pointing me in this direction.

Linked PRs

Activity

  1. added a commit that references this issue on Sep 27, 2022
  2. matthiasgoergens commented on Sep 27, 2022

    @matthiasgoergens
    ContributorAuthor

    Since I'm looking into bit-fields and ctypes anyway at the moment, I'll plan to work on a fix. But if someone else wants to give it a go, I'm happy to let them.

  3. matthiasgoergens commented on Sep 27, 2022

    @matthiasgoergens
    ContributorAuthor

    As far as I can tell the problem might be with PyCField_FromDesc in cfield.c. The code looks both a bit complicated and is not expected to be a hot inner loop (it's only for setting up a Structure from a description.) So perhaps we might want to move most of its logic into Python.

    Let's me dig some more.

  4. changed the title [-]ctypes: data does not survice round trip[/-] [+]ctypes: bit field data does not survice round trip[/+] on Sep 27, 2022
  5. added a commit that references this issue on Oct 1, 2022
  6. mdickinson commented on Oct 8, 2022

    @mdickinson
    Member

    @matthiasgoergens I think this is essentially the same issue as existing issues like #95496, #84039, #73939, #59324, etc; it would be good to try to consolidate some of these so we don't end up with an explosion of issues.

  7. matthiasgoergens commented on Oct 8, 2022

    @matthiasgoergens
    ContributorAuthor

    @mdickinson Good idea. That's actually what I am already doing with the fix I have in the works, but consolidating the issues makes sense, too.

  8. changed the title [-]ctypes: bit field data does not survice round trip[/-] [+]ctypes: bit field data does not survive round trip[/+] on Oct 9, 2022
  9. matthiasgoergens commented on Oct 9, 2022

    @matthiasgoergens
    ContributorAuthor

    @mdickinson I mentioned the issues that my PR fixes on the PR itself. Is there a way (or necessity) to also consolidate the issues themselves here?

  10. added 2 commits that reference this issue on Mar 17, 2023
  11. thesamprice commented on Mar 27, 2023

    @thesamprice

    @matthiasgoergens Could you review pull request !103052

  12. matthiasgoergens commented on Mar 27, 2023

    @matthiasgoergens
    ContributorAuthor

    @thesamprice I'll have a look, but keep in mind that I don't have any authority here.

  13. thesamprice commented on Mar 27, 2023

    @thesamprice

    More interested if the one line change passes the tests you wrote .

  14. matthiasgoergens commented on Mar 27, 2023

    @matthiasgoergens
    ContributorAuthor

    Oh, ok. Sure, I can run that against my tests.

    I guess I should publish my Python Hypothesis tests, too; and not just the tests in my PR. So you can run them yourself.

    Give me a bit of time!

    Edit: I'll answer over on #103052

  15. matthiasgoergens commented on Mar 27, 2023

    @matthiasgoergens
    ContributorAuthor

    @thesamprice I added your tests from #103052 to this PR, too. I hope you don't mind.

  16. added a commit that references this issue on May 29, 2024
  17. added a commit that references this issue on Jul 11, 2024
  18. added a commit that references this issue on Jul 17, 2024
  19. added a commit that references this issue on Sep 5, 2024
  20. encukou commented on Sep 9, 2024

    @encukou
    Member

    Thank you for the report, and the initial fix!
    The layout calculation is now done in Python, and a test suite is in place, so we're in a better place to solve the other struct/union layout issues. I'll close this one.

  21. added a commit that references this issue on Sep 12, 2024
  22. added a commit that references this issue on Sep 16, 2024
  23. added a commit that references this issue on Sep 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions