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

Delegation incorrectly assumes VaListImpl<'_> from ... will correctly pass-through to ... #127443

Closed
workingjubilee opened this issue Jul 7, 2024 · 1 comment · Fixed by #138407
Labels
A-ABI Area: Concerning the application binary interface (ABI) A-FFI Area: Foreign function interface (FFI) C-bug Category: This is a bug. F-c_variadic `#![feature(c_variadic)]` F-fn_delegation `#![feature(fn_delegation)]` requires-incomplete-features This issue requires the use of incomplete features.

Comments

@workingjubilee
Copy link
Member

To quote @petrochenkov in #127401 (comment)

Delegation is supposed to copy the wrapper function signature from the target function, and then just pass all the parameters to the target function in the body.
I assume that applies to VaListImpl or whatever is hiding under the ... in C-like variadics as well.
If passing ... through requires some additional actions, the delegation should do those actions (or report an error), but now it's not doing them.
That said, Rust should make passing ... through "just work" if it's technically possible, even if C requires some additional dance for that.

Unfortunately, ... and va_list are different types for good reason: one is arguments that have not been "discovered" yet in the function, whereas va_list is a structure that describes how to acquire those arguments, and may be acquired by rolling some or all of those arguments into the structure at the moment of its creation.

This is why, in C, there are both of these functions.

int printf ( const char * format, ... );
int vprintf ( const char * format, va_list arg );

They are not duplicates of each other, and attempting to treat them as such is a common source of UB in C programs written by people foolish enough to meddle with variadic functions in C. ( So, most of them, considering the existence of printf. )

When we receive arguments into ... using feature(c_variadic), Rust currently implicitly creates a VaListImpl<'_>, which approximately equates to C's va_list. We do this implicitly rather than requiring someone to call va_start. We are not guaranteed to know, as far as I am aware, how many arguments were passed or their shape. Some ABIs pass the necessary knowledge as a hidden parameter. But others don't. If we could control the ABI, of course, we would have the ability to do so every time, but this feature is about interfacing with C ABIs.

This interaction of features is thus currently incorrect and is undefined behavior even if a function with ... arguments is correctly called by Rust, as long as it passes through a layer of delegation.

What @petrochenkov describes is desirable, but I expect the answer will be "technically impossible, even over a reasonable subset of our supported platforms". I will do the necessary research either way. But until we confirm this, or apply the necessary changes to support delegating unsafe extern "C" fn(...), or redesign feature(c_variadic), whatever it would take to fix this, the following code must be rejected by the compiler:

//@ edition:2018
//@ aux-crate:fn_header_aux=fn-header-aux.rs

#![feature(c_variadic)]
#![feature(fn_delegation)]
#![allow(incomplete_features)]

mod to_reuse {
    pub unsafe extern "C" fn variadic_fn(n: usize, mut args: ...) {}
}

reuse to_reuse::variadic_fn;
reuse fn_header_aux::variadic_fn_extern;

fn main() {
    unsafe {
        variadic_fn(0);
        variadic_fn(0, 1);
        variadic_fn_extern(0);
        variadic_fn_extern(0, 1);
    }
    let _: unsafe extern "C" fn(usize, ...) = variadic_fn;
    let _: unsafe extern "C" fn(usize, ...) = variadic_fn_extern;
}

This bug is currently live in the most recent Rust nightlies.

rustc --version --verbose:

rustc 1.81.0-nightly (524d806c6 2024-07-05)
binary: rustc
commit-hash: 524d806c62a82ecc0cf8634b94997ae506f4d6f9
commit-date: 2024-07-05
host: x86_64-unknown-linux-gnu
release: 1.81.0-nightly
LLVM version: 18.1.7
@workingjubilee workingjubilee added C-bug Category: This is a bug. F-c_variadic `#![feature(c_variadic)]` requires-incomplete-features This issue requires the use of incomplete features. F-fn_delegation `#![feature(fn_delegation)]` labels Jul 7, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 7, 2024
@workingjubilee workingjubilee added A-FFI Area: Foreign function interface (FFI) A-ABI Area: Concerning the application binary interface (ABI) labels Jul 7, 2024
@workingjubilee
Copy link
Member Author

workingjubilee commented Jul 7, 2024

If we wish to support delegating to an unsafe extern "C" fn delegatee(...), then it would be simplest if, instead of entering a function call and then making another function call immediately, the function call in question actually resolves to calling the function delegatee during code generation. I think.

@Noratrieb Noratrieb removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 7, 2024
@bors bors closed this as completed in 9d1b62c Mar 13, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Mar 13, 2025
Rollup merge of rust-lang#138407 - Bryanskiy:delegation-variadic, r=petrochenkov

Delegation: reject C-variadics

The explanation is contained in attached issues.

Fixes rust-lang#127443
Fixes rust-lang#127413

r? `@petrochenkov`
jieyouxu pushed a commit to jieyouxu/rustc-dev-guide that referenced this issue Mar 13, 2025
Delegation: reject C-variadics

The explanation is contained in attached issues.

Fixes rust-lang/rust#127443
Fixes rust-lang/rust#127413

r? `@petrochenkov`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ABI Area: Concerning the application binary interface (ABI) A-FFI Area: Foreign function interface (FFI) C-bug Category: This is a bug. F-c_variadic `#![feature(c_variadic)]` F-fn_delegation `#![feature(fn_delegation)]` requires-incomplete-features This issue requires the use of incomplete features.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants