Skip to content

Commit 3c23ebf

Browse files
authored
gh-159184: Make the marshal order of set elements with equal dumps stable (GH-159197)
Set elements with the same dump (NaNs which are distinct objects) were written in set iteration order, which depends on their addresses. When one of them is also referenced elsewhere in the dump, the position of the reference among them changed from run to run. Elements with equal dumps are now ordered by when they were first written in this dump; floats and complex numbers not written yet are interchangeable, and which one is which is decided when one of them is referenced again. This relies on the writer knowing exactly which objects it has written (gh-129724). The elements are sorted with qsort() on the bytes of their dumps, in an array instead of a list of tuples.
1 parent 410fdda commit 3c23ebf

3 files changed

Lines changed: 467 additions & 79 deletions

File tree

‎Lib/test/test_marshal.py‎

Lines changed: 191 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -617,7 +617,6 @@ def test_unorderable_set_items(self):
617617
self.assertEqual(sorted(map(repr, new)),
618618
sorted(map(repr, sample)))
619619

620-
621620
def test_unorderable_set_items_in_code(self):
622621
# A constant set containing NaNs which are distinct objects.
623622
# Run in a subprocess: constants of code objects are immortal
@@ -662,7 +661,6 @@ def shrink(sample):
662661
"set changed size"):
663662
marshal.dumps(sample, version)
664663

665-
666664
def test_error_in_buffer(self):
667665
# The BufferError raised for a non-contiguous buffer is not replaced
668666
# with a generic error.
@@ -1238,7 +1236,8 @@ def test_nan_in_set(self):
12381236
nan1 = float('nan')
12391237
nan2 = float('nan')
12401238
nan3 = float('nan')
1241-
s = kind([nan1, 1.5, nan2, nan3])
1239+
s = kind([nan1, 1.5, nan2, nan3,
1240+
complex('nan'), complex('nan')])
12421241
for sample in (s, [s], [nan1, s], [s, nan2],
12431242
[nan3, s, nan1], [nan1, nan2, nan3, s],
12441243
[s, s], (nan2, [s, nan2], s)):
@@ -1247,6 +1246,194 @@ def test_nan_in_set(self):
12471246
new = marshal.loads(data)
12481247
self.check_same_structure(new, sample, version >= 3)
12491248

1249+
@support.cpython_only
1250+
def test_nan_in_set_order(self):
1251+
# The output does not depend on the order of the elements of a set
1252+
# which have the same marshalled representation, even if some of
1253+
# them are also referenced elsewhere.
1254+
version = self.VERSION
1255+
def flagged(data):
1256+
data = bytearray(data)
1257+
data[0] |= self.FLAG_REF
1258+
return bytes(data)
1259+
def ref(index):
1260+
return b'r' + struct.pack('<i', index)
1261+
keep = []
1262+
for make_nan in (lambda: float('nan'), lambda: complex('nan')):
1263+
data = marshal.dumps(make_nan(), version)
1264+
fdata = flagged(data)
1265+
data15 = marshal.dumps(1.5, version)
1266+
for kind, typecode in (set, b'<'), (frozenset, b'>'):
1267+
head = typecode + b'\4\0\0\0' + data15
1268+
for i in range(20):
1269+
# Different objects at different addresses every time.
1270+
keep.append(bytes(i * 8))
1271+
nans = [make_nan() for j in range(3)]
1272+
keep.append(nans)
1273+
s = kind([1.5, *nans])
1274+
a, b, c = nans[i % 3:] + nans[:i % 3]
1275+
for sample, expected in [
1276+
(s, head + data * 3),
1277+
([s, a],
1278+
b'[\2\0\0\0' + head + fdata + data * 2 + ref(0)),
1279+
([a, s],
1280+
b'[\2\0\0\0' + fdata + head + ref(0) + data * 2),
1281+
([s, a, b, a],
1282+
b'[\4\0\0\0' + head + fdata * 2 + data +
1283+
ref(0) + ref(1) + ref(0)),
1284+
([a, b, s, c],
1285+
b'[\4\0\0\0' + fdata * 2 + head + ref(0) + ref(1) +
1286+
flagged(data) + ref(2)),
1287+
([b, s, c, a, kind([a, b, c])],
1288+
b'[\5\0\0\0' + fdata + head + ref(0) + fdata * 2 +
1289+
ref(1) + ref(2) + typecode + b'\3\0\0\0' +
1290+
ref(0) + ref(1) + ref(2)),
1291+
([s, kind([b, a]), kind([c, make_nan()]), b, a, c],
1292+
b'[\6\0\0\0' + head + fdata * 3 +
1293+
typecode + b'\2\0\0\0' + ref(0) + ref(1) +
1294+
typecode + b'\2\0\0\0' + ref(2) + data +
1295+
ref(0) + ref(1) + ref(2)),
1296+
]:
1297+
self.assertEqual(marshal.dumps(sample, version),
1298+
expected)
1299+
new = marshal.loads(expected)
1300+
self.check_same_structure(new, sample, True)
1301+
1302+
@support.cpython_only
1303+
def test_nan_constants_in_code(self):
1304+
# Python has no literal for NaN, but constant folding produces one
1305+
# from arithmetic on 1e999 (inf). Such NaNs end up as a constant
1306+
# of the code object, in a tuple constant, or in a frozenset
1307+
# constant (the only kind of set the compiler creates, from a set
1308+
# display used as the right operand of "in"). Every fold creates
1309+
# a new object, so the compiler cannot produce a NaN which is an
1310+
# element of a set and is also referenced elsewhere; the ordering
1311+
# of such elements (test_nan_in_set_order) is not observable here.
1312+
# What is observable is that the code can be marshalled, and that
1313+
# the output, so the .pyc file, is the same in every process.
1314+
sources = {
1315+
'const': 'x = 1e999 - 1e999',
1316+
'tuple': 'x = (1e999 - 1e999, 2.0)',
1317+
'repeat': 'x = (1e999 - 1e999,) * 2',
1318+
'set': 'x in {1e999 - 1e999, 1e999 - 1e999, 2.0}',
1319+
'complex': 'x in {(1e999 - 1e999) * 1j, (1e999 - 1e999) * 2j}',
1320+
'tuples': 'x in {(1e999 - 1e999,) * 2, (1e999 - 1e999,) * 2}',
1321+
'nested': 'def f(x):\n return x in {1e999 - 1e999, 2j}',
1322+
}
1323+
def nan_places(code):
1324+
# Where the NaN constants of a code object are, with the
1325+
# elements of sets in any order.
1326+
places = []
1327+
def visit(x, where):
1328+
if isinstance(x, (float, complex)):
1329+
if x != x:
1330+
places.append((where, repr(x)))
1331+
elif isinstance(x, (tuple, frozenset)):
1332+
for y in sorted(x, key=repr):
1333+
visit(y, type(x).__name__)
1334+
elif isinstance(x, types.CodeType):
1335+
for y in x.co_consts:
1336+
visit(y, 'consts')
1337+
visit(code, None)
1338+
return places
1339+
expected = {
1340+
'const': [('consts', 'nan')],
1341+
'tuple': [('tuple', 'nan')],
1342+
'repeat': [('tuple', 'nan'), ('tuple', 'nan')],
1343+
'set': [('frozenset', 'nan'), ('frozenset', 'nan')],
1344+
'complex': [('frozenset', '(nan+nanj)'),
1345+
('frozenset', '(nan+nanj)')],
1346+
'tuples': [('tuple', 'nan')] * 4,
1347+
'nested': [('frozenset', 'nan')],
1348+
}
1349+
dumps = {}
1350+
for name, source in sources.items():
1351+
code = compile(source, '<marshal test>', 'exec')
1352+
self.assertEqual(nan_places(code), expected[name], name)
1353+
for version in range(marshal.version + 1):
1354+
data = marshal.dumps(code, version)
1355+
dumps[name, version] = data
1356+
new = marshal.loads(data)
1357+
self.assertEqual(nan_places(new), expected[name], name)
1358+
self.assertEqual(new.co_code, code.co_code)
1359+
# Compiling again gives other NaN objects at other
1360+
# addresses, and the same bytes.
1361+
code2 = compile(source, '<marshal test>', 'exec')
1362+
self.assertEqual(marshal.dumps(code2, version), data)
1363+
1364+
# The same bytes in other processes.
1365+
script = textwrap.dedent("""
1366+
import marshal, sys
1367+
sources = %r
1368+
for name, source in sources.items():
1369+
code = compile(source, '<marshal test>', 'exec')
1370+
for version in range(marshal.version + 1):
1371+
print(name, version, marshal.dumps(code, version).hex())
1372+
""") % sources
1373+
for seed in '1', '2', 'random':
1374+
with self.subTest(hashseed=seed):
1375+
rc, out, err = assert_python_ok('-c', script,
1376+
PYTHONHASHSEED=seed)
1377+
for line in out.decode().splitlines():
1378+
name, version, data = line.split()
1379+
self.assertEqual(bytes.fromhex(data),
1380+
dumps[name, int(version)],
1381+
(name, version))
1382+
1383+
def test_nan_in_set_changed(self):
1384+
# An element which is removed from the set, and another one added,
1385+
# while a later element is dumped (here by its __buffer__() method)
1386+
# is still written, since it was seen; but it is then referenced
1387+
# only by the writer, which only keeps track of objects that have
1388+
# several references. The set code must cope with a NaN which is
1389+
# written but not tracked.
1390+
class Item:
1391+
def __init__(self, action, h):
1392+
self.action = action
1393+
self.h = h
1394+
def __hash__(self):
1395+
return self.h
1396+
def __buffer__(self, flags):
1397+
# Called twice: to dump the element, then to write it.
1398+
action, self.action = self.action, None
1399+
if action is not None:
1400+
action()
1401+
return memoryview(b'abc')
1402+
1403+
def layout(nans, extra):
1404+
# A hash for the item and a replacement element such that the
1405+
# NaNs are iterated before the item, and the replacement lands
1406+
# in a slot which has been iterated already: the writer sees
1407+
# as many elements as the set has, and does not notice.
1408+
for h in range(128):
1409+
for k in range(128, 256):
1410+
s = set(nans) | set(extra)
1411+
item = Item(None, h)
1412+
s.add(item)
1413+
seen = []
1414+
for x in s:
1415+
seen.append(x)
1416+
if x is item:
1417+
s.discard(nans[0])
1418+
s.add(k)
1419+
if (len(seen) == len(s) and seen[-1] is item
1420+
and all(any(n is x for x in seen) for n in nans)):
1421+
return h, k
1422+
self.skipTest('no suitable set layout found')
1423+
1424+
for version in range(marshal.version + 1):
1425+
with self.subTest(version=version):
1426+
nans = [float('nan') for i in range(3)]
1427+
h, k = layout(nans, [1.5])
1428+
s = set(nans) | {1.5}
1429+
def remove_one():
1430+
s.discard(nans.pop(0))
1431+
s.add(k)
1432+
s.add(Item(remove_one, h))
1433+
new = marshal.loads(marshal.dumps(s, version))
1434+
self.assertEqual(sorted(map(repr, new)),
1435+
['1.5', "b'abc'", 'nan', 'nan', 'nan'])
1436+
12501437
def check_same_structure(self, new, sample, shared):
12511438
# Both object graphs have the same values and, if shared is true,
12521439
# share the same objects. Elements of sets are matched in any
@@ -1261,7 +1448,7 @@ def known(x):
12611448
if shared:
12621449
return seen.get(id(x), -1)
12631450
def visit(x):
1264-
if isinstance(x, float):
1451+
if isinstance(x, (float, complex)):
12651452
result.append((repr(x), number(x)))
12661453
elif isinstance(x, (set, frozenset)):
12671454
# Elements are only numbered if they occur elsewhere.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
The output of :func:`marshal.dumps` no longer depends on the order of the
2+
elements of a set which have the same marshalled form, such as NaNs which
3+
are distinct objects.

0 commit comments

Comments
 (0)