bytecodealliance / bytecodealliance/wasmtime
Cranelift: missing zero extension of i8,i16 causes undetermined behavior on x64
- Dominant language
- Rust
- Stars
- 18.6k
- Forks
- 1.8k
- Avg merge
- 1d 18h
- Merged PRs (30d)
- 126
Description
I have stumbled upon a rather nasty 'bug' this week.
When compiling IR like this:
```
function u0:0(i64) -> i64 system_v {
sig2 = (i64, i8) -> i64 system_v
fn2 = u0:112 sig2
block0(v0: i64):
v1 = icmp_imm ne v0, 0
v2 = call fn2(v0, v1)
return v2
}
```
you will get assembly that looks like this:
```asm
pushq %rbp
unwind PushFrameRegs { offset_upward_to_caller_sp: 16 }
movq %rsp, %rbp
unwind DefineNewFrame { offset_upward_to_caller_sp: 16, offset_downward_to_clobbers: 0 }
block0:
cmpq $0, %rdi
setnz %sil
load_ext_name userextname0+0, %r8
call *%r8
movq %rbp, %rsp
popq %rbp
ret
```
Note that the `icmp_imm` gets compiled to a `cmp` followed by a `setcc` instruction using `sil` as a parameter.
This means that the upper bytes of `rsi` are undefined. Now when `fn2` is a C function that looks like this:
```c
void fun(uint64_t x, bool a) {
do_smth(a ? x : 0);
}
```
and you compile this with clang the assembly looks like this:
```asm
xor eax, eax
test esi, esi
cmove rdi, rax
jmp do_smth(unsigned long)@PLT
```
while with gcc you get
```asm
xor eax, eax
test sil, sil
cmove rdi, rax
jmp do_smth(unsigned long)
```
Note that gcc emits a `test sil, sil` instruction while clang emits `test esi, esi` meaning that it relies on the 8bit argument getting zero-extended to at least 32 bit and therefore you will get random behavior depending on whether the upper bytes of `rsi` were zero or not. ([Compiler Explorer link](https://godbolt.org/z/Mn5qzjP8j))
[According to the people at LLVM](https://bugs.llvm.org/show_bug.cgi?id=44228), whether arguments should be zero-extended or not is unclear in the ABI and therefore they do not consider it a bug until this issue has been resolved on the ABI level since the major compilers do zero-extend the arguments.
Now my question is, do you consider this a bug in cranelift or at least an issue that should be addressed on the codegen level inside cranelift? (I have fixed this by manually checking if an argument is I8/I16 when emitting a function call and then passed that through a `uextend`/`inarrow` combo)
Maybe this could be done when the `enable_llvm_abi_extensions` flag is set?
And then the question would be where to fix it: When passing a variable to an external function or making sure that when instructions like `setcc` are emitted the upper parts of the destination register are cleared beforehand?
Contributor guide
Assessment
This issue has not been assessed yet.