-
Notifications
You must be signed in to change notification settings - Fork 1.6k
infinite loop test #6064
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
base: ripple/wasmi-host-functions
Are you sure you want to change the base?
infinite loop test #6064
Conversation
2227d15 to
dda7adc
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## ripple/wasmi-host-functions #6064 +/- ##
=============================================================
- Coverage 79.1% 79.1% -0.0%
=============================================================
Files 827 827
Lines 70993 70993
Branches 8304 8302 -2
=============================================================
- Hits 56133 56128 -5
- Misses 14860 14865 +5 🚀 New features to boost your workflow:
|
dda7adc to
182e542
Compare
182e542 to
ea4d668
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a test to verify that WebAssembly infinite loops are properly detected and handled. The test expects infinite loop execution to be caught and return a tecFAILED_PROCESSING error efficiently.
- Adds
testInfiniteLoop()test function with embedded WASM bytecode containing an infinite loop - Integrates the new test into the test suite execution
- Tests that infinite loops fail appropriately with
tecFAILED_PROCESSINGerror code
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
| return x; | ||
| } | ||
| */ | ||
| static std::string const infiniteWasmHex = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be in fixtures.cpp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left it here because it's only used once. Would you prefer it still go into fixtures?
| } | ||
|
|
||
| void | ||
| testInfiniteLoop() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to put it next to recursion too deep test. It is not so unique test to put it into dedicated function.
| using namespace test::jtx; | ||
| Env env{*this}; | ||
|
|
||
| /* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be in fixtures sources. here you can leave source file name
High Level Overview of Change
This PR adds a simple infinite loop test. It passes in negligible time.
Note: this PR shouldn't be merged until the Wasmi changes are added to the base branch. This test has been verified locally against that branch, but CI isn't passing yet.
Context of Change
Testing
Type of Change
API Impact
N/A
Test Plan
Added a test. It passed efficiently.