Wasmtime GitHub notifications bot (Dec 10 2024 at 19:53): Wasmtime GitHub notifications bot (Dec 10 2024 at 19:53):alexcrichton requested cfallin for a review on PR #9780.
Wasmtime GitHub notifications bot (Dec 10 2024 at 19:53):alexcrichton requested wasmtime-compiler-reviewers for a review on PR #9780.
Wasmtime GitHub notifications bot (Dec 10 2024 at 19:53):alexcrichton opened PR #9780 from alexcrichton:pulley-ungate-memory64 to bytecodealliance:main:
This commit is similar to https://github.com/bytecodealliance/wasmtime/pull/9779 in that it's removing a proposal from
the "known list of panicking features" for Pulley to allow more tests to
run on Pulley. This then fills out a few miscellaneous instructions to
get a full suite of tests passing in Pulley related to memory64 and
other instructions.
Wasmtime GitHub notifications bot (Dec 10 2024 at 19:53):alexcrichton requested fitzgen for a review on PR #9780.
Wasmtime GitHub notifications bot (Dec 10 2024 at 19:53):alexcrichton requested wasmtime-core-reviewers for a review on PR #9780.
Wasmtime GitHub notifications bot (Dec 10 2024 at 19:53):alexcrichton requested wasmtime-default-reviewers for a review on PR #9780.
Wasmtime GitHub notifications bot (Dec 10 2024 at 19:53):alexcrichton commented on PR #9780:
Note: currently stacked on https://github.com/bytecodealliance/wasmtime/pull/9779
Wasmtime GitHub notifications bot (Dec 10 2024 at 19:53):alexcrichton edited a comment on PR #9780:
note this is currently stacked on https://github.com/bytecodealliance/wasmtime/pull/9779
Wasmtime GitHub notifications bot (Dec 10 2024 at 20:21):alexcrichton updated PR #9780.
Wasmtime GitHub notifications bot (Dec 10 2024 at 21:44):github-actions[bot] commented on PR #9780:
Subscribe to Label Action
cc @fitzgen
<details>
This issue or pull request has been labeled: "cranelift", "pulley", "wasmtime:api", "wasmtime:config"Thus the following users have been cc'd because of the following labels:
- fitzgen: pulley
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 (Dec 10 2024 at 22:45):github-actions[bot] commented on PR #9780:
Label Messager: wasmtime:config
It looks like you are changing Wasmtime's configuration options. Make sure to
complete this check list:
[ ] If you added a new
Configmethod, you wrote extensive documentation for
it.<details>
Our documentation should be of the following form:
```text
Short, simple summary sentence.More details. These details can be multiple paragraphs. There should be
information about not just the method, but its parameters and results as
well.Is this method fallible? If so, when can it return an error?
Can this method panic? If so, when does it panic?
Example
Optional example here.
```</details>
[ ] If you added a new
Configmethod, or modified an existing one, you
ensured that this configuration is exercised by the fuzz targets.<details>
For example, if you expose a new strategy for allocating the next instance
slot inside the pooling allocator, you should ensure that at least one of our
fuzz targets exercises that new strategy.Often, all that is required of you is to ensure that there is a knob for this
configuration option in [wasmtime_fuzzing::Config][fuzzing-config] (or one
of its nestedstructs).Rarely, this may require authoring a new fuzz target to specifically test this
configuration. See [our docs on fuzzing][fuzzing-docs] for more details.</details>
[ ] If you are enabling a configuration option by default, make sure that it
has been fuzzed for at least two weeks before turning it on by default.[fuzzing-config]: https://github.com/bytecodealliance/wasmtime/blob/ca0e8d0a1d8cefc0496dba2f77a670571d8fdcab/crates/fuzzing/src/generators.rs#L182-L194
[fuzzing-docs]: https://docs.wasmtime.dev/contributing-fuzzing.html
<details>
To modify this label's message, edit the <code>.github/label-messager/wasmtime-config.md</code> file.
To add new label messages or remove existing label messages, edit the
<code>.github/label-messager.json</code> configuration file.</details>
Wasmtime GitHub notifications bot (Dec 11 2024 at 06:33):cfallin submitted PR review:
Last commit LGTM, with note about Spectre below (happy to discuss further re: how much of an issue and/or how or if to document this).
Wasmtime GitHub notifications bot (Dec 11 2024 at 06:33):cfallin created PR review comment:
I think we should make a note re: Spectre here --
trapnzinternally uses a branch and so one might expect there to be some exposure here. I think we're actually safe because the worst that happens is that the guest accesses an OOB address like0x1_0000_1000where0x1000is in-bounds, and gets its own in-bounds data; in other words, this check is layered on top of the actual bounds-check on the lower 32 bits, so there is still not any visibility outside the sandbox in the misspeculated path. But it's... worth noting, if only because the guest might in turn rely on OOBs not to speculatively read valid data (niche but possible).
Wasmtime GitHub notifications bot (Dec 11 2024 at 12:36):fitzgen submitted PR review:
LGTM modulo @cfallin's comment
Wasmtime GitHub notifications bot (Dec 11 2024 at 12:36):fitzgen created PR review comment:
Why were these asserts removed?
Wasmtime GitHub notifications bot (Dec 11 2024 at 15:21):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Dec 11 2024 at 15:21):alexcrichton created PR review comment:
They were basically a pain to maintain on 32-bit because I already had to modify the minimum check to use
1<<32rather thanabsolute_max(due toabsolute_maxbeing ausizeand not being able to represent1<<32) and then this PR uncovered an assertion in the next one where a 64-bit linear memory's maximum size far exceeds1<<32as well. Given that these are just debug asserts that are already basically checked in many other places it seemed best to just remove them instead of trying to contort ourselves to keep them in all portable conditions.
Wasmtime GitHub notifications bot (Dec 11 2024 at 15:25):alexcrichton updated PR #9780.
Wasmtime GitHub notifications bot (Dec 11 2024 at 15:25):alexcrichton has enabled auto merge for PR #9780.
Wasmtime GitHub notifications bot (Dec 11 2024 at 15:28):alexcrichton edited PR #9780:
This commit is similar to https://github.com/bytecodealliance/wasmtime/pull/9779 in that it's removing a proposal from
the "known list of panicking features" for Pulley to allow more tests to
run on Pulley. This then fills out a few miscellaneous instructions to
get a full suite of tests passing in Pulley related to memory64 and
other instructions.cc #9783
Wasmtime GitHub notifications bot (Dec 11 2024 at 16:05):alexcrichton merged PR #9780.
Last updated: Dec 23 2024 at 12:05 UTC