wasmtime / PR #1196 fix: do not panic on ref types while ... · git-wasmtime · Zulip Chat Archive

Stream: git-wasmtime

Topic: wasmtime / PR #1196 fix: do not panic on ref types while ...

Wasmtime GitHub notifications bot (Mar 05 2020 at 15:22):

bnjbvr requested bnjbvr for a review on PR #1196.

Wasmtime GitHub notifications bot (Mar 17 2020 at 11:20):

bnjbvr submitted PR Review.

Wasmtime GitHub notifications bot (Mar 19 2020 at 09:36):

csmoe updated PR #1196 from safepoint to master:

Closes #1156
r? @bnjbvr

Wasmtime GitHub notifications bot (Mar 19 2020 at 09:53):

bjorn3 submitted PR Review.

Wasmtime GitHub notifications bot (Mar 19 2020 at 09:53):

bjorn3 created PR Review Comment:

What about using references as function parameters, but not using them anywhere? This would skip them.

Wasmtime GitHub notifications bot (Mar 19 2020 at 09:53):

bjorn3 created PR Review Comment:

The error is reported during regalloc, not during the verifier pass.

Wasmtime GitHub notifications bot (Mar 19 2020 at 09:53):

bjorn3 submitted PR Review.

csmoe (Mar 19 2020 at 11:57):

@bjorn3 as you said, this check happens in regalloc, not verifier. So should I add an Error variant for it? https://github.com/bytecodealliance/wasmtime/blob/master/cranelift/codegen/src/result.rs#L10

bytecodealliance/wasmtime

Standalone JIT-style runtime for WebAssembly, using Cranelift - bytecodealliance/wasmtime

bjorn3 (Mar 19 2020 at 12:47):

I don't think that is necessary. Maybe move the error to the verifier though, as this is the kind of error emitted by the verifier.

Wasmtime GitHub notifications bot (Mar 20 2020 at 11:05):

bnjbvr submitted PR Review.

Wasmtime GitHub notifications bot (Mar 20 2020 at 11:05):

bnjbvr created PR Review Comment:

Good point! I think a few tests actually need to be added:

for function parameters

for EBB params

for value types in signature refs

Can you add the code and tests for this, please? Thanks!

Wasmtime GitHub notifications bot (Mar 20 2020 at 11:06):

bnjbvr submitted PR Review.

Wasmtime GitHub notifications bot (Mar 20 2020 at 11:06):

bnjbvr created PR Review Comment:

There should be an error annotation in this test, so it'd check that the verifier asserts because a ref type is used yet ref types aren't enabled, right?

Wasmtime GitHub notifications bot (Jun 25 2020 at 18:46):

alexcrichton edited PR #1196 from safepoint to main:

Closes #1156
r? @bnjbvr

Wasmtime GitHub notifications bot (Oct 11 2021 at 17:20):

cfallin closed without merge PR #1196.

Last updated: Nov 22 2024 at 17:03 UTC