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.

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

gh-84461: Fix ctypes and test_ctypes on Emscripten #94142

Merged
merged 2 commits into from Jun 24, 2022
Merged

gh-84461: Fix ctypes and test_ctypes on Emscripten #94142

merged 2 commits into from Jun 24, 2022

Conversation

tiran
Copy link
Member

@tiran tiran commented Jun 22, 2022 •
edited by bedevere-bot

  • c_longlong and c_longdouble need experimental WASM bigint.
  • Skip tests that need threading
  • Define CTYPES_MAX_ARGCOUNT for Emscripten. According to tests 122
    args is the maximum value.

@tiran tiran force-pushed the gh-84461-ctypes-tests branch from 9f4e184 to 2fb9f9d Compare Jun 22, 2022
@tiran tiran mentioned this pull request Jun 22, 2022
Closed
@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Jun 22, 2022

According to tests 122 args is the maximum value.

Increased to 1000 after: emscripten-core/emscripten#16658

@tiran
Copy link
Member Author

@tiran tiran commented Jun 22, 2022

According to tests 122 args is the maximum value.

Increased to 1000 after: emscripten-core/emscripten#16658

I'm using emcc 3.1.12 and libffi-emscripten 2022-04-03. Tests are failing for any value > 122. What am I missing?

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Jun 22, 2022

Try the following: make a file called convert_to_wasm.js containing the following:

convertJsFunctionToWasm(function(){}, "i".repeat(999));

then invoke emcc as follows:

emcc -lc --post-js convert_to_wasm.js -o main.js

then node main.js.

After emsdk install 3.1.8 I see:

CompileError: WebAssembly.Module(): unknown section code #0x7f @+248
    at convertJsFunctionToWasm (/home/hood/Documents/programming/tmptest/main.js:442:16)

After emsdk install 3.1.9 or version 3.1.12 I see no output and it exits with status 0. You sure you are using emcc 3.1.12?

@tiran
Copy link
Member Author

@tiran tiran commented Jun 23, 2022

Your example script works fine with my EMSDK. However libffi still does not accept more than 122 arguments:

# node --experimental-wasm-bigint python.js -c "import sys; print(sys._emscripten_info)"
sys._emscripten_info(emscripten_version=(3, 1, 12), runtime='Node.js v14.18.2', pthreads=False, shared_memory=False)

# node --experimental-wasm-bigint python.js -c "import _ctypes; print(_ctypes.CTYPES_MAX_ARGCOUNT)"
122

# node --experimental-wasm-bigint python.js -m test -v test_ctypes -m test_callback_too_many_args
warning: unsupported syscall: __syscall_prlimit64

== CPython 3.12.0a0 (heads/main-dirty:15bfabd1aab, Jun 23 2022, 07:16:07) [Clang 15.0.0 (https://github.com/llvm/llvm-project d58cc0839ee5935e649dbbc18c89
== Emscripten-3.1.12-wasm32-32bit little-endian
== cwd: /python-wasm/cpython/builddir/emscripten-node/build/test_python_99092æ
== CPU count: 1
== encodings: locale=UTF-8, FS=utf-8
0:00:00 load avg: 0.10 Run tests sequentially
0:00:00 load avg: 0.10 [1/1] test_ctypes
test_callback_too_many_args (test.test_ctypes.test_callbacks.SampleCallbacksTestCase.test_callback_too_many_args) ... ok

----------------------------------------------------------------------
Ran 1 test in 0.014s

OK

== Tests result: SUCCESS ==

After rebuilding with #define CTYPES_MAX_ARGCOUNT 123

# node --experimental-wasm-bigint python.js -c "import _ctypes; print(_ctypes.CTYPES_MAX_ARGCOUNT)"
123
# node --experimental-wasm-bigint python.js -m test -v test_ctypes -m test_callback_too_many_args
warning: unsupported syscall: __syscall_prlimit64

== CPython 3.12.0a0 (heads/main-dirty:15bfabd1aab, Jun 23 2022, 07:13:36) [Clang 15.0.0 (https://github.com/llvm/llvm-project d58cc0839ee5935e649dbbc18c89
== Emscripten-3.1.12-wasm32-32bit little-endian
== cwd: /python-wasm/cpython/builddir/emscripten-node/build/test_python_515224æ
== CPU count: 1
== encodings: locale=UTF-8, FS=utf-8
0:00:00 load avg: 0.10 Run tests sequentially
0:00:00 load avg: 0.10 [1/1] test_ctypes
test_callback_too_many_args (test.test_ctypes.test_callbacks.SampleCallbacksTestCase.test_callback_too_many_args) ... ERROR

======================================================================
ERROR: test_callback_too_many_args (test.test_ctypes.test_callbacks.SampleCallbacksTestCase.test_callback_too_many_args)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/python-wasm/cpython/Lib/test/test_ctypes/test_callbacks.py", line 299, in test_callback_too_many_args
    cb = proto(func)
         ^^^^^^^^^^^
RuntimeError: ffi_prep_cif failed with 1

----------------------------------------------------------------------
Ran 1 test in 0.004s

FAILED (errors=1)

@tiran
Copy link
Member Author

@tiran tiran commented Jun 23, 2022

Found the culprit: https://github.com/hoodmane/libffi-emscripten/blob/008d7aafde297703eb2f259c969aebf301b01a6d/src/wasm32/ffi.c#L105-L108

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Jun 23, 2022

Oops. Will fix!

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Jun 23, 2022

Okay I made a commit increasing MAX_ARGS to 1000, if you use tag 2022-06-23 it will work.

tiran added 2 commits Jun 24, 2022
@tiran
pythongh-84461: Fix ctypes and test_ctypes on Emscripten
6914430 
- c_longlong and c_longdouble need experimental WASM bigint.
- Skip tests that need threading
- Define ``CTYPES_MAX_ARGCOUNT`` for Emscripten. According to tests 122
  args is the maximum value.
@tiran
Supports 1000 args with libffi-emscripten 2022-06-23
bf53851
@tiran tiran force-pushed the gh-84461-ctypes-tests branch from 2fb9f9d to bf53851 Compare Jun 24, 2022
@tiran
Copy link
Member Author

@tiran tiran commented Jun 24, 2022

Okay I made a commit increasing MAX_ARGS to 1000, if you use tag 2022-06-23 it will work.

Not it works as expected, thanks!

@tiran tiran merged commit 8625802 into python:main Jun 24, 2022
12 checks passed
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Jun 24, 2022

Thanks @tiran for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@tiran tiran deleted the gh-84461-ctypes-tests branch Jun 24, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 24, 2022
@tiran @miss-islington
pythongh-84461: Fix ctypes and test_ctypes on Emscripten (pythonGH-94142
44e4a03 
)

- c_longlong and c_longdouble need experimental WASM bigint.
- Skip tests that need threading
- Define ``CTYPES_MAX_ARGCOUNT`` for Emscripten. libffi-emscripten 2022-06-23 supports up to 1000 args.
(cherry picked from commit 8625802d854ec0152177a6ff0ac092e0e3ff98a5)

Co-authored-by: Christian Heimes <christian@python.org>
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Jun 24, 2022

GH-94211 is a backport of this pull request to the 3.11 branch.

miss-islington added a commit that referenced this issue Jun 24, 2022
@miss-islington @tiran
gh-84461: Fix ctypes and test_ctypes on Emscripten (GH-94142)
4d2c972 
- c_longlong and c_longdouble need experimental WASM bigint.
- Skip tests that need threading
- Define ``CTYPES_MAX_ARGCOUNT`` for Emscripten. libffi-emscripten 2022-06-23 supports up to 1000 args.
(cherry picked from commit 8625802)

Co-authored-by: Christian Heimes <christian@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tiran @hoodmane @miss-islington @bedevere-bot