Skip to content

gh-161666: Fix undefined left shifts in stack_swap() #161667

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

Open
PavlNekrasov wants to merge 3 commits into
base: main
Choose a base branch
from
Open

gh-161666: Fix undefined left shifts in stack_swap() #161667

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
bcfc57c
Fix undefined 32-bit shifts in frameobject.c stack_swap()
PavlNekrasov Nov 5, 2026
00ab207
Add NEWS entry
PavlNekrasov Nov 10, 2026
f45c3cf
Fix stack_swap() return type
PavlNekrasov Nov 10, 2026
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
10 changes: 10 additions & 0 deletions Lib/test/test_sys_settrace.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2113,6 +2113,16 @@ def test_jump_simple_backwards(output):
output.append(1)
output.append(2)

def test_backward_jump_deep_stack(self):
src = ["def f(output):"]
src += [" " * (i + 1) + f"for a{i} in [0]:" for i in range(6)]
src += [" " * 7 + "output.append(1)",
" " * 7 + "output[0], a5 = a5, output[0]",
" " * 7 + "output.append(2)"]
ns = {}
exec("\n".join(src), ns)
self.run_test(ns["f"], 9, 7, [1, 1, 2])

@jump_test(1, 4, [5], warning=(RuntimeWarning, unbound_locals))
def test_jump_is_none_forwards(output):
x = None
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Core_and_Builtins/2026-11-10-13-08-59.gh-issue-161666.Rey68g.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix undefined shifts and corruption of the evaluation stack model when
setting ``frame.f_lineno`` in code with a deep value stack (e.g. a
comprehension with 11 or more nested ``for`` loops).
5 changes: 3 additions & 2 deletions Objects/frameobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1313,14 +1313,15 @@ peek(int64_t stack, int n)
return (stack>>(BITS_PER_BLOCK*(n-1))) & MASK;
}

static Kind
static int64_t
stack_swap(int64_t stack, int n)
{
assert(n >= 1);
Kind to_swap = peek(stack, n);
Kind top = top_of_stack(stack);
int shift = BITS_PER_BLOCK*(n-1);
int64_t replaced_low = (stack & ~(MASK << shift)) | (top << shift);
int64_t replaced_low = (stack & ~((int64_t)MASK << shift))

@iRELDevz iRELDevz Nov 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stack_swap() still returns Kind (32-bit), so the result gets truncated once the stack has 11+ entries, even with the casts. The return type needs to be int64_t:

-static Kind
+static int64_t
 stack_swap(int64_t stack, int n)

It's visible from Python too: this jump is wrongly rejected with can't jump into the body of a for loop (passes without the swap line). Could go into JumpTestCase in Lib/test/test_sys_settrace.py:

    def testbackwardjumpT(self):
        src = ["def f(output):"]
        src += [" " * (i + 1) + f"for a{i} in [0]:" for i in range(10)]
        src += [" " * 11 + "output.append(1)",
                " " * 11 + "output[0], a9 = a9, output[0]",
                " " * 11 + "output.append(2)"]
        ns = {}
        exec("\n".join(src), ns)
        self.run_test(ns["f"], 13, 11, [1, 1, 2])

@iRELDevz iRELDevz Nov 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

damn the test I suggested is too deep. 10 loops overflows the stack model and hits the top_of_stack(next_stack) == Object assert in mark_stacks on debug builds, which is what's breaking CI (it happens on main too, with or without the swap line).

6 loops is enough: it still fails on main and passes with this PR.

    def test_backward_jump_deep_stack(self):
        src = ["def f(output):"]
        src += [" " * (i + 1) + f"for a{i} in [0]:" for i in range(6)]
        src += [" " * 7 + "output.append(1)",
                " " * 7 + "output[0], a5 = a5, output[0]",
                " " * 7 + "output.append(2)"]
        ns = {}
        exec("\n".join(src), ns)
        self.run_test(ns["f"], 9, 7, [1, 1, 2])

| ((int64_t)top << shift);
int64_t replaced_top = (replaced_low & ~MASK) | to_swap;
return replaced_top;
}
Expand Down
Loading