Skip to content

gh-123171: Fix error paths in ast_opt.c for for_iter optimisations #123172

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pablogsal wants to merge 3 commits into from
Closed

gh-123171: Fix error paths in ast_opt.c for for_iter optimisations #123172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c04dbe8
gh-123171: Fix error paths in ast_opt.c for for_iter optimisations
pablogsal Aug 20, 2024
e1d5eae
Fix make_const_tuple
pablogsal Aug 20, 2024
b03efff
Refactor ast_opt.c so make_const doesn't eat errors
pablogsal Aug 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2024-08-20-12-26-34.gh-issue-123171.BVu4wg.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix potential reference leaks in the error paths of AST optimisations. Patch
by Pablo Galindo
141 changes: 91 additions & 50 deletions Python/ast_opt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,7 @@ typedef struct {
static int
make_const(expr_ty node, PyObject *val, PyArena *arena)
{
// Even if no new value was calculated, make_const may still
// need to clear an error (e.g. for division by zero)
if (val == NULL) {
if (PyErr_ExceptionMatches(PyExc_KeyboardInterrupt)) {
return 0;
}
PyErr_Clear();
return 1;
}
assert(val != NULL);
if (_PyArena_AddPyObject(arena, val) < 0) {
Py_DECREF(val);
return 0;
Expand Down Expand Up @@ -133,6 +125,9 @@ fold_unaryop(expr_ty node, PyArena *arena, _PyASTOptimizeState *state)
[USub] = PyNumber_Negative,
};
PyObject *newval = ops[node->v.UnaryOp.op](arg->v.Constant.value);
if (!newval) {
return 0;
}
return make_const(node, newval, arena);
}

Expand Down Expand Up @@ -163,30 +158,30 @@ check_complexity(PyObject *obj, Py_ssize_t limit)
#define MAX_STR_SIZE 4096 /* characters */
#define MAX_TOTAL_ITEMS 1024 /* including nested collections */

static PyObject *
safe_multiply(PyObject *v, PyObject *w)
static int
safe_multiply(PyObject *v, PyObject *w, PyObject** result)
{
if (PyLong_Check(v) && PyLong_Check(w) &&
!_PyLong_IsZero((PyLongObject *)v) && !_PyLong_IsZero((PyLongObject *)w)
) {
size_t vbits = _PyLong_NumBits(v);
size_t wbits = _PyLong_NumBits(w);
if (vbits == (size_t)-1 || wbits == (size_t)-1) {
return NULL;
return 0;
}
if (vbits + wbits > MAX_INT_SIZE) {
return NULL;
return 1;
}
}
else if (PyLong_Check(v) && PyTuple_Check(w)) {
Py_ssize_t size = PyTuple_GET_SIZE(w);
if (size) {
long n = PyLong_AsLong(v);
if (n < 0 || n > MAX_COLLECTION_SIZE / size) {
return NULL;
return PyErr_Occurred() == NULL;
}
if (n && check_complexity(w, MAX_TOTAL_ITEMS / n) < 0) {
return NULL;
return 1;
}
}
}
Expand All @@ -196,65 +191,69 @@ safe_multiply(PyObject *v, PyObject *w)
if (size) {
long n = PyLong_AsLong(v);
if (n < 0 || n > MAX_STR_SIZE / size) {
return NULL;
return PyErr_Occurred() == NULL;
}
}
}
else if (PyLong_Check(w) &&
(PyTuple_Check(v) || PyUnicode_Check(v) || PyBytes_Check(v)))
{
return safe_multiply(w, v);
return safe_multiply(w, v, result);
}

return PyNumber_Multiply(v, w);
*result = PyNumber_Multiply(v, w);
return *result == NULL;
}

static PyObject *
safe_power(PyObject *v, PyObject *w)
static int
safe_power(PyObject *v, PyObject *w, PyObject** result)
{
if (PyLong_Check(v) && PyLong_Check(w) &&
!_PyLong_IsZero((PyLongObject *)v) && _PyLong_IsPositive((PyLongObject *)w)
) {
size_t vbits = _PyLong_NumBits(v);
size_t wbits = PyLong_AsSize_t(w);
if (vbits == (size_t)-1 || wbits == (size_t)-1) {
return NULL;
return 0;
}
if (vbits > MAX_INT_SIZE / wbits) {
return NULL;
return 1;
}
}

return PyNumber_Power(v, w, Py_None);
*result = PyNumber_Power(v, w, Py_None);
return *result == NULL;
}

static PyObject *
safe_lshift(PyObject *v, PyObject *w)
static int
safe_lshift(PyObject *v, PyObject *w, PyObject** result)
{
if (PyLong_Check(v) && PyLong_Check(w) &&
!_PyLong_IsZero((PyLongObject *)v) && !_PyLong_IsZero((PyLongObject *)w)
) {
size_t vbits = _PyLong_NumBits(v);
size_t wbits = PyLong_AsSize_t(w);
if (vbits == (size_t)-1 || wbits == (size_t)-1) {
return NULL;
return 0;
}
if (wbits > MAX_INT_SIZE || vbits > MAX_INT_SIZE - wbits) {
return NULL;
return 1;
}
}

return PyNumber_Lshift(v, w);
*result = PyNumber_Lshift(v, w);
return *result == NULL;
}

static PyObject *
safe_mod(PyObject *v, PyObject *w)
static int
safe_mod(PyObject *v, PyObject *w, PyObject** result)
{
if (PyUnicode_Check(v) || PyBytes_Check(v)) {
return NULL;
return 1;
}

return PyNumber_Remainder(v, w);
*result = PyNumber_Remainder(v, w);
return *result == NULL;
}


Expand Down Expand Up @@ -481,6 +480,7 @@ fold_binop(expr_ty node, PyArena *arena, _PyASTOptimizeState *state)
return 1;
}

int res = 1;
PyObject *rv = rhs->v.Constant.value;
PyObject *newval = NULL;

Expand All @@ -492,22 +492,34 @@ fold_binop(expr_ty node, PyArena *arena, _PyASTOptimizeState *state)
newval = PyNumber_Subtract(lv, rv);
break;
case Mult:
newval = safe_multiply(lv, rv);
res = safe_multiply(lv, rv, &newval);
if (res == 1 && newval == NULL) {
return 1;
}
break;
case Div:
newval = PyNumber_TrueDivide(lv, rv);
break;
break;
case FloorDiv:
newval = PyNumber_FloorDivide(lv, rv);
break;
case Mod:
newval = safe_mod(lv, rv);
case Mod:
res = safe_mod(lv, rv, &newval);
if (res == 1 && newval == NULL) {
return 1;
}
break;
case Pow:
newval = safe_power(lv, rv);
res = safe_power(lv, rv, &newval);
if (res == 1 && newval == NULL) {
return 1;
}
break;
case LShift:
newval = safe_lshift(lv, rv);
res = safe_lshift(lv, rv, &newval);
if (res == 1 && newval == NULL) {
return 1;
}
break;
case RShift:
newval = PyNumber_Rshift(lv, rv);
Expand All @@ -528,47 +540,61 @@ fold_binop(expr_ty node, PyArena *arena, _PyASTOptimizeState *state)
// operators are added without being handled here
}

if (!newval) {
assert(PyErr_Occurred());
if (PyErr_ExceptionMatches(PyExc_ZeroDivisionError)) {
PyErr_Clear();
return 1;
}
return 0;
}

return make_const(node, newval, arena);
}

static PyObject*
make_const_tuple(asdl_expr_seq *elts)
static int
make_const_tuple(asdl_expr_seq *elts, PyObject** res)
{
for (Py_ssize_t i = 0; i < asdl_seq_LEN(elts); i++) {
expr_ty e = (expr_ty)asdl_seq_GET(elts, i);
if (e->kind != Constant_kind) {
return NULL;
return 1;
}
}

PyObject *newval = PyTuple_New(asdl_seq_LEN(elts));
if (newval == NULL) {
return NULL;
return 0;
}

for (Py_ssize_t i = 0; i < asdl_seq_LEN(elts); i++) {
expr_ty e = (expr_ty)asdl_seq_GET(elts, i);
PyObject *v = e->v.Constant.value;
PyTuple_SET_ITEM(newval, i, Py_NewRef(v));
}
return newval;
*res = newval;
return 1;
}

static int
fold_tuple(expr_ty node, PyArena *arena, _PyASTOptimizeState *state)
{
PyObject *newval;
PyObject *newval = NULL;

if (node->v.Tuple.ctx != Load)
return 1;

newval = make_const_tuple(node->v.Tuple.elts);
int res = make_const_tuple(node->v.Tuple.elts, &newval);
if (!res || newval == NULL) {
return res;
}
return make_const(node, newval, arena);
}

static int
fold_subscr(expr_ty node, PyArena *arena, _PyASTOptimizeState *state)
{
return 1;
PyObject *newval;
expr_ty arg, idx;

Expand All @@ -594,7 +620,7 @@ fold_subscr(expr_ty node, PyArena *arena, _PyASTOptimizeState *state)
static int
fold_iter(expr_ty arg, PyArena *arena, _PyASTOptimizeState *state)
{
PyObject *newval;
PyObject *newval = NULL;
if (arg->kind == List_kind) {
/* First change a list into tuple. */
asdl_expr_seq *elts = arg->v.List.elts;
Expand All @@ -606,12 +632,23 @@ fold_iter(expr_ty arg, PyArena *arena, _PyASTOptimizeState *state)
arg->v.Tuple.elts = elts;
arg->v.Tuple.ctx = ctx;
/* Try to create a constant tuple. */
newval = make_const_tuple(elts);
int res = make_const_tuple(elts, &newval);
if (!res || newval == NULL) {
return res;
}
}
else if (arg->kind == Set_kind) {
newval = make_const_tuple(arg->v.Set.elts);
int res = make_const_tuple(arg->v.Set.elts, &newval);
if (!res || newval == NULL) {
return res;
}
if (newval) {
Py_SETREF(newval, PyFrozenSet_New(newval));
PyObject* the_set = PyFrozenSet_New(newval);
if (the_set == NULL) {
Py_DECREF(newval);
return 0;
}
Py_SETREF(newval, the_set);
}
}
else {
Expand Down Expand Up @@ -821,7 +858,11 @@ astfold_expr(expr_ty node_, PyArena *ctx_, _PyASTOptimizeState *state)
if (node_->v.Name.ctx == Load &&
_PyUnicode_EqualToASCIIString(node_->v.Name.id, "__debug__")) {
LEAVE_RECURSIVE(state);
return make_const(node_, PyBool_FromLong(!state->optimize), ctx_);
PyObject* the_bool = PyBool_FromLong(!state->optimize);
if (!the_bool) {
return 0;
}
return make_const(node_, the_bool, ctx_);
}
break;
case NamedExpr_kind:
Expand Down