Stream: git-wasmtime

Topic: wasmtime / PR #7602 winch: Ensure stack pointer for br_table

Wasmtime GitHub notifications bot (Nov 29 2023 at 17:31):

saulecabrera opened PR #7602 from saulecabrera:winch-ensure-sp-for-br-table to bytecodealliance:main:

Follow up to:

https://github.com/bytecodealliance/wasmtime/pull/7547

In which I overlooked this change and the fuzzer found an issue with the following program:

(module
  (func (export "") (result i32)
    block (result i32)
       i32.const 0
    end
    i32.const 0
    i32.const 0
    br_table 0
  )
)

This commit ensures that the stack pointer is correctly positioned when emitting br_table.

We can't know for sure which branch will be taken, but since all branches must share the same type information, we can be certain that the expectations regarding the stack pointer are the same and thus can we use the default target in order to ensure the correct placement.

This change also adds some missing spec test cases for br_table.

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Nov 29 2023 at 17:31):

saulecabrera requested abrown for a review on PR #7602.

Wasmtime GitHub notifications bot (Nov 29 2023 at 17:31):

saulecabrera requested wasmtime-compiler-reviewers for a review on PR #7602.

Wasmtime GitHub notifications bot (Nov 29 2023 at 17:31):

saulecabrera requested wasmtime-core-reviewers for a review on PR #7602.

Wasmtime GitHub notifications bot (Nov 29 2023 at 17:31):

saulecabrera requested pchickey for a review on PR #7602.

Wasmtime GitHub notifications bot (Nov 29 2023 at 17:32):

saulecabrera requested elliottt for a review on PR #7602.

Wasmtime GitHub notifications bot (Nov 29 2023 at 17:46):

saulecabrera updated PR #7602.

Wasmtime GitHub notifications bot (Nov 29 2023 at 18:44):

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

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:

• saulecabrera: winch

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

Learn more.
</details>

Wasmtime GitHub notifications bot (Nov 29 2023 at 19:38):

elliottt submitted PR review:

Looks good to me!

As an aside, is there a good way that we can be confident that we've flushed out any remaining stack balance issues? Perhaps fuzzing is enough?

Wasmtime GitHub notifications bot (Nov 29 2023 at 19:43):

saulecabrera commented on PR #7602:

As an aside, is there a good way that we can be confident that we've flushed out any remaining stack balance issues? Perhaps fuzzing is enough?

We've been locally fuzzing after each new change; I think that's probably the best way to ensure that we've addressed all the stack balancing issues (or any other issues really), given that many of the edge cases that we've found are not covered through the spec tests.

Wasmtime GitHub notifications bot (Nov 29 2023 at 19:46):

saulecabrera commented on PR #7602:

Perhaps, we could also add additional run tests with these edge cases, that way they'll break, at runtime, even if they successfully compile. I'll remove this one from the queue momentarily and add an additional run test.

Wasmtime GitHub notifications bot (Nov 29 2023 at 19:47):

saulecabrera edited a comment on PR #7602:

Perhaps, we could also add additional run tests with these edge cases, that way they'll break, at runtime, even if they successfully compile when something goes wrong. I'll remove this one from the queue momentarily and add an additional run test.

Wasmtime GitHub notifications bot (Nov 29 2023 at 20:00):

saulecabrera updated PR #7602.

Wasmtime GitHub notifications bot (Nov 29 2023 at 20:01):

saulecabrera has enabled auto merge for PR #7602.

Wasmtime GitHub notifications bot (Nov 29 2023 at 20:41):

saulecabrera merged PR #7602.

Last updated: Dec 23 2024 at 12:05 UTC