Skip to content

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

Open
@workingjubilee

Description

@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

Activity

added
needs-triageThis issue may need triage. Remove it if it has been sufficiently triaged.
F-c_variadic`#![feature(c_variadic)]`
T-langRelevant to the language team
T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.
on May 26, 2025
added
A-ABIArea: Concerning the application binary interface (ABI)
and removed
needs-triageThis issue may need triage. Remove it if it has been sufficiently triaged.
on May 26, 2025
bjorn3

bjorn3 commented on May 27, 2025

@bjorn3
Member

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

folkertdev commented on May 27, 2025

@folkertdev
Contributor

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

workingjubilee commented on May 27, 2025

@workingjubilee
MemberAuthor

Happy to rule things out if we want.

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

workingjubilee

workingjubilee commented on May 27, 2025

@workingjubilee
MemberAuthor

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

folkertdev commented on May 27, 2025

@folkertdev
Contributor

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

workingjubilee commented on May 28, 2025

@workingjubilee
MemberAuthor

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

workingjubilee commented on May 28, 2025

@workingjubilee
MemberAuthor

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: ...) {

}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-ABIArea: Concerning the application binary interface (ABI)C-bugCategory: This is a bug.F-c_variadic`#![feature(c_variadic)]`T-langRelevant to the language teamT-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @folkertdev@fmease@bjorn3@workingjubilee@rustbot

        Issue actions

          `VaList<'_>` does not carry its ABI in its type · Issue #141618 · rust-lang/rust