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

Fix Wasm translator bug: end of toplevel frame is branched-to only for fallthrough returns. #2450

Merged
merged 1 commit into from
Nov 25, 2020

Conversation

cfallin
Copy link
Member

@cfallin cfallin commented Nov 25, 2020

This makes the value of state.reachable() inaccurate when observing at
the tail of functions (in the post-function hook) after an ordinary
return instruction.

@cfallin cfallin requested a review from fitzgen November 25, 2020 02:58
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:wasm labels Nov 25, 2020
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

I guess? I'll take your word for it.

Is it possible to include a regression test?

@cfallin
Copy link
Member Author

cfallin commented Nov 25, 2020

I'll see what I can whip up -- there currently aren't any direct unit tests for the state presented to the FuncEnvironment, though this does fix a test failure over in bytecodealliance/lucet#612.

…r fallthrough returns.

This makes the value of `state.reachable()` inaccurate when observing at
the tail of functions (in the post-function hook) after an ordinary
return instruction.
@cfallin
Copy link
Member Author

cfallin commented Nov 25, 2020

Added a few tests of reachability info, with some plumbing on the dummy environment.

@cfallin cfallin requested a review from fitzgen November 25, 2020 18:58
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for adding these test.

@fitzgen fitzgen merged commit 31bac3e into main Nov 25, 2020
@fitzgen fitzgen deleted the cfallin/fix-wasm-reachable branch November 25, 2020 21:09
julian-seward1 added a commit to mozilla-spidermonkey/wasmtime that referenced this pull request Dec 9, 2020
…s merging

in the test case):

commit 31bac3e
Merge: 93c1993 34d9931
Author: Nick Fitzgerald <[email protected]>
Date:   Wed Nov 25 13:09:05 2020 -0800

    Merge pull request bytecodealliance#2450 from bytecodealliance/cfallin/fix-wasm-reachable

    Fix Wasm translator bug: end of toplevel frame is branched-to only for fallthrough returns.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:wasm cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants