diff --git a/Include/internal/pycore_compile.h b/Include/internal/pycore_compile.h index 7e248429af8eb8..dc325919ce21c6 100644 --- a/Include/internal/pycore_compile.h +++ b/Include/internal/pycore_compile.h @@ -201,6 +201,10 @@ int _PyCodegen_Module(struct _PyCompiler *c, _Py_SourceLocation loc, asdl_stmt_s bool is_interactive); int _PyCompile_ConstCacheMergeOne(PyObject *const_cache, PyObject **obj); +int _PyCompile_ConstCacheMergeOneLocal(PyObject *const_cache, + PyObject *local_const_cache, + PyObject **obj); +int _PyCompile_ConstCacheAddLocal(PyObject *local_const_cache, PyObject *obj); PyCodeObject *_PyCompile_OptimizeAndAssemble(struct _PyCompiler *c, int addNone); diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py index df473d59fff3d8..6d12d76ecd7d5c 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -816,12 +816,30 @@ def check_same_constant(const): f2.__code__.co_consts[1]) # {0} is converted to a constant frozenset({0}) by the peephole - # optimizer - f1, f2 = lambda x: x in {0}, lambda x: x in {0} - self.assertIs(f1.__code__.co_consts, f2.__code__.co_consts) + # optimizer. In free-threaded builds, frozenset-bearing co_consts are + # not shared across code units because later string interning can + # replace them. + ns = {} + exec("f1, f2 = lambda x: x in {0}, lambda x: x in {0}", ns) + f1, f2 = ns["f1"], ns["f2"] + if support.Py_GIL_DISABLED: + self.assertIsNot(f1.__code__.co_consts, f2.__code__.co_consts) + else: + self.assertIs(f1.__code__.co_consts, f2.__code__.co_consts) self.check_constant(f1, frozenset({0})) + self.check_constant(f2, frozenset({0})) self.assertTrue(f1(0)) + def repeated_set_constants(x): + return x in {1, 2, 3}, x in {1, 2, 3} + + set_constants = [ + value + for value in repeated_set_constants.__code__.co_consts + if isinstance(value, frozenset) + ] + self.assertEqual(set_constants, [frozenset({1, 2, 3})]) + # Merging equal co_linetable is not a strict requirement # for the Python semantics, it's a more an implementation detail. @support.cpython_only diff --git a/Lib/test/test_marshal.py b/Lib/test/test_marshal.py index b5bacadbfd381f..0945ff44cc3aad 100644 --- a/Lib/test/test_marshal.py +++ b/Lib/test/test_marshal.py @@ -15,6 +15,27 @@ except ImportError: _testcapi = None + +def _make_ambiguous_set_codes(): + template = (lambda: None).__code__ + left = complex(float("nan"), 0) + right = complex(float("nan"), 0) + code = template.replace(co_consts=(frozenset((left, right)),)) + nested_code = template.replace( + co_consts=(frozenset((frozenset((left, right)),)),) + ) + return code, nested_code + + +if support.Py_GIL_DISABLED: + # Code construction immortalizes numeric constants in free-threaded + # builds. Create these constants once so refleak runs do not accumulate + # intentionally immortal objects. + _AMBIGUOUS_SET_CODES = _make_ambiguous_set_codes() +else: + _AMBIGUOUS_SET_CODES = () + + class HelperMixin: def helper(self, sample, *extra): new = marshal.loads(marshal.dumps(sample, *extra)) @@ -363,6 +384,144 @@ def test_shared_reference_tuple(self): self.assertEqual(b[0], big) self.assertIs(b[0], b[1]) + @unittest.skipUnless( + support.Py_GIL_DISABLED, + "free-threaded builds require deterministic reference tracking", + ) + def test_code_reference_tracking_does_not_depend_on_refcount(self): + for version in range(3, marshal.version + 1): + with self.subTest(version=version): + constant = (bytes(bytearray(b"payload")),) + code = (lambda: None).__code__.replace(co_consts=(constant,)) + del constant + + without_extra_reference = marshal.dumps(code, version) + extra_reference = code.co_consts[0] + with_extra_reference = marshal.dumps(code, version) + self.assertEqual(without_extra_reference, with_extra_reference) + self.assertEqual( + marshal.loads(with_extra_reference).co_consts, + code.co_consts, + ) + self.assertIs(extra_reference, code.co_consts[0]) + + @unittest.skipUnless( + support.Py_GIL_DISABLED, + "free-threaded builds require compile-order-independent constants", + ) + def test_code_serialization_does_not_depend_on_compile_order(self): + script = textwrap.dedent(""" + import marshal + {prelude} + code = compile({source!r}, "/repro/target.py", "exec") + print(marshal.dumps(code).hex()) + """) + + cases = ( + ( + """ + value = {"foo", 2, 3} + + def function(): + nested = {"foo", 2, 3} + """, + 'compile("import foo", "/repro/prelude.py", "exec")', + ), + ( + """ + value = target["foo":"bar"] + + def function(target): + return target["foo":"bar"] + """, + 'compile(\'target["foo":"bar"]\', ' + '"/repro/prelude.py", "exec")', + ), + ) + for source, prelude in cases: + source = textwrap.dedent(source) + with self.subTest(source=source): + _, without_prelude, _ = assert_python_ok( + "-c", + script.format(prelude="", source=source), + PYTHONHASHSEED="0", + ) + _, with_prelude, _ = assert_python_ok( + "-c", + script.format(prelude=prelude, source=source), + PYTHONHASHSEED="0", + ) + self.assertEqual(without_prelude, with_prelude) + + @unittest.skipUnless( + support.Py_GIL_DISABLED, + "only deterministic code serialization rejects ambiguous set order", + ) + def test_code_serialization_set_sort_key_ties(self): + code, nested_code = _AMBIGUOUS_SET_CODES + for version in range(marshal.version + 1): + with self.subTest(version=version): + with self.assertRaisesRegex( + ValueError, + "cannot deterministically marshal set elements", + ): + marshal.dumps(code, version) + with self.assertRaisesRegex( + ValueError, + "cannot deterministically marshal set elements", + ): + marshal.dumps(nested_code, version) + + source = """ + def contains(value): + return value in { + (1e300 * 1e300) - (1e300 * 1e300), + (1e300 * 1e300) - (1e300 * 1e300), + } + """ + code = compile(textwrap.dedent(source), "/repro/nan_set.py", "exec") + marshal.dumps(code) + + def test_code_serialization_set_shared_subobjects(self): + shared = (0,) + for _ in range(21): + shared = (shared, shared) + code = (lambda: None).__code__.replace( + co_consts=(frozenset(((shared,),)),) + ) + + payload = marshal.dumps(code) + self.assertLess(len(payload), 1000) + self.assertEqual(marshal.loads(payload).co_consts, code.co_consts) + + shared = frozenset((0,)) + for _ in range(10): + shared = frozenset(((0, shared), (1, shared))) + code = (lambda: None).__code__.replace(co_consts=(shared,)) + + payload = marshal.dumps(code) + self.assertLess(len(payload), 2000) + self.assertEqual(marshal.loads(payload).co_consts, code.co_consts) + + def test_code_serialization_recursive_constants(self): + recursive = [] + recursive_slice = slice(recursive) + recursive.append(recursive_slice) + code = (lambda: None).__code__.replace( + co_consts=(recursive_slice,) + ) + for version in range(marshal.version + 1): + with self.subTest(type="slice", version=version): + self.assertRaises(ValueError, marshal.dumps, code, version) + + recursive = [] + recursive_frozendict = frozendict({None: recursive}) + recursive.append(recursive_frozendict) + code = code.replace(co_consts=(recursive_frozendict,)) + for version in range(marshal.version + 1): + with self.subTest(type="frozendict", version=version): + self.assertRaises(ValueError, marshal.dumps, code, version) + def test_reference_loop_code(self): def f(): return 1234.5 diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-08-31-15-55-00.gh-129724.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-31-15-55-00.gh-129724.rst new file mode 100644 index 00000000000000..cad448afbac8c0 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-31-15-55-00.gh-129724.rst @@ -0,0 +1,4 @@ +Make reference tracking for marshalled code objects in free-threaded builds +independent of live reference counts. Avoid unstable sharing of immutable +constants in free-threaded compiler output. This makes free-threaded bytecode +compilation deterministic across compilation order and concurrent compilation. diff --git a/Python/compile.c b/Python/compile.c index f3852041bce69c..ee3e1ff4373769 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -339,10 +339,38 @@ _PyCompile_SetQualname(compiler *c) /* Merge const *o* and return constant key object. * If recursive, insert all elements if o is a tuple or frozen set. */ +static int +const_cache_contains_rebuilt_container(PyObject *o) +{ +#ifdef Py_GIL_DISABLED + if (PyFrozenSet_CheckExact(o)) { + return 1; + } + if (PySlice_Check(o)) { + return 1; + } + if (PyTuple_CheckExact(o)) { + for (Py_ssize_t i = 0; i < PyTuple_GET_SIZE(o); i++) { + if (const_cache_contains_rebuilt_container( + PyTuple_GET_ITEM(o, i))) + { + return 1; + } + } + } +#else + (void)o; +#endif + return 0; +} + static PyObject* -const_cache_insert(PyObject *const_cache, PyObject *o, bool recursive) +const_cache_insert(PyObject *const_cache, PyObject *local_const_cache, + PyObject *o, bool recursive) { - assert(PyDict_CheckExact(const_cache)); + assert(const_cache == NULL || PyDict_CheckExact(const_cache)); + assert(local_const_cache == NULL || + PyDict_CheckExact(local_const_cache)); // None and Ellipsis are immortal objects, and key is the singleton. // No need to merge object and key. if (o == Py_None || o == Py_Ellipsis) { @@ -354,28 +382,34 @@ const_cache_insert(PyObject *const_cache, PyObject *o, bool recursive) return NULL; } - PyObject *t; - int res = PyDict_SetDefaultRef(const_cache, key, key, &t); - if (res != 0) { - // o was not inserted into const_cache. t is either the existing value - // or NULL (on error). - Py_DECREF(key); - return t; + // Code construction can rebuild some containers after interning their + // elements. Do not let prior compilation decide whether code units share + // the original object. + PyObject *cache = const_cache_contains_rebuilt_container(o) + ? local_const_cache : const_cache; + if (cache != NULL) { + PyObject *t; + int res = PyDict_SetDefaultRef(cache, key, key, &t); + if (res != 0) { + // o was not inserted into const_cache. t is either the existing + // value or NULL (on error). + Py_DECREF(key); + return t; + } + Py_DECREF(t); } - Py_DECREF(t); if (!recursive) { return key; } - // We registered o in const_cache. - // When o is a tuple or frozenset, we want to merge its - // items too. + // When o is a tuple or frozenset, merge its items too. if (PyTuple_CheckExact(o)) { Py_ssize_t len = PyTuple_GET_SIZE(o); for (Py_ssize_t i = 0; i < len; i++) { PyObject *item = PyTuple_GET_ITEM(o, i); - PyObject *u = const_cache_insert(const_cache, item, recursive); + PyObject *u = const_cache_insert( + const_cache, local_const_cache, item, recursive); if (u == NULL) { Py_DECREF(key); return NULL; @@ -417,7 +451,8 @@ const_cache_insert(PyObject *const_cache, PyObject *o, bool recursive) PyObject *item; Py_hash_t hash; while (_PySet_NextEntry(o, &pos, &item, &hash)) { - PyObject *k = const_cache_insert(const_cache, item, recursive); + PyObject *k = const_cache_insert( + const_cache, local_const_cache, item, recursive); if (k == NULL) { Py_DECREF(tuple); Py_DECREF(key); @@ -454,7 +489,7 @@ const_cache_insert(PyObject *const_cache, PyObject *o, bool recursive) static PyObject* merge_consts_recursive(PyObject *const_cache, PyObject *o) { - return const_cache_insert(const_cache, o, true); + return const_cache_insert(const_cache, NULL, o, true); } Py_ssize_t @@ -1385,7 +1420,28 @@ _PyCompile_Metadata(compiler *c) int _PyCompile_ConstCacheMergeOne(PyObject *const_cache, PyObject **obj) { - PyObject *key = const_cache_insert(const_cache, *obj, false); + PyObject *key = const_cache_insert(const_cache, NULL, *obj, false); + if (key == NULL) { + return ERROR; + } + if (PyTuple_CheckExact(key)) { + PyObject *item = PyTuple_GET_ITEM(key, 1); + Py_SETREF(*obj, Py_NewRef(item)); + Py_DECREF(key); + } + else { + Py_SETREF(*obj, key); + } + return SUCCESS; +} + +int +_PyCompile_ConstCacheMergeOneLocal(PyObject *const_cache, + PyObject *local_const_cache, + PyObject **obj) +{ + PyObject *key = const_cache_insert( + const_cache, local_const_cache, *obj, false); if (key == NULL) { return ERROR; } @@ -1400,6 +1456,20 @@ _PyCompile_ConstCacheMergeOne(PyObject *const_cache, PyObject **obj) return SUCCESS; } +int +_PyCompile_ConstCacheAddLocal(PyObject *local_const_cache, PyObject *obj) +{ + if (!const_cache_contains_rebuilt_container(obj)) { + return SUCCESS; + } + PyObject *key = const_cache_insert(NULL, local_const_cache, obj, false); + if (key == NULL) { + return ERROR; + } + Py_DECREF(key); + return SUCCESS; +} + static PyObject * consts_dict_keys_inorder(PyObject *dict) { diff --git a/Python/flowgraph.c b/Python/flowgraph.c index a5138d1a1fa284..211c1c7e269084 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -7,6 +7,9 @@ #include "pycore_pymem.h" // _PyMem_IsPtrFreed() #include "pycore_long.h" // _PY_IS_SMALL_INT() #include "pycore_hashtable.h" // _Py_hashtable_t +#ifdef Py_GIL_DISABLED +# include "pycore_setobject.h" // _PySet_NextEntry() +#endif #include "pycore_opcode_utils.h" #include "pycore_opcode_metadata.h" // OPCODE_HAS_ARG, etc @@ -1345,9 +1348,11 @@ get_const_value(int opcode, int oparg, PyObject *co_consts) // Steals a reference to newconst. static int add_const(PyObject *newconst, PyObject *consts, PyObject *const_cache, - _Py_hashtable_t *consts_index) + PyObject *local_const_cache, _Py_hashtable_t *consts_index) { - if (_PyCompile_ConstCacheMergeOne(const_cache, &newconst) < 0) { + if (_PyCompile_ConstCacheMergeOneLocal( + const_cache, local_const_cache, &newconst) < 0) + { Py_DECREF(newconst); return -1; } @@ -1496,6 +1501,7 @@ maybe_instr_make_load_common_const(cfg_instr *instr, PyObject *newconst) static int instr_make_load_const(cfg_instr *instr, PyObject *newconst, PyObject *consts, PyObject *const_cache, + PyObject *local_const_cache, _Py_hashtable_t *consts_index) { int res = maybe_instr_make_load_smallint(instr, newconst, consts, const_cache); @@ -1514,7 +1520,8 @@ instr_make_load_const(cfg_instr *instr, PyObject *newconst, if (res > 0) { return SUCCESS; } - int oparg = add_const(newconst, consts, const_cache, consts_index); + int oparg = add_const( + newconst, consts, const_cache, local_const_cache, consts_index); RETURN_IF_ERROR(oparg); INSTR_SET_OP1(instr, LOAD_CONST, oparg); return SUCCESS; @@ -1528,7 +1535,8 @@ instr_make_load_const(cfg_instr *instr, PyObject *newconst, */ static int fold_tuple_of_constants(basicblock *bb, int i, PyObject *consts, - PyObject *const_cache, _Py_hashtable_t *consts_index) + PyObject *const_cache, PyObject *local_const_cache, + _Py_hashtable_t *consts_index) { /* Pre-conditions */ assert(PyDict_CheckExact(const_cache)); @@ -1565,7 +1573,9 @@ fold_tuple_of_constants(basicblock *bb, int i, PyObject *consts, } nop_out(const_instrs, seq_size); - return instr_make_load_const(instr, const_tuple, consts, const_cache, consts_index); + return instr_make_load_const( + instr, const_tuple, consts, const_cache, local_const_cache, + consts_index); } /* Replace: @@ -1587,9 +1597,49 @@ fold_tuple_of_constants(basicblock *bb, int i, PyObject *consts, matching BUILD_LIST/BUILD_SET start is selected from its opcode, and for sets the result is wrapped in a frozenset. */ + +#ifdef Py_GIL_DISABLED +static int +constant_contains_nan(PyObject *value) +{ + if (PyFloat_CheckExact(value)) { + return isnan(PyFloat_AS_DOUBLE(value)); + } + if (PyComplex_CheckExact(value)) { + Py_complex number = PyComplex_AsCComplex(value); + return isnan(number.real) || isnan(number.imag); + } + if (PyTuple_CheckExact(value)) { + for (Py_ssize_t i = 0; i < PyTuple_GET_SIZE(value); i++) { + if (constant_contains_nan(PyTuple_GET_ITEM(value, i))) { + return 1; + } + } + } + else if (PyFrozenSet_CheckExact(value)) { + Py_ssize_t pos = 0; + PyObject *item; + Py_hash_t hash; + while (_PySet_NextEntry(value, &pos, &item, &hash)) { + if (constant_contains_nan(item)) { + return 1; + } + } + } + else if (PySlice_Check(value)) { + PySliceObject *slice = (PySliceObject *)value; + return (constant_contains_nan(slice->start) || + constant_contains_nan(slice->stop) || + constant_contains_nan(slice->step)); + } + return 0; +} +#endif + static int fold_constant_seq_into_load_const(basicblock *bb, int i, PyObject *consts, PyObject *const_cache, + PyObject *local_const_cache, _Py_hashtable_t *consts_index) { assert(PyDict_CheckExact(const_cache)); @@ -1648,11 +1698,16 @@ fold_constant_seq_into_load_const(basicblock *bb, int i, assert(consts_found > 0); PyTuple_SET_ITEM(newconst, --consts_found, constant); } - nop_out(&instr, 1); } assert(consts_found == 0); if (build_op == BUILD_SET) { +#ifdef Py_GIL_DISABLED + if (constant_contains_nan(newconst)) { + Py_DECREF(newconst); + return SUCCESS; + } +#endif PyObject *frozen = PyFrozenSet_New(newconst); Py_DECREF(newconst); if (frozen == NULL) { @@ -1660,7 +1715,15 @@ fold_constant_seq_into_load_const(basicblock *bb, int i, } newconst = frozen; } - return instr_make_load_const(target, newconst, consts, const_cache, consts_index); + for (int newpos = newpos_start; newpos >= pos; newpos--) { + instr = &bb->b_instr[newpos]; + if (instr->i_opcode != NOP) { + nop_out(&instr, 1); + } + } + return instr_make_load_const( + target, newconst, consts, const_cache, local_const_cache, + consts_index); } if (expect_append) { @@ -1697,6 +1760,7 @@ Optimize lists and sets for: static int optimize_lists_and_sets(basicblock *bb, int i, int nextop, PyObject *consts, PyObject *const_cache, + PyObject *local_const_cache, _Py_hashtable_t *consts_index) { assert(PyDict_CheckExact(const_cache)); @@ -1739,6 +1803,12 @@ optimize_lists_and_sets(basicblock *bb, int i, int nextop, } if (instr->i_opcode == BUILD_SET) { +#ifdef Py_GIL_DISABLED + if (constant_contains_nan(const_result)) { + Py_DECREF(const_result); + return SUCCESS; + } +#endif PyObject *frozenset = PyFrozenSet_New(const_result); if (frozenset == NULL) { Py_DECREF(const_result); @@ -1747,7 +1817,8 @@ optimize_lists_and_sets(basicblock *bb, int i, int nextop, Py_SETREF(const_result, frozenset); } - int index = add_const(const_result, consts, const_cache, consts_index); + int index = add_const( + const_result, consts, const_cache, local_const_cache, consts_index); RETURN_IF_ERROR(index); nop_out(const_instrs, seq_size); @@ -1945,7 +2016,8 @@ eval_const_binop(PyObject *left, int op, PyObject *right) static int fold_const_binop(basicblock *bb, int i, PyObject *consts, - PyObject *const_cache, _Py_hashtable_t *consts_index) + PyObject *const_cache, PyObject *local_const_cache, + _Py_hashtable_t *consts_index) { #define BINOP_OPERAND_COUNT 2 assert(PyDict_CheckExact(const_cache)); @@ -1987,7 +2059,9 @@ fold_const_binop(basicblock *bb, int i, PyObject *consts, } nop_out(operands_instrs, BINOP_OPERAND_COUNT); - return instr_make_load_const(binop, newconst, consts, const_cache, consts_index); + return instr_make_load_const( + binop, newconst, consts, const_cache, local_const_cache, + consts_index); } static PyObject * @@ -2034,7 +2108,8 @@ eval_const_unaryop(PyObject *operand, int opcode, int oparg) static int fold_const_unaryop(basicblock *bb, int i, PyObject *consts, - PyObject *const_cache, _Py_hashtable_t *consts_index) + PyObject *const_cache, PyObject *local_const_cache, + _Py_hashtable_t *consts_index) { #define UNARYOP_OPERAND_COUNT 1 assert(PyDict_CheckExact(const_cache)); @@ -2071,7 +2146,9 @@ fold_const_unaryop(basicblock *bb, int i, PyObject *consts, assert(PyBool_Check(newconst)); } nop_out(&operand_instr, UNARYOP_OPERAND_COUNT); - return instr_make_load_const(unaryop, newconst, consts, const_cache, consts_index); + return instr_make_load_const( + unaryop, newconst, consts, const_cache, local_const_cache, + consts_index); } #define VISITED (-1) @@ -2266,8 +2343,10 @@ apply_static_swaps(basicblock *block, int i) } static int -basicblock_optimize_load_const(PyObject *const_cache, basicblock *bb, - PyObject *consts, _Py_hashtable_t *consts_index) +basicblock_optimize_load_const(PyObject *const_cache, + PyObject *local_const_cache, basicblock *bb, + PyObject *consts, + _Py_hashtable_t *consts_index) { assert(PyDict_CheckExact(const_cache)); assert(PyList_CheckExact(consts)); @@ -2385,7 +2464,9 @@ basicblock_optimize_load_const(PyObject *const_cache, basicblock *bb, return ERROR; } cnt = PyBool_FromLong(is_true); - int index = add_const(cnt, consts, const_cache, consts_index); + int index = add_const( + cnt, consts, const_cache, local_const_cache, + consts_index); if (index < 0) { return ERROR; } @@ -2410,16 +2491,20 @@ basicblock_optimize_load_const(PyObject *const_cache, basicblock *bb, } static int -optimize_load_const(PyObject *const_cache, cfg_builder *g, PyObject *consts, - _Py_hashtable_t *consts_index) { +optimize_load_const(PyObject *const_cache, PyObject *local_const_cache, + cfg_builder *g, PyObject *consts, + _Py_hashtable_t *consts_index) +{ for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { - RETURN_IF_ERROR(basicblock_optimize_load_const(const_cache, b, consts, consts_index)); + RETURN_IF_ERROR(basicblock_optimize_load_const( + const_cache, local_const_cache, b, consts, consts_index)); } return SUCCESS; } static int -optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts, +optimize_basic_block(PyObject *const_cache, PyObject *local_const_cache, + basicblock *bb, PyObject *consts, _Py_hashtable_t *consts_index) { assert(PyDict_CheckExact(const_cache)); @@ -2460,11 +2545,15 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts, continue; } } - RETURN_IF_ERROR(fold_tuple_of_constants(bb, i, consts, const_cache, consts_index)); + RETURN_IF_ERROR(fold_tuple_of_constants( + bb, i, consts, const_cache, local_const_cache, + consts_index)); break; case BUILD_LIST: case BUILD_SET: - RETURN_IF_ERROR(optimize_lists_and_sets(bb, i, nextop, consts, const_cache, consts_index)); + RETURN_IF_ERROR(optimize_lists_and_sets( + bb, i, nextop, consts, const_cache, local_const_cache, + consts_index)); break; case POP_JUMP_IF_NOT_NONE: case POP_JUMP_IF_NONE: @@ -2599,28 +2688,37 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts, _Py_FALLTHROUGH; case UNARY_INVERT: case UNARY_NEGATIVE: - RETURN_IF_ERROR(fold_const_unaryop(bb, i, consts, const_cache, consts_index)); + RETURN_IF_ERROR(fold_const_unaryop( + bb, i, consts, const_cache, local_const_cache, + consts_index)); break; case CALL_INTRINSIC_1: if (oparg == INTRINSIC_LIST_TO_TUPLE) { - RETURN_IF_ERROR(fold_constant_seq_into_load_const(bb, i, consts, const_cache, consts_index)); + RETURN_IF_ERROR(fold_constant_seq_into_load_const( + bb, i, consts, const_cache, local_const_cache, + consts_index)); if (inst->i_opcode == CALL_INTRINSIC_1 && nextop == GET_ITER) { INSTR_SET_OP0(inst, NOP); } } else if (oparg == INTRINSIC_UNARY_POSITIVE) { - RETURN_IF_ERROR(fold_const_unaryop(bb, i, consts, const_cache, consts_index)); + RETURN_IF_ERROR(fold_const_unaryop( + bb, i, consts, const_cache, local_const_cache, + consts_index)); } break; case LIST_APPEND: case SET_ADD: if (oparg == 1 && (nextop == GET_ITER || nextop == CONTAINS_OP)) { RETURN_IF_ERROR(fold_constant_seq_into_load_const( - bb, i, consts, const_cache, consts_index)); + bb, i, consts, const_cache, local_const_cache, + consts_index)); } break; case BINARY_OP: - RETURN_IF_ERROR(fold_const_binop(bb, i, consts, const_cache, consts_index)); + RETURN_IF_ERROR(fold_const_binop( + bb, i, consts, const_cache, local_const_cache, + consts_index)); break; } } @@ -2666,6 +2764,7 @@ remove_redundant_nops_and_jumps(cfg_builder *g) */ static int optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache, + PyObject *local_const_cache, _Py_hashtable_t *consts_index, int firstlineno) { assert(PyDict_CheckExact(const_cache)); @@ -2673,9 +2772,11 @@ optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache, RETURN_IF_ERROR(inline_small_or_no_lineno_blocks(g->g_entryblock)); RETURN_IF_ERROR(remove_unreachable(g->g_entryblock)); RETURN_IF_ERROR(resolve_line_numbers(g, firstlineno)); - RETURN_IF_ERROR(optimize_load_const(const_cache, g, consts, consts_index)); + RETURN_IF_ERROR(optimize_load_const( + const_cache, local_const_cache, g, consts, consts_index)); for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { - RETURN_IF_ERROR(optimize_basic_block(const_cache, b, consts, consts_index)); + RETURN_IF_ERROR(optimize_basic_block( + const_cache, local_const_cache, b, consts, consts_index)); } RETURN_IF_ERROR(remove_redundant_nops_and_pairs(g->g_entryblock)); RETURN_IF_ERROR(remove_unreachable(g->g_entryblock)); @@ -3799,9 +3900,22 @@ _PyCfg_OptimizeCodeUnit(cfg_builder *g, PyObject *consts, PyObject *const_cache, /** Optimization **/ + PyObject *local_const_cache = PyDict_New(); + if (local_const_cache == NULL) { + return ERROR; + } + for (Py_ssize_t i = 0; i < PyList_GET_SIZE(consts); i++) { + PyObject *item = PyList_GET_ITEM(consts, i); + if (_PyCompile_ConstCacheAddLocal(local_const_cache, item) < 0) { + Py_DECREF(local_const_cache); + return ERROR; + } + } + _Py_hashtable_t *consts_index = _Py_hashtable_new( _Py_hashtable_hash_ptr, _Py_hashtable_compare_direct); if (consts_index == NULL) { + Py_DECREF(local_const_cache); PyErr_NoMemory(); return ERROR; } @@ -3814,14 +3928,17 @@ _PyCfg_OptimizeCodeUnit(cfg_builder *g, PyObject *consts, PyObject *const_cache, if (_Py_hashtable_set(consts_index, (void *)item, (void *)(uintptr_t)i) < 0) { _Py_hashtable_destroy(consts_index); + Py_DECREF(local_const_cache); PyErr_NoMemory(); return ERROR; } } - int ret = optimize_cfg(g, consts, const_cache, consts_index, firstlineno); + int ret = optimize_cfg( + g, consts, const_cache, local_const_cache, consts_index, firstlineno); _Py_hashtable_destroy(consts_index); + Py_DECREF(local_const_cache); RETURN_IF_ERROR(ret); diff --git a/Python/marshal.c b/Python/marshal.c index ef5a8d3840cd80..b4021c41690ca2 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -106,6 +106,7 @@ module marshal #define WFERR_NESTEDTOODEEP 2 #define WFERR_NOMEMORY 3 #define WFERR_CODE_NOT_ALLOWED 4 +#define WFERR_DETERMINISTIC_SET 5 typedef struct { FILE *fp; @@ -118,8 +119,23 @@ typedef struct { _Py_hashtable_t *hashtable; int version; int allow_code; + enum { + WF_REF_DEFAULT, + WF_REF_DETERMINISTIC, + } ref_mode; } WFILE; +static int +w_ref_mode_for_object(PyObject *x) +{ +#ifdef Py_GIL_DISABLED + if (PyCode_Check(x)) { + return WF_REF_DETERMINISTIC; + } +#endif + return WF_REF_DEFAULT; +} + #define w_byte(c, p) do { \ if ((p)->ptr != (p)->end || w_reserve((p), 1)) \ *(p)->ptr++ = (c); \ @@ -251,7 +267,8 @@ w_short_pstring(const void *s, Py_ssize_t n, WFILE *p) } while(0) static PyObject * -_PyMarshal_WriteObjectToString(PyObject *x, int version, int allow_code); +w_object_to_string(PyObject *x, int version, int allow_code, + int ref_mode, int *error); #define _r_digits(bitsize) \ static void \ @@ -386,12 +403,9 @@ w_ref(PyObject *v, char *flag, WFILE *p) if (p->version < 3 || p->hashtable == NULL) return 0; /* not writing object references */ - /* If it has only one reference, it definitely isn't shared. - * But we use TYPE_REF always for interned string, to PYC file stable - * as possible. - */ - if (_PyObject_IsUniquelyReferenced(v) && - !(PyUnicode_CheckExact(v) && PyUnicode_CHECK_INTERNED(v))) { + if (p->ref_mode == WF_REF_DEFAULT && + _PyObject_IsUniquelyReferenced(v) && + !(PyUnicode_CheckExact(v) && PyUnicode_CHECK_INTERNED(v))) { return 0; } @@ -441,7 +455,8 @@ w_complete(PyObject *v, WFILE *p) if (p->version < 3 || p->hashtable == NULL) { return; } - if (_PyObject_IsUniquelyReferenced(v)) { + if (p->ref_mode == WF_REF_DEFAULT && + _PyObject_IsUniquelyReferenced(v)) { return; } @@ -657,22 +672,46 @@ w_complex_object(PyObject *v, char flag, WFILE *p) Py_ssize_t i = 0; Py_BEGIN_CRITICAL_SECTION(v); while (_PySet_NextEntryRef(v, &pos, &value, &hash)) { - PyObject *dump = _PyMarshal_WriteObjectToString(value, - p->version, p->allow_code); + int error = WFERR_UNMARSHALLABLE; + PyObject *dump = w_object_to_string( + value, p->version, p->allow_code, p->ref_mode, &error); if (dump == NULL) { - p->error = WFERR_UNMARSHALLABLE; + p->error = error; Py_DECREF(value); break; } - PyObject *pair = _PyTuple_FromPairSteal(dump, value); - if (pair == NULL) { - p->error = WFERR_NOMEMORY; - break; + PyObject *pair; + if (p->ref_mode == WF_REF_DETERMINISTIC) { + PyObject *ordinal = PyLong_FromSsize_t(i); + if (ordinal == NULL) { + p->error = WFERR_NOMEMORY; + Py_DECREF(dump); + Py_DECREF(value); + break; + } + pair = PyTuple_New(3); + if (pair == NULL) { + p->error = WFERR_NOMEMORY; + Py_DECREF(dump); + Py_DECREF(ordinal); + Py_DECREF(value); + break; + } + PyTuple_SET_ITEM(pair, 0, dump); + PyTuple_SET_ITEM(pair, 1, ordinal); + PyTuple_SET_ITEM(pair, 2, value); + } + else { + pair = _PyTuple_FromPairSteal(dump, value); + if (pair == NULL) { + p->error = WFERR_NOMEMORY; + break; + } } PyList_SET_ITEM(pairs, i++, pair); } Py_END_CRITICAL_SECTION(); - if (p->error == WFERR_UNMARSHALLABLE || p->error == WFERR_NOMEMORY) { + if (p->error != WFERR_OK) { Py_DECREF(pairs); return; } @@ -682,9 +721,31 @@ w_complex_object(PyObject *v, char flag, WFILE *p) Py_DECREF(pairs); return; } + if (p->ref_mode == WF_REF_DETERMINISTIC) { + for (Py_ssize_t i = 1; i < n; i++) { + PyObject *previous = PyTuple_GET_ITEM( + PyList_GET_ITEM(pairs, i - 1), 0); + PyObject *current = PyTuple_GET_ITEM( + PyList_GET_ITEM(pairs, i), 0); + if (PyBytes_GET_SIZE(previous) == PyBytes_GET_SIZE(current) && + memcmp(PyBytes_AS_STRING(previous), + PyBytes_AS_STRING(current), + PyBytes_GET_SIZE(previous)) == 0) + { + PyErr_SetString( + PyExc_ValueError, + "cannot deterministically marshal set elements " + "with identical encodings"); + p->error = WFERR_DETERMINISTIC_SET; + Py_DECREF(pairs); + return; + } + } + } + int value_index = p->ref_mode == WF_REF_DETERMINISTIC ? 2 : 1; for (Py_ssize_t i = 0; i < n; i++) { PyObject *pair = PyList_GET_ITEM(pairs, i); - value = PyTuple_GET_ITEM(pair, 1); + value = PyTuple_GET_ITEM(pair, value_index); w_object(value, p); } Py_DECREF(pairs); @@ -817,6 +878,7 @@ PyMarshal_WriteObjectToFile(PyObject *x, FILE *fp, int version) wf.error = WFERR_OK; wf.version = version; wf.allow_code = 1; + wf.ref_mode = w_ref_mode_for_object(x); if (w_init_refs(&wf, version)) { return; /* caller must check PyErr_Occurred() */ } @@ -1909,34 +1971,56 @@ PyMarshal_ReadObjectFromString(const char *str, Py_ssize_t len) } static PyObject * -_PyMarshal_WriteObjectToString(PyObject *x, int version, int allow_code) +w_object_to_string(PyObject *x, int version, int allow_code, + int ref_mode, int *error) { WFILE wf; + if (error != NULL) { + *error = WFERR_UNMARSHALLABLE; + } if (PySys_Audit("marshal.dumps", "Oi", x, version) < 0) { return NULL; } memset(&wf, 0, sizeof(wf)); wf.str = PyBytes_FromStringAndSize((char *)NULL, 50); - if (wf.str == NULL) + if (wf.str == NULL) { + if (error != NULL) { + *error = WFERR_NOMEMORY; + } return NULL; + } wf.ptr = wf.buf = PyBytes_AS_STRING(wf.str); wf.end = wf.ptr + PyBytes_GET_SIZE(wf.str); wf.error = WFERR_OK; wf.version = version; wf.allow_code = allow_code; + wf.ref_mode = ref_mode; if (w_init_refs(&wf, version)) { Py_DECREF(wf.str); + if (error != NULL) { + *error = WFERR_NOMEMORY; + } return NULL; } w_object(x, &wf); w_clear_refs(&wf); - if (wf.str != NULL) { + if (wf.str == NULL) { + wf.error = WFERR_NOMEMORY; + } + else { const char *base = PyBytes_AS_STRING(wf.str); - if (_PyBytes_Resize(&wf.str, (Py_ssize_t)(wf.ptr - base)) < 0) + if (_PyBytes_Resize(&wf.str, (Py_ssize_t)(wf.ptr - base)) < 0) { + if (error != NULL) { + *error = WFERR_NOMEMORY; + } return NULL; + } } if (wf.error != WFERR_OK) { + if (error != NULL) { + *error = wf.error; + } Py_XDECREF(wf.str); switch (wf.error) { case WFERR_NOMEMORY: @@ -1950,6 +2034,12 @@ _PyMarshal_WriteObjectToString(PyObject *x, int version, int allow_code) PyErr_SetString(PyExc_ValueError, "marshalling code objects is disallowed"); break; + case WFERR_DETERMINISTIC_SET: + PyErr_SetString( + PyExc_ValueError, + "cannot deterministically marshal set elements " + "with identical encodings"); + break; default: case WFERR_UNMARSHALLABLE: PyErr_SetString(PyExc_ValueError, @@ -1961,6 +2051,13 @@ _PyMarshal_WriteObjectToString(PyObject *x, int version, int allow_code) return wf.str; } +static PyObject * +_PyMarshal_WriteObjectToString(PyObject *x, int version, int allow_code) +{ + int ref_mode = w_ref_mode_for_object(x); + return w_object_to_string(x, version, allow_code, ref_mode, NULL); +} + PyObject * PyMarshal_WriteObjectToString(PyObject *x, int version) {