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
test_super_deep()
for test_call
triggers a stack overflow protection under a debug build for WASI
#111798
Comments
Lines 992 to 1026 in 804575b
Looks like the C stack blows up past a depth of 85. |
@markshannon what's the best way to update this test so it passes with a more limited stack size? Should I update |
I also saw this issue on Linux (Fedora). But I failed to reproduce it in a reliable way. cc @hroncok |
When Python is built in debug mode, We already had issues with other tests which rely on how much stack memory is available: https://pythondev.readthedocs.io/unstable_tests.html#unlimited-recursion Well, I suppose that here the purpose of the test is to prove that Python no longer consumes any stack memory. But maybe this assumption is wrong when Python is built in debug mode? I propose to reduce the maximum depth is |
I can reproduce the issue by building Python with
Output:
I tested |
Using |
Measure with c_limit=90 py_limit=1,000 recursion_limit=100,000 on Fedora 39 (gcc 13.2.1 on x86-64):
I suppose that the fact that Script to measure import _testcapi
import sys
sys.setrecursionlimit(100_000)
py_limit = 1000
c_limit = 90
lowest = None
def py_recurse(n, m):
global lowest
sp = _testcapi.stack_pointer()
if lowest is None or sp < lowest:
lowest = sp
if n >= 1:
py_recurse(n-1, m)
else:
c_py_recurse(m-1)
def c_py_recurse(m):
global lowest
sp = _testcapi.stack_pointer()
if lowest is None or sp < lowest:
lowest = sp
if m >= 1:
_testcapi.pyobject_vectorcall(py_recurse, (py_limit, m), ())
def measure(func, depth):
global lowest
lowest = None
high = _testcapi.stack_pointer()
func(depth)
low = lowest
per_call = (high - low) / depth
print(f"{func.__name__}: {per_call:.1f} bytes/call "
f"-- measured with {c_limit=:,} {py_limit=:,} "
f"recursion_limit={sys.getrecursionlimit():,}")
measure(c_py_recurse, c_limit) |
Latest major change related to recursion:
This change added test_super_deep() and skips it on s390x architecture. |
Reenable test_call.test_super_deep() on s390x architecture.
Reenable test_call.test_super_deep() on the s390x architecture.
Reenable test_call.test_super_deep() on the s390x architecture.
I'm personally tempted to skip this on debug builds under WASI since 85 is not a reasonable stack depth for |
I submitted 5 debug builds and 5 normal builds to the Fedora builds system. All normal builds succeeded on each architecture. All debug builds failed on x86_64/aarch64/ppc64le and succeeded on i686/s390x. All failures were |
@markshannon could you shed some light as to the purpose of the failing test so we can update it appropriately? This no longer seems to be a WebAssembly issue. |
Maybe we should skip the test on all debug builds? |
Did you mean on the debug builds? |
My PR #112124 does the opposite: it enables again I prefer to reduce limits rather than skipping tests on debug builds. Most of our CIs are running tests on Python built in debug mode. Also, if a test crash on a debug build, it also means that end users can be affected by the issue. My change should fix the crash on deep recursive C function calls. |
Yes. 😅 Fixed my comment. |
My PR adds |
* Run again test_ast_recursion_limit() on WASI platform. * Add _testinternalcapi.get_c_recursion_remaining(). * Fix test_ast and test_sys_settrace: test_ast_recursion_limit() and test_trace_unpack_long_sequence() now adjust the maximum recursion depth depending on the the remaining C recursion.
* Run again test_ast_recursion_limit() on WASI platform. * Add _testinternalcapi.get_c_recursion_remaining(). * Fix test_ast and test_sys_settrace: test_ast_recursion_limit() and test_trace_unpack_long_sequence() now adjust the maximum recursion depth depending on the the remaining C recursion.
On Fedora, I cannot reproduce the test_call crash on Python 3.12 when I build Python with |
@brettcannon: How do you build Python for WASI in debug mode? I made the following quick & dirty changes: diff --git a/Tools/wasm/wasm_assets.py b/Tools/wasm/wasm_assets.py
index ffa5e303412..f198782f48f 100755
--- a/Tools/wasm/wasm_assets.py
+++ b/Tools/wasm/wasm_assets.py
@@ -99,6 +99,7 @@
"_sysconfigdata__emscripten_wasm32-emscripten",
"_sysconfigdata__wasi_wasm32-wasi",
"_sysconfigdata__wasi_wasm64-wasi",
+ "_sysconfigdata_d_wasi_wasm32-wasi",
)
diff --git a/Tools/wasm/wasm_build.py b/Tools/wasm/wasm_build.py
index c0b9999a5da..1cb7b510597 100755
--- a/Tools/wasm/wasm_build.py
+++ b/Tools/wasm/wasm_build.py
@@ -329,7 +329,7 @@ def _check_wasi() -> None:
# workaround for https://github.com/python/cpython/issues/95952
"HOSTRUNNER": (
"wasmtime run "
- "--env PYTHONPATH=/{relbuilddir}/build/lib.wasi-wasm32-{version}:/Lib "
+ "--env PYTHONPATH=/{relbuilddir}/build/lib.wasi-wasm32-{version}-pydebug:/Lib "
"--mapdir /::{srcdir} --"
),
"PATH": [WASI_SDK_PATH / "bin", os.environ["PATH"]],
@@ -475,7 +475,7 @@ def configure_cmd(self) -> List[str]:
# use relative path, so WASI tests can find lib prefix.
# pathlib.Path.relative_to() does not work here.
configure = os.path.relpath(CONFIGURE, self.builddir)
- cmd = [configure, "-C"]
+ cmd = [configure, "-C", "--with-pydebug"]
platform = self.host.platform
if platform.configure_wrapper:
cmd.insert(0, os.fspath(platform.configure_wrapper)) I'm using On the current main branch (at
|
I'm developing https://github.com/brettcannon/cpython/blob/wasi.py/Tools/wasm/wasi.py to make this all easier, so that's how I have been doing this personally.
That works and essentially what my script does.
I'm still seeing a failure under WASI SDK 20 and wasmtime 14 after merging up to 762eb58 . |
Alright, i'm not surprised to see a different behavior on this specific recursion stress test depending on the compiler version and flags. I'm always using the same container, i don't know which versions are used inside. |
It is to test that the interpreter doesn't crash when the recursion limit is set to some silly number. |
Tools/wasm/README.md says:
I'm only using So the problem is that the test_call fails if Python is built in debug mode with WASI SDK 20 and wasmtime 14? |
Neither do I, hence why we install the requisite tools into the devcontainer.
I'm working on it (and it's technically WASI-SDK 16+ and any version of wasmtime that fully implements WASI preview1, but in general just some recent version).
Correct as that's how I discovered the failure, but I don't know how strict those versions are in terms of causing the issue. But since those are the common/preferred tools for working on WASI and (nearly) the latest versions it's at least worth my time to try and make sure they work (and it's what the WASI buildbots use today, so fixing this means I can make them do pydebug builds). |
Tested using WASI-SDK 20 and wasmtime 14.
Linked PRs
The text was updated successfully, but these errors were encountered: