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

bpo-38588: Fix segfaults when dict comparison with modifying operand #17734

Open
wants to merge 3 commits into
base: master
from

Conversation

@corona10
Copy link
Member

corona10 commented Dec 29, 2019

Based on LCatro's report

https://bugs.python.org/issue38588

@corona10 corona10 requested a review from methane as a code owner Dec 29, 2019
@corona10 corona10 changed the title bpo-38588: Fix segfaults when dict comparision with modifying operand bpo-38588: Fix segfaults when dict comparison with modifying operand Dec 29, 2019
@pablogsal

This comment has been minimized.

Copy link
Member

pablogsal commented Dec 29, 2019

Could you include the rest of the cases that LCatro was mentioning? He mentioned another 2 for lists.

@corona10

This comment has been minimized.

Copy link
Member Author

corona10 commented Dec 30, 2019

@pablogsal

poc3 is not reproducible on my local mac machine and Linux machine with the master branch.
Looks like the master branch code was changed

cmp = PyObject_RichCompareBool(PyList_GET_ITEM(a, i), el, Py_EQ);

However poc2 is reproducible, so I will add the patch on this PR.

$ cat poc3.py
class poc() :
    def __eq__(self,other) :
        list1.clear()
        return NotImplemented

list1 = [ set() ]
poc() in list1
print('end')

$ ./python poc3.py
end
@ZackerySpytz

This comment has been minimized.

Copy link
Contributor

ZackerySpytz commented Dec 30, 2019

FWIW, LCatro had opened another issue (https://bugs.python.org/issue38610) for similar crashes in the index(), count(), and remove() methods of list. I had created GH-17022 to address them.

@corona10

This comment has been minimized.

Copy link
Member Author

corona10 commented Dec 30, 2019

@pablogsal
Please take a look,
I 've added the list case patch except which is not reproducible in the master branch.
(I've attached the script above)

And for the list_richcompare I've added more cases that were not reported by LCatro.
Both cases are added in the unit test and fixed.

And IMHO, we can close bpo-38588 with this patch
:)

@corona10 corona10 requested a review from pablogsal Dec 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.