Stream: git-wasmtime

Topic: wasmtime / issue #3144 threads: Addresses both out-of-bou...

Wasmtime GitHub notifications bot (Aug 04 2021 at 19:49):

alexcrichton opened issue #3144:

With the threads proposal atomic operations can be invalid with their addresses for two reasons: either they're out-of-bounds or they're misaligned. I'm not sure what the intention is in the official spec which of these traps should happen if both are true.

Currently the implementation of threads (after #3143) will sort of "nondeterministically" reveal either trap. The exact trap depends on the configuration of the runtime. The problem is that the heap_addr in cranelift doesn't actually verify the address, it just ensures that if it's accessed it's either guaranteed to segfault or it's guaranteed to be valid. In the "guaranteed to be valid" case we'd give an out-of-bounds trap, whereas in the "guaranteed to segfault" case we'd give a misalignment trap because the alignment check happens before the actual memory access.

We should probably clarify with the upstream specification which trap is intended, and we'll need to modify codegen to match this. My guess is that we'll want to guarantee that the misalignment trap happens first, and only properly aligned addresses are reported as out-of-bounds. That should allow us to have a boolean flag on the heap_addr clif instruction to check for alignment, and it'd add the two operands as the first thing to check the alignment, and then the overflow check afterwards tests whether the addition was correct.

Wasmtime GitHub notifications bot (Aug 04 2021 at 20:00):

alexcrichton commented on issue #3144:

Hm no nevermind I'm just gonna fix this to be alignment is checked first and then out-of-bounds.

Wasmtime GitHub notifications bot (Aug 04 2021 at 20:00):

alexcrichton closed issue #3144:

With the threads proposal atomic operations can be invalid with their addresses for two reasons: either they're out-of-bounds or they're misaligned. I'm not sure what the intention is in the official spec which of these traps should happen if both are true.

Currently the implementation of threads (after #3143) will sort of "nondeterministically" reveal either trap. The exact trap depends on the configuration of the runtime. The problem is that the heap_addr in cranelift doesn't actually verify the address, it just ensures that if it's accessed it's either guaranteed to segfault or it's guaranteed to be valid. In the "guaranteed to be valid" case we'd give an out-of-bounds trap, whereas in the "guaranteed to segfault" case we'd give a misalignment trap because the alignment check happens before the actual memory access.

We should probably clarify with the upstream specification which trap is intended, and we'll need to modify codegen to match this. My guess is that we'll want to guarantee that the misalignment trap happens first, and only properly aligned addresses are reported as out-of-bounds. That should allow us to have a boolean flag on the heap_addr clif instruction to check for alignment, and it'd add the two operands as the first thing to check the alignment, and then the overflow check afterwards tests whether the addition was correct.

Wasmtime GitHub notifications bot (Aug 04 2021 at 20:12):

cfallin commented on issue #3144:

:+1: that seems reasonable to me; the alignment check is a specific part of these instructions' semantics, and so it makes sense that we don't try to access the heap at all (from an semantics point of view anyway) if the address is misaligned.

Wasmtime GitHub notifications bot (Aug 04 2021 at 20:12):

cfallin edited a comment on issue #3144:

:+1: that seems reasonable to me; the alignment check is a specific part of these instructions' semantics, and so it makes sense that we don't try to access the heap at all (from a semantics point of view anyway) if the address is misaligned.

Last updated: Dec 23 2024 at 12:05 UTC