Wasmtime GitHub notifications bot (Mar 18 2024 at 22:55): Wasmtime GitHub notifications bot (Mar 18 2024 at 22:55):cfallin opened PR #8176 from cfallin:fix-pcc-imported-memories to bytecodealliance:main:
Exposed by a fuzzbug (https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=67429); rather than exclude from fuzzing, it seemed easier to just implement. We need to define a new memory type to describe the memory definition struct pointed to by vmctx, and set up points-to facts appropriately.
Stacked on top of #8174; only last commit is new.
Wasmtime GitHub notifications bot (Mar 18 2024 at 22:55):cfallin requested wasmtime-compiler-reviewers for a review on PR #8176.
Wasmtime GitHub notifications bot (Mar 18 2024 at 22:55):cfallin requested fitzgen for a review on PR #8176.
Wasmtime GitHub notifications bot (Mar 18 2024 at 22:56):cfallin requested wasmtime-core-reviewers for a review on PR #8176.
Wasmtime GitHub notifications bot (Mar 19 2024 at 00:48):cfallin updated PR #8176.
Wasmtime GitHub notifications bot (Mar 19 2024 at 00:49):cfallin edited PR #8176:
Exposed by a fuzzbug (https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=67429); rather than exclude from fuzzing, it seemed easier to just implement. We need to define a new memory type to describe the memory definition struct pointed to by vmctx, and set up points-to facts appropriately.
Wasmtime GitHub notifications bot (Mar 19 2024 at 03:35):alexcrichton commented on PR #8176:
I don't know enough here to really review what's going on, but could the pcc-related pieces get factored into their own functions/methods? Right now they're suffering quite a bit from a huge indentation due to being so nested so breaking them out might help with readability
Wasmtime GitHub notifications bot (Mar 19 2024 at 15:33):cfallin commented on PR #8176:
Perhaps; the original intent was to put the PCC logic alongside the CLIF generation because they "mirror" each other (both have largely the same case breakdown, so we'd mirror the control flow elsewhere, which seems more brittle). Happy to consider refactors, unfortunately don't have much time to do a big refactor now. @fitzgen and I cowrote the original; @fitzgen would you be willing to review the fuzzbug fix here?
Wasmtime GitHub notifications bot (Mar 19 2024 at 18:53):cfallin updated PR #8176.
Wasmtime GitHub notifications bot (Mar 19 2024 at 18:54):cfallin commented on PR #8176:
@alexcrichton On second look, I did a small refactor on some of the logic so hopefully it's at least a little easier to digest now -- let me know what you think. Thanks!
Wasmtime GitHub notifications bot (Mar 19 2024 at 19:57):alexcrichton commented on PR #8176:
Seems reasonable to me! Alas though I fear I know so little about how this is all supposed to work that I still can't really make heads or tails of it without diving much deeper into pcc stuff, so I'll leave @fitzgen to review
Wasmtime GitHub notifications bot (Mar 20 2024 at 15:11):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Mar 20 2024 at 15:39):fitzgen merged PR #8176.
Last updated: Dec 23 2024 at 12:05 UTC