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

[johnchen902]

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]

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.

[rhettinger]

Do you want to submit a patch?

[rhettinger]

Serhiy, do you want to take this one?

[embg]

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.

[serhiy-storchaka]

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]

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