Don’t panic when http handle function returns before invoking set#10256
Closed
primoly wants to merge 23 commits intobytecodealliance:mainfrom
Closed
Don’t panic when http handle function returns before invoking set#10256primoly wants to merge 23 commits intobytecodealliance:mainfrom
handle function returns before invoking set#10256primoly wants to merge 23 commits intobytecodealliance:mainfrom
Conversation
…ce#10261) fixes bytecodealliance#10257 Github now has a builtin choice for a blank issue, so it does not require a template anymore.
…liance#10247) * Winch: Add SIMD float arithmetic support for x64 with AVX * Add ensure_has_avx to v128_neg method
Member
|
Thanks! Could you add a test for this as well? |
* Bump MSRV to 1.83.0 This commit updates the minimum supported Rust version for Wasmtime and Cranelift to 1.83.0. This is coupled with Rust's release today of 1.85.0. Additionally the nightly version used in testing was updated to today's nightly too. prtest:full * Update path_rename test to work on Windows Looks like the Rust standard library and Windows now has enough support to act the same across all platforms. * Fix compile warning * Looks like windows has new error codes now too * This error is still different
* ci: generate a list of generated files This fix is necessary for Windows users who may be using absolute-path target directories: the previous solution, separating the paths by `:`, runs into issues with Windows absolute paths (e.g., `C:\...`). This change is similar to bytecodealliance#10266 but should avoid any further OS compatibility issues during a hypothetical cross-compilation. prtest:full * fix: debug string
Contributor
Author
|
I’ve added tests, but they are useless, as they pass even without the changes of this PR. Here a component that returns before calling set. Invoke with proxy.wat(component
(type (;0;)
(instance
(export (;0;) "incoming-request" (type (sub resource)))
(export (;1;) "response-outparam" (type (sub resource)))
)
)
(import "wasi:http/types@0.2.0" (instance (;0;) (type 0)))
(core module (;0;)
(type (;0;) (func (param i32 i32)))
(export "cm32p2|wasi:http/incoming-handler@0.2|handle" (func $handle))
(func $handle (;0;) (type 0) (param i32 i32)
return
)
)
(core instance (;0;) (instantiate 0))
(alias export 0 "incoming-request" (type (;1;)))
(type (;2;) (own 1))
(alias export 0 "response-outparam" (type (;3;)))
(type (;4;) (own 3))
(type (;5;) (func (param "request" 2) (param "response-out" 4)))
(alias core export 0 "cm32p2|wasi:http/incoming-handler@0.2|handle" (core func (;0;)))
(func (;0;) (type 5) (canon lift (core func 0)))
(alias export 0 "incoming-request" (type (;6;)))
(alias export 0 "response-outparam" (type (;7;)))
(component (;0;)
(import "import-type-incoming-request" (type (;0;) (sub resource)))
(import "import-type-response-outparam" (type (;1;) (sub resource)))
(import "import-type-incoming-request0" (type (;2;) (eq 0)))
(type (;3;) (own 2))
(import "import-type-response-outparam0" (type (;4;) (eq 1)))
(type (;5;) (own 4))
(type (;6;) (func (param "request" 3) (param "response-out" 5)))
(import "import-func-handle" (func (;0;) (type 6)))
(export (;7;) "incoming-request" (type 0))
(export (;8;) "response-outparam" (type 1))
(type (;9;) (own 7))
(type (;10;) (own 8))
(type (;11;) (func (param "request" 9) (param "response-out" 10)))
(export (;1;) "handle" (func 0) (func (type 11)))
)
(instance (;1;) (instantiate 0
(with "import-func-handle" (func 0))
(with "import-type-incoming-request" (type 6))
(with "import-type-response-outparam" (type 7))
(with "import-type-incoming-request0" (type 1))
(with "import-type-response-outparam0" (type 3))
)
)
(export (;2;) "wasi:http/incoming-handler@0.2.0" (instance 1))
) |
Member
|
I think the difference between binaries is For the test here could the output of |
…ytecodealliance#10260) * x64: Use priority overlap in new instruction lowerings Use overlapping priorities when the only difference is in the type being matched on, and otherwise use priorities to disambiguate the arms of `GprMemImm` and the various possibilities of extraction. * x64: ensure 8-bit sign extensions have higher priorities In cases where an 8-bit immediate is used, x64 can emit a shorter encoding and rely on the CPU to sign extend it. Several Cranelift tests rely on this (as they should--this improves code size). The previous commit breaks the previous priorities; this change restores them, ensuring we use the smallest encoding when possible. * x64: remove old emission replaced by new assembler We expect the new assembler rules to prevent us from reaching the old emission rules. This change removes the old rules completely. * x64: re-prioritize based review, add line breaks * review: remove `is_mem` extractor for now --------- Co-authored-by: Alex Crichton <alex@alexcrichton.com>
…ytecodealliance#10201) * winch(aarch64): Correct treatment for stores and other trapping ops This commit is one more in the series of executing spec tests for aarch64. It's mostly a small follow-up to bytecodealliance#10146, in which I omitted contextualizing the memory flags for stores as well as ensuring that the SP is aligned when emitting other trapping instructions like `checked_uadd`. * Add note around why SP alignment is needed in `checked_uadd`
alexcrichton
approved these changes
Feb 24, 2025
For the error from `handle_request` use `e.context(...)` instead of stringifying `e` into a message. Additionally when logging a guest error use `:?` instead of `:#?` to give a more standard `anyhow` rendering of the error.
* Upgrade Windows builder to `windows-2025` This is an attempt to address bytecodealliance#10289 and unblock the upgrade of Wasmtime in the wasmtime-go bindings. Honestly I'm lost in the number of MinGW bugs we're dodging at this point. Regardless though this is something that will need to be done at some point anyway and theoretically shouldn't cause any other regressions, so I figured I might as well go ahead and do this and hopefully fix some MinGW issues while I'm at it. * Try a new way of getting windows cpu information prtest:full * Another attempt
Member
|
@primoly would you like some help in debugging the CI failure? |
Contributor
Author
|
I think I just broke the PR. I’ll open a new one. I don’t understand how Git works, sorry ._. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
wasmtime servecurrently presumes that a failure to callresponse-outparam::setinsidehandlemeans the component trapped. But it can also happen by returning before invokingset. This PR accommodates for that.