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 wasm_runtime_detect_native_stack_overflow_size #3355

Merged
merged 21 commits into from
Apr 26, 2024

Conversation

yamt
Copy link
Collaborator

@yamt yamt commented Apr 25, 2024

  • Add a few API (per-host function override of native stack requirement #3325)
    wasm_runtime_detect_native_stack_overflow_size
    wasm_runtime_detect_native_stack_overflow
  • Adapt the runtime to use them
  • Adapt samples/native-stack-overflow to use them
  • Add a few missing overflow checks in the interpreters
  • build and run the sample on the CI

@yamt
Copy link
Collaborator Author

yamt commented Apr 25, 2024

windows failure looks like #2776

macos failure looks unrelated:

-- Check for working C compiler: /usr/bin/cc
CMake Error at /usr/local/Cellar/cmake/3.29.2/share/cmake/Modules/CMakeTestCCompiler.cmake:67 (message):
  The C compiler

    "/usr/bin/cc"

  is not able to compile a simple test program.

  It fails with the following output:

    Change Dir: '/Users/runner/work/wasm-micro-runtime/wasm-micro-runtime/product-mini/platforms/darwin/build/CMakeFiles/CMakeScratch/TryCompile-soOD6B'
    
    Run Build Command(s): /usr/local/Cellar/cmake/3.29.2/bin/cmake -E env VERBOSE=1 /usr/bin/make -f Makefile cmTC_bee20/fast
    sh: line 1:  6878 Killed: 9               /Applications/Xcode_14.2.app/Contents/Developer/usr/bin/xcodebuild -sdk '' -find make 2> /dev/null
    make: error: sh -c '/Applications/Xcode_14.2.app/Contents/Developer/usr/bin/xcodebuild -sdk '' -find make 2> /dev/null' failed with exit code 35072: (null) (errno=No such file or directory)
    xcode-select: Failed to locate 'make', requesting installation of command line developer tools.
    
    

  

  CMake will not be able to correctly generate this project.
Call Stack (most recent call first):
  CMakeLists.txt:6 (project)


-- Check for working C compiler: /usr/bin/cc - broken
-- Configuring incomplete, errors occurred!

@yamt yamt marked this pull request as ready for review April 25, 2024 10:26
@wenyongh
Copy link
Contributor

windows failure looks like #2776

macos failure looks unrelated:

-- Check for working C compiler: /usr/bin/cc
CMake Error at /usr/local/Cellar/cmake/3.29.2/share/cmake/Modules/CMakeTestCCompiler.cmake:67 (message):
  The C compiler

    "/usr/bin/cc"

  is not able to compile a simple test program.

  It fails with the following output:

    Change Dir: '/Users/runner/work/wasm-micro-runtime/wasm-micro-runtime/product-mini/platforms/darwin/build/CMakeFiles/CMakeScratch/TryCompile-soOD6B'
    
    Run Build Command(s): /usr/local/Cellar/cmake/3.29.2/bin/cmake -E env VERBOSE=1 /usr/bin/make -f Makefile cmTC_bee20/fast
    sh: line 1:  6878 Killed: 9               /Applications/Xcode_14.2.app/Contents/Developer/usr/bin/xcodebuild -sdk '' -find make 2> /dev/null
    make: error: sh -c '/Applications/Xcode_14.2.app/Contents/Developer/usr/bin/xcodebuild -sdk '' -find make 2> /dev/null' failed with exit code 35072: (null) (errno=No such file or directory)
    xcode-select: Failed to locate 'make', requesting installation of command line developer tools.
    
    

  

  CMake will not be able to correctly generate this project.
Call Stack (most recent call first):
  CMakeLists.txt:6 (project)


-- Check for working C compiler: /usr/bin/cc - broken
-- Configuring incomplete, errors occurred!

The two CIs were re-run and are OK now.

Copy link
Contributor

@wenyongh wenyongh left a comment

Choose a reason for hiding this comment

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

LGTM with minor issues

* Otherwise, raise a "native stack overflow" trap and returns false.
*
* Note: please do not expect a very strict detection. it's a good idea
* to give some margins. wasm_runtime_detect_native_stack_overflow itslf
Copy link
Contributor

Choose a reason for hiding this comment

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

Had better itself

* Ensure that the calling thread still has a reasonable amount of
* native stack (WASM_STACK_GUARD_SIZE bytes) available.
*
* If enough stack is left, returns true.
Copy link
Contributor

Choose a reason for hiding this comment

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

had better returns -> return?

* native stack (WASM_STACK_GUARD_SIZE bytes) available.
*
* If enough stack is left, returns true.
* Otherwise, raise a "native stack overflow" trap and returns false.
Copy link
Contributor

Choose a reason for hiding this comment

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

had better returns -> return?

@yamt yamt force-pushed the native-stack-overflow-wip2 branch from dd6e020 to e0eb25f Compare April 25, 2024 13:16
@yamt
Copy link
Collaborator Author

yamt commented Apr 25, 2024

force pushed to resolve a conflict with #3358

yamt added 20 commits April 26, 2024 16:00
this changes the accounting by one small function call.
i don't think it can cause real problems though.
This allows each native function to require more native stack.

cf. bytecodealliance#3325
To avoid:
LLVM ERROR: Only small, tiny and large code models are allowed on AArch64
as i have no good way to debug right now.

the failure seen on the CI:
```
====== Interpreter test1
 stack size   | fail?  | leak?  | exception
---------------------------------------------------------------------------
    0 - 24576 | failed |     ok | Exception: native stack overflow

====== Interpreter WAMR_DISABLE_HW_BOUND_CHECK=1 test1
 stack size   | fail?  | leak?  | exception
---------------------------------------------------------------------------
    0 -  3568 | failed |     ok | Exception: native stack overflow
 3568 - 24576 | failed |     ok | Exception: wasm operand stack overflow

====== AOT test1
 stack size   | fail?  | leak?  | exception
---------------------------------------------------------------------------
    0 - 24576 | failed |     ok | Exception: native stack overflow

====== AOT WAMR_DISABLE_HW_BOUND_CHECK=1 test1
 stack size   | fail?  | leak?  | exception
---------------------------------------------------------------------------
    0 -  4000 | failed |     ok | Exception: native stack overflow
 4000 - 24576 |     ok |     ok |
====== Interpreter test2
 stack size   | fail?  | leak?  | exception
---------------------------------------------------------------------------
    0 - 24576 | failed |     ok | Exception: native stack overflow

====== Interpreter WAMR_DISABLE_HW_BOUND_CHECK=1 test2
./run.sh: line 12: 14708 Segmentation fault: 11  out/native-stack-overflow.WAMR_DISABLE_HW_BOUND_CHECK out/wasm-apps/testapp.wasm ${NAME}
```
fix an infinite loop in consume_stack1
@yamt yamt force-pushed the native-stack-overflow-wip2 branch from e971376 to 6196f1c Compare April 26, 2024 07:00
@yamt
Copy link
Collaborator Author

yamt commented Apr 26, 2024

rebased after #3366

@wenyongh wenyongh merged commit 410ee58 into bytecodealliance:main Apr 26, 2024
669 checks passed
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 1, 2024
…3355)

- Add a few API (bytecodealliance#3325)
   ```c
   wasm_runtime_detect_native_stack_overflow_size
   wasm_runtime_detect_native_stack_overflow
   ```
- Adapt the runtime to use them
- Adapt samples/native-stack-overflow to use them
- Add a few missing overflow checks in the interpreters
- Build and run the sample on the CI
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 2, 2024
…3355)

- Add a few API (bytecodealliance#3325)
   ```c
   wasm_runtime_detect_native_stack_overflow_size
   wasm_runtime_detect_native_stack_overflow
   ```
- Adapt the runtime to use them
- Adapt samples/native-stack-overflow to use them
- Add a few missing overflow checks in the interpreters
- Build and run the sample on the CI

Signed-off-by: victoryang00 <victoryang00@ucsc.edu>
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
…3355)

- Add a few API (bytecodealliance#3325)
   ```c
   wasm_runtime_detect_native_stack_overflow_size
   wasm_runtime_detect_native_stack_overflow
   ```
- Adapt the runtime to use them
- Adapt samples/native-stack-overflow to use them
- Add a few missing overflow checks in the interpreters
- Build and run the sample on the CI
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
…3355)

- Add a few API (bytecodealliance#3325)
   ```c
   wasm_runtime_detect_native_stack_overflow_size
   wasm_runtime_detect_native_stack_overflow
   ```
- Adapt the runtime to use them
- Adapt samples/native-stack-overflow to use them
- Add a few missing overflow checks in the interpreters
- Build and run the sample on the CI

Signed-off-by: victoryang00 <victoryang00@ucsc.edu>
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
…3355)

- Add a few API (bytecodealliance#3325)
   ```c
   wasm_runtime_detect_native_stack_overflow_size
   wasm_runtime_detect_native_stack_overflow
   ```
- Adapt the runtime to use them
- Adapt samples/native-stack-overflow to use them
- Add a few missing overflow checks in the interpreters
- Build and run the sample on the CI

Signed-off-by: victoryang00 <victoryang00@ucsc.edu>
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request Jun 7, 2024
…3355)

- Add a few API (bytecodealliance#3325)
   ```c
   wasm_runtime_detect_native_stack_overflow_size
   wasm_runtime_detect_native_stack_overflow
   ```
- Adapt the runtime to use them
- Adapt samples/native-stack-overflow to use them
- Add a few missing overflow checks in the interpreters
- Build and run the sample on the CI

Signed-off-by: victoryang00 <victoryang00@ucsc.edu>
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.

None yet

2 participants