Cranelift: Crash when compiling pinned reg on `x86_64`

[afonso360]

trafficstars

👋 Hey,

When implementing #4375 I found some crashes when compiling some examples that use the pinned reg on x86_64.

.clif Test Case

test run
set enable_pinned_reg
target x86_64

function %read_write_iadd(i64) -> i64 {
block0(v0: i64):
    set_pinned_reg v0
    v1 = get_pinned_reg.i64
    v2 = iadd_imm.i64 v1, 1
    return v2
}
; run: %read_write_iadd(0) == 1
; run: %read_write_iadd(-1) == 0
; run: %read_write_iadd(0xDEADBEEF_C0FFEEEE) == 0xDEADBEEF_C0FFEEEF

Steps to Reproduce

• clif-util test ./the-above.clif

Expected Results

The tests to pass

Actual Results

clif-util crashes seemingly with a regalloc issue.

     Running `C:\Users\Afonso\CLionProjects\wasmtime\target\debug\clif-util.exe test .\lmao.clif`
thread 'worker #0' panicked at 'assertion failed: `(left == right)`
  left: `p15i`,
 right: `p0i`', cranelift\codegen\src\machinst\reg.rs:469:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
FAIL .\lmao.clif: panicked in worker #0: assertion failed: `(left == right)`
  left: `p15i`,
 right: `p0i`
1 tests
Error: 1 failure
error: process didn't exit successfully: `C:\Users\Afonso\CLionProjects\wasmtime\target\debug\clif-util.exe test .\lmao.clif` (exit code: 1)

Versions and Environment

Cranelift version or commit: a2197ebbeb207f72b0a26b0fedcc81b43486cd99 (main as of writing) Operating system: Windows Architecture: x86_64

Extra Info

In my machine the above fails, but the simplified test commited in #4375 works.

function %read_write(i64) -> i64 {
block0(v0: i64):
    set_pinned_reg v0
    v1 = get_pinned_reg.i64
    return v1
}

But it looks like in CI it failed with:

running 1 test
error: test failed, to rerun pass '-p cranelift-tools --test filetests'
Caused by:
  process didn't exit successfully: `/home/runner/work/wasmtime/wasmtime/target/debug/deps/filetests-28b9c918e0c0996e` (signal: 11, SIGSEGV: invalid memory reference)

cc @cfallin since this looks like a regalloc issue

[cfallin]

@afonso360 I wasn't able to reproduce on Linux or macOS (and I don't have a Windows VM handy to test at the moment). However, I did run into some segfaults that eventually led me to realize: the enable_pinned_reg setting in general alters the ABI, and the above test-case overwrites r15 (on x86-64) which is normally a callee-saved register but not with the setting enabled. So Wasmtime happily calls the function, assuming it is SysV, and gets a corrupted register as a result.

I think that probably we should disallow enable_pinned_reg = true from run-tests unless we can adapt the harness to handle the alternate convention (this would include updating the trampolines at least); happy to review a PR for that if you want to tackle it.

The regalloc assertion might be an impossible constraint generated by the opcodes. If you can find a way to repro it on Linux I can look into it more!

[afonso360]

I think that probably we should disallow enable_pinned_reg = true from run-tests unless we can adapt the harness to handle the alternate convention (this would include updating the trampolines at least); happy to review a PR for that if you want to tackle it.

I've added a PR to disable it for now. Maybe we can pick it up later, I really want to focus on finishing the interpreter right now.