Stream: git-wasmtime

Topic: wasmtime / PR #6156 cranelift-interpreter: Propagate trap...

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 18:23):

afonso360 opened PR #6156 from afonso360:interp-call-trap to bytecodealliance:main:

:wave: Hey,

Fuzzgen found a bug in the interpreter (#6155) where if a function called another function and the child function trapped, the interpreter was panicking instead of propagating the trap correctly.

This PR fixes that by propagating the trap up instead of panicking.

Fixes: #6155

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 18:23):

afonso360 requested cfallin for a review on PR #6156.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 18:23):

afonso360 requested wasmtime-compiler-reviewers for a review on PR #6156.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 18:25):

afonso360 updated PR #6156.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 18:26):

afonso360 edited PR #6156:

:wave: Hey,

Fuzzgen found a bug in the interpreter (#6155) where if a function called another function and the child function trapped, the interpreter was panicking instead of propagating the trap correctly.

This PR fixes that by propagating the trap up instead of panicking.

Fixes: #6155

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 18:27):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 18:27):

bjorn3 created PR review comment:

Maybe remove the unwrap_return method to avoid this footgun for others?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 18:47):

afonso360 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 18:47):

afonso360 created PR review comment:

We use it a lot in the rest of the tests so it still has some use. But I'll add a cfg(test)

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 18:48):

afonso360 updated PR #6156.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 18:49):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 18:49):

bjorn3 created PR review comment:

The tests can wrap the expected return value in ControlFlow::Return I think if they avoid unwrap_return.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 18:49):

afonso360 edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 18:54):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 18:55):

cfallin has enabled auto merge for PR #6156.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 19:05):

afonso360 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 19:05):

afonso360 created PR review comment:

ControlFlow doesen't implement PartialEq so we can't just do something like assert_eq!(ControlFlow::Return(vec![...]), rets)

It doesn't implement PartialEq because it has some function pointers in some of the arms.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 19:09):

afonso360 edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2023 at 19:48):

cfallin merged PR #6156.

Last updated: Dec 23 2024 at 12:05 UTC