classification
Title: SystemError in _collections._count_element() in case of a bad mapping argument
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: Oren Milman, rhettinger, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2017-09-26 06:32 by Oren Milman, last changed 2017-09-27 03:46 by rhettinger. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 3763 merged Oren Milman, 2017-09-26 09:41
PR 3776 merged python-dev, 2017-09-27 03:18
Messages (5)
msg303014 - (view) Author: Oren Milman (Oren Milman) * Date: 2017-09-26 06:32
The following code causes a SystemError:
class BadMapping:
    get = dict.get
    __setitem__ = dict.__setitem__

import _collections
_collections._count_elements(BadMapping(), [42])


This is because _count_elements() (in Modules/_collectionsmodule.c) assumes
that the mapping argument is a dictionary in case it has the same get() and
__setitem__() methods as dict. And so, _count_elements() passes the mapping
argument to _PyDict_GetItem_KnownHash(), which raises the SystemError.


ISTM it is a very unlikely corner case, so that adding a test (as well as
a NEWS.d item) for it is unnecessary.
What do you think?
msg303016 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-26 06:48
I agree. _collections._count_elements() is an internal function. It should be called only with Counter (which is a dict subclass) or its subclass.

But I think it is worth to make the code more reliable.
msg303077 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-09-26 20:47
Oren, thank you for the work you've been doing.
msg303088 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-09-27 03:18
New changeset 31aca4bf79217e6ec4c1d056d3ad7ed4dd469c78 by Raymond Hettinger (Oren Milman) in branch 'master':
bpo-31586: Use _count_element fast path for real dicts.
https://github.com/python/cpython/commit/31aca4bf79217e6ec4c1d056d3ad7ed4dd469c78
msg303089 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-09-27 03:45
New changeset a1c49f6f09150f70f063417c8d67a38e59dde7ed by Raymond Hettinger (Miss Islington (bot)) in branch '3.6':
[3.6] bpo-31586: Use _count_element fast path for real dicts. (#3776)
https://github.com/python/cpython/commit/a1c49f6f09150f70f063417c8d67a38e59dde7ed
History
Date User Action Args
2017-09-27 03:46:36rhettingersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-09-27 03:45:59rhettingersetmessages: + msg303089
2017-09-27 03:18:37python-devsetpull_requests: + pull_request3763
2017-09-27 03:18:26rhettingersetmessages: + msg303088
2017-09-26 20:47:24rhettingersetassignee: rhettinger
messages: + msg303077
2017-09-26 09:41:54Oren Milmansetkeywords: + patch
stage: patch review
pull_requests: + pull_request3748
2017-09-26 06:48:26serhiy.storchakasetmessages: + msg303016
2017-09-26 06:44:57serhiy.storchakasetnosy: + rhettinger, serhiy.storchaka

versions: + Python 3.6, Python 3.7, - Python 3.8
2017-09-26 06:32:28Oren Milmancreate