classification
Title: Double decref and dereferencing after decref in int()
Type: crash Stage:
Components: Interpreter Core Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: mark.dickinson Nosy List: benjamin.peterson, georg.brandl, jcea, mark.dickinson, python-dev, serhiy.storchaka
Priority: high Keywords: patch

Created on 2012-09-27 08:46 by serhiy.storchaka, last changed 2012-09-29 07:27 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
float2int_dbl_decref.patch serhiy.storchaka, 2012-09-27 08:46 review
float2int_dbl_decref_2.patch mark.dickinson, 2012-09-27 17:08 review
Messages (12)
msg171371 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-09-27 08:46
In function convert_integral_to_int() in file Objects/abstract.c integral double decrefed and dereferenced after decrefing if returned value of __int__() is not int. Python 3.3 only affected.

Here is a patch.
msg171376 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-09-27 15:33
Nice catch!  And indeed, the following code generates a segfault on my machine:


    class B(object):
        def __int__(self):
            return 43.0

    class A(object):
        def __trunc__(self):
            return B()

    int(A())


The patch should probably include a regression test.
msg171380 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-09-27 16:48
> And indeed, the following code generates a segfault on my machine:

I was going to give similar example and assign crash type to issue, but on my machine it does not generate a segfault.

> The patch should probably include a regression test.

Someone borrowed Guido's time machine and have already done this test (see NonIntegral in Lib/test/test_int.py).
msg171382 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-09-27 17:02
That test doesn't look quite the same, though:  the return value of __trunc__ is an object that has __trunc__ rather than __int__.  And all the existing tests pass on my machine without issues.
msg171383 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-09-27 17:08
Here's patch that adds a regression test.
msg171385 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-09-27 17:29
Ah, it generates a segfault in debug mode.

LGTM.
msg171386 - (view) Author: Roundup Robot (python-dev) Date: 2012-09-27 18:39
New changeset 690287f8ea95 by Mark Dickinson in branch 'default':
Issue #16060: Fix a double DECREF in int() implementation.  Thanks Serhiy Storchaka.
http://hg.python.org/cpython/rev/690287f8ea95
msg171388 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-09-27 18:43
Applied.  Thanks!

I'm not sure whether this is worthy of inclusion in Python 3.3.0;  on one hand, it's a segfault from core Python.  On the other, it doesn't look that easy to trigger by accident.

On balance, I'd say it's safe to wait for Python 3.3.1 for this.  Adding Georg to the nosy in case he disagrees.
msg171415 - (view) Author: Jesús Cea Avión (jcea) * (Python committer) Date: 2012-09-28 08:36
Serhiy, I wonder how you found this :)
msg171424 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-09-28 09:37
> Serhiy, I wonder how you found this :)

I just looked at the code for issue16036.
msg171461 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-09-28 13:35
Applied: d23eb81bd482.
msg171556 - (view) Author: Roundup Robot (python-dev) Date: 2012-09-29 07:27
New changeset d23eb81bd482 by Mark Dickinson in branch 'default':
Issue #16060: Fix a double DECREF in int() implementation.  Thanks Serhiy Storchaka.
http://hg.python.org/cpython/rev/d23eb81bd482
History
Date User Action Args
2012-09-29 07:27:39python-devsetmessages: + msg171556
2012-09-28 13:35:29georg.brandlsetstatus: open -> closed
resolution: fixed
messages: + msg171461
2012-09-28 09:37:59serhiy.storchakasetmessages: + msg171424
2012-09-28 08:36:15jceasetnosy: + jcea
messages: + msg171415
2012-09-27 18:51:51mark.dickinsonsetassignee: mark.dickinson
2012-09-27 18:43:07mark.dickinsonsetnosy: + georg.brandl
messages: + msg171388
2012-09-27 18:39:18python-devsetnosy: + python-dev
messages: + msg171386
2012-09-27 17:29:42serhiy.storchakasettype: behavior -> crash
messages: + msg171385
2012-09-27 17:08:47mark.dickinsonsetfiles: + float2int_dbl_decref_2.patch

messages: + msg171383
2012-09-27 17:02:51mark.dickinsonsetmessages: + msg171382
2012-09-27 16:48:13serhiy.storchakasetmessages: + msg171380
2012-09-27 15:33:54mark.dickinsonsetpriority: normal -> high
nosy: + mark.dickinson
messages: + msg171376

2012-09-27 08:46:57serhiy.storchakacreate