Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Miri doesn't understand C-variadic functions #1892

Open
bugadani opened this issue Sep 24, 2021 · 11 comments
Open

Miri doesn't understand C-variadic functions #1892

bugadani opened this issue Sep 24, 2021 · 11 comments
Labels
A-interpreter Area: affects the core interpreter C-bug Category: This is a bug.

Comments

@bugadani
Copy link

I have a program written in C and Rust. This program has a function that is variadic and written in C. To enable testing my Rust code, I needed to mock this function using the #[feature(c_variadic)] unstable feature.

This means I have not a function in my test code defined like the following:

pub unsafe extern "C" fn variadic_fn(
        some_arg: *const c_char,
        mut args: ...
    ) {
...
}

When running Miri on my test code, I'm greeted with the following error:
error: Undefined Behavior: calling a function with argument of type std::ffi::VaListImpl passing data of type *const i8

@RalfJung
Copy link
Member

Yeah, Miri probably does not support varargs at all currently. I assume this is another thing that is blocked on rust-lang/rust#56166.

If you could provide a self-contained example demonstrating the problem, that would be helpful as a testcase. :)

@bugadani
Copy link
Author

Sure thing.

#![feature(c_variadic)]

use std::os::raw::c_char;

macro_rules! c_str {
    ($lit:expr) => {
        concat!($lit, "\0").as_ptr() as *const c_char
    };
}

#[allow(unused_unsafe)]
pub unsafe extern "C" fn variadic_fn(_: *const c_char, mut _args: ...) {
    todo!()
}

fn main() {
    unsafe {
        variadic_fn(c_str!("c string"), 1, 2, 3);
    }
}

@RalfJung RalfJung changed the title Miri doesn't understand C-variadic functions defined in Rust Miri doesn't understand C-variadic functions Nov 28, 2021
@RalfJung RalfJung added the A-shims Area: This affects the external function shims label Dec 6, 2021
@RalfJung
Copy link
Member

RalfJung commented Dec 6, 2021

FWIW, rust-lang/rust#91342 should help with this.

But actually implementing VaList and the associated functions to retrieve the arguments would probably still require a bunch of work, fiddling with many horrible platform-specific details.

@RalfJung RalfJung added A-interpreter Area: affects the core interpreter C-bug Category: This is a bug. and removed A-shims Area: This affects the external function shims labels Dec 16, 2021
@RalfJung
Copy link
Member

To be clear, I don't currently have plans to work on those platform-specific parts -- it will be a lot of work for very little gain, as such Rust-to-Rust calls of c-variadic functions are exceedingly rare, to my knowledge.

An alternative for your usecase might be to use cfg(miri) for mocking, so you can replace the c-variadic calls with regular Rust functions when testing under Miri.

@bugadani
Copy link
Author

bugadani commented Dec 16, 2021

No worries, no worries, as a workaround I banished 99% of my C code (so I could also remove the C-variadic workaround for my tests), so this isn't important to me any more :) (I guess I should have said this earlier? Sorry 😳)

@saethlin

This comment was marked as off-topic.

@RalfJung

This comment was marked as off-topic.

@RalfJung

This comment was marked as off-topic.

@saethlin
Copy link
Member

saethlin commented Jun 5, 2022

Could Miri report this as an unsupported operation, not UB? Would that close this issue?

@RalfJung
Copy link
Member

RalfJung commented Jun 5, 2022

It does:

https://github.com/rust-lang/rust/blob/d7a2d9ae0e7e4b3c5811bdfd4809cfc772062140/compiler/rustc_const_eval/src/interpret/terminator.rs#L362

However, the issue you reported is separately detected before and considered UB:

https://github.com/rust-lang/rust/blob/d7a2d9ae0e7e4b3c5811bdfd4809cfc772062140/compiler/rustc_const_eval/src/interpret/terminator.rs#L357

Possibly we should remove that particular check. For our FFI implementation we ended up saying that we don't want to make a difference between variadic and non-variadic arguments, so this would make us more consistent. (But to my knowledge, at least on Apple's aarch64 targets, that is actually unsound.)

Would that close this issue?

No, what would close this issue is actually getting the testcase to pass.

@bjorn3
Copy link
Member

bjorn3 commented Jun 5, 2022

(But to my knowledge, at least on Apple's aarch64 targets, that is actually unsound.)

Indeed. Variadic arguments are always passed on the stack even if the same argument would end up in a register for a non-variadic function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-interpreter Area: affects the core interpreter C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

4 participants