[issue26811] segfault due to null pointer in tuple

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
pull_requests: +1384

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[Roundup Robot]

Roundup Robot added the comment:

New changeset a98ef122d73d by Serhiy Storchaka in branch '3.5':
Issue #26811: gc.get_objects() no longer contains a broken tuple with NULL
https://hg.python.org/cpython/rev/a98ef122d73d

New changeset 3fe1c7ad3b58 by Serhiy Storchaka in branch 'default':
Issue #26811: gc.get_objects() no longer contains a broken tuple with NULL
https://hg.python.org/cpython/rev/3fe1c7ad3b58

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

May be using _PyObject_GC_UNTRACK() is more correct than setting the size to 0 
or setting the item to None. But property_descr_get_gc_untrack.patch makes the 
code a little slower. Here is optimized version of Victor's patch.

--
Added file: 
http://bugs.python.org/file42577/property_descr_get_gc_untrack_2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[STINNER Victor]

STINNER Victor added the comment:

Oh, I see "The property() getter calls are up to 25% faster.
(Contributed by Joe Jevnik in issue 23910.)" in
https://docs.python.org/dev/whatsnew/3.5.html#optimizations

Hum... This is embarrassing :-/ Ok, let's keep it, but we must fix it ;-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

> I suggest to remove the micro-optimization from Python 3.5 for safety.

Then we should remove the promise from What's New In Python 3.5. That will 
cause a regression in namedtuple attributes access about 30%.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[STINNER Victor]

STINNER Victor added the comment:

I created the issue #26814: "Add a new _PyObject_CallStack() function which 
avoids the creation of a tuple or dict for arguments".

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[STINNER Victor]

STINNER Victor added the comment:

I suggest to remove the micro-optimization from Python 3.5 for safety.

I'm ok to experiment a new safer implementation on Python 3.6 ;-) We have more 
time to fix the code in Python 3.6 if new issues are found. Setting the tuple 
size to zero looks simple and safe, but the overall hack deserves a comment to 
explain:

* why you use a cached tuple
* why the reference count can be different than 2: recursive calls
* why do you change the tuple size

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

References:

Issue23910 -- added original optimization (661cdbd617b8).
Issue24276 -- it was significant rewritten (5dbf3d932a59).

My suggestion in msg263886 to revert the optimization was related to the 
original code. Now, after reminding the background, I think it is worth to keep 
the optimization and try to fix the code.

Making the cached tuple to save None adds additional complexity and overhead 
(about 5%). Here is a patch that uses different trick. The size of saved tuple 
is set to 0.

--
stage:  -> patch review
type:  -> crash
Added file: 
http://bugs.python.org/file42547/property_cached_args_set_size_0.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[STINNER Victor]

STINNER Victor added the comment:

> Maybe a new C function can be designed to call a function: it would uses 
> PyEval_EvalCodeEx() to call functions implemented in Python, or create the 
> tuple for functions implemented in C.

I would expect C code to be more optimized that Python functions, but for the 
specific case of function calls: Python are more optimized with the fast-path 
fast_function()!

I recall vaguely that Larry Hasting shared me his idea of passing the Python 
stack to C functions to avoid the creation of a tuple.

Maybe we can add a new API for C functions accepted a C array of PyObject*, a 
number of positional arguments and a number of (key, value) pairs for keyword 
arguments. Something similar to PyEval_EvalCodeEx() but for C functions. It 
means that we should add a new flavor of 
PyArg_ParseTuple/PyArg_ParseTupleAndKeywords to accepts this format.

Larry's idea was to use the argument clinic to handle that for us.

--
nosy: +larry

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[STINNER Victor]

STINNER Victor added the comment:

Serhiy: "Ah, yes, 5dbf3d932a59 partially fixed this optimization."

Since we already talking about hacks, another hack is to hide this private 
tuple from the GC using _PyObject_GC_UNTRACK(): see attached 
property_descr_get_gc_untrack.patch.

Note: I dislike my own patch :-D


Serhiy: "661cdbd617b8 can cause problems with recursion and with cached 
getters. I suggest to revert it."

Even if I don't understand the issue, I concur that the micro-optimization must 
be reverted. A micro-optimization must not introduce any behaviour change nor 
segfault :-)


Raymond Hettinger: "Serhiy, I'll thinking the whole optimization ought to be 
reverted as being too tricky and error-prone for the small benefit.  A less 
aggressive approach would be to have the tuple save a valid object (such as 
None) instead of NULL.  What do you think?"

PyEval_EvalCodeEx() accepts a C array of PyObject* for function arguments, it's 
used by fast_function() in Pythn/ceval.c. Maybe a new C function can be 
designed to call a function: it would uses PyEval_EvalCodeEx() to call 
functions implemented in Python, or create the tuple for functions implemented 
in C.

It looks like the issue #23910 which introduced the optimization was created 
for functions implemented in C. So I don't know it's acceptable. But it would 
be safer and more generic, no?

--
keywords: +patch
nosy: +haypo
Added file: http://bugs.python.org/file42546/property_descr_get_gc_untrack.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Sorry, I'm late with my comments. All my posts are written without seeing your 
previous message.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Ah, yes, 5dbf3d932a59 partially fixed this optimization. Saving None in a tuple 
(as I believe done in itertools/zip/enumerate optimization) should fix this 
issue. I agree that the code is too tricky (as well as the code in 
itertools/zip/enumerate). It would be better to make tuple creation faster 
instead of adding special caches in separate functions (see also issue23507). 
But for now this optimization makes attribute access in named tuples 30% 
faster, and this is good argument for keeping it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[Raymond Hettinger]

Raymond Hettinger added the comment:

I concur.  Please do the honors and revert it from 3.5 and 3.6.

--
assignee: rhettinger -> serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[Raymond Hettinger]

Changes by Raymond Hettinger :


--
priority: normal -> high

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

661cdbd617b8 can cause problems with recursion and with cached getters. I 
suggest to revert it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[Raymond Hettinger]

Raymond Hettinger added the comment:

Also, see https://hg.python.org/cpython/rev/5dbf3d932a59

Serhiy, I'll thinking the whole optimization ought to be reverted as being too 
tricky and error-prone for the small benefit.  A less aggressive approach would 
be to have the tuple save a valid object (such as None) instead of NULL.  What 
do you think?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[Raymond Hettinger]

Changes by Raymond Hettinger :


--
assignee:  -> rhettinger
nosy: +rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[Xiang Zhang]

Xiang Zhang added the comment:

hg bisect tells me changeset 95830:661cdbd617b8 introduces this behaviour.

--
nosy: +xiang.zhang
type: crash -> 

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
nosy: +brett.cannon, eric.snow, ncoghlan, serhiy.storchaka
type:  -> crash

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26811] segfault due to null pointer in tuple

[random832]

New submission from random832:

I was writing something that iterates over all objects in gc.get_objects(). I 
don't know precisely where the offending tuple is coming from, but nearby 
objects in gc.get_objects() appear to be related to frozen_importlib.

A tuple exists somewhere with a null pointer in it. Attempting to iterate over 
it or access its item results in a segmentation fault.

I confirmed this on both Linux and OSX.

Python 3.6.0a0 (default:778ccbe3cf74, Apr 20 2016, 20:17:38)

--
components: Interpreter Core
files: fnt.py
messages: 263866
nosy: random832
priority: normal
severity: normal
status: open
title: segfault due to null pointer in tuple
versions: Python 3.6
Added file: http://bugs.python.org/file42545/fnt.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com