From 7ecd3425d45a37efbc745788dfe21df0286c785a Mon Sep 17 00:00:00 2001 From: Erlend Egeberg Aasland Date: Wed, 25 Aug 2021 12:59:42 +0200 Subject: [PATCH 1/7] bpo-27334: roll back transaction if sqlite3 context manager fails to commit (GH-26202) Co-authored-by: Luca Citi Co-authored-by: Berker Peksag --- Lib/sqlite3/test/dbapi.py | 80 ++++++++++++++++++- .../2021-05-18-00-17-21.bpo-27334.32EJZi.rst | 2 + Modules/_sqlite/connection.c | 28 +++++-- 3 files changed, 102 insertions(+), 8 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2021-05-18-00-17-21.bpo-27334.32EJZi.rst diff --git a/Lib/sqlite3/test/dbapi.py b/Lib/sqlite3/test/dbapi.py index 5d7e5bba05bc45..bb9d5a7ce3e001 100644 --- a/Lib/sqlite3/test/dbapi.py +++ b/Lib/sqlite3/test/dbapi.py @@ -22,11 +22,17 @@ import contextlib import sqlite3 as sqlite +import subprocess import sys import threading import unittest -from test.support import check_disallow_instantiation, threading_helper, bigmemtest +from test.support import ( + bigmemtest, + check_disallow_instantiation, + threading_helper, + SHORT_TIMEOUT, +) from test.support.os_helper import TESTFN, unlink @@ -986,6 +992,77 @@ def test_on_conflict_replace(self): self.assertEqual(self.cu.fetchall(), [('Very different data!', 'foo')]) +class MultiprocessTests(unittest.TestCase): + CONNECTION_TIMEOUT = SHORT_TIMEOUT / 1000. # Defaults to 30 ms + + def tearDown(self): + unlink(TESTFN) + + def test_ctx_mgr_rollback_if_commit_failed(self): + # bpo-27334: ctx manager does not rollback if commit fails + SCRIPT = f"""if 1: + import sqlite3 + def wait(): + print("started") + assert "database is locked" in input() + + cx = sqlite3.connect("{TESTFN}", timeout={self.CONNECTION_TIMEOUT}) + cx.create_function("wait", 0, wait) + with cx: + cx.execute("create table t(t)") + try: + # execute two transactions; both will try to lock the db + cx.executescript(''' + -- start a transaction and wait for parent + begin transaction; + select * from t; + select wait(); + rollback; + + -- start a new transaction; would fail if parent holds lock + begin transaction; + select * from t; + rollback; + ''') + finally: + cx.close() + """ + + # spawn child process + proc = subprocess.Popen( + [sys.executable, "-c", SCRIPT], + encoding="utf-8", + bufsize=0, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + ) + self.addCleanup(proc.communicate) + + # wait for child process to start + self.assertEqual("started", proc.stdout.readline().strip()) + + cx = sqlite.connect(TESTFN, timeout=self.CONNECTION_TIMEOUT) + try: # context manager should correctly release the db lock + with cx: + cx.execute("insert into t values('test')") + except sqlite.OperationalError as exc: + proc.stdin.write(str(exc)) + else: + proc.stdin.write("no error") + finally: + cx.close() + + # terminate child process + self.assertIsNone(proc.returncode) + try: + proc.communicate(input="end", timeout=SHORT_TIMEOUT) + except subprocess.TimeoutExpired: + proc.kill() + proc.communicate() + raise + self.assertEqual(proc.returncode, 0) + + def suite(): tests = [ ClosedConTests, @@ -995,6 +1072,7 @@ def suite(): CursorTests, ExtensionTests, ModuleTests, + MultiprocessTests, OpenTests, SqliteOnConflictTests, ThreadTests, diff --git a/Misc/NEWS.d/next/Library/2021-05-18-00-17-21.bpo-27334.32EJZi.rst b/Misc/NEWS.d/next/Library/2021-05-18-00-17-21.bpo-27334.32EJZi.rst new file mode 100644 index 00000000000000..dc0cdf33ec5acf --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-05-18-00-17-21.bpo-27334.32EJZi.rst @@ -0,0 +1,2 @@ +The :mod:`sqlite3` context manager now performs a rollback (thus releasing the +database lock) if commit failed. Patch by Luca Citi and Erlend E. Aasland. diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 8ad5f5f061da5d..1bc045523a252e 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -1785,17 +1785,31 @@ pysqlite_connection_exit_impl(pysqlite_Connection *self, PyObject *exc_type, PyObject *exc_value, PyObject *exc_tb) /*[clinic end generated code: output=0705200e9321202a input=bd66f1532c9c54a7]*/ { - const char* method_name; + int commit = 0; PyObject* result; if (exc_type == Py_None && exc_value == Py_None && exc_tb == Py_None) { - method_name = "commit"; - } else { - method_name = "rollback"; + commit = 1; + result = pysqlite_connection_commit_impl(self); } - - result = PyObject_CallMethod((PyObject*)self, method_name, NULL); - if (!result) { + else { + result = pysqlite_connection_rollback_impl(self); + } + + if (result == NULL) { + if (commit) { + /* Commit failed; try to rollback in order to unlock the database. + * If rollback also fails, chain the exceptions. */ + PyObject *exc, *val, *tb; + PyErr_Fetch(&exc, &val, &tb); + result = pysqlite_connection_rollback_impl(self); + if (result == NULL) { + _PyErr_ChainExceptions(exc, val, tb); + } + else { + PyErr_Restore(exc, val, tb); + } + } return NULL; } Py_DECREF(result); From 33d95c6facdfda3c8c0feffa7a99184e4abc2f63 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 25 Aug 2021 04:14:34 -0700 Subject: [PATCH 2/7] bpo-37596: Make `set` and `frozenset` marshalling deterministic (GH-27926) --- Lib/test/test_marshal.py | 25 +++++++++++++++ .../2021-08-23-21-39-59.bpo-37596.ojRcwB.rst | 2 ++ Python/marshal.c | 32 +++++++++++++++++++ 3 files changed, 59 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2021-08-23-21-39-59.bpo-37596.ojRcwB.rst diff --git a/Lib/test/test_marshal.py b/Lib/test/test_marshal.py index d20b9d2c1ff391..bdfe79fbbecb35 100644 --- a/Lib/test/test_marshal.py +++ b/Lib/test/test_marshal.py @@ -344,6 +344,31 @@ def test_eof(self): for i in range(len(data)): self.assertRaises(EOFError, marshal.loads, data[0: i]) + def test_deterministic_sets(self): + # bpo-37596: To support reproducible builds, sets and frozensets need to + # have their elements serialized in a consistent order (even when they + # have been scrambled by hash randomization): + for kind in ("set", "frozenset"): + for elements in ( + "float('nan'), b'a', b'b', b'c', 'x', 'y', 'z'", + # Also test for bad interactions with backreferencing: + "('string', 1), ('string', 2), ('string', 3)", + ): + s = f"{kind}([{elements}])" + with self.subTest(s): + # First, make sure that our test case still has different + # orders under hash seeds 0 and 1. If this check fails, we + # need to update this test with different elements: + args = ["-c", f"print({s})"] + _, repr_0, _ = assert_python_ok(*args, PYTHONHASHSEED="0") + _, repr_1, _ = assert_python_ok(*args, PYTHONHASHSEED="1") + self.assertNotEqual(repr_0, repr_1) + # Then, perform the actual test: + args = ["-c", f"import marshal; print(marshal.dumps({s}))"] + _, dump_0, _ = assert_python_ok(*args, PYTHONHASHSEED="0") + _, dump_1, _ = assert_python_ok(*args, PYTHONHASHSEED="1") + self.assertEqual(dump_0, dump_1) + LARGE_SIZE = 2**31 pointer_size = 8 if sys.maxsize > 0xFFFFFFFF else 4 diff --git a/Misc/NEWS.d/next/Library/2021-08-23-21-39-59.bpo-37596.ojRcwB.rst b/Misc/NEWS.d/next/Library/2021-08-23-21-39-59.bpo-37596.ojRcwB.rst new file mode 100644 index 00000000000000..81fdfeb6294569 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-08-23-21-39-59.bpo-37596.ojRcwB.rst @@ -0,0 +1,2 @@ +Ensure that :class:`set` and :class:`frozenset` objects are always +:mod:`marshalled ` reproducibly. diff --git a/Python/marshal.c b/Python/marshal.c index 1260704c74c0be..b69c4d09641da8 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -503,9 +503,41 @@ w_complex_object(PyObject *v, char flag, WFILE *p) W_TYPE(TYPE_SET, p); n = PySet_GET_SIZE(v); W_SIZE(n, p); + // bpo-37596: To support reproducible builds, sets and frozensets need + // to have their elements serialized in a consistent order (even when + // they have been scrambled by hash randomization). To ensure this, we + // use an order equivalent to sorted(v, key=marshal.dumps): + PyObject *pairs = PyList_New(0); + if (pairs == NULL) { + p->error = WFERR_NOMEMORY; + return; + } while (_PySet_NextEntry(v, &pos, &value, &hash)) { + PyObject *dump = PyMarshal_WriteObjectToString(value, p->version); + if (dump == NULL) { + p->error = WFERR_UNMARSHALLABLE; + goto anyset_done; + } + PyObject *pair = PyTuple_Pack(2, dump, value); + Py_DECREF(dump); + if (pair == NULL || PyList_Append(pairs, pair)) { + p->error = WFERR_NOMEMORY; + Py_XDECREF(pair); + goto anyset_done; + } + Py_DECREF(pair); + } + if (PyList_Sort(pairs)) { + p->error = WFERR_NOMEMORY; + goto anyset_done; + } + for (Py_ssize_t i = 0; i < n; i++) { + PyObject *pair = PyList_GET_ITEM(pairs, i); + value = PyTuple_GET_ITEM(pair, 1); w_object(value, p); } + anyset_done: + Py_DECREF(pairs); } else if (PyCode_Check(v)) { PyCodeObject *co = (PyCodeObject *)v; From 214c2e5d916d3ce5e7b1db800210b93001850bbb Mon Sep 17 00:00:00 2001 From: Pablo Galindo Salgado Date: Wed, 25 Aug 2021 13:41:14 +0100 Subject: [PATCH 3/7] Format the Python-tokenize module and fix exit path (GH-27935) --- Python/Python-tokenize.c | 93 ++++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 47 deletions(-) diff --git a/Python/Python-tokenize.c b/Python/Python-tokenize.c index b9fb1693ce117e..2933b5b7b1e202 100644 --- a/Python/Python-tokenize.c +++ b/Python/Python-tokenize.c @@ -4,16 +4,15 @@ static struct PyModuleDef _tokenizemodule; typedef struct { - PyTypeObject* TokenizerIter; + PyTypeObject *TokenizerIter; } tokenize_state; -static tokenize_state* -get_tokenize_state(PyObject* module) -{ - return (tokenize_state*)PyModule_GetState(module); +static tokenize_state * +get_tokenize_state(PyObject *module) { + return (tokenize_state *)PyModule_GetState(module); } -#define _tokenize_get_state_by_type(type) \ +#define _tokenize_get_state_by_type(type) \ get_tokenize_state(_PyType_GetModuleByDef(type, &_tokenizemodule)) #include "clinic/Python-tokenize.c.h" @@ -24,9 +23,9 @@ class _tokenizer.tokenizeriter "tokenizeriterobject *" "_tokenize_get_state_by_t [clinic start generated code]*/ /*[clinic end generated code: output=da39a3ee5e6b4b0d input=96d98ee2fef7a8bc]*/ -typedef struct { - PyObject_HEAD - struct tok_state* tok; +typedef struct +{ + PyObject_HEAD struct tok_state *tok; } tokenizeriterobject; /*[clinic input] @@ -40,27 +39,28 @@ static PyObject * tokenizeriter_new_impl(PyTypeObject *type, const char *source) /*[clinic end generated code: output=7fd9f46cf9263cbb input=4384b368407375c6]*/ { - tokenizeriterobject* self = (tokenizeriterobject*)type->tp_alloc(type, 0); + tokenizeriterobject *self = (tokenizeriterobject *)type->tp_alloc(type, 0); if (self == NULL) { return NULL; } - PyObject* filename = PyUnicode_FromString(""); + PyObject *filename = PyUnicode_FromString(""); if (filename == NULL) { return NULL; } self->tok = PyTokenizer_FromUTF8(source, 1); if (self->tok == NULL) { + Py_DECREF(filename); return NULL; } self->tok->filename = filename; - return (PyObject*)self; + return (PyObject *)self; } -static PyObject* -tokenizeriter_next(tokenizeriterobject* it) +static PyObject * +tokenizeriter_next(tokenizeriterobject *it) { - const char* start; - const char* end; + const char *start; + const char *end; int type = PyTokenizer_Get(it->tok, &start, &end); if (type == ERRORTOKEN && PyErr_Occurred()) { return NULL; @@ -69,10 +69,11 @@ tokenizeriter_next(tokenizeriterobject* it) PyErr_SetString(PyExc_StopIteration, "EOF"); return NULL; } - PyObject* str = NULL; + PyObject *str = NULL; if (start == NULL || end == NULL) { str = PyUnicode_FromString(""); - } else { + } + else { str = PyUnicode_FromStringAndSize(start, end - start); } if (str == NULL) { @@ -80,12 +81,12 @@ tokenizeriter_next(tokenizeriterobject* it) } Py_ssize_t size = it->tok->inp - it->tok->buf; - PyObject* line = PyUnicode_DecodeUTF8(it->tok->buf, size, "replace"); + PyObject *line = PyUnicode_DecodeUTF8(it->tok->buf, size, "replace"); if (line == NULL) { Py_DECREF(str); return NULL; } - const char* line_start = type == STRING ? it->tok->multi_line_start : it->tok->line_start; + const char *line_start = type == STRING ? it->tok->multi_line_start : it->tok->line_start; int lineno = type == STRING ? it->tok->first_lineno : it->tok->lineno; int end_lineno = it->tok->lineno; int col_offset = -1; @@ -101,41 +102,39 @@ tokenizeriter_next(tokenizeriterobject* it) } static void -tokenizeriter_dealloc(tokenizeriterobject* it) +tokenizeriter_dealloc(tokenizeriterobject *it) { - PyTypeObject* tp = Py_TYPE(it); + PyTypeObject *tp = Py_TYPE(it); PyTokenizer_Free(it->tok); tp->tp_free(it); Py_DECREF(tp); } static PyType_Slot tokenizeriter_slots[] = { - {Py_tp_new, tokenizeriter_new}, - {Py_tp_dealloc, tokenizeriter_dealloc}, - {Py_tp_getattro, PyObject_GenericGetAttr}, - {Py_tp_iter, PyObject_SelfIter}, - {Py_tp_iternext, tokenizeriter_next}, - {0, NULL}, + {Py_tp_new, tokenizeriter_new}, + {Py_tp_dealloc, tokenizeriter_dealloc}, + {Py_tp_getattro, PyObject_GenericGetAttr}, + {Py_tp_iter, PyObject_SelfIter}, + {Py_tp_iternext, tokenizeriter_next}, + {0, NULL}, }; static PyType_Spec tokenizeriter_spec = { - .name = "_tokenize.TokenizerIter", - .basicsize = sizeof(tokenizeriterobject), - .flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_IMMUTABLETYPE), - .slots = tokenizeriter_slots, + .name = "_tokenize.TokenizerIter", + .basicsize = sizeof(tokenizeriterobject), + .flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_IMMUTABLETYPE), + .slots = tokenizeriter_slots, }; - static int -tokenizemodule_exec(PyObject* m) +tokenizemodule_exec(PyObject *m) { - tokenize_state* state = get_tokenize_state(m); + tokenize_state *state = get_tokenize_state(m); if (state == NULL) { return -1; } - state->TokenizerIter = (PyTypeObject *)PyType_FromModuleAndSpec( - m, &tokenizeriter_spec, NULL); + state->TokenizerIter = (PyTypeObject *)PyType_FromModuleAndSpec(m, &tokenizeriter_spec, NULL); if (state->TokenizerIter == NULL) { return -1; } @@ -147,11 +146,11 @@ tokenizemodule_exec(PyObject* m) } static PyMethodDef tokenize_methods[] = { - {NULL, NULL, 0, NULL} /* Sentinel */ + {NULL, NULL, 0, NULL} /* Sentinel */ }; static PyModuleDef_Slot tokenizemodule_slots[] = { - {Py_mod_exec, tokenizemodule_exec}, + {Py_mod_exec, tokenizemodule_exec}, {0, NULL} }; @@ -178,14 +177,14 @@ tokenizemodule_free(void *m) } static struct PyModuleDef _tokenizemodule = { - PyModuleDef_HEAD_INIT, - .m_name = "_tokenize", - .m_size = sizeof(tokenize_state), - .m_slots = tokenizemodule_slots, - .m_methods = tokenize_methods, - .m_traverse = tokenizemodule_traverse, - .m_clear = tokenizemodule_clear, - .m_free = tokenizemodule_free, + PyModuleDef_HEAD_INIT, + .m_name = "_tokenize", + .m_size = sizeof(tokenize_state), + .m_slots = tokenizemodule_slots, + .m_methods = tokenize_methods, + .m_traverse = tokenizemodule_traverse, + .m_clear = tokenizemodule_clear, + .m_free = tokenizemodule_free, }; PyMODINIT_FUNC From f9242d50b18572ef0d584a1c815ed08d1a38e4f4 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 25 Aug 2021 13:44:20 +0100 Subject: [PATCH 4/7] bpo-44990: Change layout of evaluation frames. "Layout B" (GH-27933) Places the locals between the specials and stack. This is the more "natural" layout for a C struct, makes the code simpler and gives a slight speedup (~1%) --- Include/internal/pycore_frame.h | 44 ++++++++++++++++--- Objects/frameobject.c | 24 +++-------- Objects/genobject.c | 10 ++--- Objects/typeobject.c | 2 +- Python/ceval.c | 76 +++++++++++++++++---------------- Python/frame.c | 32 ++++++-------- Python/pystate.c | 20 ++++----- Tools/gdb/libpython.py | 6 ++- 8 files changed, 114 insertions(+), 100 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index cd465d73bc6bdb..6afb95c3ad62e0 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -29,10 +29,9 @@ typedef struct _interpreter_frame { PyObject *generator; struct _interpreter_frame *previous; int f_lasti; /* Last instruction if called */ - int stackdepth; /* Depth of value stack */ - int nlocalsplus; - PyFrameState f_state; /* What state the frame is in */ - PyObject *stack[1]; + int stacktop; /* Offset of TOS from localsplus */ + PyFrameState f_state; /* What state the frame is in */ + PyObject *localsplus[1]; } InterpreterFrame; static inline int _PyFrame_IsRunnable(InterpreterFrame *f) { @@ -47,6 +46,26 @@ static inline int _PyFrameHasCompleted(InterpreterFrame *f) { return f->f_state > FRAME_EXECUTING; } +static inline PyObject **_PyFrame_Stackbase(InterpreterFrame *f) { + return f->localsplus + f->f_code->co_nlocalsplus; +} + +static inline PyObject *_PyFrame_StackPeek(InterpreterFrame *f) { + assert(f->stacktop > f->f_code->co_nlocalsplus); + return f->localsplus[f->stacktop-1]; +} + +static inline PyObject *_PyFrame_StackPop(InterpreterFrame *f) { + assert(f->stacktop > f->f_code->co_nlocalsplus); + f->stacktop--; + return f->localsplus[f->stacktop]; +} + +static inline void _PyFrame_StackPush(InterpreterFrame *f, PyObject *value) { + f->localsplus[f->stacktop] = value; + f->stacktop++; +} + #define FRAME_SPECIALS_SIZE ((sizeof(InterpreterFrame)-1)/sizeof(PyObject *)) InterpreterFrame * @@ -61,8 +80,7 @@ _PyFrame_InitializeSpecials( frame->f_builtins = Py_NewRef(con->fc_builtins); frame->f_globals = Py_NewRef(con->fc_globals); frame->f_locals = Py_XNewRef(locals); - frame->nlocalsplus = nlocalsplus; - frame->stackdepth = 0; + frame->stacktop = nlocalsplus; frame->frame_obj = NULL; frame->generator = NULL; frame->f_lasti = -1; @@ -75,7 +93,19 @@ _PyFrame_InitializeSpecials( static inline PyObject** _PyFrame_GetLocalsArray(InterpreterFrame *frame) { - return ((PyObject **)frame) - frame->nlocalsplus; + return frame->localsplus; +} + +static inline PyObject** +_PyFrame_GetStackPointer(InterpreterFrame *frame) +{ + return frame->localsplus+frame->stacktop; +} + +static inline void +_PyFrame_SetStackPointer(InterpreterFrame *frame, PyObject **stack_pointer) +{ + frame->stacktop = (int)(stack_pointer - frame->localsplus); } /* For use by _PyFrame_GetFrameObject diff --git a/Objects/frameobject.c b/Objects/frameobject.c index 2af69597c396ee..00d6888ff2a2ac 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -397,9 +397,7 @@ first_line_not_before(int *lines, int len, int line) static void frame_stack_pop(PyFrameObject *f) { - assert(f->f_frame->stackdepth > 0); - f->f_frame->stackdepth--; - PyObject *v = f->f_frame->stack[f->f_frame->stackdepth]; + PyObject *v = _PyFrame_StackPop(f->f_frame); Py_DECREF(v); } @@ -633,14 +631,10 @@ frame_dealloc(PyFrameObject *f) Py_CLEAR(frame->f_builtins); Py_CLEAR(frame->f_locals); PyObject **locals = _PyFrame_GetLocalsArray(frame); - for (int i = 0; i < co->co_nlocalsplus; i++) { + for (int i = 0; i < frame->stacktop; i++) { Py_CLEAR(locals[i]); } - /* stack */ - for (int i = 0; i < frame->stackdepth; i++) { - Py_CLEAR(frame->stack[i]); - } - PyMem_Free(locals); + PyMem_Free(frame); } Py_CLEAR(f->f_back); Py_CLEAR(f->f_trace); @@ -686,17 +680,13 @@ frame_tp_clear(PyFrameObject *f) Py_CLEAR(f->f_trace); - /* locals */ + /* locals and stack */ PyObject **locals = _PyFrame_GetLocalsArray(f->f_frame); - for (int i = 0; i < f->f_frame->nlocalsplus; i++) { + assert(f->f_frame->stacktop >= 0); + for (int i = 0; i < f->f_frame->stacktop; i++) { Py_CLEAR(locals[i]); } - - /* stack */ - for (int i = 0; i < f->f_frame->stackdepth; i++) { - Py_CLEAR(f->f_frame->stack[i]); - } - f->f_frame->stackdepth = 0; + f->f_frame->stacktop = 0; return 0; } diff --git a/Objects/genobject.c b/Objects/genobject.c index 86cd9cf7254589..7a687ce7f7cdfb 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -190,8 +190,7 @@ gen_send_ex2(PyGenObject *gen, PyObject *arg, PyObject **presult, /* Push arg onto the frame's value stack */ result = arg ? arg : Py_None; Py_INCREF(result); - frame->stack[frame->stackdepth] = result; - frame->stackdepth++; + _PyFrame_StackPush(frame, result); frame->previous = tstate->frame; @@ -350,8 +349,7 @@ _PyGen_yf(PyGenObject *gen) if (code[(frame->f_lasti+1)*sizeof(_Py_CODEUNIT)] != YIELD_FROM) return NULL; - assert(frame->stackdepth > 0); - yf = frame->stack[frame->stackdepth-1]; + yf = _PyFrame_StackPeek(frame); Py_INCREF(yf); } @@ -469,9 +467,7 @@ _gen_throw(PyGenObject *gen, int close_on_genexit, if (!ret) { PyObject *val; /* Pop subiterator from stack */ - assert(gen->gi_xframe->stackdepth > 0); - gen->gi_xframe->stackdepth--; - ret = gen->gi_xframe->stack[gen->gi_xframe->stackdepth]; + ret = _PyFrame_StackPop(gen->gi_xframe); assert(ret == yf); Py_DECREF(ret); /* Termination repetition of YIELD_FROM */ diff --git a/Objects/typeobject.c b/Objects/typeobject.c index a38690aed5c818..ec274a025de30c 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -8874,7 +8874,7 @@ super_init_without_args(PyFrameObject *f, PyCodeObject *co, return -1; } - assert(f->f_frame->nlocalsplus > 0); + assert(f->f_frame->f_code->co_nlocalsplus > 0); PyObject *firstarg = _PyFrame_GetLocalsArray(f->f_frame)[0]; // The first argument might be a cell. if (firstarg != NULL && (_PyLocals_GetKind(co->co_localspluskinds, 0) & CO_FAST_CELL)) { diff --git a/Python/ceval.c b/Python/ceval.c index 333c54f50e2e3a..5fec90b10483ab 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1367,7 +1367,7 @@ eval_frame_handle_pending(PyThreadState *tstate) /* The stack can grow at most MAXINT deep, as co_nlocals and co_stacksize are ints. */ -#define STACK_LEVEL() ((int)(stack_pointer - frame->stack)) +#define STACK_LEVEL() ((int)(stack_pointer - _PyFrame_Stackbase(frame))) #define EMPTY() (STACK_LEVEL() == 0) #define TOP() (stack_pointer[-1]) #define SECOND() (stack_pointer[-2]) @@ -1413,7 +1413,7 @@ eval_frame_handle_pending(PyThreadState *tstate) /* Local variable macros */ -#define GETLOCAL(i) (localsplus[i]) +#define GETLOCAL(i) (frame->localsplus[i]) /* The SETLOCAL() macro must not DECREF the local variable in-place and then store the new value; it must copy the old value to a temporary @@ -1559,7 +1559,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr PyObject *names = co->co_names; PyObject *consts = co->co_consts; - PyObject **localsplus = _PyFrame_GetLocalsArray(frame); _Py_CODEUNIT *first_instr = co->co_firstinstr; /* frame->f_lasti refers to the index of the last instruction, @@ -1578,14 +1577,14 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr */ assert(frame->f_lasti >= -1); _Py_CODEUNIT *next_instr = first_instr + frame->f_lasti + 1; - PyObject **stack_pointer = frame->stack + frame->stackdepth; + PyObject **stack_pointer = _PyFrame_GetStackPointer(frame); /* Set stackdepth to -1. * Update when returning or calling trace function. - Having f_stackdepth <= 0 ensures that invalid + Having stackdepth <= 0 ensures that invalid values are not visible to the cycle GC. We choose -1 rather than 0 to assist debugging. */ - frame->stackdepth = -1; + frame->stacktop = -1; frame->f_state = FRAME_EXECUTING; #ifdef LLTRACE @@ -1668,7 +1667,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr int err; /* see maybe_call_line_trace() for expository comments */ - frame->stackdepth = (int)(stack_pointer - frame->stack); + _PyFrame_SetStackPointer(frame, stack_pointer); err = maybe_call_line_trace(tstate->c_tracefunc, tstate->c_traceobj, @@ -1679,8 +1678,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr } /* Reload possibly changed frame fields */ JUMPTO(frame->f_lasti); - stack_pointer = frame->stack+frame->stackdepth; - frame->stackdepth = -1; + + stack_pointer = _PyFrame_GetStackPointer(frame); + frame->stacktop = -1; TRACING_NEXTOPARG(); } PRE_DISPATCH_GOTO(); @@ -2439,7 +2439,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr retval = POP(); assert(EMPTY()); frame->f_state = FRAME_RETURNED; - frame->stackdepth = 0; + _PyFrame_SetStackPointer(frame, stack_pointer); goto exiting; } @@ -2627,7 +2627,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr assert(frame->f_lasti > 0); frame->f_lasti -= 1; frame->f_state = FRAME_SUSPENDED; - frame->stackdepth = (int)(stack_pointer - frame->stack); + _PyFrame_SetStackPointer(frame, stack_pointer); goto exiting; } @@ -2644,7 +2644,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr retval = w; } frame->f_state = FRAME_SUSPENDED; - frame->stackdepth = (int)(stack_pointer - frame->stack); + _PyFrame_SetStackPointer(frame, stack_pointer); goto exiting; } @@ -4346,7 +4346,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr oparg = cache->adaptive.original_oparg; STAT_DEC(LOAD_METHOD, unquickened); JUMP_TO_INSTRUCTION(LOAD_METHOD); - } + } } TARGET(LOAD_METHOD_CACHED): { @@ -4364,7 +4364,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr assert(cache1->tp_version != 0); assert(self_cls->tp_dictoffset >= 0); assert(Py_TYPE(self_cls)->tp_dictoffset > 0); - + // inline version of _PyObject_GetDictPtr for offset >= 0 PyObject *dict = self_cls->tp_dictoffset != 0 ? *(PyObject **) ((char *)self + self_cls->tp_dictoffset) : NULL; @@ -4817,17 +4817,20 @@ MISS_WITH_OPARG_COUNTER(BINARY_SUBSCR) assert(_PyErr_Occurred(tstate)); /* Pop remaining stack entries. */ - while (!EMPTY()) { + PyObject **stackbase = _PyFrame_Stackbase(frame); + while (stack_pointer > stackbase) { PyObject *o = POP(); Py_XDECREF(o); } - frame->stackdepth = 0; + assert(STACK_LEVEL() == 0); + _PyFrame_SetStackPointer(frame, stack_pointer); frame->f_state = FRAME_RAISED; goto exiting; } assert(STACK_LEVEL() >= level); - while (STACK_LEVEL() > level) { + PyObject **new_top = _PyFrame_Stackbase(frame) + level; + while (stack_pointer > new_top) { PyObject *v = POP(); Py_XDECREF(v); } @@ -4981,7 +4984,7 @@ missing_arguments(PyThreadState *tstate, PyCodeObject *co, end = start + co->co_kwonlyargcount; } for (i = start; i < end; i++) { - if (GETLOCAL(i) == NULL) { + if (localsplus[i] == NULL) { PyObject *raw = PyTuple_GET_ITEM(co->co_localsplusnames, i); PyObject *name = PyObject_Repr(raw); if (name == NULL) { @@ -5010,7 +5013,7 @@ too_many_positional(PyThreadState *tstate, PyCodeObject *co, assert((co->co_flags & CO_VARARGS) == 0); /* Count missing keyword-only args. */ for (i = co_argcount; i < co_argcount + co->co_kwonlyargcount; i++) { - if (GETLOCAL(i) != NULL) { + if (localsplus[i] != NULL) { kwonly_given++; } } @@ -5217,7 +5220,8 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, if (co->co_flags & CO_VARARGS) { i++; } - SETLOCAL(i, kwdict); + assert(localsplus[i] == NULL); + localsplus[i] = kwdict; } else { kwdict = NULL; @@ -5234,7 +5238,8 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, for (j = 0; j < n; j++) { PyObject *x = args[j]; Py_INCREF(x); - SETLOCAL(j, x); + assert(localsplus[j] == NULL); + localsplus[j] = x; } /* Pack other positional arguments into the *args argument */ @@ -5243,7 +5248,8 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, if (u == NULL) { goto fail; } - SETLOCAL(total_args, u); + assert(localsplus[total_args] == NULL); + localsplus[total_args] = u; } /* Handle keyword arguments */ @@ -5307,14 +5313,14 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, continue; kw_found: - if (GETLOCAL(j) != NULL) { + if (localsplus[j] != NULL) { _PyErr_Format(tstate, PyExc_TypeError, "%U() got multiple values for argument '%S'", con->fc_qualname, keyword); goto fail; } Py_INCREF(value); - SETLOCAL(j, value); + localsplus[j] = value; } } @@ -5331,7 +5337,7 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, Py_ssize_t m = co->co_argcount - defcount; Py_ssize_t missing = 0; for (i = argcount; i < m; i++) { - if (GETLOCAL(i) == NULL) { + if (localsplus[i] == NULL) { missing++; } } @@ -5347,10 +5353,10 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, if (defcount) { PyObject **defs = &PyTuple_GET_ITEM(con->fc_defaults, 0); for (; i < defcount; i++) { - if (GETLOCAL(m+i) == NULL) { + if (localsplus[m+i] == NULL) { PyObject *def = defs[i]; Py_INCREF(def); - SETLOCAL(m+i, def); + localsplus[m+i] = def; } } } @@ -5360,14 +5366,14 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, if (co->co_kwonlyargcount > 0) { Py_ssize_t missing = 0; for (i = co->co_argcount; i < total_args; i++) { - if (GETLOCAL(i) != NULL) + if (localsplus[i] != NULL) continue; PyObject *varname = PyTuple_GET_ITEM(co->co_localsplusnames, i); if (con->fc_kwdefaults != NULL) { PyObject *def = PyDict_GetItemWithError(con->fc_kwdefaults, varname); if (def) { Py_INCREF(def); - SETLOCAL(i, def); + localsplus[i] = def; continue; } else if (_PyErr_Occurred(tstate)) { @@ -5407,18 +5413,17 @@ make_coro_frame(PyThreadState *tstate, assert(con->fc_defaults == NULL || PyTuple_CheckExact(con->fc_defaults)); PyCodeObject *code = (PyCodeObject *)con->fc_code; int size = code->co_nlocalsplus+code->co_stacksize + FRAME_SPECIALS_SIZE; - PyObject **localsarray = PyMem_Malloc(sizeof(PyObject *)*size); - if (localsarray == NULL) { + InterpreterFrame *frame = (InterpreterFrame *)PyMem_Malloc(sizeof(PyObject *)*size); + if (frame == NULL) { PyErr_NoMemory(); return NULL; } for (Py_ssize_t i=0; i < code->co_nlocalsplus; i++) { - localsarray[i] = NULL; + frame->localsplus[i] = NULL; } - InterpreterFrame *frame = (InterpreterFrame *)(localsarray + code->co_nlocalsplus); _PyFrame_InitializeSpecials(frame, con, locals, code->co_nlocalsplus); assert(frame->frame_obj == NULL); - if (initialize_locals(tstate, con, localsarray, args, argcount, kwnames)) { + if (initialize_locals(tstate, con, frame->localsplus, args, argcount, kwnames)) { _PyFrame_Clear(frame, 1); return NULL; } @@ -5497,7 +5502,7 @@ _PyEval_Vector(PyThreadState *tstate, PyFrameConstructor *con, } assert (tstate->interp->eval_frame != NULL); PyObject *retval = _PyEval_EvalFrame(tstate, frame, 0); - assert(frame->stackdepth == 0); + assert(_PyFrame_GetStackPointer(frame) == _PyFrame_Stackbase(frame)); if (_PyEvalFrameClearAndPop(tstate, frame)) { retval = NULL; } @@ -6764,7 +6769,6 @@ unicode_concatenate(PyThreadState *tstate, PyObject *v, PyObject *w, switch (opcode) { case STORE_FAST: { - PyObject **localsplus = _PyFrame_GetLocalsArray(frame); if (GETLOCAL(oparg) == v) SETLOCAL(oparg, NULL); break; diff --git a/Python/frame.c b/Python/frame.c index ae4284346ea12f..3d2415fee70978 100644 --- a/Python/frame.c +++ b/Python/frame.c @@ -14,13 +14,11 @@ _PyFrame_Traverse(InterpreterFrame *frame, visitproc visit, void *arg) Py_VISIT(frame->f_code); /* locals */ PyObject **locals = _PyFrame_GetLocalsArray(frame); - for (int i = 0; i < frame->nlocalsplus; i++) { + int i = 0; + /* locals and stack */ + for (; i stacktop; i++) { Py_VISIT(locals[i]); } - /* stack */ - for (int i = 0; i stackdepth; i++) { - Py_VISIT(frame->stack[i]); - } return 0; } @@ -47,17 +45,15 @@ _PyFrame_MakeAndSetFrameObject(InterpreterFrame *frame) static InterpreterFrame * copy_frame_to_heap(InterpreterFrame *frame) { - - Py_ssize_t size = ((char*)&frame->stack[frame->stackdepth]) - (char *)_PyFrame_GetLocalsArray(frame); - PyObject **copy = PyMem_Malloc(size); + assert(frame->stacktop >= frame->f_code->co_nlocalsplus); + Py_ssize_t size = ((char*)&frame->localsplus[frame->stacktop]) - (char *)frame; + InterpreterFrame *copy = PyMem_Malloc(size); if (copy == NULL) { PyErr_NoMemory(); return NULL; } - PyObject **locals = _PyFrame_GetLocalsArray(frame); - memcpy(copy, locals, size); - InterpreterFrame *res = (InterpreterFrame *)(copy + frame->nlocalsplus); - return res; + memcpy(copy, frame, size); + return copy; } static inline void @@ -103,7 +99,6 @@ take_ownership(PyFrameObject *f, InterpreterFrame *frame) int _PyFrame_Clear(InterpreterFrame * frame, int take) { - PyObject **localsarray = ((PyObject **)frame)-frame->nlocalsplus; if (frame->frame_obj) { PyFrameObject *f = frame->frame_obj; frame->frame_obj = NULL; @@ -120,16 +115,13 @@ _PyFrame_Clear(InterpreterFrame * frame, int take) } Py_DECREF(f); } - for (int i = 0; i < frame->nlocalsplus; i++) { - Py_XDECREF(localsarray[i]); - } - assert(frame->stackdepth >= 0); - for (int i = 0; i < frame->stackdepth; i++) { - Py_DECREF(frame->stack[i]); + assert(_PyFrame_GetStackPointer(frame) >= _PyFrame_Stackbase(frame)); + for (int i = 0; i < frame->stacktop; i++) { + Py_XDECREF(frame->localsplus[i]); } clear_specials(frame); if (take) { - PyMem_Free(_PyFrame_GetLocalsArray(frame)); + PyMem_Free(frame); } return 0; } diff --git a/Python/pystate.c b/Python/pystate.c index 6a15b54d1dd84f..45272142e41734 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -2045,21 +2045,21 @@ _PyThreadState_PushFrame(PyThreadState *tstate, PyFrameConstructor *con, PyObjec size_t size = nlocalsplus + code->co_stacksize + FRAME_SPECIALS_SIZE; assert(size < INT_MAX/sizeof(PyObject *)); - PyObject **localsarray = tstate->datastack_top; - PyObject **top = localsarray + size; + PyObject **base = tstate->datastack_top; + PyObject **top = base + size; if (top >= tstate->datastack_limit) { - localsarray = push_chunk(tstate, (int)size); - if (localsarray == NULL) { + base = push_chunk(tstate, (int)size); + if (base == NULL) { return NULL; } } else { tstate->datastack_top = top; } - InterpreterFrame * frame = (InterpreterFrame *)(localsarray + nlocalsplus); + InterpreterFrame *frame = (InterpreterFrame *)base; _PyFrame_InitializeSpecials(frame, con, locals, nlocalsplus); for (int i=0; i < nlocalsplus; i++) { - localsarray[i] = NULL; + frame->localsplus[i] = NULL; } return frame; } @@ -2067,8 +2067,8 @@ _PyThreadState_PushFrame(PyThreadState *tstate, PyFrameConstructor *con, PyObjec void _PyThreadState_PopFrame(PyThreadState *tstate, InterpreterFrame * frame) { - PyObject **locals = _PyFrame_GetLocalsArray(frame); - if (locals == &tstate->datastack_chunk->data[0]) { + PyObject **base = (PyObject **)frame; + if (base == &tstate->datastack_chunk->data[0]) { _PyStackChunk *chunk = tstate->datastack_chunk; _PyStackChunk *previous = chunk->previous; tstate->datastack_top = &previous->data[previous->top]; @@ -2077,8 +2077,8 @@ _PyThreadState_PopFrame(PyThreadState *tstate, InterpreterFrame * frame) tstate->datastack_limit = (PyObject **)(((char *)previous) + previous->size); } else { - assert(tstate->datastack_top >= locals); - tstate->datastack_top = locals; + assert(tstate->datastack_top >= base); + tstate->datastack_top = base; } } diff --git a/Tools/gdb/libpython.py b/Tools/gdb/libpython.py index 8b09563c18c9b1..c11b23e74b9bed 100755 --- a/Tools/gdb/libpython.py +++ b/Tools/gdb/libpython.py @@ -958,9 +958,11 @@ def iter_locals(self): if self.is_optimized_out(): return + obj_ptr_ptr = gdb.lookup_type("PyObject").pointer().pointer() - base = self._gdbval.cast(obj_ptr_ptr) - localsplus = base - self._f_nlocalsplus() + + localsplus = self._gdbval["localsplus"].cast(obj_ptr_ptr) + for i in safe_range(self.co_nlocals): pyop_value = PyObjectPtr.from_pyobject_ptr(localsplus[i]) if pyop_value.is_null(): From 15d50d7ed8afd3ab26b00210b5b4feaaadd5af51 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 25 Aug 2021 14:28:43 +0100 Subject: [PATCH 5/7] bpo-44946: Streamline operators and creation of ints for common case of single 'digit'. (GH-27832) --- Objects/longobject.c | 252 +++++++++++++++++++++++++++---------------- 1 file changed, 160 insertions(+), 92 deletions(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index d9127b31fd4867..18b0839adb6b05 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -26,15 +26,27 @@ class int "PyObject *" "&PyLong_Type" _Py_IDENTIFIER(little); _Py_IDENTIFIER(big); -/* convert a PyLong of size 1, 0 or -1 to an sdigit */ -#define MEDIUM_VALUE(x) (assert(-1 <= Py_SIZE(x) && Py_SIZE(x) <= 1), \ - Py_SIZE(x) < 0 ? -(sdigit)(x)->ob_digit[0] : \ - (Py_SIZE(x) == 0 ? (sdigit)0 : \ - (sdigit)(x)->ob_digit[0])) +/* Is this PyLong of size 1, 0 or -1? */ +#define IS_MEDIUM_VALUE(x) (((size_t)Py_SIZE(x)) + 1U < 3U) + +/* convert a PyLong of size 1, 0 or -1 to a C integer */ +static inline stwodigits +medium_value(PyLongObject *x) +{ + assert(IS_MEDIUM_VALUE(x)); + return ((stwodigits)Py_SIZE(x)) * x->ob_digit[0]; +} #define IS_SMALL_INT(ival) (-NSMALLNEGINTS <= (ival) && (ival) < NSMALLPOSINTS) #define IS_SMALL_UINT(ival) ((ival) < NSMALLPOSINTS) +static inline int is_medium_int(stwodigits x) +{ + /* Take care that we are comparing unsigned values. */ + twodigits x_plus_mask = ((twodigits)x) + PyLong_MASK; + return x_plus_mask < ((twodigits)PyLong_MASK) + PyLong_BASE; +} + static PyObject * get_small_int(sdigit ival) { @@ -47,33 +59,16 @@ get_small_int(sdigit ival) static PyLongObject * maybe_small_long(PyLongObject *v) { - if (v && Py_ABS(Py_SIZE(v)) <= 1) { - sdigit ival = MEDIUM_VALUE(v); + if (v && IS_MEDIUM_VALUE(v)) { + stwodigits ival = medium_value(v); if (IS_SMALL_INT(ival)) { Py_DECREF(v); - return (PyLongObject *)get_small_int(ival); + return (PyLongObject *)get_small_int((sdigit)ival); } } return v; } -/* If a freshly-allocated int is already shared, it must - be a small integer, so negating it must go to PyLong_FromLong */ -Py_LOCAL_INLINE(void) -_PyLong_Negate(PyLongObject **x_p) -{ - PyLongObject *x; - - x = (PyLongObject *)*x_p; - if (Py_REFCNT(x) == 1) { - Py_SET_SIZE(x, -Py_SIZE(x)); - return; - } - - *x_p = (PyLongObject *)PyLong_FromLong(-MEDIUM_VALUE(x)); - Py_DECREF(x); -} - /* For int multiplication, use the O(N**2) school algorithm unless * both operands contain more than KARATSUBA_CUTOFF digits (this * being an internal Python int digit, in base BASE). @@ -121,18 +116,21 @@ PyLongObject * _PyLong_New(Py_ssize_t size) { PyLongObject *result; - /* Number of bytes needed is: offsetof(PyLongObject, ob_digit) + - sizeof(digit)*size. Previous incarnations of this code used - sizeof(PyVarObject) instead of the offsetof, but this risks being - incorrect in the presence of padding between the PyVarObject header - and the digits. */ if (size > (Py_ssize_t)MAX_LONG_DIGITS) { PyErr_SetString(PyExc_OverflowError, "too many digits in integer"); return NULL; } + /* Fast operations for single digit integers (including zero) + * assume that there is always at least one digit present. */ + Py_ssize_t ndigits = size ? size : 1; + /* Number of bytes needed is: offsetof(PyLongObject, ob_digit) + + sizeof(digit)*size. Previous incarnations of this code used + sizeof(PyVarObject) instead of the offsetof, but this risks being + incorrect in the presence of padding between the PyVarObject header + and the digits. */ result = PyObject_Malloc(offsetof(PyLongObject, ob_digit) + - size*sizeof(digit)); + ndigits*sizeof(digit)); if (!result) { PyErr_NoMemory(); return NULL; @@ -152,9 +150,9 @@ _PyLong_Copy(PyLongObject *src) if (i < 0) i = -(i); if (i < 2) { - sdigit ival = MEDIUM_VALUE(src); + stwodigits ival = medium_value(src); if (IS_SMALL_INT(ival)) { - return get_small_int(ival); + return get_small_int((sdigit)ival); } } result = _PyLong_New(i); @@ -167,65 +165,126 @@ _PyLong_Copy(PyLongObject *src) return (PyObject *)result; } -/* Create a new int object from a C long int */ +static PyObject * +_PyLong_FromMedium(sdigit x) +{ + assert(!IS_SMALL_INT(x)); + assert(is_medium_int(x)); + /* We could use a freelist here */ + PyLongObject *v = PyObject_Malloc(sizeof(PyLongObject)); + if (v == NULL) { + PyErr_NoMemory(); + return NULL; + } + Py_ssize_t sign = x < 0 ? -1: 1; + digit abs_x = x < 0 ? -x : x; + _PyObject_InitVar((PyVarObject*)v, &PyLong_Type, sign); + v->ob_digit[0] = abs_x; + return (PyObject*)v; +} -PyObject * -PyLong_FromLong(long ival) +static PyObject * +_PyLong_FromLarge(stwodigits ival) { - PyLongObject *v; - unsigned long abs_ival; - unsigned long t; /* unsigned so >> doesn't propagate sign bit */ - int ndigits = 0; + twodigits abs_ival; int sign; + assert(!is_medium_int(ival)); + if (ival < 0) { + /* negate: can't write this as abs_ival = -ival since that + invokes undefined behaviour when ival is LONG_MIN */ + abs_ival = 0U-(twodigits)ival; + sign = -1; + } + else { + abs_ival = (twodigits)ival; + sign = 1; + } + /* Must be at least two digits */ + assert(abs_ival >> PyLong_SHIFT != 0); + twodigits t = abs_ival >> (PyLong_SHIFT * 2); + Py_ssize_t ndigits = 2; + while (t) { + ++ndigits; + t >>= PyLong_SHIFT; + } + PyLongObject *v = _PyLong_New(ndigits); + if (v != NULL) { + digit *p = v->ob_digit; + Py_SET_SIZE(v, ndigits * sign); + t = abs_ival; + while (t) { + *p++ = Py_SAFE_DOWNCAST( + t & PyLong_MASK, twodigits, digit); + t >>= PyLong_SHIFT; + } + } + return (PyObject *)v; +} + +/* Create a new int object from a C word-sized int */ +static inline PyObject * +_PyLong_FromSTwoDigits(stwodigits x) +{ + if (IS_SMALL_INT(x)) { + return get_small_int((sdigit)x); + } + assert(x != 0); + if (is_medium_int(x)) { + return _PyLong_FromMedium((sdigit)x); + } + return _PyLong_FromLarge(x); +} + +/* If a freshly-allocated int is already shared, it must + be a small integer, so negating it must go to PyLong_FromLong */ +Py_LOCAL_INLINE(void) +_PyLong_Negate(PyLongObject **x_p) +{ + PyLongObject *x; + + x = (PyLongObject *)*x_p; + if (Py_REFCNT(x) == 1) { + Py_SET_SIZE(x, -Py_SIZE(x)); + return; + } + + *x_p = (PyLongObject *)_PyLong_FromSTwoDigits(-medium_value(x)); + Py_DECREF(x); +} + +/* Create a new int object from a C long int */ +PyObject * +PyLong_FromLong(long ival) +{ if (IS_SMALL_INT(ival)) { return get_small_int((sdigit)ival); } - + unsigned long abs_ival; + int sign; if (ival < 0) { /* negate: can't write this as abs_ival = -ival since that invokes undefined behaviour when ival is LONG_MIN */ - abs_ival = 0U-(unsigned long)ival; + abs_ival = 0U-(twodigits)ival; sign = -1; } else { abs_ival = (unsigned long)ival; - sign = ival == 0 ? 0 : 1; + sign = 1; } - /* Fast path for single-digit ints */ if (!(abs_ival >> PyLong_SHIFT)) { - v = _PyLong_New(1); - if (v) { - Py_SET_SIZE(v, sign); - v->ob_digit[0] = Py_SAFE_DOWNCAST( - abs_ival, unsigned long, digit); - } - return (PyObject*)v; - } - -#if PyLong_SHIFT==15 - /* 2 digits */ - if (!(abs_ival >> 2*PyLong_SHIFT)) { - v = _PyLong_New(2); - if (v) { - Py_SET_SIZE(v, 2 * sign); - v->ob_digit[0] = Py_SAFE_DOWNCAST( - abs_ival & PyLong_MASK, unsigned long, digit); - v->ob_digit[1] = Py_SAFE_DOWNCAST( - abs_ival >> PyLong_SHIFT, unsigned long, digit); - } - return (PyObject*)v; + return _PyLong_FromMedium((sdigit)ival); } -#endif - - /* Larger numbers: loop to determine number of digits */ - t = abs_ival; + /* Must be at least two digits. + * Do shift in two steps to avoid undefined behavior. */ + unsigned long t = (abs_ival >> PyLong_SHIFT) >> PyLong_SHIFT; + Py_ssize_t ndigits = 2; while (t) { ++ndigits; t >>= PyLong_SHIFT; } - v = _PyLong_New(ndigits); + PyLongObject *v = _PyLong_New(ndigits); if (v != NULL) { digit *p = v->ob_digit; Py_SET_SIZE(v, ndigits * sign); @@ -2860,12 +2919,12 @@ PyLong_AsDouble(PyObject *v) PyErr_SetString(PyExc_TypeError, "an integer is required"); return -1.0; } - if (Py_ABS(Py_SIZE(v)) <= 1) { + if (IS_MEDIUM_VALUE(v)) { /* Fast path; single digit long (31 bits) will cast safely to double. This improves performance of FP/long operations by 20%. */ - return (double)MEDIUM_VALUE((PyLongObject *)v); + return (double)medium_value((PyLongObject *)v); } x = _PyLong_Frexp((PyLongObject *)v, &exponent); if ((x == -1.0 && PyErr_Occurred()) || exponent > DBL_MAX_EXP) { @@ -3067,8 +3126,8 @@ long_add(PyLongObject *a, PyLongObject *b) CHECK_BINOP(a, b); - if (Py_ABS(Py_SIZE(a)) <= 1 && Py_ABS(Py_SIZE(b)) <= 1) { - return PyLong_FromLong(MEDIUM_VALUE(a) + MEDIUM_VALUE(b)); + if (IS_MEDIUM_VALUE(a) && IS_MEDIUM_VALUE(b)) { + return _PyLong_FromSTwoDigits(medium_value(a) + medium_value(b)); } if (Py_SIZE(a) < 0) { if (Py_SIZE(b) < 0) { @@ -3101,8 +3160,8 @@ long_sub(PyLongObject *a, PyLongObject *b) CHECK_BINOP(a, b); - if (Py_ABS(Py_SIZE(a)) <= 1 && Py_ABS(Py_SIZE(b)) <= 1) { - return PyLong_FromLong(MEDIUM_VALUE(a) - MEDIUM_VALUE(b)); + if (IS_MEDIUM_VALUE(a) && IS_MEDIUM_VALUE(b)) { + return _PyLong_FromSTwoDigits(medium_value(a) - medium_value(b)); } if (Py_SIZE(a) < 0) { if (Py_SIZE(b) < 0) { @@ -3536,9 +3595,9 @@ long_mul(PyLongObject *a, PyLongObject *b) CHECK_BINOP(a, b); /* fast path for single-digit multiplication */ - if (Py_ABS(Py_SIZE(a)) <= 1 && Py_ABS(Py_SIZE(b)) <= 1) { - stwodigits v = (stwodigits)(MEDIUM_VALUE(a)) * MEDIUM_VALUE(b); - return PyLong_FromLongLong((long long)v); + if (IS_MEDIUM_VALUE(a) && IS_MEDIUM_VALUE(b)) { + stwodigits v = medium_value(a) * medium_value(b); + return _PyLong_FromSTwoDigits(v); } z = k_mul(a, b); @@ -4343,8 +4402,8 @@ long_invert(PyLongObject *v) { /* Implement ~x as -(x+1) */ PyLongObject *x; - if (Py_ABS(Py_SIZE(v)) <=1) - return PyLong_FromLong(-(MEDIUM_VALUE(v)+1)); + if (IS_MEDIUM_VALUE(v)) + return _PyLong_FromSTwoDigits(~medium_value(v)); x = (PyLongObject *) long_add(v, (PyLongObject *)_PyLong_GetOne()); if (x == NULL) return NULL; @@ -4358,8 +4417,8 @@ static PyObject * long_neg(PyLongObject *v) { PyLongObject *z; - if (Py_ABS(Py_SIZE(v)) <= 1) - return PyLong_FromLong(-MEDIUM_VALUE(v)); + if (IS_MEDIUM_VALUE(v)) + return _PyLong_FromSTwoDigits(-medium_value(v)); z = (PyLongObject *)_PyLong_Copy(v); if (z != NULL) Py_SET_SIZE(z, -(Py_SIZE(v))); @@ -4704,28 +4763,37 @@ long_bitwise(PyLongObject *a, static PyObject * long_and(PyObject *a, PyObject *b) { - PyObject *c; CHECK_BINOP(a, b); - c = long_bitwise((PyLongObject*)a, '&', (PyLongObject*)b); - return c; + PyLongObject *x = (PyLongObject*)a; + PyLongObject *y = (PyLongObject*)b; + if (IS_MEDIUM_VALUE(x) && IS_MEDIUM_VALUE(y)) { + return _PyLong_FromSTwoDigits(medium_value(x) & medium_value(y)); + } + return long_bitwise(x, '&', y); } static PyObject * long_xor(PyObject *a, PyObject *b) { - PyObject *c; CHECK_BINOP(a, b); - c = long_bitwise((PyLongObject*)a, '^', (PyLongObject*)b); - return c; + PyLongObject *x = (PyLongObject*)a; + PyLongObject *y = (PyLongObject*)b; + if (IS_MEDIUM_VALUE(x) && IS_MEDIUM_VALUE(y)) { + return _PyLong_FromSTwoDigits(medium_value(x) ^ medium_value(y)); + } + return long_bitwise(x, '^', y); } static PyObject * long_or(PyObject *a, PyObject *b) { - PyObject *c; CHECK_BINOP(a, b); - c = long_bitwise((PyLongObject*)a, '|', (PyLongObject*)b); - return c; + PyLongObject *x = (PyLongObject*)a; + PyLongObject *y = (PyLongObject*)b; + if (IS_MEDIUM_VALUE(x) && IS_MEDIUM_VALUE(y)) { + return _PyLong_FromSTwoDigits(medium_value(x) | medium_value(y)); + } + return long_bitwise(x, '|', y); } static PyObject * From a3c11cebf174e0c822eda8c545f7548269ce7a25 Mon Sep 17 00:00:00 2001 From: Erlend Egeberg Aasland Date: Wed, 25 Aug 2021 15:57:54 +0200 Subject: [PATCH 6/7] bpo-27334: Fix reference leak introduced by GH-26202 (GH-27942) --- Modules/_sqlite/connection.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 1bc045523a252e..19d30d24d7f2e7 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -1807,6 +1807,7 @@ pysqlite_connection_exit_impl(pysqlite_Connection *self, PyObject *exc_type, _PyErr_ChainExceptions(exc, val, tb); } else { + Py_DECREF(result); PyErr_Restore(exc, val, tb); } } From 24da544014f78e6f1440d5ce5c2d14794a020340 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Salgado Date: Wed, 25 Aug 2021 15:24:32 +0100 Subject: [PATCH 7/7] bpo-44929: [Enum] Fix global repr (GH-27789) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix typo in __repr__ code * Add more tests for global int flag reprs * use last module if multi-module string - when an enum's `__module__` contains several module names, only use the last one Co-authored-by: Łukasz Langa Co-authored-by: Ethan Furman --- Lib/enum.py | 19 ++++-- Lib/test/test_enum.py | 68 ++++++++++++++++--- .../2021-08-16-23-16-17.bpo-44929.qpMEky.rst | 2 + 3 files changed, 74 insertions(+), 15 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2021-08-16-23-16-17.bpo-44929.qpMEky.rst diff --git a/Lib/enum.py b/Lib/enum.py index 84e3cc17bbc531..0776761ae6e735 100644 --- a/Lib/enum.py +++ b/Lib/enum.py @@ -1390,17 +1390,28 @@ def _power_of_two(value): return value == 2 ** _high_bit(value) def global_enum_repr(self): - return '%s.%s' % (self.__class__.__module__, self._name_) + """ + use module.enum_name instead of class.enum_name + + the module is the last module in case of a multi-module name + """ + module = self.__class__.__module__.split('.')[-1] + return '%s.%s' % (module, self._name_) def global_flag_repr(self): - module = self.__class__.__module__ + """ + use module.flag_name instead of class.flag_name + + the module is the last module in case of a multi-module name + """ + module = self.__class__.__module__.split('.')[-1] cls_name = self.__class__.__name__ if self._name_ is None: - return "%x" % (module, cls_name, self._value_) + return "%s.%s(0x%x)" % (module, cls_name, self._value_) if _is_single_bit(self): return '%s.%s' % (module, self._name_) if self._boundary_ is not FlagBoundary.KEEP: - return module + module.join(self.name.split('|')) + return '|'.join(['%s.%s' % (module, name) for name in self.name.split('|')]) else: name = [] for n in self._name_.split('|'): diff --git a/Lib/test/test_enum.py b/Lib/test/test_enum.py index 116f61e5cf0924..52d7756da98bd8 100644 --- a/Lib/test/test_enum.py +++ b/Lib/test/test_enum.py @@ -28,6 +28,9 @@ def load_tests(loader, tests, ignore): )) return tests +MODULE = ('test.test_enum', '__main__')[__name__=='__main__'] +SHORT_MODULE = MODULE.split('.')[-1] + # for pickle tests try: class Stooges(Enum): @@ -143,6 +146,23 @@ def __init__(self, fget=None, fset=None, fdel=None, doc=None): def __get__(self, instance, ownerclass): return self.fget(ownerclass) +# for global repr tests + +@enum.global_enum +class HeadlightsK(IntFlag, boundary=enum.KEEP): + OFF_K = 0 + LOW_BEAM_K = auto() + HIGH_BEAM_K = auto() + FOG_K = auto() + + +@enum.global_enum +class HeadlightsC(IntFlag, boundary=enum.CONFORM): + OFF_C = 0 + LOW_BEAM_C = auto() + HIGH_BEAM_C = auto() + FOG_C = auto() + # tests @@ -3224,6 +3244,34 @@ def test_repr(self): self.assertEqual(repr(~(Open.WO | Open.CE)), 'Open.RW') self.assertEqual(repr(Open(~4)), '-5') + def test_global_repr_keep(self): + self.assertEqual( + repr(HeadlightsK(0)), + '%s.OFF_K' % SHORT_MODULE, + ) + self.assertEqual( + repr(HeadlightsK(2**0 + 2**2 + 2**3)), + '%(m)s.LOW_BEAM_K|%(m)s.FOG_K|0x8' % {'m': SHORT_MODULE}, + ) + self.assertEqual( + repr(HeadlightsK(2**3)), + '%(m)s.HeadlightsK(0x8)' % {'m': SHORT_MODULE}, + ) + + def test_global_repr_conform1(self): + self.assertEqual( + repr(HeadlightsC(0)), + '%s.OFF_C' % SHORT_MODULE, + ) + self.assertEqual( + repr(HeadlightsC(2**0 + 2**2 + 2**3)), + '%(m)s.LOW_BEAM_C|%(m)s.FOG_C' % {'m': SHORT_MODULE}, + ) + self.assertEqual( + repr(HeadlightsC(2**3)), + '%(m)s.OFF_C' % {'m': SHORT_MODULE}, + ) + def test_format(self): Perm = self.Perm self.assertEqual(format(Perm.R, ''), '4') @@ -4085,7 +4133,7 @@ def setUp(self): def test_convert_value_lookup_priority(self): test_type = enum.IntEnum._convert_( 'UnittestConvert', - ('test.test_enum', '__main__')[__name__=='__main__'], + MODULE, filter=lambda x: x.startswith('CONVERT_TEST_')) # We don't want the reverse lookup value to vary when there are # multiple possible names for a given value. It should always @@ -4095,7 +4143,7 @@ def test_convert_value_lookup_priority(self): def test_convert(self): test_type = enum.IntEnum._convert_( 'UnittestConvert', - ('test.test_enum', '__main__')[__name__=='__main__'], + MODULE, filter=lambda x: x.startswith('CONVERT_TEST_')) # Ensure that test_type has all of the desired names and values. self.assertEqual(test_type.CONVERT_TEST_NAME_F, @@ -4115,7 +4163,7 @@ def test_convert_warn(self): with self.assertWarns(DeprecationWarning): enum.IntEnum._convert( 'UnittestConvert', - ('test.test_enum', '__main__')[__name__=='__main__'], + MODULE, filter=lambda x: x.startswith('CONVERT_TEST_')) @unittest.skipUnless(python_version >= (3, 9), @@ -4124,16 +4172,15 @@ def test_convert_raise(self): with self.assertRaises(AttributeError): enum.IntEnum._convert( 'UnittestConvert', - ('test.test_enum', '__main__')[__name__=='__main__'], + MODULE, filter=lambda x: x.startswith('CONVERT_TEST_')) def test_convert_repr_and_str(self): - module = ('test.test_enum', '__main__')[__name__=='__main__'] test_type = enum.IntEnum._convert_( 'UnittestConvert', - module, + MODULE, filter=lambda x: x.startswith('CONVERT_STRING_TEST_')) - self.assertEqual(repr(test_type.CONVERT_STRING_TEST_NAME_A), '%s.CONVERT_STRING_TEST_NAME_A' % module) + self.assertEqual(repr(test_type.CONVERT_STRING_TEST_NAME_A), '%s.CONVERT_STRING_TEST_NAME_A' % SHORT_MODULE) self.assertEqual(str(test_type.CONVERT_STRING_TEST_NAME_A), 'CONVERT_STRING_TEST_NAME_A') self.assertEqual(format(test_type.CONVERT_STRING_TEST_NAME_A), '5') @@ -4151,7 +4198,7 @@ def setUp(self): def test_convert(self): test_type = enum.StrEnum._convert_( 'UnittestConvert', - ('test.test_enum', '__main__')[__name__=='__main__'], + MODULE, filter=lambda x: x.startswith('CONVERT_STR_')) # Ensure that test_type has all of the desired names and values. self.assertEqual(test_type.CONVERT_STR_TEST_1, 'hello') @@ -4162,12 +4209,11 @@ def test_convert(self): [], msg='Names other than CONVERT_STR_* found.') def test_convert_repr_and_str(self): - module = ('test.test_enum', '__main__')[__name__=='__main__'] test_type = enum.StrEnum._convert_( 'UnittestConvert', - module, + MODULE, filter=lambda x: x.startswith('CONVERT_STR_')) - self.assertEqual(repr(test_type.CONVERT_STR_TEST_1), '%s.CONVERT_STR_TEST_1' % module) + self.assertEqual(repr(test_type.CONVERT_STR_TEST_1), '%s.CONVERT_STR_TEST_1' % SHORT_MODULE) self.assertEqual(str(test_type.CONVERT_STR_TEST_2), 'goodbye') self.assertEqual(format(test_type.CONVERT_STR_TEST_1), 'hello') diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-08-16-23-16-17.bpo-44929.qpMEky.rst b/Misc/NEWS.d/next/Core and Builtins/2021-08-16-23-16-17.bpo-44929.qpMEky.rst new file mode 100644 index 00000000000000..e883e031a4afd8 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2021-08-16-23-16-17.bpo-44929.qpMEky.rst @@ -0,0 +1,2 @@ +Fix some edge cases of ``enum.Flag`` string representation in the REPL. +Patch by Pablo Galindo.