classification
Title: Coverity scan: Python/dtoa.c resource leak
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.8, Python 3.7, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: cstratak, mark.dickinson, vstinner
Priority: normal Keywords: patch

Created on 2019-03-11 13:53 by cstratak, last changed 2019-03-14 16:20 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 12276 merged vstinner, 2019-03-11 14:50
PR 12331 merged vstinner, 2019-03-14 15:25
PR 12332 merged vstinner, 2019-03-14 15:25
Messages (8)
msg337668 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2019-03-11 13:53
Coverity report on dtoa.c. It was run on python2 but the same code resides on python3.

Error: RESOURCE_LEAK (CWE-772): [#def89]
Python-2.7.15/Python/dtoa.c:1846: alloc_fn: Storage is returned from allocation function "s2b".
Python-2.7.15/Python/dtoa.c:526:9: alloc_fn: Storage is returned from allocation function "multadd".
Python-2.7.15/Python/dtoa.c:479:13: alloc_fn: Storage is returned from allocation function "Balloc".
Python-2.7.15/Python/dtoa.c:371:13: alloc_fn: Storage is returned from allocation function "PyMem_Malloc".
Python-2.7.15/Objects/object.c:2348:5: alloc_fn: Storage is returned from allocation function "malloc".
Python-2.7.15/Objects/object.c:2348:5: return_alloc_fn: Directly returning storage allocated by "malloc".
Python-2.7.15/Python/dtoa.c:371:13: var_assign: Assigning: "rv" = "PyMem_Malloc(len * 8UL)".
Python-2.7.15/Python/dtoa.c:379:5: return_alloc: Returning allocated memory "rv".
Python-2.7.15/Python/dtoa.c:479:13: var_assign: Assigning: "b1" = "Balloc(b->k + 1)".
Python-2.7.15/Python/dtoa.c:486:13: var_assign: Assigning: "b" = "b1".
Python-2.7.15/Python/dtoa.c:491:5: return_alloc: Returning allocated memory "b".
Python-2.7.15/Python/dtoa.c:526:9: var_assign: Assigning: "b" = "multadd(b, 10, *s++ - 48)".
Python-2.7.15/Python/dtoa.c:530:5: return_alloc: Returning allocated memory "b".
Python-2.7.15/Python/dtoa.c:1846: var_assign: Assigning: "bd0" = storage returned from "s2b(s0, nd0, nd, y)".
Python-2.7.15/Python/dtoa.c:2249: leaked_storage: Variable "bd0" going out of scope leaks the storage it points to.
 2247|   
 2248|     undfl:
 2249|->     return sign ? -0.0 : 0.0;
 2250|   
 2251|     ovfl:

Error: RESOURCE_LEAK (CWE-772): [#def90]
Python-2.7.15/Python/dtoa.c:2006: alloc_fn: Storage is returned from allocation function "diff".
Python-2.7.15/Python/dtoa.c:952:5: alloc_fn: Storage is returned from allocation function "Balloc".
Python-2.7.15/Python/dtoa.c:371:13: alloc_fn: Storage is returned from allocation function "PyMem_Malloc".
Python-2.7.15/Objects/object.c:2348:5: alloc_fn: Storage is returned from allocation function "malloc".
Python-2.7.15/Objects/object.c:2348:5: return_alloc_fn: Directly returning storage allocated by "malloc".
Python-2.7.15/Python/dtoa.c:371:13: var_assign: Assigning: "rv" = "PyMem_Malloc(len * 8UL)".
Python-2.7.15/Python/dtoa.c:379:5: return_alloc: Returning allocated memory "rv".
Python-2.7.15/Python/dtoa.c:952:5: var_assign: Assigning: "c" = "Balloc(a->k)".
Python-2.7.15/Python/dtoa.c:962:5: var_assign: Assigning: "xc" = "c".
Python-2.7.15/Python/dtoa.c:996:5: return_alloc: Returning allocated memory "c".
Python-2.7.15/Python/dtoa.c:2006: var_assign: Assigning: "delta" = storage returned from "diff(bb, bd)".
Python-2.7.15/Python/dtoa.c:2016: noescape: Resource "delta" is not freed or pointed-to in "cmp".
Python-2.7.15/Python/dtoa.c:890:13: noescape: "cmp(Bigint *, Bigint *)" does not free or save its parameter "a".
Python-2.7.15/Python/dtoa.c:2129: noescape: Resource "delta" is not freed or pointed-to in "ratio".
Python-2.7.15/Python/dtoa.c:1179:15: noescape: "ratio(Bigint *, Bigint *)" does not free or save its parameter "a".
Python-2.7.15/Python/dtoa.c:2249: leaked_storage: Variable "delta" going out of scope leaks the storage it points to.
 2247|   
 2248|     undfl:
 2249|->     return sign ? -0.0 : 0.0;
 2250|   
 2251|     ovfl:
msg337669 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-03-11 14:03
* bpo-9009 discussed maintenance of Python/dtoa.c
* Python/dtoa.c asks to frequently update it from "upstream" http://www.netlib.org/fp/dtoa.c
* The upstream is also mentioned in the license: https://docs.python.org/dev/license.html#strtod-and-dtoa

... in practice, it seems like Python became the "upstream". I see lot of changes, but I'm not sure that version maintained by David M. Gay on http://www.netlib.org/fp/dtoa.c has been updated since Mark Dickinson copied it to Python/dtoa.c:

commit b08a53a99def3fa949643974f713b5b189e21bc7
Author: Mark Dickinson <dickinsm@gmail.com>
Date:   Thu Apr 16 19:52:09 2009 +0000

    Issue #1580: use short float repr where possible.
     - incorporate and adapt David Gay's dtoa and strtod
       into the Python core
     - on platforms where we can use Gay's code (almost
       all!), repr(float) is based on the shortest
       sequence of decimal digits that rounds correctly.
     - add sys.float_repr_style attribute to indicate
       whether we're using Gay's code or not
     - add autoconf magic to detect and enable SSE2
       instructions on x86/gcc
     - slight change to repr and str:  repr switches
       to exponential notation at 1e16 instead of
       1e17, str switches at 1e11 instead of 1e12
msg337670 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-03-11 14:18
Julia copied the same file. See:

* https://bugs.llvm.org//show_bug.cgi?id=31928
* https://bugs.freebsd.org/216770
* https://bugs.python.org/issue30104: "It was decided to not touch dtoa.c to not diverge from upstream."
msg337690 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2019-03-11 18:41
> ... in practice, it seems like Python became the "upstream".

Yes; unfortunately, we changed things enough that updating from upstream became impractical. At some point we should take a look at changes made to the upstream dtoa.c since our adoption of it, and figure out whether any of those changes need to be applied to our copy. That's not going to be an easy task. It would be easier if there were upstream testcases (and regression tests in particular), but as far as I'm aware there aren't any.
msg337863 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-03-13 16:55
New changeset 9776b0636ae39668d3ce1c006d4be01dad01bf9f by Victor Stinner in branch 'master':
bpo-36262: Fix _Py_dg_strtod() memory leak (goto undfl) (GH-12276)
https://github.com/python/cpython/commit/9776b0636ae39668d3ce1c006d4be01dad01bf9f
msg337943 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-03-14 16:12
New changeset 9818360ed94b39c4605951077b73ae798cddbfb9 by Victor Stinner in branch '3.7':
bpo-36262: Fix _Py_dg_strtod() memory leak (goto undfl) (GH-12276) (GH-12331)
https://github.com/python/cpython/commit/9818360ed94b39c4605951077b73ae798cddbfb9
msg337945 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-03-14 16:20
New changeset b14057877fd897eaee7bc6626682fc6092b6bbd2 by Victor Stinner in branch '2.7':
bpo-36262: Fix _Py_dg_strtod() memory leak (goto undfl) (GH-12276) (GH-12332)
https://github.com/python/cpython/commit/b14057877fd897eaee7bc6626682fc6092b6bbd2
msg337946 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-03-14 16:20
Thanks for the report Charalampos. I fixed dtoa.c in 2.7, 3.7 and master branches.
History
Date User Action Args
2019-03-14 16:20:37vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg337946

stage: patch review -> resolved
2019-03-14 16:20:04vstinnersetmessages: + msg337945
2019-03-14 16:12:08vstinnersetmessages: + msg337943
2019-03-14 15:25:51vstinnersetpull_requests: + pull_request12303
2019-03-14 15:25:47vstinnersetpull_requests: + pull_request12302
2019-03-13 16:55:11vstinnersetmessages: + msg337863
2019-03-11 18:41:38mark.dickinsonsetmessages: + msg337690
2019-03-11 14:50:01vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request12256
2019-03-11 14:18:31vstinnersetmessages: + msg337670
2019-03-11 14:03:27vstinnersetmessages: + msg337669
2019-03-11 13:54:31cstrataksetnosy: + mark.dickinson, vstinner
2019-03-11 13:53:27cstratakcreate