Skip to content

GH-130397: use __stack_high and __stack_low LLVM WASM attributes #131855

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

FFY00
Copy link
Member

@FFY00 FFY00 commented Mar 28, 2025

@hoodmane
Copy link
Contributor

This same approach would also work on emscripten but instead I think we're going the route of making pthread_getattr_np work so it can use the usual Linux code path.

@@ -429,6 +429,10 @@ int pthread_attr_destroy(pthread_attr_t *a)

#endif

#if defined(__wasi__) & _Py__has_attribute(weak)
extern __attribute__((weak)) unsigned char __stack_high;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be size_t, not char right?

@brettcannon
Copy link
Member

CI suggests this isn't enough to fix the issue.

Do we need to manually set the stack size, roll back the stack checking changes that are causing the issue, or something else?

@hoodmane
Copy link
Contributor

hoodmane commented Apr 4, 2025

I was just looking into this today and I think a big part of the problem is that _Py_get_machine_stack_pointer(); does not seem to grow very fast at all in the stack overflow tests. I adjusted _Py_ReachedRecursionLimit() to occasionally log the value it got from _Py_get_machine_stack_pointer() and then ran test_ast_recursion_limit to segfault.

Not only does it get nowhere close to exhausting the 5mb of shadow stack, the shadow stack pointer only seems to move up/down by about ~4000. I am not sure if you'll get the same results on WASI, but if this is true and not just some measurement artifact, the whole approach to stack checks might not be workable at all.

         soft_limit: 3807680
calls: 62 here_addr: 4305520
calls: 63 here_addr: 4305920
calls: 64 here_addr: 4305008
calls: 65 here_addr: 4303776
calls: 66 here_addr: 4302336
calls: 67 here_addr: 4301776
calls: 68 here_addr: 4302768
calls: 69 here_addr: 4303200
calls: 70 here_addr: 4303776
calls: 71 here_addr: 4305024
calls: 72 here_addr: 4305024
calls: 73 here_addr: 4305024
calls: 74 here_addr: 4305024
calls: 75 here_addr: 4305920
calls: 76 here_addr: 4305920
RangeError: Maximum call stack size exceeded

@python-cla-bot
Copy link

python-cla-bot bot commented Apr 6, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants