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

PyThread_create_key(): fix comparison between signed and unsigned numbers in Python/thread_pthread.h #66402

Closed
vstinner opened this issue Aug 15, 2014 · 4 comments
Labels
type-bug An unexpected behavior, bug, or error

Comments

@vstinner
Copy link
Member

BPO 22206
Nosy @pitrou, @vstinner
Files
  • PyThread_create_key.patch
  • pthread_key_create_overflow.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2014-08-17.20:15:23.609>
    created_at = <Date 2014-08-15.21:45:30.341>
    labels = ['type-bug']
    title = 'PyThread_create_key(): fix comparison between signed and unsigned numbers in Python/thread_pthread.h'
    updated_at = <Date 2014-08-17.20:15:23.607>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2014-08-17.20:15:23.607>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2014-08-17.20:15:23.609>
    closer = 'vstinner'
    components = []
    creation = <Date 2014-08-15.21:45:30.341>
    creator = 'vstinner'
    dependencies = []
    files = ['36380', '36386']
    hgrepos = []
    issue_num = 22206
    keywords = ['patch']
    message_count = 4.0
    messages = ['225367', '225390', '225467', '225468']
    nosy_count = 4.0
    nosy_names = ['pitrou', 'vstinner', 'neologix', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'patch review'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue22206'
    versions = ['Python 3.5']

    @vstinner
    Copy link
    Member Author

    The issue bpo-22110 enabled more compiler warnings. I would like to fix this one:
    ---
    gcc -pthread -c -Wno-unused-result -Wsign-compare -g -O0 -Wall -Wstrict-prototypes -Werror=declaration-after-statement -I. -IInclude -I./Include -DPy_BUILD_CORE -o Python/thread.o Python/thread.c
    In file included from Python/thread.c:86:0:
    Python/thread_pthread.h: In function ‘PyThread_create_key’:
    Python/thread_pthread.h:611:22: attention : signed and unsigned type in conditional expression [-Wsign-compare]
    return fail ? -1 : key;
    ^
    ---

    Attached patch uses Py_SAFE_DOWNCAST() to explicitly downcast to int.

    On Linux (on my Fedora 20/amd64), pthread_key_t is defined as an unsigned int, whereas the result type of PyThread_create_key is a signed int.

    Nobody complained before, so I get that nobody noticed the possible overflow for a key > INT_MAX. I checked the code, we only check if PyThread_create_key() returns -1, if you reach UINT_MAX keys. UINT_MAX keys sounds insane, you probably hit another limit before.

    On Linux, it looks like the key is a counter and deleted values are reused:

    haypo@selma$ ./python
    Python 3.5.0a0 (default:a0b38f4eb79e, Aug 15 2014, 23:37:42) 
    [GCC 4.8.3 20140624 (Red Hat 4.8.3-1)] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import ctypes
    >>> ctypes.pythonapi.PyThread_create_key()
    2
    >>> ctypes.pythonapi.PyThread_create_key()
    3
    >>> ctypes.pythonapi.PyThread_create_key()
    4
    >>> ctypes.pythonapi.PyThread_delete_key(3)
    0
    >>> ctypes.pythonapi.PyThread_create_key()
    3

    @vstinner vstinner changed the title PyThread_create_key(): fix comparison between signed and unsigned numbers PyThread_create_key(): fix comparison between signed and unsigned numbers in Python/thread_pthread.h Aug 15, 2014
    @vstinner
    Copy link
    Member Author

    pthread_key_create_overflow.patch: safer patch, delete the newly created key and return an error on integer overflow.

    @serhiy-storchaka serhiy-storchaka added the type-bug An unexpected behavior, bug, or error label Aug 16, 2014
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 17, 2014

    New changeset 1b898b5d5ffe by Victor Stinner in branch 'default':
    Issue bpo-22206: Using pthread, PyThread_create_key() now sets errno to ENOMEM and
    http://hg.python.org/cpython/rev/1b898b5d5ffe

    @vstinner
    Copy link
    Member Author

    I fixed the issue in Python 3.5, I close the issue.

    Even if Python 2.7 and 3.4 are also affected, I prefer to not modify them to not take the risk of introducing a regression for a corner case.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants