i128 abi incompatibility with cg_llvm

[DrewRidley]

Hi all,

I was informed a little over a week ago that it should be possible to mix LLVM and cranelift in the same rust target. I went about writing a game in bevy, and using opt = 3 with LLVM for my dependencies. I used the following toml file to achieve this:

[profile.dev.package."*"]
codegen-backend = "llvm"

[profile.dev]
codegen-backend = "cranelift"

I then added bevy as a dependency, and ran the simple example provided. All was well and the examples listed on their website run as expected. However, as soon as a Plugin is written in a cranelift context, and added to the LLVM emitted bevy code, a segmentation fault occurs. I suspect it might have to do with calling conventions, but my knowledge about LLVM and Cranelift is limited so I won't be much help here.

Here is a minimal example that should illustrate the problem:

use bevy::prelude::*;

pub struct MyCraneliftPlugin;

impl Plugin for MyCraneliftPlugin {
    fn build(&self, app: &mut App) {
       app.add_systems(Update, || { debug!("Hello world!") });
    }
}

   fn main() {
    App::new()
        .add_plugins(MinimalPlugins)
        .add_plugins(MyCraneliftPlugin)
        .run();
}

Compiling the following exclusively with LLVM or exclusively with Cranelift works as expected. Mixing the backends with the config provided earlier causes a segfault. I tried to investigate where exactly the segmentation fault occurs with lldb but was largely unsuccessful as it was not clear which code emitted was from cranelift and which was llvm.

Any help would be much appreciated, and I apologize I could not be more help here.

[bjorn3]

I can reproduce the crash with the following Cargo.toml:

[package]
name = "crash_repro"
version = "0.1.0"
edition = "2021"

[dependencies]
bevy = "0.12.1"

[profile.dev.package."*"]
codegen-backend = "llvm"
opt-level = 3

[profile.dev]
codegen-backend = "cranelift"

[bjorn3]

Found the issue: LLVM allows returning values in up to 3 registers, while Cranelift uses an implicit return value argument after the second register even when enable_llvm_abi_extensions is used: https://github.com/bytecodealliance/wasmtime/blob/536cf88ce9864cdddfe52bf625c81b06e8eb1d43/cranelift/codegen/src/isa/x64/abi.rs#L1058-L1059 This should be easy to fix.

[bjorn3]

https://github.com/bytecodealliance/wasmtime/pull/7770 should fix this.

[bjorn3]

By the way thanks for providing the repro! It was really useful for figuring out the problem.

[bjorn3]

The Cranelift PR has been accepted and backported to the branch that will be released in a little longer than a week. Once it has been released I will update cg_clif.

[bjorn3]

Updated Cranelift in https://github.com/rust-lang/rustc_codegen_cranelift/commit/dc7ed1680c093ea7667f841ecaa7b89f205aef6f. Got a couple of other things I want to do first before syncing to the rust repo to get it on nightly. Will let you know when this is in a new nightly.

[bjorn3]

Should be fixed on the latest nightly.

[bjorn3]

Seems like it isn't fixed after all.

[bjorn3]

Tried it again. Can't reproduce this crash anymore. abi-cafe does still show abi incompatibilities with cg_llvm for the i128 handling though, possibly because of https://blog.rust-lang.org/2024/03/30/i128-layout-update.html

[bjorn3]

Will be fixed by https://github.com/bytecodealliance/wasmtime/pull/8270 on x86_64-unknown-linux-gnu at least.

[bjorn3]

With a cg_clif patched to use current Cranelift main (which contains https://github.com/bytecodealliance/wasmtime/pull/8270) https://github.com/rust-lang/rustc_codegen_cranelift/actions/runs/8508177743 shows that abi-cafe now passes on x86_64-unknown-linux-gnu with the i128 test expectations updated to indicate that they are no longer broken. On x86_64-apple-darwin interoperability with cg_llvm works again too, but the clang used on GHA doesn't yet have the ABI fixes, so cg_clif <-> C is still broken for i128.

[bjorn3]

Updated cranelift in https://github.com/rust-lang/rustc_codegen_cranelift/commit/6ec27fe9dac839a15cb27ff8551503a5d94759fc. Will sync it back to the rust repo tomorrow.

[bjorn3]

This has been fixed on nightly now.