[issue27945] Various segfaults with dict

[Ned Deily]

Change by Ned Deily :


--
Removed message: https://bugs.python.org/msg342104

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Ned Deily]

Ned Deily  added the comment:


New changeset f7344798e57da6b9c4ed9372e8eaecde80989c86 by larryhastings (Serhiy 
Storchaka) in branch '3.4':
[3.4] [3.5] bpo-27945: Fixed various segfaults with dict. (GH-1657) (GH-1678) 
(#2248)
https://github.com/python/cpython/commit/f7344798e57da6b9c4ed9372e8eaecde80989c86


--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Ned Deily]

Changes by Ned Deily :


--
priority: release blocker -> 
resolution:  -> fixed
stage: commit 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

[issue27945] Various segfaults with dict

[Ned Deily]

Ned Deily added the comment:


New changeset 8fbdab50fc8f2b71f19b54f3a0208cfbf2be7713 by Ned Deily (Serhiy 
Storchaka) in branch '3.3':
[3.3] [3.5] bpo-27945: Fixed various segfaults with dict. (GH-1657) (GH-1678) 
(#2396)
https://github.com/python/cpython/commit/8fbdab50fc8f2b71f19b54f3a0208cfbf2be7713


--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Larry Hastings]

Larry Hastings added the comment:


New changeset f7344798e57da6b9c4ed9372e8eaecde80989c86 by larryhastings (Serhiy 
Storchaka) in branch '3.4':
[3.4] [3.5] bpo-27945: Fixed various segfaults with dict. (GH-1657) (GH-1678) 
(#2248)
https://github.com/python/cpython/commit/f7344798e57da6b9c4ed9372e8eaecde80989c86


--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

For the context see issue30484. If the segfault is caused by garbage collection 
this perhaps is not an expluatable vulnerability, but a severe bug that can 
affect any multithread application and doesn't have a workaround.

--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Ned Deily]

Ned Deily added the comment:

Since Serhiy created backport PRs for 3.4 and 3.3, I'm reopening the issue and 
marking it as Release Blocker (for those releases) so we don't lose track of 
them and agree they meet the criteria for security-fix-only releases.

--
nosy: +benjamin.peterson, georg.brandl
priority: critical -> release blocker
resolution: fixed -> 
stage: resolved -> commit review
status: closed -> open
versions: +Python 3.3, Python 3.4

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
pull_requests: +2442

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
pull_requests: +2296

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thank you for your patches Duane and Tim. Thank you for your very detailed 
report that allow writing these patches tehybel.

I excluded changes in dict.fromkeys() since they look unnecessary for this 
issue after fixing insertdict(). There are other reasons for adding these 
changes (this makes the behavior with exact dicts and sets and their subclasses 
more consistent), but this is other issue. There are also reasons for not 
adding them.

We need to check also the set implementation whether it is vulnerable to 
similar issues.

--
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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Serhiy Storchaka added the comment:


New changeset e6a0b5982973e64b9fa28e5e3e54eb8c47882780 by Serhiy Storchaka in 
branch '2.7':
[2.7] bpo-27945: Fixed various segfaults with dict. (GH-1657) (#1681)
https://github.com/python/cpython/commit/e6a0b5982973e64b9fa28e5e3e54eb8c47882780


--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
pull_requests: +1776

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Serhiy Storchaka added the comment:


New changeset 564398af6ccb34d0db8b6e2537830eca285689e5 by Serhiy Storchaka in 
branch '3.6':
[3.6] bpo-27945: Fixed various segfaults with dict. (GH-1657) (#1677)
https://github.com/python/cpython/commit/564398af6ccb34d0db8b6e2537830eca285689e5


--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Serhiy Storchaka added the comment:


New changeset 2f7f533cf6fb57fcedcbc7bd454ac59fbaf2c655 by Serhiy Storchaka in 
branch '3.5':
[3.5] bpo-27945: Fixed various segfaults with dict. (GH-1657) (#1678)
https://github.com/python/cpython/commit/2f7f533cf6fb57fcedcbc7bd454ac59fbaf2c655


--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
pull_requests: +1773

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
pull_requests: +1772

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Serhiy Storchaka added the comment:


New changeset 753bca3934a7618a4fa96e107ad1c5c18633a683 by Serhiy Storchaka in 
branch 'master':
bpo-27945: Fixed various segfaults with dict. (#1657)
https://github.com/python/cpython/commit/753bca3934a7618a4fa96e107ad1c5c18633a683


--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
pull_requests: +1751

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

This issue is severe, but I don't consider it as release critical for 3.6.0. 
The patch fixes segfaults, but it can add unneeded overhead, and the dict 
performance is critical for Python core. The segfaults are not new. I'm trying 
to minimize the overhead of changes, but this doesn't suffer a haste.

--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Ned Deily]

Ned Deily added the comment:

Where do we stand on this issue?  At the moment, 3.6.0 is going to be released 
without these fixes.

--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[INADA Naoki]

INADA Naoki added the comment:

LGTM.

Performance on Azure VM (AMD Opteron(tm) Processor 4171 HE):

$ ~/local/py36/bin/patched -m perf compare_to master.json patched.json -G
Slower (10):
- spectral_norm: 915 ms +- 17 ms -> 967 ms +- 25 ms: 1.06x slower
- nbody: 774 ms +- 28 ms -> 805 ms +- 22 ms: 1.04x slower
- regex_dna: 965 ms +- 18 ms -> 993 ms +- 22 ms: 1.03x slower
- go: 1.93 sec +- 0.05 sec -> 1.99 sec +- 0.06 sec: 1.03x slower
- python_startup_no_site: 29.7 ms +- 2.0 ms -> 30.5 ms +- 2.0 ms: 1.03x slower
- xml_etree_parse: 1.02 sec +- 0.03 sec -> 1.04 sec +- 0.04 sec: 1.02x slower
- python_startup: 52.7 ms +- 3.6 ms -> 53.7 ms +- 3.6 ms: 1.02x slower
- xml_etree_generate: 962 ms +- 24 ms -> 978 ms +- 25 ms: 1.02x slower
- pickle_dict: 188 us +- 8 us -> 191 us +- 8 us: 1.02x slower
- scimark_fft: 2.19 sec +- 0.04 sec -> 2.20 sec +- 0.05 sec: 1.00x slower

Faster (26):
- fannkuch: 3.76 sec +- 0.07 sec -> 3.55 sec +- 0.09 sec: 1.06x faster
- call_method_slots: 59.4 ms +- 9.0 ms -> 56.6 ms +- 3.4 ms: 1.05x faster
- sqlite_synth: 27.6 us +- 1.4 us -> 26.4 us +- 1.2 us: 1.05x faster
- nqueens: 852 ms +- 19 ms -> 813 ms +- 20 ms: 1.05x faster
- django_template: 1.35 sec +- 0.04 sec -> 1.29 sec +- 0.03 sec: 1.05x faster
- unpack_sequence: 407 ns +- 16 ns -> 390 ns +- 16 ns: 1.04x faster
- call_method: 60.6 ms +- 1.4 ms -> 58.1 ms +- 1.6 ms: 1.04x faster
- xml_etree_iterparse: 918 ms +- 29 ms -> 881 ms +- 32 ms: 1.04x faster
- unpickle_pure_python: 3.44 ms +- 0.14 ms -> 3.31 ms +- 0.19 ms: 1.04x faster
- richards: 608 ms +- 20 ms -> 585 ms +- 15 ms: 1.04x faster
- chaos: 921 ms +- 22 ms -> 891 ms +- 24 ms: 1.03x faster
- mako: 189 ms +- 8 ms -> 183 ms +- 8 ms: 1.03x faster
- meteor_contest: 699 ms +- 23 ms -> 677 ms +- 26 ms: 1.03x faster
- regex_compile: 1.52 sec +- 0.03 sec -> 1.48 sec +- 0.04 sec: 1.03x faster
- chameleon: 101 ms +- 5 ms -> 97.8 ms +- 4.0 ms: 1.03x faster
- regex_v8: 153 ms +- 7 ms -> 149 ms +- 6 ms: 1.03x faster
- 2to3: 2.59 sec +- 0.05 sec -> 2.52 sec +- 0.05 sec: 1.03x faster
- crypto_pyaes: 797 ms +- 20 ms -> 776 ms +- 18 ms: 1.03x faster
- genshi_xml: 653 ms +- 23 ms -> 637 ms +- 27 ms: 1.03x faster
- pickle_pure_python: 4.54 ms +- 0.19 ms -> 4.42 ms +- 0.20 ms: 1.03x faster
- regex_effbot: 18.4 ms +- 0.7 ms -> 18.1 ms +- 0.6 ms: 1.02x faster
- unpickle: 107 us +- 4 us -> 105 us +- 5 us: 1.02x faster
- deltablue: 61.2 ms +- 3.4 ms -> 60.0 ms +- 3.5 ms: 1.02x faster
- raytrace: 4.34 sec +- 0.14 sec -> 4.26 sec +- 0.10 sec: 1.02x faster
- telco: 63.9 ms +- 3.1 ms -> 62.9 ms +- 3.2 ms: 1.02x faster
- pidigits: 965 ms +- 14 ms -> 955 ms +- 20 ms: 1.01x faster

Benchmark hidden because not significant (28): call_method_unknown, 
call_simple, dulwich_log, float, genshi_text, hexiom, html5lib, json_dumps, 
json_loads, logging_format, logging_silent, logging_simple, pathlib, pickle, 
pickle_list, scimark_lu, scimark_monte_carlo, scimark_sor, 
scimark_sparse_mat_mult, sqlalchemy_declarative, sqlalchemy_imperative, 
sympy_expand, sympy_integrate, sympy_str, sympy_sum, tornado_http, 
unpickle_list, xml_etree_process

--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

The change to dict_equal() LGTM. It doesn't add an overhead.

For dictiter_iternextitem() I propose other change. It doesn't add an overhead.

There are bugs in the patch for _PyDict_FromKeys().

The change to dictitems_contains() adds an overhead, but it is small and seems 
unavoidable.

I wondering whether it is worth to use PySequence_Tuple() instead of 
PySequence_Fast() in PyDict_MergeFromSeq2(). This would add a cost of creating 
new tuple if items are not tuples, but save increfing/decrefing the key and the 
value in common case.

I have doubts about insertdict(). Additional incref duplicates increfs in 
dict_merge(). Is it worth to move it outside of insertdict()?

I have doubts about _PyDict_FromKeys(). It seems to me that it is safe to use 
_PyDict_Next() in a loop that mutates the dict (not checked _PySet_Next()). 
With guarded insertdict() additional check is not needed (and it doesn't help 
against clearing the dict inside insertdict()).

In updated patch fixed some bugs and used different solution for 
dictiter_iternextitem().

--
Added file: http://bugs.python.org/file45550/27945-dict-segv-py37-2.patch

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Here is a consolidated patch for review.

--
Added file: http://bugs.python.org/file45549/27945-dict-segv-py36.patch

___
Python tracker 

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

[issue27945] Various segfaults with dict

[INADA Naoki]

INADA Naoki added the comment:

I modified the patch to avoid incref when pair is not list, because 
tuple is common for such case.
But I can't get back original performance.

(python 2.7 is modified version of patch)

inada-n@test1:~/work/bench$ ~/local/py27/bin/master -m perf timeit --rigorous  
--python ~/local/py27/bin/python2.7 --compare-to ~/local/py27/bin/master -s 'L 
= [(i,i) for i in range(1)]' -- 'dict(L)'
master: . 1.47 ms +- 0.06 ms
python2.7: . 1.55 ms +- 0.06 ms

Median +- std dev: [master] 1.47 ms +- 0.06 ms -> [python2.7] 1.55 ms +- 0.06 
ms: 1.05x slower
inada-n@test1:~/work/bench$ ~/local/py27/bin/master -m perf timeit --rigorous  
--python ~/local/py27/bin/python2.7 --compare-to ~/local/py27/bin/patched -s 'L 
= [(i,i) for i in range(1)]' -- 'dict(L)'
patched: . 1.56 ms +- 0.08 ms
python2.7: . 1.55 ms +- 0.09 ms

Median +- std dev: [patched] 1.56 ms +- 0.08 ms -> [python2.7] 1.55 ms +- 0.09 
ms: 1.01x faster
Not significant!

--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[INADA Naoki]

INADA Naoki added the comment:

I'm sorry, dict.fromkeys() didn't use PyDict_MergeFromSeq2().

This may be microbench for worst case:

$ ~/local/py35/bin/master -m perf timeit --rigorous  --python 
~/local/py35/bin/patched --compare-to ~/local/py35/bin/master -s 'L = [(i,i) 
for i in range(1)]' -- 'dict(L)'
master: . 2.06 ms +- 0.11 ms
patched: . 2.16 ms +- 0.09 ms

Median +- std dev: [master] 2.06 ms +- 0.11 ms -> [patched] 2.16 ms +- 0.09 ms: 
1.05x slower

$ ~/local/py27/bin/master -m perf timeit --rigorous  --python 
~/local/py27/bin/patched --compare-to ~/local/py27/bin/master -s 'L = [(i,i) 
for i in range(1)]' -- 'dict(L)'
master: . 1.48 ms +- 0.06 ms
patched: . 1.57 ms +- 0.09 ms

Median +- std dev: [master] 1.48 ms +- 0.06 ms -> [patched] 1.57 ms +- 0.09 ms: 
1.06x slower

--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[INADA Naoki]

INADA Naoki added the comment:

Only patch which affects to hot loop is:

--- a/Objects/dictobject.c  Tue Nov 15 21:21:35 2016 -0500
+++ b/Objects/dictobject.c  Wed Nov 16 11:40:51 2016 +
@@ -1550,11 +1550,18 @@ PyDict_MergeFromSeq2(PyObject *d, PyObje
 /* Update/merge with this (key, value) pair. */
 key = PySequence_Fast_GET_ITEM(fast, 0);
 value = PySequence_Fast_GET_ITEM(fast, 1);
+Py_INCREF(key);
+Py_INCREF(value);
 if (override || PyDict_GetItem(d, key) == NULL) {
 int status = PyDict_SetItem(d, key, value);
-if (status < 0)
+if (status < 0) {
+Py_DECREF(key);
+Py_DECREF(value);
 goto Fail;
+}
 }
+Py_DECREF(key);
+Py_DECREF(value);
 Py_DECREF(fast);
 Py_DECREF(item);
 }

Microbenchmark for it:

$ ~/local/py35/bin/master -m perf timeit --rigorous -t --python 
~/local/py35/bin/patched --compare-to ~/local/py35/bin/master -s 'L = [(i,i) 
for i in range(1)]' -- 'dict.fromkeys(L)'
Benchmark master


.Total duration: 21.2 sec
Start date: 2016-11-17 12:21:39
End date: 2016-11-17 12:22:13
Raw sample minimum: 109 ms
Raw sample maximum: 144 ms

Number of runs: 41
Total number of samples: 120
Number of samples per run: 3
Number of warmups per run: 1
Loop iterations per sample: 32

Minimum: 3.41 ms (-13%)
Median +- std dev: 3.92 ms +- 0.19 ms
Mean +- std dev: 3.92 ms +- 0.19 ms
Maximum: 4.50 ms (+15%)

Median +- std dev: 3.92 ms +- 0.19 ms

Benchmark patched
=

.Total duration: 21.3 sec
Start date: 2016-11-17 12:22:13
End date: 2016-11-17 12:22:47
Raw sample minimum: 108 ms
Raw sample maximum: 152 ms

Number of runs: 41
Total number of samples: 120
Number of samples per run: 3
Number of warmups per run: 1
Loop iterations per sample: 32

Minimum: 3.39 ms (-14%)
Median +- std dev: 3.92 ms +- 0.23 ms
Mean +- std dev: 3.95 ms +- 0.23 ms
Maximum: 4.74 ms (+21%)

Median +- std dev: 3.92 ms +- 0.23 ms

Compare
===

Median +- std dev: [master] 3.92 ms +- 0.19 ms -> [patched] 3.92 ms +- 0.23 ms: 
1.00x slower
Not significant!

--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[INADA Naoki]

INADA Naoki added the comment:

I run performance 0.5.0 on Python 3.5.
Since it took long time even without -b all option, I haven't run it for Python 
2.7 yet.

On Python 3.5:

$ ./venv/cpython3.5-846d5b1f0b61/bin/python -m perf compare_to py35-master.json 
py35-patched.json  -G
Slower (14):
- logging_silent: 2.92 us +- 0.15 us -> 3.05 us +- 0.18 us: 1.04x slower
- pickle: 93.2 us +- 4.7 us -> 95.8 us +- 5.0 us: 1.03x slower
- python_startup: 72.6 ms +- 4.7 ms -> 74.6 ms +- 4.8 ms: 1.03x slower
- meteor_contest: 766 ms +- 13 ms -> 779 ms +- 15 ms: 1.02x slower
- scimark_sor: 2.00 sec +- 0.04 sec -> 2.04 sec +- 0.03 sec: 1.02x slower
- fannkuch: 3.86 sec +- 0.05 sec -> 3.91 sec +- 0.04 sec: 1.01x slower
- python_startup_no_site: 37.4 ms +- 2.4 ms -> 38.0 ms +- 2.6 ms: 1.01x slower
- regex_dna: 958 ms +- 13 ms -> 973 ms +- 16 ms: 1.01x slower
- regex_compile: 1.41 sec +- 0.02 sec -> 1.43 sec +- 0.02 sec: 1.01x slower
- raytrace: 5.46 sec +- 0.06 sec -> 5.52 sec +- 0.07 sec: 1.01x slower
- scimark_lu: 1.88 sec +- 0.09 sec -> 1.90 sec +- 0.12 sec: 1.01x slower
- nbody: 887 ms +- 19 ms -> 895 ms +- 21 ms: 1.01x slower
- 2to3: 3.01 sec +- 0.03 sec -> 3.03 sec +- 0.04 sec: 1.01x slower
- scimark_fft: 2.52 sec +- 0.05 sec -> 2.54 sec +- 0.04 sec: 1.01x slower

Faster (16):
- scimark_sparse_mat_mult: 35.0 ms +- 1.8 ms -> 32.7 ms +- 2.0 ms: 1.07x faster
- xml_etree_process: 1.13 sec +- 0.03 sec -> 1.07 sec +- 0.03 sec: 1.06x faster
- xml_etree_generate: 1.35 sec +- 0.03 sec -> 1.29 sec +- 0.03 sec: 1.05x faster
- mako: 154 ms +- 8 ms -> 149 ms +- 7 ms: 1.04x faster
- telco: 87.4 ms +- 3.7 ms -> 84.2 ms +- 4.1 ms: 1.04x faster
- tornado_http: 1.75 sec +- 0.03 sec -> 1.70 sec +- 0.03 sec: 1.03x faster
- call_simple: 45.5 ms +- 2.1 ms -> 44.3 ms +- 1.4 ms: 1.03x faster
- sympy_integrate: 202 ms +- 9 ms -> 198 ms +- 7 ms: 1.02x faster
- sympy_str: 2.08 sec +- 0.06 sec -> 2.04 sec +- 0.06 sec: 1.02x faster
- django_template: 1.65 sec +- 0.03 sec -> 1.61 sec +- 0.03 sec: 1.02x faster
- call_method_unknown: 65.1 ms +- 0.9 ms -> 64.3 ms +- 0.9 ms: 1.01x faster
- xml_etree_parse: 1.34 sec +- 0.04 sec -> 1.32 sec +- 0.04 sec: 1.01x faster
- sympy_sum: 1.01 sec +- 0.03 sec -> 998 ms +- 35 ms: 1.01x faster
- call_method_slots: 59.3 ms +- 0.8 ms -> 58.6 ms +- 1.0 ms: 1.01x faster
- float: 1.24 sec +- 0.03 sec -> 1.23 sec +- 0.02 sec: 1.01x faster
- chaos: 1.20 sec +- 0.02 sec -> 1.19 sec +- 0.02 sec: 1.01x faster

Benchmark hidden because not significant (31): call_method, chameleon, 
crypto_pyaes, deltablue, dulwich_log, genshi_text, genshi_xml, go, hexiom, 
html5lib, json_dumps, json_loads, logging_format, logging_simple, nqueens, 
pathlib, pickle_dict, pickle_list, pickle_pure_python, pidigits, regex_effbot, 
regex_v8, richards, scimark_monte_carlo, spectral_norm, sympy_expand, 
unpack_sequence, unpickle, unpickle_list, unpickle_pure_python, 
xml_etree_iterparse

--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[INADA Naoki]

INADA Naoki added the comment:

OK, I'll run benchmark in this week.
But three patches seems don't affects to performance critical APIs.

--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

I worry about performance. Additional increment/decrement of reference counter 
can add significant overhead in critical places of Python interpreter. Before 
committing any patches we should measure the performance lost.

--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[INADA Naoki]

INADA Naoki added the comment:

0001-Issue-27945-fix-dictiter_iternextitem-use-after-free.patch LGTM and OK too.
But 0001-Issue-27945-Fixed-segfaults-in-dict.fromkeys-when-it.patch cause 
conflict.

I want to commit first three patches.
For another reviewer, here is the patch merging three patches. Please review it.

--
Added file: http://bugs.python.org/file45500/27945-dict-segv-py27.patch

___
Python tracker 

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

[issue27945] Various segfaults with dict

[INADA Naoki]

INADA Naoki added the comment:

0001-Issue-27945-fix-dictitems_contains-use-after-free.patch

LGTM.
This patch can be applied to 2.7 and 3.5, without conflict against previous 
patch.
It passes `make quicktest`.

--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[INADA Naoki]

INADA Naoki added the comment:

0001-Issue-27945-fix-PyDict_MergeFromSeq2-use-after-free.patch:

LGTM. I've checked it can be applied to 2.7 and 3.5 branch and passes
`make quicktest`.

--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
nosy: +inada.naoki
versions: +Python 2.7, Python 3.7

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Duane Griffin]

Duane Griffin added the comment:

Note that I think most or all of these issues apply to 2.7 and while I didn't 
do a proper check I think the fixes also apply.

--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Tim Mitchell]

Tim Mitchell added the comment:

Here is my patch for parts 3 and 4.
Core issue for part 4 appears to be dk_lookup calling arbitrary python code may 
free the key.
dk_lookup is also used in _PyDict_LoadGlobal not sure if this bug can occur 
here.

--
nosy: +Tim Mitchell
Added file: 
http://bugs.python.org/file44587/0001-Issue-27945-Fixed-segfaults-in-dict.fromkeys-when-it.patch

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
assignee:  -> serhiy.storchaka
stage: needs patch -> patch review

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Duane Griffin]

Changes by Duane Griffin :


Removed file: 
http://bugs.python.org/file44573/0001-Issue-27945-fix-dictiter_iternextitem-use-after-free.patch

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Duane Griffin]

Duane Griffin added the comment:

Ah, my first fix (for the fifth issue) was incomplete. Please see attached 
patch which I think correctly fixes the problem.

--
Added file: 
http://bugs.python.org/file44585/0001-Issue-27945-fix-dictiter_iternextitem-use-after-free.patch

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Duane Griffin]

Duane Griffin added the comment:

Fix for the second issue: with this fix there is no segfault or valgrind issue 
reported on during execution or on exit.

--
Added file: 
http://bugs.python.org/file44582/0001-Issue-27945-fix-dictitems_contains-use-after-free.patch

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Duane Griffin]

Duane Griffin added the comment:

Fix for the first issue: with this fix there is no segfault or valgrind issue 
reported on during execution or on exit.

--
Added file: 
http://bugs.python.org/file44579/0001-Issue-27945-fix-PyDict_MergeFromSeq2-use-after-free.patch

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Duane Griffin]

Duane Griffin added the comment:

Apologies: compiling python with --with-pydebug all of these issues are 
reproducible on head after all. Furthermore while my patch fixes the reported 
crash it still crashes on exit:

Program received signal SIGSEGV, Segmentation fault.
0x00437193 in visit_decref (op=0x768a4c98, data=0x0) at 
Modules/gcmodule.c:374
374 if (PyObject_IS_GC(op)) {
(gdb) bt
#0  0x00437193 in visit_decref (op=0x768a4c98, data=0x0) at 
Modules/gcmodule.c:374
#1  0x004a9112 in tupletraverse (o=0x768a49f8, visit=0x43716d 
, arg=0x0) at Objects/tupleobject.c:571
#2  0x0043690a in subtract_refs (containers=containers@entry=0x87cda0 
) at Modules/gcmodule.c:399
#3  0x00437ac3 in collect (generation=generation@entry=2, 
n_collected=n_collected@entry=0x7fffd838, 
n_uncollectable=n_uncollectable@entry=0x7fffd840, 
nofail=nofail@entry=0) at Modules/gcmodule.c:956
#4  0x00437f57 in collect_with_callback (generation=generation@entry=2) 
at Modules/gcmodule.c:1128
#5  0x004383a6 in PyGC_Collect () at Modules/gcmodule.c:1592
#6  0x004383cf in _PyGC_CollectIfEnabled () at Modules/gcmodule.c:1605
#7  0x00420c76 in Py_FinalizeEx () at Python/pylifecycle.c:603
#8  0x0043682b in Py_Main (argc=argc@entry=2, argv=argv@entry=0x90d010) 
at Modules/main.c:798
#9  0x0041d153 in main (argc=2, argv=0x7fffdae8) at 
./Programs/python.c:69

--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Duane Griffin]

Duane Griffin added the comment:

I cannot reproduce the segfaults for the first four issues however valgrind 
still reports problems for all but the second. The fifth (last) one still 
segfaults.

I have a patch for the fifth issue. The other remaining issues are all 
reporting the same invalid read at exit. I'm not sure whether that is the same 
issue reported here or something else that they all trigger:

Invalid read of size 8
at 0x43C5FA: delete_garbage (gcmodule.c:868)
by 0x43C5FA: collect (gcmodule.c:1019)
by 0x43D2F0: _PyGC_CollectNoFail (gcmodule.c:1623)
by 0x55D95E: PyImport_Cleanup (import.c:420)
by 0x4224E5: Py_FinalizeEx.part.3 (pylifecycle.c:612)
by 0x43B0CB: Py_Main (main.c:798)
by 0x41DC71: main (python.c:69)

--
keywords: +patch
nosy: +duaneg
Added file: 
http://bugs.python.org/file44573/0001-Issue-27945-fix-dictiter_iternextitem-use-after-free.patch

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Emanuel Barry]

Emanuel Barry added the comment:

Ping. The built-in dict was considerably changed in #27350; do any of these 
issues still persist?

--

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Christian Heimes]

Changes by Christian Heimes :


--
nosy: +christian.heimes

___
Python tracker 

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

[issue27945] Various segfaults with dict

[Emanuel Barry]

Changes by Emanuel Barry :


--
nosy: +ebarry, larry, ned.deily, rhettinger, serhiy.storchaka
priority: normal -> critical
stage:  -> needs patch
title: five dictobject issues -> Various segfaults with dict
type:  -> crash

___
Python tracker 

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