VaList<'_> does not carry its ABI in its type

[workingjubilee]

Consider the following code that will be valid with extended_varargs_abi_support and c_variadic:

unsafe extern "sysv64" {
    fn nix_valist(vl: VaList<'_>);
}
unsafe extern "win64" {
    fn win_valist(vl: VaList<'_>);
}

For VaList<'_>, part of the c_variadic feature, the type does not contain the ABI it is used with. However, if we want to support defining functions with ..., that use VaList<'_> internally, for every ABI that we would accept with declarations of functions that accept ..., like so:

unsafe extern "sysv64" fn nix_valist(vl: VaList<'_>) {
    todo!()
}
unsafe extern "win64" fn win_valist(vl: VaList<'_>) {
    todo!()
}

then we have two problems:

1. We no longer can expand to the same type, as we currently assume a target can only have one VaList implementation, but in actuality it can have an implementation for each ABI it supports!
2. The VaList that we construct can now be passed to a function like vsnprintf (or the previous nix_valist and win_valist) which accepts the VaList as a parameter. Because VaList does not include the ABI it is constructed with in its type, it is valid to pass it to a function that will assume it uses a different varargs ABI. This can cause incorrect behavior in programs that otherwise take sensible precautions with using variable arguments, like using other data to transfer argument counts and types.

While this is not yet decided as behavior we wish to support, and the relevant functions of VaList are currently unsafe, it may be wise to consider embedding the ABI in VaList to address these problems. The goal would be to have something equivalent to VaList<'_, extern "sysv64">.

@rustbot label: +F-c_variadic +T-lang +T-libs-api

[bjorn3]

The VaList type is supposed to correspond to va_list in C, of which there only exist one version for each target, just like other fundamental types like c_int or c_long. In C even when you explicitly specify the calling convention, the va_list implementation doesn't change from the platform default: https://godbolt.org/z/5s1G8zxGr Clang simply denies va_start in functions with a non-standard calling convention: https://godbolt.org/z/qxh8PM5o5

[folkertdev]

Even so, we'd still need T-lang to formally decide that we want to close off the road of being more flexible than clang.

But given that clang does not support per-abi va_list, and the main goal of c_variadic is compatibility with C, I think that should really limit how much language design budget we're willing to spend here.

[workingjubilee]

Happy to rule things out if we want.

Our ...-to-VaList<'_> desugaring is equivalent to calling va_start, right?

[workingjubilee]

I ask about that because I'm wondering if we'd have to error on the definition existing at all, in effect. I know sometimes C functions are declared with variable arguments but they don't actually "use" them in a traditional sense, but I'm not sure we'd have that same leeway.

[folkertdev]

Yes, the current desugaring is equivalent to creating a stack value of type va_list and calling the va_start macro:

 va_list foo;
 va_start(foo);

We also automatically add a call to (the equivalent of) the va_end macro at the end of the function.

[workingjubilee]

This was very briefly discussed in T-lang's meeting on May 28th and @joshtriplett suggested that we could handle this in the future if we wanted to add such support by using new types. This offers the choice of

• new types per ABI (his suggestion)
• a defaulted generic (my suggestion)

...and both seem basically fine to me.

[workingjubilee]

Note that we probably shouldn't even support, even when compiling for x86_64-unknown-linux, the following, without deciding on how we are addressing this issue (closing the door or what), one way or another:

pub extern "sysv64" fn has_varargs(args: ...) {
    let args = args; // this needs to already have the final type we want picked for it
}

So we should only support these for now:

pub extern "C" fn has_varargs(args: ...) {

}

pub extern "C-unwind" fn unwind_with_varargs(args: ...) {

}