Skip to content
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

Jump to bottom

Lower marshal.c recursion depth limit for WASI #98925

Closed
brettcannon opened this issue Oct 31, 2022 · 1 comment
Closed

Lower marshal.c recursion depth limit for WASI #98925

brettcannon opened this issue Oct 31, 2022 · 1 comment
Assignees
@brettcannon
Labels
3.11 only security fixes 3.12 bugs and security fixes OS-wasi type-bug An unexpected behavior, bug, or error

Comments

@brettcannon
Copy link
Member

brettcannon commented Oct 31, 2022 •
edited by bedevere-bot
Loading

Based on bytecodealliance/wasmtime#5152 it looks like the stack depth specified in:

cpython/Python/marshal.c

Line 38 in 4702552

#define MAX_MARSHAL_STACK_DEPTH 2000

and:

cpython/Lib/test/test_marshal.py

Line 264 in 7644935

MAX_MARSHAL_STACK_DEPTH = 2000

are too high for wasmtime 2.0 (in development). We should drop it to either 1000 to match Windows or something like 1500 if we want a higher, but still acceptable, limit.
@brettcannon brettcannon added type-bug An unexpected behavior, bug, or error 3.11 only security fixes 3.12 bugs and security fixes OS-wasi labels Oct 31, 2022
@brettcannon brettcannon self-assigned this Oct 31, 2022
@brettcannon brettcannon mentioned this issue Oct 31, 2022
Closed
@brettcannon
Copy link
Member Author

FYI this was detected via https://github.com/tiran/cpython-wasm-test/actions/workflows/wasi.yml .

@bedevere-bot bedevere-bot mentioned this issue Nov 1, 2022
Merged
brettcannon added a commit to brettcannon/cpython that referenced this issue Nov 1, 2022
@brettcannon
pythongh-98925: Lower marshal recursion depth for WASI
921f4c5 
For (in-development) wasmtime 2.0, the stack depth cost is 6% higher. As the default is 2000 and Windows is 1000, split the difference and choose 1500 for WASI.
miss-islington pushed a commit that referenced this issue Nov 1, 2022
@brettcannon
gh-98925: Lower marshal recursion depth for WASI (GH-98938)
9711265 
For wasmtime 2.0, the stack depth cost is 6% higher. This causes the default max `marshal` recursion depth to blow the stack.

As the default marshal depth is 2000 and Windows is set to 1000, split the difference and choose 1500 for WASI to be safe.
@bedevere-bot bedevere-bot mentioned this issue Nov 1, 2022
Merged
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Nov 1, 2022
@brettcannon @miss-islington
pythongh-98925: Lower marshal recursion depth for WASI (pythonGH-98938)
c1e2a00 
For wasmtime 2.0, the stack depth cost is 6% higher. This causes the default max `marshal` recursion depth to blow the stack.

As the default marshal depth is 2000 and Windows is set to 1000, split the difference and choose 1500 for WASI to be safe.
(cherry picked from commit 9711265)

Co-authored-by: Brett Cannon <brett@python.org>
brettcannon added a commit that referenced this issue Nov 1, 2022
@miss-islington @brettcannon
[3.11] gh-98925: Lower marshal recursion depth for WASI (GH-98938) (G…
39e0627 
…H-98979)

* gh-98925: Lower marshal recursion depth for WASI (GH-98938)

For wasmtime 2.0, the stack depth cost is 6% higher. This causes the default max `marshal` recursion depth to blow the stack.

As the default marshal depth is 2000 and Windows is set to 1000, split the difference and choose 1500 for WASI to be safe.
(cherry picked from commit 9711265)

Co-authored-by: Brett Cannon <brett@python.org>
@kieferrm kieferrm mentioned this issue Nov 6, 2022
Closed
39 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@brettcannon brettcannon

Labels
3.11 only security fixes 3.12 bugs and security fixes OS-wasi type-bug An unexpected behavior, bug, or error
Projects
No open projects
WASI to tier 2
Status: Done
Milestone
No milestone
Development

No branches or pull requests
1 participant
@brettcannon