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

ms.key_compare is not initialized in all paths of list_sort_impl() #78170

Closed
johnchen902 mannequin opened this issue Jun 28, 2018 · 6 comments
Closed

ms.key_compare is not initialized in all paths of list_sort_impl() #78170

johnchen902 mannequin opened this issue Jun 28, 2018 · 6 comments
Assignees
Labels
3.7 (EOL) end of life 3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@johnchen902
Copy link
Mannequin

johnchen902 mannequin commented Jun 28, 2018

BPO 33989
Nosy @rhettinger, @serhiy-storchaka, @embg, @ZackerySpytz, @johnchen902, @miss-islington
PRs
  • bpo-33989: Ensure that ms.key_compare is always initialized in list_sort_impl() #8710
  • [3.7] bpo-33989: Ensure that ms.key_compare is always initialized in list_sort_impl(). (GH-8710) #11970
  • 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 = 'https://github.com/serhiy-storchaka'
    closed_at = <Date 2019-02-21.08:14:44.903>
    created_at = <Date 2018-06-28.17:43:06.071>
    labels = ['interpreter-core', '3.7', '3.8', 'type-crash']
    title = 'ms.key_compare is not initialized in all paths of list_sort_impl()'
    updated_at = <Date 2019-02-21.08:14:44.902>
    user = 'https://github.com/johnchen902'

    bugs.python.org fields:

    activity = <Date 2019-02-21.08:14:44.902>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2019-02-21.08:14:44.903>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2018-06-28.17:43:06.071>
    creator = 'johnchen902'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 33989
    keywords = ['patch']
    message_count = 6.0
    messages = ['320679', '320749', '320751', '320752', '336188', '336189']
    nosy_count = 6.0
    nosy_names = ['rhettinger', 'serhiy.storchaka', 'elliot.gorokhovsky', 'ZackerySpytz', 'johnchen902', 'miss-islington']
    pr_nums = ['8710', '11970']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue33989'
    versions = ['Python 3.7', 'Python 3.8']

    @johnchen902
    Copy link
    Mannequin Author

    johnchen902 mannequin commented Jun 28, 2018

    Relevant code (Objects/listobject.c lines 2268 -- 2286 as of commit e76ac9d):

            /* Choose the best compare, given what we now know about the keys. */
            if (keys_are_all_same_type) {
    
                if (key_type == &PyUnicode_Type && strings_are_latin) {
                    ms.key_compare = unsafe_latin_compare;
                }
                else if (key_type == &PyLong_Type && ints_are_bounded) {
                    ms.key_compare = unsafe_long_compare;
                }
                else if (key_type == &PyFloat_Type) {
                    ms.key_compare = unsafe_float_compare;
                }
                else if ((ms.key_richcompare = key_type->tp_richcompare) != NULL) {
                    ms.key_compare = unsafe_object_compare;
                }
            }
            else {
                ms.key_compare = safe_object_compare;
            }

    Clearly, ms.key_compare is not assigned here if keys_are_all_same_type is true but key_type->tp_richcompare is NULL.

    I don't know how to obtain an object with ob_type->tp_richcompare being NULL, though.

    @johnchen902 johnchen902 mannequin added 3.7 (EOL) end of life 3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump labels Jun 28, 2018
    @rhettinger
    Copy link
    Contributor

    Do you want to submit a patch?

    @rhettinger
    Copy link
    Contributor

    Serhiy, do you want to take this one?

    @embg
    Copy link
    Mannequin

    embg mannequin commented Jun 30, 2018

    You can always fall back on safe_object_compare. So unless there's an
    obvious reason why your edge case can't be triggered, it would be worth
    putting that in as a failsafe. The additional branch should be 100%
    predictable, so there shouldn't be any performance penalty.

    @ZackerySpytz ZackerySpytz mannequin changed the title ms.key_compare is not initialized in all pathes of list_sort_impl ms.key_compare is not initialized in all paths of list_sort_impl() Aug 9, 2018
    @serhiy-storchaka
    Copy link
    Member

    New changeset ebc793d by Serhiy Storchaka (Zackery Spytz) in branch 'master':
    bpo-33989: Ensure that ms.key_compare is always initialized in list_sort_impl(). (GH-8710)
    ebc793d

    @miss-islington
    Copy link
    Contributor

    New changeset 0e73ea2 by Miss Islington (bot) in branch '3.7':
    bpo-33989: Ensure that ms.key_compare is always initialized in list_sort_impl(). (GH-8710)
    0e73ea2

    @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
    3.7 (EOL) end of life 3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants