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

add another set of tests to examine "return from _start" #46

Merged
merged 2 commits into from
Aug 23, 2023

Conversation

yamt
Copy link
Contributor

@yamt yamt commented Jul 6, 2023

a return from _start is an equivalent of proc_exit(0) and thus should terminate other threads.
at least it's what wasi-libc as of today assumes.

cf. #21

(venv) spacetanuki% python3 test-runner/wasi_test_runner.py -t ../wasi-threads/test/testsuite -r ~/git/toywasm/test/wasi-testsuite-adapter.py
Test wasi_threads_exit_nonmain_wasi passed
Test wasi_threads_exit_main_busy passed
Test wasi_threads_return_main_wasi passed
Test wasi_threads_return_main_wasi_read passed
Test wasi_threads_return_main_block passed
Test wasi_threads_exit_main_wasi passed
Test wasi_threads_return_main_busy passed
Test wasi_threads_exit_nonmain_busy passed
Test wasi_threads_spawn passed
Test wasi_threads_exit_main_block passed
Test wasi_threads_exit_nonmain_wasi_read passed
Test wasi_threads_exit_nonmain_block passed
Test wasi_threads_exit_main_wasi_read passed

===== Test results =====
Runtime: toywasm v28.0.0
Suite: WASI threads proposal
  Total: 13
  Passed:  13
  Failed:  0
  Skipped: 0

Test suites: 1 passed, 0 total
Tests:       13 passed, 0 total
(venv) spacetanuki%

yamt added 2 commits July 6, 2023 12:20
a return from _start is an equivalent of proc_exit(0) and thus should
terminate other threads.
at least it's what wasi-libc as of today assumes.

cf. WebAssembly#21

```
(venv) spacetanuki% python3 test-runner/wasi_test_runner.py -t ../wasi-threads/test/testsuite -r ~/git/toywasm/test/wasi-testsuite-adapter.py
Test wasi_threads_exit_nonmain_wasi passed
Test wasi_threads_exit_main_busy passed
Test wasi_threads_return_main_wasi passed
Test wasi_threads_return_main_wasi_read passed
Test wasi_threads_return_main_block passed
Test wasi_threads_exit_main_wasi passed
Test wasi_threads_return_main_busy passed
Test wasi_threads_exit_nonmain_busy passed
Test wasi_threads_spawn passed
Test wasi_threads_exit_main_block passed
Test wasi_threads_exit_nonmain_wasi_read passed
Test wasi_threads_exit_nonmain_block passed
Test wasi_threads_exit_main_wasi_read passed

===== Test results =====
Runtime: toywasm v28.0.0
Suite: WASI threads proposal
  Total: 13
  Passed:  13
  Failed:  0
  Skipped: 0

Test suites: 1 passed, 0 total
Tests:       13 passed, 0 total
(venv) spacetanuki%
```
@yamt
Copy link
Contributor Author

yamt commented Jul 19, 2023

@abrown @loganek ping

@loganek
Copy link
Collaborator

loganek commented Jul 20, 2023

@yamt thanks a lot for the change, and really sorry for late review. I can see that return from _start is only equivalent to proc_exit if the return code is not 0 (https://github.com/WebAssembly/wasi-libc/blob/main/libc-bottom-half/crt/crt1-command.c#L51). Do you have any context about that?

@yamt
Copy link
Contributor Author

yamt commented Jul 21, 2023

I can see that return from _start is only equivalent to proc_exit if the return code is not 0 (https://github.com/WebAssembly/wasi-libc/blob/main/libc-bottom-half/crt/crt1-command.c#L51).

you can read the code as "if r != 0, we can replace proc_exit with a return"
if you want to return non-zero value, you need to use proc_exit explicitly.

Do you have any context about that?

see #21
this PR is the option b.

WebAssembly/wasi-libc#367 is option c.

@yamt
Copy link
Contributor Author

yamt commented Aug 18, 2023

@abrown @loganek ping (again)

@abrown
Copy link
Collaborator

abrown commented Aug 21, 2023

@yamt, I'll take a look this week.

Copy link
Collaborator

@loganek loganek left a comment

Choose a reason for hiding this comment

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

Those tests look fine to me. I think for those kind of tests it'd be good to extend wasi testsuite config file spec and implement WebAssembly/wasi-testsuite#42 so tests can be terminated gracefully on failure.

@abrown abrown merged commit c92e3f0 into WebAssembly:main Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants