Stream: git-wasmtime

Topic: wasmtime / PR #5794 Fix handling of jumps in bugpoint

Wasmtime GitHub notifications bot (Feb 15 2023 at 20:25):

elliottt opened PR #5794 from trevor/5792 to main:

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Feb 15 2023 at 20:27):

elliottt edited PR #5794 from trevor/5792 to main:

Fixes #5794.

The introduction of brif didn't take into account the branch mutation behavior in bugpoint, which led to a crash when trying to reduce a fuzz bug. This PR fixes the bug by handling block terminators that branch to a single destination as before.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Feb 15 2023 at 20:27):

elliottt edited PR #5794 from trevor/5792 to main:

Fixes #5794.

The introduction of brif didn't take into account the branch mutation behavior in bugpoint, which led to a crash when trying to reduce a fuzz bug. This PR fixes the bug by handling block terminators that branch to a single destination as before.

• [ ] Add the test case from #5794

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Feb 15 2023 at 20:28):

bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Feb 15 2023 at 21:06):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Feb 15 2023 at 21:06):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Feb 15 2023 at 21:06):

jameysharp created PR review comment:

There's a handy to_vec method on slices that I think you can use here:

        let branch_args = branch_dests[0].args_slice(&func.dfg.value_lists).to_vec();

Wasmtime GitHub notifications bot (Feb 15 2023 at 21:06):

jameysharp created PR review comment:

I think I'd have an easier time convincing myself that this is correct if it were structured a little differently. I believe it's correct as-is but I had to think hard to come to that conclusion.

// FIXME: maybe free the detached block param value list?
let block_params = func.dfg.detach_block_params(block).as_slice(&func.dfg.value_lists).to_vec();
assert_eq!(block_params.len(), branch_args.len());
for (block_param, arg) in block_params.into_iter().zip(branch_args) {
...

Wasmtime GitHub notifications bot (Feb 15 2023 at 21:42):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Feb 15 2023 at 21:42):

elliottt created PR review comment:

Nice!

Wasmtime GitHub notifications bot (Feb 15 2023 at 21:44):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Feb 15 2023 at 21:44):

elliottt created PR review comment:

I like this more -- I too had to stare a lot at the input to that for loop, and this refactoring cleans it up quite a bit!

Wasmtime GitHub notifications bot (Feb 15 2023 at 21:56):

elliottt updated PR #5794 from trevor/5792 to main.

Wasmtime GitHub notifications bot (Feb 15 2023 at 21:57):

elliottt edited PR #5794 from trevor/5792 to main:

Fixes #5794.

The introduction of brif didn't take into account the branch mutation behavior in bugpoint, which led to a crash when trying to reduce a fuzz bug. This PR fixes the bug by handling block terminators that branch to a single destination as before.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Feb 15 2023 at 21:59):

elliottt edited PR #5794 from trevor/5792 to main:

Fixes #5792.

The introduction of brif didn't take into account the branch mutation behavior in bugpoint, which led to a crash when trying to reduce a fuzz bug. This PR fixes the bug by handling block terminators that branch to a single destination as before.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Feb 15 2023 at 23:07):

elliottt merged PR #5794.

Last updated: Nov 22 2024 at 17:03 UTC