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

Adjust fuel consumption to be empty when fuel is at 0 rather than 1 #5013

Merged
merged 1 commit into from
Oct 5, 2022

Conversation

itsrainy
Copy link
Contributor

@itsrainy itsrainy commented Oct 4, 2022

I ran into this when attempting to use set_fuel in a fuzz target (bytecodealliance/wasm-tools#769). set_fuel calls consume_fuel(0) in order to get the current amount of fuel in a Store. However, consume_fuel(0) will return an out of fuel error if there is 0 fuel remaining, which is the case with a new Store.

Switching the < to a <= also has the effect of making this work:

...
let mut store = Store::new(&engine, ());
store.add_fuel(1).unwrap();
store.consume_fuel(1).unwrap();

previously this would have returned an "out of fuel" error, which doesn't seem correct, though maybe there's some nuance I'm missing?

@itsrainy itsrainy requested a review from alexcrichton October 4, 2022 23:06
@github-actions github-actions bot added fuzzing Issues related to our fuzzing infrastructure wasmtime:api Related to the API of the `wasmtime` crate itself labels Oct 4, 2022
@github-actions
Copy link

github-actions bot commented Oct 4, 2022

Subscribe to Label Action

cc @fitzgen, @peterhuene

This issue or pull request has been labeled: "fuzzing", "wasmtime:api"

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

  • fitzgen: fuzzing
  • peterhuene: wasmtime:api

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

Learn more.

Co-authored-by: Jamey Sharp <jsharp@fastly.com>
@itsrainy itsrainy force-pushed the rainy/minor-fuel-changes branch from eebbda2 to 48b7bb9 Compare October 4, 2022 23:34
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me, thanks!

@alexcrichton alexcrichton merged commit 6d1bce9 into main Oct 5, 2022
@alexcrichton alexcrichton deleted the rainy/minor-fuel-changes branch October 5, 2022 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuzzing Issues related to our fuzzing infrastructure wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants