Stream: git-wasmtime

Topic: wasmtime / issue #5612 fix issue 5569.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 21 2023 at 10:59):

afonso360 commented on issue #5612:

When I investigated that issue I think it was due to a veneer being inserted right in the middle of a load_constant sequence, that uses fixed offset jumps. Its likely that the other fixes changed the generated code enough that that particular test case no longer triggers that same issue.

I'll run the fuzzer again to find another counterexample.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 22 2023 at 01:11):

yuyang-ok commented on issue #5612:

Yeah, sounds reasonable. @afonso360

view this post on Zulip Wasmtime GitHub notifications bot (Jan 22 2023 at 02:10):

yuyang-ok commented on issue #5612:

I think I have locate the problem.

Here I put a Minstrel::RawData.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L80

And when emit I try to emit_island.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L473
Those load constant instruction must execute together.

I also think this struct LoadConstant should only used in emit stage not lower stage.
I test at 299be327d.

<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897473-6e6af2b9-1f17-4a0f-90b9-389cfd2b4478.png">

<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897490-e992da26-1899-4bbf-9a05-2e43ad11b3e1.png">

view this post on Zulip Wasmtime GitHub notifications bot (Jan 22 2023 at 02:11):

yuyang-ok commented on issue #5612:

I checked the code LoadConstant is only used at emit stage.Maybe I use this in lower stage too early.
I don't know who modify the code.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 22 2023 at 02:12):

yuyang-ok edited a comment on issue #5612:

I think I have locate the problem.

Here I put a Minstrel::RawData.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L80

And when emit I try to emit_island.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L470
Those load constant instruction must execute together.

I also think this struct LoadConstant should only used in emit stage not lower stage.
I test at 299be327d.

<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897473-6e6af2b9-1f17-4a0f-90b9-389cfd2b4478.png">

<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897490-e992da26-1899-4bbf-9a05-2e43ad11b3e1.png">

view this post on Zulip Wasmtime GitHub notifications bot (Jan 22 2023 at 02:15):

yuyang-ok deleted a comment on issue #5612:

I checked the code LoadConstant is only used at emit stage.Maybe I use this in lower stage too early.
I don't know who modify the code.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 22 2023 at 02:26):

yuyang-ok edited a comment on issue #5612:

I think I have locate the problem.

Here I put a Minstrel::RawData.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L80

And when emit I try to emit_island.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L470
Those load constant instruction must execute together.

I also think this struct LoadConstant should only used in emit stage not lower stage.
I test at commit 299be327d.

<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897473-6e6af2b9-1f17-4a0f-90b9-389cfd2b4478.png">

<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897490-e992da26-1899-4bbf-9a05-2e43ad11b3e1.png">

view this post on Zulip Wasmtime GitHub notifications bot (Jan 22 2023 at 21:16):

afonso360 commented on issue #5612:

That looks like it could fix the issue! I'm going to run the fuzzer for a while to check if it finds another counter example on this branch!

view this post on Zulip Wasmtime GitHub notifications bot (Jan 22 2023 at 23:34):

yuyang-ok edited a comment on issue #5612:

I think I have locate the problem.

Here I put a Minst::RawData.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L80

And when emit I try to emit_island.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L470
Those load constant instruction must execute together.

I also think this struct LoadConstant should only used in emit stage not lower stage.
I test at commit 299be327d.

<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897473-6e6af2b9-1f17-4a0f-90b9-389cfd2b4478.png">

<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897490-e992da26-1899-4bbf-9a05-2e43ad11b3e1.png">

view this post on Zulip Wasmtime GitHub notifications bot (Jan 22 2023 at 23:34):

yuyang-ok edited a comment on issue #5612:

I think I have locate the problem.

Here I put a Minst::RawData.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L80

And when emit I try to emit_island.
https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/riscv64/inst/emit.rs#L470
Those load constant instructions must execute together.

I also think this struct LoadConstant should only used in emit stage not lower stage.
I test at commit 299be327d.

<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897473-6e6af2b9-1f17-4a0f-90b9-389cfd2b4478.png">

<img width="1182" alt="image" src="https://user-images.githubusercontent.com/96557710/213897490-e992da26-1899-4bbf-9a05-2e43ad11b3e1.png">

view this post on Zulip Wasmtime GitHub notifications bot (Jan 23 2023 at 10:05):

afonso360 commented on issue #5612:

It ran the fuzzer overnight without crashing, It looks like it's fixed!

view this post on Zulip Wasmtime GitHub notifications bot (Jan 23 2023 at 10:11):

yuyang-ok commented on issue #5612:

@afonso360 I think this issue is fixed.:-)

view this post on Zulip Wasmtime GitHub notifications bot (Jan 23 2023 at 10:15):

yuyang-ok edited a comment on issue #5612:

@afonso360 I think this issue is fixed too.:-)

view this post on Zulip Wasmtime GitHub notifications bot (Jan 29 2023 at 06:58):

yuyang-ok commented on issue #5612:

@afonso360 What are we waiting for?

view this post on Zulip Wasmtime GitHub notifications bot (Jan 29 2023 at 14:57):

afonso360 commented on issue #5612:

Hey, someone else has to review this and merge it.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 30 2023 at 03:35):

yuyang-ok commented on issue #5612:

@afonso360 ok, thanks

Last updated: Nov 22 2024 at 16:03 UTC