Stream: git-wasmtime

Topic: wasmtime / Issue #2316 Cranelift: Incorrect values with m...

Wasmtime GitHub notifications bot (Oct 24 2020 at 18:24):

chrisvittal edited Issue #2316:

Thanks for opening a bug report! Please answer the questions below
if they're relevant and delete this text before submitting.

• What are the steps to reproduce the issue? Can you include a CLIF test case,
  ideally reduced with the bugpoint clif-util command?
  Run the code attached at the bottom of this post. It builds a very simple function that takes 3 pointer parameters, prints them with printf then returns them in a permuted order. The clif is as follows:

function u0:0(i64, i64, i64) -> i64, i64, i64 system_v {
    gv0 = symbol colocated u1:0
    gv1 = symbol colocated u1:1
    gv2 = symbol colocated u1:2
    sig0 = (i64, i64) -> i32 system_v
    fn0 = u0:0 sig0

block0(v0: i64, v1: i64, v2: i64):
    v3 = symbol_value.i64 gv0
    v4 = call fn0(v3, v0)
    v5 = symbol_value.i64 gv1
    v6 = call fn0(v5, v1)
    v7 = symbol_value.i64 gv2
    v8 = call fn0(v7, v2)
    jump block1(v2, v0, v1)

block1(v9: i64, v10: i64, v11: i64):
    return v9, v10, v11
}

I build the code with a FunctionBuilder much like the toy example. Then I call it from rust as follows, printing out diagnostic information along the way.

    unsafe {
        let mut zero = 0usize;
        let mut one = 1usize;
        let mut two = 2usize;

        let zero_r = &mut zero;
        let one_r = &mut one;
        let two_r = &mut two;

        println!("from rust:");
        println!("0: {:p}", zero_r);
        println!("1: {:p}", one_r);
        println!("2: {:p}", two_r);

        #[repr(C)]
        struct Retval(*const u8, *const u8, *const u8);
        let func = std::mem::transmute::<
            _,
            unsafe extern "C" fn(*mut usize, *mut usize, *mut usize) -> Retval,
        >(func);
        println!("from compiled:");
        let Retval(_2, _0, _1) = func(zero_r, one_r, two_r);

        println!("from returned:");
        println!("0: {:p}", _0);
        println!("1: {:p}", _1);
        println!("2: {:p}", _2);
    }

The output I then see (in debug mode), (with variation on exact addresses but the offsets are usually similar) is the following:

from rust:
0: 0x7ffe818bf6c0
1: 0x7ffe818bf6c8
2: 0x7ffe818bf6d0
from compiled:
0: 0x7ffe818bf840
1: 0x7ffe818bf6c0
2: 0x7ffe818bf6c8
from returned:
0: 0x7ffe818bfa88
1: 0x7ffe818bf880
2: 0x557e358e62a0

• What do you expect to happen? What does actually happen? Does it panic, and
  if so, with which assertion?

I expect the block parameters to have the same values that were passed into the function. I expect the return values to be what the function should return.

• Which Cranelift version / commit hash / branch are you using?
  0.67.0

• If relevant, can you include some extra information about your environment?
  (Rust version, operating system, architecture...)
  rustc 1.49.0-nightly (8dae8cdcc 2020-10-12)
  x86_64-unknown-linux-gnu

Thank you for your assistance.

Full code here:

# Cargo.toml
[package]
name = "cranelift-repro"
version = "0.1.0"
authors = ["Chris Vittal <chris@vittal.dev>"]
edition = "2018"

[dependencies]
cranelift = "0.67.0"
cranelift-module = "0.67.0"
cranelift-simplejit = "0.67.0"

// main.rs
use cranelift::prelude::*;
use cranelift_module::{DataContext, Linkage, Module};
use cranelift_simplejit::{SimpleJITBackend, SimpleJITBuilder};

use std::ffi::CString;

#[repr(C)]
struct Retval(*const u8, *const u8, *const u8);

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let builder = SimpleJITBuilder::new(cranelift_module::default_libcall_names());
    let mut builder_context = FunctionBuilderContext::new();
    let mut data_ctx = DataContext::new();
    let mut module: Module<SimpleJITBackend> = Module::new(builder);
    let mut ctx = module.make_context();
    let ptr_ty = module.target_config().pointer_type();
    let mut fn_sig = module.make_signature();
    fn_sig.params.extend(&[AbiParam::new(ptr_ty); 3]);
    fn_sig.returns.extend(&[AbiParam::new(ptr_ty); 3]);
    ctx.func.signature = fn_sig;

    // this is hacky
    let mut printf_sig = module.make_signature();
    printf_sig.params.extend(&[AbiParam::new(ptr_ty); 2]);
    printf_sig.returns.push(AbiParam::new(types::I32));

    let mut builder = FunctionBuilder::new(&mut ctx.func, &mut builder_context);
    let printf = module.declare_function("printf", Linkage::Import, &printf_sig)?;
    let printf = module.declare_func_in_func(printf, &mut builder.func);

    let entry_block = builder.create_block();
    builder.append_block_params_for_function_params(entry_block);
    builder.switch_to_block(entry_block);
    builder.seal_block(entry_block);
    let params: Vec<Value> = Vec::from(builder.block_params(entry_block));
    for (i, param) in params.iter().enumerate() {
        let s = CString::new(format!("{}: %p\n", i)).expect("no nulls");
        data_ctx.define(s.into_bytes_with_nul().into_boxed_slice());
        let name = format!("data_{}", i);
        let id = module.declare_data(&name, Linkage::Local, false, false, None)?;
        module.define_data(id, &data_ctx)?;
        data_ctx.clear();
        let id = module.declare_data_in_func(id, &mut builder.func);
        let ptr = builder.ins().symbol_value(ptr_ty, id);
        builder.ins().call(printf, &[ptr, *param]);
    }

    let exit_block = builder.create_block();
    builder.append_block_params_for_function_returns(exit_block);
    let ret_params = [params[2], params[0], params[1]];
    builder.ins().jump(exit_block, &ret_params);
    builder.switch_to_block(exit_block);
    builder.seal_block(exit_block);

    let params = Vec::from(builder.block_params(exit_block));
    builder.ins().return_(&params);

    println!("{}", builder.func.display(None));
    builder.finalize();
    let id = module.declare_function("test_fn", Linkage::Export, &ctx.func.signature)?;
    module.define_function(id, &mut ctx, &mut codegen::binemit::NullTrapSink {})?;
    module.finalize_definitions();
    let func: *const u8 = module.get_finalized_function(id);

    let mut zero = 0usize;
    let mut one = 1usize;
    let mut two = 2usize;

    let zero_r = &mut zero;
    let one_r = &mut one;
    let two_r = &mut two;

    unsafe {
        let func = std::mem::transmute::<
            _,
            unsafe extern "C" fn(*mut usize, *mut usize, *mut usize) -> Retval,
        >(func);
        println!("from rust:");
        println!("0: {:p}", zero_r);
        println!("1: {:p}", one_r);
        println!("2: {:p}", two_r);
        println!("from compiled:");
        let Retval(_2, _0, _1) = func(zero_r, one_r, two_r);
        println!("from returned:");
        println!("0: {:p}", _0);
        println!("1: {:p}", _1);
        println!("2: {:p}", _2);
    }

    Ok(())
}

Wasmtime GitHub notifications bot (Oct 24 2020 at 19:09):

bjorn3 commented on Issue #2316:

It isn't allowed to return more than 3 values. This should result in an error when you run the verifier.

Wasmtime GitHub notifications bot (Oct 24 2020 at 19:13):

bjorn3 commented on Issue #2316:

Also using multiple return values to represent returning a tuple struct is not correct in the general case. For the two primitive elements case it happens to be correct on x86_64 with the System-V call conv, but for three or more elements the function in most cases actually takes a pointer to where the struct needs to be stored.

Wasmtime GitHub notifications bot (Oct 24 2020 at 19:55):

chrisvittal commented on Issue #2316:

Thank you for the explanation. I added a call to verify_function and got no errors.

Wasmtime GitHub notifications bot (Oct 24 2020 at 20:01):

chrisvittal commented on Issue #2316:

More specifically,

I added this line after finishing, but before finalizing the builder.

    dbg!(codegen::verify_function(&builder.func, dbg!(module.isa())))?;

And It gave me:

[src/main.rs:59] module.isa() = TargetIsa { triple: Triple { architecture: X86_64, vendor: Unknown, operating_system: Linux, environment: Gnu, binary_format: Elf }, pointer_width: U64}
[src/main.rs:59] codegen::verify_function(&builder.func, dbg!(module . isa())) = Ok(
    (),
)

Wasmtime GitHub notifications bot (Oct 24 2020 at 20:11):

chrisvittal commented on Issue #2316:

Thank you for your help. I'll update my program with this in mind.

Wasmtime GitHub notifications bot (Oct 24 2020 at 20:14):

bjorn3 commented on Issue #2316:

The three return value case is an extension of the System-V abi, it only gives a verifier error for four or more return values. While this extension is supported by both Cranelift and I think LLVM, it will never be used for #[repr(C)] structs with three register sized elements, as that would be in conflict with the System-V abi. I am pretty sure there is no way to convince rustc to use this extension. You should instead just take an extra argument that points to a place to write the return value to.

Wasmtime GitHub notifications bot (Oct 24 2020 at 20:45):

chrisvittal commented on Issue #2316:

I'm updating my program to use output parameters now.

I'm going to close this.

This is a really nice project. Thanks for you help and support.

Wasmtime GitHub notifications bot (Oct 24 2020 at 20:45):

chrisvittal closed Issue #2316:

Thanks for opening a bug report! Please answer the questions below
if they're relevant and delete this text before submitting.

• What are the steps to reproduce the issue? Can you include a CLIF test case,
  ideally reduced with the bugpoint clif-util command?
  Run the code attached at the bottom of this post. It builds a very simple function that takes 3 pointer parameters, prints them with printf then returns them in a permuted order. The clif is as follows:

function u0:0(i64, i64, i64) -> i64, i64, i64 system_v {
    gv0 = symbol colocated u1:0
    gv1 = symbol colocated u1:1
    gv2 = symbol colocated u1:2
    sig0 = (i64, i64) -> i32 system_v
    fn0 = u0:0 sig0

block0(v0: i64, v1: i64, v2: i64):
    v3 = symbol_value.i64 gv0
    v4 = call fn0(v3, v0)
    v5 = symbol_value.i64 gv1
    v6 = call fn0(v5, v1)
    v7 = symbol_value.i64 gv2
    v8 = call fn0(v7, v2)
    jump block1(v2, v0, v1)

block1(v9: i64, v10: i64, v11: i64):
    return v9, v10, v11
}

I build the code with a FunctionBuilder much like the toy example. Then I call it from rust as follows, printing out diagnostic information along the way.

    unsafe {
        let mut zero = 0usize;
        let mut one = 1usize;
        let mut two = 2usize;

        let zero_r = &mut zero;
        let one_r = &mut one;
        let two_r = &mut two;

        println!("from rust:");
        println!("0: {:p}", zero_r);
        println!("1: {:p}", one_r);
        println!("2: {:p}", two_r);

        #[repr(C)]
        struct Retval(*const u8, *const u8, *const u8);
        let func = std::mem::transmute::<
            _,
            unsafe extern "C" fn(*mut usize, *mut usize, *mut usize) -> Retval,
        >(func);
        println!("from compiled:");
        let Retval(_2, _0, _1) = func(zero_r, one_r, two_r);

        println!("from returned:");
        println!("0: {:p}", _0);
        println!("1: {:p}", _1);
        println!("2: {:p}", _2);
    }

The output I then see (in debug mode), (with variation on exact addresses but the offsets are usually similar) is the following:

from rust:
0: 0x7ffe818bf6c0
1: 0x7ffe818bf6c8
2: 0x7ffe818bf6d0
from compiled:
0: 0x7ffe818bf840
1: 0x7ffe818bf6c0
2: 0x7ffe818bf6c8
from returned:
0: 0x7ffe818bfa88
1: 0x7ffe818bf880
2: 0x557e358e62a0

• What do you expect to happen? What does actually happen? Does it panic, and
  if so, with which assertion?

I expect the block parameters to have the same values that were passed into the function. I expect the return values to be what the function should return.

• Which Cranelift version / commit hash / branch are you using?
  0.67.0

• If relevant, can you include some extra information about your environment?
  (Rust version, operating system, architecture...)
  rustc 1.49.0-nightly (8dae8cdcc 2020-10-12)
  x86_64-unknown-linux-gnu

Thank you for your assistance.

Full code here:

# Cargo.toml
[package]
name = "cranelift-repro"
version = "0.1.0"
authors = ["Chris Vittal <chris@vittal.dev>"]
edition = "2018"

[dependencies]
cranelift = "0.67.0"
cranelift-module = "0.67.0"
cranelift-simplejit = "0.67.0"

// main.rs
use cranelift::prelude::*;
use cranelift_module::{DataContext, Linkage, Module};
use cranelift_simplejit::{SimpleJITBackend, SimpleJITBuilder};

use std::ffi::CString;

#[repr(C)]
struct Retval(*const u8, *const u8, *const u8);

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let builder = SimpleJITBuilder::new(cranelift_module::default_libcall_names());
    let mut builder_context = FunctionBuilderContext::new();
    let mut data_ctx = DataContext::new();
    let mut module: Module<SimpleJITBackend> = Module::new(builder);
    let mut ctx = module.make_context();
    let ptr_ty = module.target_config().pointer_type();
    let mut fn_sig = module.make_signature();
    fn_sig.params.extend(&[AbiParam::new(ptr_ty); 3]);
    fn_sig.returns.extend(&[AbiParam::new(ptr_ty); 3]);
    ctx.func.signature = fn_sig;

    // this is hacky
    let mut printf_sig = module.make_signature();
    printf_sig.params.extend(&[AbiParam::new(ptr_ty); 2]);
    printf_sig.returns.push(AbiParam::new(types::I32));

    let mut builder = FunctionBuilder::new(&mut ctx.func, &mut builder_context);
    let printf = module.declare_function("printf", Linkage::Import, &printf_sig)?;
    let printf = module.declare_func_in_func(printf, &mut builder.func);

    let entry_block = builder.create_block();
    builder.append_block_params_for_function_params(entry_block);
    builder.switch_to_block(entry_block);
    builder.seal_block(entry_block);
    let params: Vec<Value> = Vec::from(builder.block_params(entry_block));
    for (i, param) in params.iter().enumerate() {
        let s = CString::new(format!("{}: %p\n", i)).expect("no nulls");
        data_ctx.define(s.into_bytes_with_nul().into_boxed_slice());
        let name = format!("data_{}", i);
        let id = module.declare_data(&name, Linkage::Local, false, false, None)?;
        module.define_data(id, &data_ctx)?;
        data_ctx.clear();
        let id = module.declare_data_in_func(id, &mut builder.func);
        let ptr = builder.ins().symbol_value(ptr_ty, id);
        builder.ins().call(printf, &[ptr, *param]);
    }

    let exit_block = builder.create_block();
    builder.append_block_params_for_function_returns(exit_block);
    let ret_params = [params[2], params[0], params[1]];
    builder.ins().jump(exit_block, &ret_params);
    builder.switch_to_block(exit_block);
    builder.seal_block(exit_block);

    let params = Vec::from(builder.block_params(exit_block));
    builder.ins().return_(&params);

    println!("{}", builder.func.display(None));
    builder.finalize();
    let id = module.declare_function("test_fn", Linkage::Export, &ctx.func.signature)?;
    module.define_function(id, &mut ctx, &mut codegen::binemit::NullTrapSink {})?;
    module.finalize_definitions();
    let func: *const u8 = module.get_finalized_function(id);

    let mut zero = 0usize;
    let mut one = 1usize;
    let mut two = 2usize;

    let zero_r = &mut zero;
    let one_r = &mut one;
    let two_r = &mut two;

    unsafe {
        let func = std::mem::transmute::<
            _,
            unsafe extern "C" fn(*mut usize, *mut usize, *mut usize) -> Retval,
        >(func);
        println!("from rust:");
        println!("0: {:p}", zero_r);
        println!("1: {:p}", one_r);
        println!("2: {:p}", two_r);
        println!("from compiled:");
        let Retval(_2, _0, _1) = func(zero_r, one_r, two_r);
        println!("from returned:");
        println!("0: {:p}", _0);
        println!("1: {:p}", _1);
        println!("2: {:p}", _2);
    }

    Ok(())
}

Last updated: Nov 22 2024 at 16:03 UTC