stabilize c-style varargs for sysv64, win64, efiapi, aapcs

[RalfJung]

This has been split up so the PR now only contains the extended_varargs_abi_support stabilization; "system" has been moved to #145954.

Previous (combined) PR description:

This stabilizes extern block declarations of variadic functions with the system, sysv64, win64, efiapi, aapcs ABIs. This corresponds to the extended_varargs_abi_support and extern_system_varargs feature gates.

The feature gates were split up since it seemed like there might be further discussion needed for what exactly "system" ABI variadic functions should do, but a consensus has meanwhile been reached: they shall behave like "C" functions. IOW, the ABI of a "system" function is (bold part is new in this PR):

• "stdcall" for win32 targets for non-variadic functions
• "C" for everything else

This had been previously stabilized without FCP in #116161, which got reverted in #136897. There was also a "fun" race condition involved with the system ABI being added to the list of variadic-supporting ABIs between the creation and merge of #116161.

There was a question raised here whether t-lang even needs to be involved for a change like this. Not sure if that has meanwhile been clarified? The behavior of the "system" ABI (a Rust-specific ABI) definitely feels like t-lang territory to me.

Fixes #100189
Cc @rust-lang/lang

Stabilization report

General design

What is the RFC for this feature and what changes have occurred to the user-facing design since the RFC was finalized?

AFAIK there is no RFC. The tracking issues are

• Tracking Issue for extended_varargs_abi_support #100189
• Tracking Issue for extern_system_varargs #136946

What behavior are we committing to that has been controversial? Summarize the major arguments pro/con.

The only controversial point is whether "system" ABI functions should support variadics.

• Pro: This allows crates like windows-rs to consistently use "system", see e.g. Should use extern "system" as much as possible microsoft/windows-rs#3626.
• Cons: @workingjubilee had some implementation concerns, but I think those have been resolved. EDIT: turns out Jubilee still has concerns (she mentioned that in a DM); I'll let her express those.

Note that "system" is already a magic ABI we introduced to "do the right thing". This just makes it do the right thing in more cases. In particular, it means that on Windows one can almost always just do

extern "system" {
  // put all the things here
}

and it'll do the right thing, rather than having to split imports into non-varargs and varargs, with the varargs in a separate extern "C" block (and risking accidentally putting a non-vararg there).

(I am saying "almost" always because some Windows API functions actually use cdecl, not stdcall, on x86. Those of course need to go in extern "C" blocks.)

Are there extensions to this feature that remain unstable? How do we know that we are not accidentally committing to those?

Actually defining variadic functions in Rust remains unstable, under the c_variadic feature gate.

Has a Call for Testing period been conducted? If so, what feedback was received?

Does any OSS nightly users use this feature? For instance, a useful indication might be "search <grep.app> for #![feature(FEATURE_NAME)] and had N results".

There was no call for testing.

A search brings up https://github.com/rust-osdev/uefi-rs/blob/main/uefi-raw/src/table/boot.rs using this for "efiapi". This doesn't seem widely used, but it is an "obvious" gap in our support for c-variadics.

Implementation quality

All rustc does here is forward the ABI to LLVM so there's lot a lot to say here...

Summarize the major parts of the implementation and provide links into the code (or to PRs)

An example for async closures: https://rustc-dev-guide.rust-lang.org/coroutine-closures.html.

The check for allowed variadic ABIs is here.

The special handling of "system" is here.

Summarize existing test coverage of this feature

Consider what the "edges" of this feature are. We're particularly interested in seeing tests that assure us about exactly what nearby things we're not stabilizing.

Within each test, include a comment at the top describing the purpose of the test and what set of invariants it intends to demonstrate. This is a great help to those reviewing the tests at stabilization time.

• What does the test coverage landscape for this feature look like?
  • Tests for compiler errors when you use the feature wrongly or make mistakes?
  • Tests for the feature itself:
    • Limits of the feature (so failing compilation)
    • Exercises of edge cases of the feature
    • Tests that checks the feature works as expected (where applicable, //@ run-pass).
  • Are there any intentional gaps in test coverage?

Link to test folders or individual tests (ui/codegen/assembly/run-make tests, etc.).

Prior PRs add a codegen test for all ABIs and tests actually calling extern variadic functions for sysv64 and win64:

• add codegen test for variadics #144359
• test using multiple c-variadic ABIs in the same program #144379

We don't have a way of executing uefi target code in the test suite, so it's unclear how to fully test efiapi. aapcs could probably be done? (But note that we have hardly an such actually-calling-functions tests for ABI things, we almost entirely rely on codegen tests.)

The test ensuring that we do not stabilize defining c-variadic functions is tests/ui/feature-gates/feature-gate-c_variadic.rs.

What outstanding bugs in the issue tracker involve this feature? Are they stabilization-blocking?

None that I am aware of.

What FIXMEs are still in the code for that feature and why is it ok to leave them there?

None that I am aware of.

Summarize contributors to the feature by name for recognition and assuredness that people involved in the feature agree with stabilization

@Soveu added sysv64, win64, efiapi, aapcs to the list of ABIs that allow variadics, @beepster4096 added system. @workingjubilee recently refactored the ABI handling in the compiler, also affecting this feature.

Which tools need to be adjusted to support this feature. Has this work been done?

Consider rustdoc, clippy, rust-analyzer, rustfmt, rustup, docs.rs.

Maybe RA needs to be taught about the new allowed ABIs? No idea how precisely they mirror what exactly rustc accepts and rejects here.

Type system and execution rules

What compilation-time checks are done that are needed to prevent undefined behavior?

(Be sure to link to tests demonstrating that these tests are being done.)

Nothing new here, this just expands the existing support for calling variadic functions to more ABIs.

Does the feature's implementation need checks to prevent UB or is it sound by default and needs opt in in places to perform the dangerous/unsafe operations? If it is not sound by default, what is the rationale?

Nothing new here, this just expands the existing support for calling variadic functions to more ABIs.

Can users use this feature to introduce undefined behavior, or use this feature to break the abstraction of Rust and expose the underlying assembly-level implementation? (Describe.)

Nothing new here, this just expands the existing support for calling variadic functions to more ABIs.

What updates are needed to the reference/specification? (link to PRs when they exist)

• update for more ABIs supporting c-variadics reference#1936

Common interactions

Does this feature introduce new expressions and can they produce temporaries? What are the lifetimes of those temporaries?

No.

What other unstable features may be exposed by this feature?

None.

[rustbot]

r? @oli-obk

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

HIR ty lowering was modified

cc @fmease

[Soveu]

Looks great :) Thanks for pushing this topic

[workingjubilee]

I drafted a PR of similar nature but halted on the fact that there are no tests that verify the functional correctness of code compiled with this feature. It is untested.

[workingjubilee]

@RalfJung I still wanted extern_system_varargs to be addressed separately, please.

[RalfJung]

That seems like a lot of unnecessary bureaucratic overhead to me and the two PRs would necessarily conflict. Why should it be separate?

[RalfJung]

Regarding test coverage -- checking the generated LLVM IR represents our usual level of coverage here. I agree it'd be great to have end-to-end tests that actually invoke C code, but we rarely ever do that, we don't have great infrastructure to do it, and I am not convinced that now is the time we have to start enforcing this.

[RalfJung]

Pinging @kennykerr @ChrisDenton @dpaoliello for some Windows input specifically on the question of the "system" ABI. Does the behavior proposed for stabilization here make sense for you and the windows-rs crate?

[dpaoliello]

Makes sense to me.

[workingjubilee]

There is not really any testing for the ABIs being used in this way. That is, whether we actually perform the call correctly and receive an intelligible result. Because of the quirks of the codegen backend's implementation, and some of these ABIs (aapcs especially) having close friends, there is reason to question whether the result is exactly equivalent even if the ABI is conceptually the same as one that is implemented.

The past few days, I have not really had access to my usual computer so I can't attend meetings or submit PRs myself and even leaving comments is kinda challenging or I would have tried to offer an alternative.

I would prefer that we have the ability to make a coherent decision regarding how all of the Windows/x86 related ABIs work, together, instead of shipping it as piecewise PRs. Because I do feel like they are not so large a subject they cannot be considered together, and I would rather we decide them together, even if we eventually also decide to implement some details more gradually. Right now we are demanding everyone churn their existing uses of e.g. extern "stdcall" anyways due to #141435 and we should probably reflect on the result of that, "oh, the largest set of windows bindings in Rust triggers this lint", more deeply.

We have yet to reflect, and I do not think we should rush in again.

More personally, this is not the first time I have asked for simply altering the pace of something because I struggle to respond in time and would rather not thrash, panic, and be out of energy for actually doing useful work. When I did have my computer, not long ago, I was mostly trying to take a look at our many extern "interrupt" ABIs and do some work on those and hopefully a replacement for them.

[RalfJung]

I don't think it is fair to say that anything is being "rushed" here. There hasn't been any activity in both tracking issues for more than a month, and the most recent activity there all signals general consensus, at least that's how I read it. This is a proposal for stabilization, which can easily take weeks or months, and will definitely take at least 10 days once t-lang actually reached consensus.

Right now we are demanding everyone churn their existing uses of e.g. extern "stdcall" anyways due to #141435 and we should probably reflect on the result of that, "oh, the largest set of windows bindings in Rust triggers this lint", more deeply.

That is a fair point. Maybe it is not too surprising that when Microsoft writes Rust bindings, that they assume MSVC conventions such as "stdcall works everywhere even if that does not make sense" -- which we knew to be an MSVC convention when we made #137018 a future-compat lint.

What is not entirely clear to me is how you think this should influence the decision about "system" varargs. I have presented my vision for that: "system" is already a "do what I mean" ABI that we came up with so that it is easier to just use the Right ABI when calling Windows ABI functions; it seems quite natural to extend that behavior to varargs. In contrast, while "stdcall" was actually equivalent to "system" for non-vararg functions until we started linting in #137018, I think we can call that an accident (evidence: the "system" ABI exists, since Rust 1.0, to do explicitly what the "stdcall" ABI did implicitly).

So what is the alternative? Go full MSVC and accept varargs on stdcall functions, silently falling back to "C"? That would be a full 180 degree turn compared to #137018 and #87678, and would mean that the "system" ABI is pretty pointless.

Could you spell out what you think the story around varargs and the Windows ABIs should look like?

[workingjubilee]

I did say I would try to give my explanation but I kinda expected to have my computer back by now. I can't really explain my full rationales currently typing away on my phone like this, unfortunately, so it won't be available in time for the triage meeting. Sorry.

But yes it would at least partly be an attempt to give a rundown of the final places to arrive at here and the costs/benefits insofar as I am aware.

[workingjubilee]

Though I hope I do not have to elaborate in great detail to say that any final place that "makes extern "system" kinda pointless" is also one that gives reason to not complicate it further.

[workingjubilee]

It is complicated by things like target modifiers, which raise questions like "what does extern "system" mean if codegen has created a different C ABI due to flags?" For instance, the VDSO of Linux, as I understand it, is compiled with a "classic" C ABI, even if the rest of the kernel uses a modified ABI due to x86 regparm flags and whatnot. Is that extern "system"? Should it be? After all, it literally is code being compiled so it can expose interfaces that conform with the platform ABI that you use to talk to the system.

[workingjubilee]

@bors r+ rollup

[bors]

📌 Commit e10735d has been approved by workingjubilee

It is now in the queue for this repository.

[jhpratt]

presumptive cause of #146104 (comment)

@bors r-

[RalfJung]

@bors r=workingjubilee

[bors]

📌 Commit f6d55ae has been approved by workingjubilee

It is now in the queue for this repository.