Skip to content

Commit 544bf89

Browse files
committed
gh-159184: Do not compare set elements when marshalling a set
Set elements are written in the order of their dumps. They were sorted as (dump, element) pairs, so elements with the same dump were compared. That raised TypeError for a set of several complex NaNs, which the compiler can produce by constant folding, making the module impossible to marshal. Sort (dump, index, element) tuples, in which the indexes are all different. Elements with the same dump stay in the order of the set, as float NaNs did before.
1 parent 7893950 commit 544bf89

3 files changed

Lines changed: 63 additions & 18 deletions

File tree

‎Lib/test/test_marshal.py‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -600,13 +600,44 @@ def test_unmarshallable(self):
600600
"cannot marshal type objects"):
601601
marshal.dumps((arg, memoryview(b'')))
602602

603-
def test_error_in_set_item(self):
604-
# Set items are sorted by their marshalled representation, and NaNs
605-
# are only distinguished by identity, so they are compared as
606-
# complex numbers.
603+
def test_unorderable_set_items(self):
604+
# Set items are sorted by their marshalled representation, they
605+
# are not compared, even if NaNs which are distinct objects have
606+
# the same marshalled representation.
607607
nan = float('nan')
608-
with self.assertRaisesRegex(TypeError, "'<' not supported"):
609-
marshal.dumps({complex(nan, 0), complex(nan, 0)})
608+
for kind in set, frozenset:
609+
for version in range(marshal.version + 1):
610+
with self.subTest(kind=kind, version=version):
611+
sample = kind([complex(nan, 0), complex(nan, 0), 1j,
612+
(complex(nan, 0),), (complex(nan, 0),),
613+
float('nan'), float('nan')])
614+
self.assertEqual(len(sample), 7)
615+
new = marshal.loads(marshal.dumps(sample, version))
616+
self.assertIs(type(new), kind)
617+
self.assertEqual(sorted(map(repr, new)),
618+
sorted(map(repr, sample)))
619+
620+
621+
def test_unorderable_set_items_in_code(self):
622+
# A constant set containing NaNs which are distinct objects.
623+
# Run in a subprocess: constants of code objects are immortal
624+
# in the free-threaded build.
625+
script = textwrap.dedent("""
626+
import marshal
627+
source = 'x in {(1e999-1e999)*1j, (1e999-1e999)*2j, 1.5}'
628+
code = compile(source, '<marshal test>', 'eval')
629+
def consts(code):
630+
return [sorted(map(repr, x)) if isinstance(x, frozenset) else x
631+
for x in code.co_consts]
632+
assert ['(nan+nanj)', '(nan+nanj)', '1.5'] in consts(code)
633+
for version in range(marshal.version + 1):
634+
new = marshal.loads(marshal.dumps(code, version))
635+
assert new.co_code == code.co_code
636+
assert consts(new) == consts(code)
637+
assert eval(new, {'x': 1.5}) is True
638+
assert eval(new, {'x': complex('nan')}) is False
639+
""")
640+
assert_python_ok('-c', script)
610641

611642
def test_set_changed_size(self):
612643
# A set is modified while its items are being marshalled.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix :func:`marshal.dumps` raising :exc:`TypeError` for a set or frozenset
2+
containing several complex NaNs, which also made it impossible to write a
3+
:file:`.pyc` file for source code containing such a constant.

‎Python/marshal.c‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -845,9 +845,13 @@ w_complex_object(PyObject *v, WFILE *p)
845845
// bpo-37596: To support reproducible builds, sets and frozensets need
846846
// to have their elements serialized in a consistent order (even when
847847
// they have been scrambled by hash randomization). To ensure this, we
848-
// use an order equivalent to sorted(v, key=marshal.dumps):
849-
PyObject *pairs = PyList_New(n);
850-
if (pairs == NULL) {
848+
// use an order equivalent to sorted(v, key=marshal.dumps).
849+
// The list has a (dump, index, element) tuple for each element.
850+
// The indexes are all different, so that sorting never compares
851+
// two elements: elements with the same dump (NaNs which are
852+
// distinct objects) may not be orderable.
853+
PyObject *items = PyList_New(n);
854+
if (items == NULL) {
851855
p->error = true;
852856
return;
853857
}
@@ -871,12 +875,19 @@ w_complex_object(PyObject *v, WFILE *p)
871875
Py_DECREF(value);
872876
break;
873877
}
874-
PyObject *pair = _PyTuple_FromPairSteal(dump, value);
875-
if (pair == NULL) {
878+
PyObject *index = PyLong_FromSsize_t(i);
879+
PyObject *item = NULL;
880+
if (index != NULL) {
881+
item = PyTuple_Pack(3, dump, index, value);
882+
Py_DECREF(index);
883+
}
884+
Py_DECREF(dump);
885+
Py_DECREF(value);
886+
if (item == NULL) {
876887
p->error = true;
877888
break;
878889
}
879-
PyList_SET_ITEM(pairs, i++, pair);
890+
PyList_SET_ITEM(items, i++, item);
880891
}
881892
Py_END_CRITICAL_SECTION();
882893
if (!p->error && (changed || i != n)) {
@@ -885,20 +896,20 @@ w_complex_object(PyObject *v, WFILE *p)
885896
p->error = true;
886897
}
887898
if (p->error) {
888-
Py_DECREF(pairs);
899+
Py_DECREF(items);
889900
return;
890901
}
891-
if (PyList_Sort(pairs)) {
902+
if (PyList_Sort(items)) {
892903
p->error = true;
893-
Py_DECREF(pairs);
904+
Py_DECREF(items);
894905
return;
895906
}
896907
for (Py_ssize_t i = 0; i < n; i++) {
897-
PyObject *pair = PyList_GET_ITEM(pairs, i);
898-
value = PyTuple_GET_ITEM(pair, 1);
908+
PyObject *item = PyList_GET_ITEM(items, i);
909+
value = PyTuple_GET_ITEM(item, 2);
899910
w_object(value, p);
900911
}
901-
Py_DECREF(pairs);
912+
Py_DECREF(items);
902913
}
903914
else if (PyCode_Check(v)) {
904915
if (!p->allow_code) {

0 commit comments

Comments
 (0)