Stream: git-wasmtime

Topic: wasmtime / PR #14297 winch: emit fuel before throwing exc...

view this post on Zulip Wasmtime GitHub notifications bot (Sep 08 2026 at 10:35):

macovedj opened PR #14297 from macovedj:winch-throw-fuel-emission to bytecodealliance:main:

Resolves https://github.com/bytecodealliance/wasmtime/issues/14292.
Looks like I hadn't accounted for fuel emission in the new throw and throw_ref instructions.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 08 2026 at 10:35):

macovedj requested cfallin for a review on PR #14297.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 08 2026 at 10:35):

macovedj requested wasmtime-compiler-reviewers for a review on PR #14297.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 08 2026 at 10:35):

macovedj requested wasmtime-core-reviewers for a review on PR #14297.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 08 2026 at 11:44):

github-actions[bot] added the label winch on PR #14297.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 08 2026 at 11:45):

github-actions[bot] commented on PR #14297:

Subscribe to Label Action

cc @saulecabrera

<details>
This issue or pull request has been labeled: "winch"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Sep 08 2026 at 18:25):

:thumbs_up: cfallin submitted PR review:

Thanks for the fix -- subjective design question below but I'm fine with this going in either way.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 08 2026 at 18:25):

:speech_balloon: cfallin created PR review comment:

I'm not a huge fan of the wildcard / non-exhaustive match here -- I understand why it's written this way (only a small minority of opcodes will need to emit fuel checks -- only the "control flow"-ish ones -- and a few hundred lines of SIMD ops here would clutter the logic) but I wonder if it would make sense to at least write a predicate somewhere called is_control_flow_or_call with an actually exhaustive match and then use it here? Basically, I wonder if we can leverage the type system to make sure we don't miss this in the future if/when new opcodes are added.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 09 2026 at 09:06):

:memo: macovedj submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 09 2026 at 09:06):

:speech_balloon: macovedj created PR review comment:

While evaluating this I noticed that there is similar logic in fuel_before_op in func_environ.rs, and was wondering if we had any interest in having a shared function for classifying the fuel action for both backends due to the non-exhaustive match over there. I went ahead and made a commit on a separate branch to see what you think. At the end of the day, the Operator enum is sourced from wasmparser and is non-exhaustive, so I created a macro that picks up instructions added there and that will produce compilation errors if there isn't a match branch for it. How does this feel?

view this post on Zulip Wasmtime GitHub notifications bot (Sep 10 2026 at 23:35):

:memo: cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 10 2026 at 23:35):

:speech_balloon: cfallin created PR review comment:

This unfortunately fell off my plate in prepping for a work trip and now I'm out tomorrow and all next week; I will be able to review this on Mon Sep 21 unless someone else wants to get to it first. Sorry!

view this post on Zulip Wasmtime GitHub notifications bot (Sep 11 2026 at 18:44):

:memo: cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 11 2026 at 18:44):

:speech_balloon: cfallin created PR review comment:

(Got a little time to look at this and want to unblock it)

I like the new approach but probably it's best to review it as a followup PR. I didn't realize that wasmparser defines Operator as non-exhaustive; I like the idea of locally defining an exhaustive variant to ensure we reason about new opcodes but it has semver implications and we want to be careful about returning a clean error (as the Wasm-to-CLIF translator does here) rather than failing to compile with a semver-compatible upgrade. So I suspect we could skip the OperatorKind and From impl, and instead have the full operator list and error on other opcodes seen at runtime. That'll still give us an indication when the testsuite starts to include new opcodes that we need to properly classify their fuel behavior.

In the meantime I'll merge this so we have a fuzzbug fix -- thanks again.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 11 2026 at 18:44):

cfallin added PR #14297 winch: emit fuel before throwing exceptions to the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 11 2026 at 19:11):

:check: cfallin merged PR #14297.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 11 2026 at 19:11):

cfallin removed PR #14297 winch: emit fuel before throwing exceptions from the merge queue.

Last updated: Sep 20 2026 at 18:08 UTC