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

Add CI for WASI threads #1819

Merged
merged 1 commit into from
Dec 22, 2022
Merged

Add CI for WASI threads #1819

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 23 additions & 4 deletions .github/workflows/compilation_on_android_ubuntu.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ jobs:
os: [ubuntu-20.04, ubuntu-22.04]
wasi_sdk_release:
[
"https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-12/wasi-sdk-12.0-linux.tar.gz",
"https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-16/wasi-sdk-16.0-linux.tar.gz",
]
wabt_release:
[
Expand Down Expand Up @@ -299,7 +299,7 @@ jobs:
os: [ubuntu-20.04, ubuntu-22.04]
wasi_sdk_release:
[
"https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-12/wasi-sdk-12.0-linux.tar.gz",
"https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-16/wasi-sdk-16.0-linux.tar.gz",
]
wabt_release:
[
Expand All @@ -313,8 +313,8 @@ jobs:
run: |
cd /opt
sudo wget ${{ matrix.wasi_sdk_release }}
sudo tar -xzf wasi-sdk-12.0-*.tar.gz
sudo mv wasi-sdk-12.0 wasi-sdk
sudo tar -xzf wasi-sdk-16.0-*.tar.gz
sudo mv wasi-sdk-16.0 wasi-sdk

- name: download and install wabt
run: |
Expand All @@ -323,6 +323,17 @@ jobs:
sudo tar -xzf wabt-1.0.24-*.tar.gz
sudo mv wabt-1.0.24 wabt

- name: build wasi-libc (needed for wasi-threads)
run: |
git clone --branch wasi-sdk-17 https://github.com/WebAssembly/wasi-libc
Copy link
Contributor

Choose a reason for hiding this comment

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

Branch wasi-sdk-17 isn't found in the wasi-libc repo, seems that main branch is OK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a tag https://github.com/WebAssembly/wasi-libc/tree/wasi-sdk-17.

I'd prefer to pin the version/tag rather than using main. Unless we want to use the current latest commit from main and we checkout that specific one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I remember that the combination wasi-sdk-16 + main from wasi-libc was failing on Ubuntu.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK. So shall we merge the PR now? @loganek, are there any comments from you? Thanks.

Copy link
Contributor Author

@eloparco eloparco Dec 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I think so, to recap:

  • we're using wasi-sdk-16 because wasi-sdk-17 would require a newer version of libc wrt the one on Ubuntu-20.04 used in the pipeline (the alternative would be to update the Ubuntu image)
  • --export=malloc and --export=free are used here otherwise the pipeline would fail when executing the wasi-threads samples (the alternative would be to comment the assert in samples/wasi-threads/wasm-apps/no_pthread.c); the behavior will be fixed in a new PR

cd wasi-libc
make \
AR=/opt/wasi-sdk/bin/llvm-ar \
NM=/opt/wasi-sdk/bin/llvm-nm \
CC=/opt/wasi-sdk/bin/clang \
THREAD_MODEL=posix
working-directory: core/deps

- name: Build Sample [basic]
run: |
cd samples/basic
Expand Down Expand Up @@ -376,6 +387,14 @@ jobs:
exit $?
working-directory: ./samples/simple

- name: Build Sample [wasi-threads]
run: |
cd samples/wasi-threads
mkdir build && cd build
cmake -DWASI_SYSROOT=`pwd`/../../../core/deps/wasi-libc/sysroot ..
cmake --build . --config Release --parallel 4
./iwasm wasm-apps/no_pthread.wasm

spec_test:
needs: [build_iwasm, build_llvm_libraries, build_wamrc]
runs-on: ubuntu-20.04
Expand Down
27 changes: 23 additions & 4 deletions .github/workflows/compilation_on_macos.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ jobs:
#$AOT_BUILD_OPTIONS,
]
os: [macos-latest]
wasi_sdk_release: ["https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-12/wasi-sdk-12.0-macos.tar.gz"]
wasi_sdk_release: ["https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-16/wasi-sdk-16.0-macos.tar.gz"]
wabt_release: ["https://github.com/WebAssembly/wabt/releases/download/1.0.24/wabt-1.0.24-macos.tar.gz"]
steps:
- name: checkout
Expand Down Expand Up @@ -246,7 +246,7 @@ jobs:
strategy:
matrix:
os: [macos-latest]
wasi_sdk_release: ["https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-12/wasi-sdk-12.0-macos.tar.gz"]
wasi_sdk_release: ["https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-16/wasi-sdk-16.0-macos.tar.gz"]
wabt_release: ["https://github.com/WebAssembly/wabt/releases/download/1.0.24/wabt-1.0.24-macos.tar.gz"]
steps:
- name: checkout
Expand All @@ -256,8 +256,8 @@ jobs:
run: |
cd /opt
sudo wget ${{ matrix.wasi_sdk_release }}
sudo tar -xzf wasi-sdk-12.0-*.tar.gz
sudo mv wasi-sdk-12.0 wasi-sdk
sudo tar -xzf wasi-sdk-16.0-*.tar.gz
sudo mv wasi-sdk-16.0 wasi-sdk

- name: download and install wabt
run: |
Expand All @@ -266,6 +266,17 @@ jobs:
sudo tar -xzf wabt-1.0.24-*.tar.gz
sudo mv wabt-1.0.24 wabt

- name: build wasi-libc (needed for wasi-threads)
run: |
git clone --branch wasi-sdk-17 https://github.com/WebAssembly/wasi-libc
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

cd wasi-libc
make \
AR=/opt/wasi-sdk/bin/llvm-ar \
NM=/opt/wasi-sdk/bin/llvm-nm \
CC=/opt/wasi-sdk/bin/clang \
THREAD_MODEL=posix
working-directory: core/deps

- name: Build Sample [basic]
run: |
cd samples/basic
Expand Down Expand Up @@ -311,3 +322,11 @@ jobs:
cmake ..
cmake --build . --config Release --parallel 4
./hello

- name: Build Sample [wasi-threads]
run: |
cd samples/wasi-threads
mkdir build && cd build
cmake -DWASI_SYSROOT=`pwd`/../../../core/deps/wasi-libc/sysroot ..
cmake --build . --config Release --parallel 4
./iwasm wasm-apps/no_pthread.wasm
25 changes: 22 additions & 3 deletions .github/workflows/compilation_on_sgx.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ jobs:
os: [ubuntu-20.04]
wasi_sdk_release:
[
"https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-12/wasi-sdk-12.0-linux.tar.gz",
"https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-16/wasi-sdk-16.0-linux.tar.gz",
]
wabt_release:
[
Expand All @@ -258,8 +258,8 @@ jobs:
run: |
cd /opt
sudo wget ${{ matrix.wasi_sdk_release }}
sudo tar -xzf wasi-sdk-12.0-*.tar.gz
sudo mv wasi-sdk-12.0 wasi-sdk
sudo tar -xzf wasi-sdk-16.0-*.tar.gz
sudo mv wasi-sdk-16.0 wasi-sdk

- name: download and install wabt
run: |
Expand All @@ -268,6 +268,17 @@ jobs:
sudo tar -xzf wabt-1.0.24-*.tar.gz
sudo mv wabt-1.0.24 wabt

- name: build wasi-libc (needed for wasi-threads)
run: |
git clone --branch wasi-sdk-17 https://github.com/WebAssembly/wasi-libc
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

cd wasi-libc
make \
AR=/opt/wasi-sdk/bin/llvm-ar \
NM=/opt/wasi-sdk/bin/llvm-nm \
CC=/opt/wasi-sdk/bin/clang \
THREAD_MODEL=posix
working-directory: core/deps

- name: install SGX SDK and necessary libraries
run: |
mkdir -p /opt/intel
Expand Down Expand Up @@ -327,6 +338,14 @@ jobs:
cmake --build . --config Release --parallel 4
./hello

- name: Build Sample [wasi-threads]
run: |
cd samples/wasi-threads
mkdir build && cd build
cmake -DWASI_SYSROOT=`pwd`/../../../core/deps/wasi-libc/sysroot ..
cmake --build . --config Release --parallel 4
./iwasm wasm-apps/no_pthread.wasm

spec_test_default:
needs: [build_iwasm, build_llvm_libraries, build_wamrc]
runs-on: ubuntu-20.04
Expand Down
2 changes: 2 additions & 0 deletions samples/wasi-threads/wasm-apps/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ function (compile_sample SOURCE_FILE)
LINKER:--export=__data_end
LINKER:--shared-memory,--max-memory=1966080
LINKER:--export=wasi_thread_start
LINKER:--export=malloc
LINKER:--export=free
Copy link
Collaborator

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because I was getting this error without the additional exports:

[09:52:39:510 - 101150580]: Memory instantiate success.
[09:52:39:614 - 101150580]: Error: app heap is corrupted, if the wasm file is compiled by wasi-sdk-12.0 or higher version, please add -Wl,--export=malloc -Wl,--export=free to export malloc and free functions. If it is compiled by asc, please add --exportRuntime to export the runtime helpers.
[09:52:39:618 - 101150580]: thread manager error: failed to allocate aux stack space for new thread
[09:52:39:620 - 101150580]: Failed to spawn a new thread
[09:52:39:623 - 101150580]: /Users/eloparco/dev/forks/wasm-micro-runtime/core/iwasm/interpreter/wasm_interp_fast.c, line 3959, meet an exception Exception: app heap corrupted
Exception: app heap corrupted

Copy link
Collaborator

Choose a reason for hiding this comment

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

the aux stack allocation should be disabled for wasi-threads because it's libc's responsibility to allocate stack for new threads.

no_pthread.c seems broken. its wasi_thread_start should either update the stack pointer global or avoid using C stack.

Copy link
Contributor Author

@eloparco eloparco Dec 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

the aux stack allocation should be disabled for wasi-threads because it's libc's responsibility to allocate stack for new threads.

https://github.com/bytecodealliance/wasm-micro-runtime/blob/dev/wasi_threads/core/config.h#L172

no_pthread.c seems broken. its wasi_thread_start should either update the stack pointer global or avoid using C stack.

How do I use the app heap? if I don't export the malloc and free functions I get Error: app heap is corrupted

Copy link
Contributor

Choose a reason for hiding this comment

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

According to this,

app heap and libc heap: the heap to allocate memory for wasm app, note that app heap is created only when the malloc/free functions (or __new/__release functions for AssemblyScript) are not exported and runtime can not detect the libc heap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've read that one already, but if I don't export malloc and free I'm getting the corruption error

Copy link
Collaborator

Choose a reason for hiding this comment

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

the aux stack allocation should be disabled for wasi-threads because it's libc's responsibility to allocate stack for new threads.

https://github.com/bytecodealliance/wasm-micro-runtime/blob/dev/wasi_threads/core/config.h#L172

#1799 (comment)

no_pthread.c seems broken. its wasi_thread_start should either update the stack pointer global or avoid using C stack.

How do I use the app heap? if I don't export the malloc and free functions I get Error: app heap is corrupted

wamr doesn't need to (and shouldn't) use the heap (app heap or libc heap) at least for this purpose (stack allocation for wasi-threads) because the stack is managed inside the module itself. (usually by libc)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Threads implementation in WAMR is not fully completed yet. One of the things is to always (regardless of compilation flags) use on-heap aux stack allocation. I have change for that already but didn't test it yet; I'll make the change public beginning of next month when I'm back from holidays, until then I'd suggest waiting with merging this PR.

)
endfunction ()

Expand Down