-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
bpo-42609: Check recursion depth in the AST validator and optimizer #23744
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.
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
Changes from 1 commit
8c5cb1e
8d3a7a2
364d0d2
e0e64e2
8c53a01
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,6 +87,9 @@ PyAPI_FUNC(int) PyCompile_OpcodeStackEffectWithJump(int opcode, int oparg, int j | |
typedef struct { | ||
int optimize; | ||
int ff_features; | ||
|
||
int recursion_depth; /* current recursion depth */ | ||
int recursion_limit; /* recursion limit */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you not use Py_EnterRecursiveCall here instead? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code is copied from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. C99 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but it was just copied from |
||
} _PyASTOptimizeState; | ||
|
||
PyAPI_FUNC(int) _PyAST_Optimize(struct _mod *, PyArena *arena, _PyASTOptimizeState *state); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -543,21 +543,24 @@ def test_compiler_recursion_limit(self): | |
# XXX (ncoghlan): duplicating the scaling factor here is a little | ||
# ugly. Perhaps it should be exposed somewhere... | ||
fail_depth = sys.getrecursionlimit() * 3 | ||
crash_depth = sys.getrecursionlimit() * 300 | ||
success_depth = int(fail_depth * 0.75) | ||
|
||
def check_limit(prefix, repeated): | ||
def check_limit(prefix, repeated, mode="single"): | ||
expect_ok = prefix + repeated * success_depth | ||
self.compile_single(expect_ok) | ||
broken = prefix + repeated * fail_depth | ||
details = "Compiling ({!r} + {!r} * {})".format( | ||
prefix, repeated, fail_depth) | ||
with self.assertRaises(RecursionError, msg=details): | ||
self.compile_single(broken) | ||
compile(expect_ok, '<test>', mode) | ||
for depth in (fail_depth, crash_depth): | ||
broken = prefix + repeated * depth | ||
details = "Compiling ({!r} + {!r} * {})".format( | ||
prefix, repeated, depth) | ||
Comment on lines
+554
to
+555
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An f-string could be used. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but this code already was here. |
||
with self.assertRaises(RecursionError, msg=details): | ||
compile(broken, '<test>', mode) | ||
|
||
check_limit("a", "()") | ||
check_limit("a", ".b") | ||
check_limit("a", "[0]") | ||
check_limit("a", "*a") | ||
#check_limit("if a: pass", "\nelif a: pass", mode="exec") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this commented-out code should be added. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is still a bug not fixed by this PR. |
||
|
||
def test_null_terminated(self): | ||
# The source code is null-terminated internally, but bytes-like | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
Prevented crashes in the AST validator and optimizer when compile some | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. compile -> compiling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. |
||
absurdly long expressions like ``"+0"*1000000``. :exc:`RecursionError` is | ||
now raised instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty line separates groups of fields.