`c_variadic`: use `Clone` instead of LLVM `va_copy` #150436

folkertdev · 2025-12-27T16:20:33Z

tracking issue: #44930

Implement va_copy as (the rust equivalent of) memcpy, which is the behavior of all current LLVM targets. By providing our own implementation, we can guarantee its behavior. These guarantees are important for implementing c-variadics in e.g. const-eval.

Discussed in #t-compiler/const-eval > c-variadics in const-eval.

I've also updated the comment for Drop a bit. The background here is that the C standard requires that va_end is used in the same function (and really, in the same scope) as the corresponding va_start or va_copy. That is because historically va_start would start a scope, which va_end would then close. e.g.

https://softwarepreservation.computerhistory.org/c_plus_plus/cfront/release_3.0.3/source/incl-master/proto-headers/stdarg.sol

#define         va_start(ap, parmN)     {\
        va_buf  _va;\
        _vastart(ap = (va_list)_va, (char *)&parmN + sizeof parmN)
#define         va_end(ap)      }
#define         va_arg(ap, mode)        *((mode *)_vaarg(ap, sizeof (mode)))

The C standard still has to consider such implementations, but for rust they are irrelevant. Hence we can use Clone for va_copy and Drop for va_end.

folkertdev · 2025-12-27T16:21:09Z

library/core/src/ffi/va_list.rs

        /// Basic implementation of a `va_list`.
        #[repr(transparent)]
-        #[derive(Debug)]
+        #[derive(Debug, Clone, Copy)]
        struct VaListInner {
            ptr: *const c_void,
        }


https://github.com/llvm/llvm-project/blob/87e8e7d8f0db53060ef2f6ef4ab612fc0f2b4490/llvm/lib/Transforms/IPO/ExpandVariadics.cpp#L127-L129

folkertdev · 2025-12-27T16:21:34Z

library/core/src/ffi/va_list.rs

        /// https://github.com/llvm/llvm-project/blob/0cdc1b6dd4a870fc41d4b15ad97e0001882aba58/clang/lib/CodeGen/Targets/Hexagon.cpp#L407-L417
        #[repr(C)]
-        #[derive(Debug)]
+        #[derive(Debug, Clone, Copy)]


https://github.com/llvm/llvm-project/blob/5aee01a3df011e660f26660bc30a8c94a1651d8e/llvm/lib/Target/Hexagon/HexagonISelLowering.cpp#L1087

folkertdev · 2025-12-27T16:21:47Z

library/core/src/ffi/va_list.rs

        /// https://github.com/llvm/llvm-project/blob/af9a4263a1a209953a1d339ef781a954e31268ff/llvm/lib/Target/Xtensa/XtensaISelLowering.cpp#L1211-L1215
        #[repr(C)]
-        #[derive(Debug)]
+        #[derive(Debug, Clone, Copy)]


https://github.com/llvm/llvm-project/blob/5aee01a3df011e660f26660bc30a8c94a1651d8e/llvm/lib/Target/Xtensa/XtensaISelLowering.cpp#L1260

folkertdev · 2025-12-27T16:23:59Z

library/core/src/ffi/va_list.rs

        /// https://docs.google.com/gview?embedded=true&url=https://github.com/IBM/s390x-abi/releases/download/v1.7/lzsabi_s390x.pdf
        #[repr(C)]
-        #[derive(Debug)]
+        #[derive(Debug, Clone, Copy)]


https://github.com/llvm/llvm-project/blob/5aee01a3df011e660f26660bc30a8c94a1651d8e/llvm/lib/Target/SystemZ/SystemZISelLowering.h#L339

folkertdev · 2025-12-27T16:24:20Z

library/core/src/ffi/va_list.rs

        /// [GCC header]: https://web.mit.edu/darwin/src/modules/gcc/gcc/ginclude/va-ppc.h
        #[repr(C)]
-        #[derive(Debug)]
+        #[derive(Debug, Clone, Copy)]


https://github.com/llvm/llvm-project/blob/5aee01a3df011e660f26660bc30a8c94a1651d8e/llvm/lib/Target/PowerPC/PPCISelLowering.h#L713

folkertdev · 2025-12-27T16:24:35Z

library/core/src/ffi/va_list.rs

        /// http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
        #[repr(C)]
-        #[derive(Debug)]
+        #[derive(Debug, Clone, Copy)]


https://github.com/llvm/llvm-project/blob/5aee01a3df011e660f26660bc30a8c94a1651d8e/llvm/lib/Target/AArch64/AArch64ISelLowering.h#L709

folkertdev · 2026-01-02T21:04:28Z

r? @workingjubilee

The codegen of VaList::clone is target-specific now, and because minicore does not define VaList it is hard to test. I think it's fine to just test the behavior, and have LLVM use a memcpy when VaList is a struct, or just re-use the pointer value if it's just a pointer.

rustbot · 2026-01-02T21:04:33Z

workingjubilee is currently at their maximum review capacity.
They may take a while to respond.

rustbot · 2026-01-02T21:04:34Z

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

RalfJung · 2026-01-03T15:13:28Z

Implement va_copy as (the rust equivalent of) memcpy, which is the behavior of all current LLVM targets. By providing our own implementation, we can guarantee its behavior.

Didn't we say we would avoid that assumption and also support implementations that do malloc/free in va_copy / va_end? See #t-compiler/const-eval > c-variadics in const-eval @ 💬.

folkertdev · 2026-01-03T23:53:02Z

That's right, we don't actually impl Copy for VaList, so that a future target could implement a meaningful Clone and Drop. It's just that in practice for all current targets Clone happens to be a memcpy and Drop a no-op

workingjubilee

Offering a suggested historical note.

Please feel free to trim it or move it into the PR description itself, down to the amounts you find worthwhile or just amusing to keep (including 0 lines).

r=me after

Thanks!

View changes since this review

workingjubilee · 2026-01-06T21:53:13Z

library/core/src/ffi/va_list.rs

+        //
+        // Rust requires that not calling `va_end` on a `va_list` does not cause undefined
+        // behaviour: it is safe to leak values.
+    }


Suggested change

}

// The only target with a possibly-non-conformant C implementation

// was Pyramid Technology (usually `#ifdef pyr` in old C headers)

// which used `#define`s to replace `va_start` and `va_end` with `{ }`

// and thus needed to be called in the same scope.

// This should still be irrelevant for *binary* compatibility.

//

// Pyramid Technology was acquired by Siemens AG in 1995.

// Its proprietary arch was replaced by MIPS in 1991[^1][^2]

// and DCOS/X, its Unix implementation, was replaced by SINIX in 1995[^3].

//

// [^1]: ComputerWorld, Vol XXV, No. 16, 1991 April 22nd, from digitized text: https://archive.org/stream/computerworld2516unse/computerworld2516unse_djvu.txt

// [^2]: "Pyramid Server Uses Mips Chip" from digitized collection: https://archive.computerhistory.org/resources/access/text/2022/03/102739303-05-01-acc.pdf

// [^3]: https://en.wikipedia.org/wiki/DC/OSx

}

This is fun, but I don't think it is relevant for this comment. The same-scope restriction is a C implementation detail (portable macro assembler and all that), rust (and modern C implementations) don't have that problem.

I'm satisfied with it being part of the PR history, then.

I just wanted to make sure it was incredibly, completely, totally dead. :^)

RalfJung · 2026-01-07T07:57:19Z

@folkertdev what is your plan for va_copy regarding the const-eval support for this?
ISTM that without the intrinsic, it'll not be possible to catch a bunch of the UB that we had planned to catch?

RalfJung · 2026-01-07T10:45:51Z

library/core/src/intrinsics/mod.rs

-///
-#[rustc_intrinsic]
-#[rustc_nounwind]
-pub unsafe fn va_copy<'f>(dest: *mut VaList<'f>, src: &VaList<'f>);


The docs for va_end still reference va_copy. Does va_end even still make sense after this PR?

Also, seems like it is now newly legal to call va_arg multiple times on the same raw VaList representation after duplicating it. The entire protocol for how the code may interact with va_arg is changed quite a bit. That should also be reflected in the docs.

rustbot · 2026-01-07T20:15:55Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

folkertdev · 2026-01-07T20:19:22Z

Based on further discussion in #t-compiler/const-eval > c-variadics in const-eval, a slight change of plans

we still no longer call LLVM's va_copy
we do still have our own va_copy intrinsic that serves as a hook for const evaluation
va_copy has a fallback body, code generation backends should not provide their own implementation

I've changed the signature of the intrinsic to be more ergonomic, and made it safe because while the hook is used to detect UB, the intrinsic itself is completely safe.

workingjubilee · 2026-01-07T21:00:06Z

Typo in a field name?

workingjubilee · 2026-01-08T01:42:14Z

cool!

@bors r+

rust-bors · 2026-01-08T01:42:18Z

📌 Commit 97e26cb has been approved by workingjubilee

It is now in the queue for this repository.

workingjubilee · 2026-01-08T01:45:48Z

Actually, hm, right this second, I'm not sure if I...

@bors r-

You! The one who is reviewing now! Answer!
r? @RalfJung

rust-bors · 2026-01-08T01:45:52Z

Commit 97e26cb has been unapproved.

folkertdev · 2026-01-08T09:29:12Z

Locally with #150601 I can now get this to emit the error from the comment. So this setup should work in practice.

fn manual_copy() {
    const unsafe extern "C" fn helper(ap: ...) {
        // A copy created using Clone is valid, and can be used to read arguments.
        let mut copy = ap.clone();
        assert!(copy.arg::<i32>() == 1i32);

        let mut u = core::mem::MaybeUninit::uninit();
        unsafe { core::ptr::copy_nonoverlapping(&ap, u.as_mut_ptr(), 1) };

        // Manually creating the copy is fine.
        let mut copy = unsafe { u.assume_init() };

        // Even using it is fine.
        let _ = copy.arg::<i32>();

        // But then using (or dropping) the original is not.
        drop(copy);
        drop(ap);
    }

    const { unsafe { helper(1, 2, 3) } };
    //~^ ERROR va_end on unknown va_list allocation ALLOC0 [E0080]
}

The above also requires some changes to va_end (like having Drop actually call it), but I want to handle that separately because I suspect LLVM may be finicky about that.

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 27, 2025

folkertdev commented Dec 27, 2025

View reviewed changes

folkertdev added the F-c_variadic `#![feature(c_variadic)]` label Dec 27, 2025

This comment has been minimized.

Sign in to view

folkertdev force-pushed the va-list-copy branch 3 times, most recently from 7a78640 to 8e09112 Compare January 2, 2026 21:02

rustbot added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Jan 2, 2026

rustbot assigned workingjubilee Jan 2, 2026

folkertdev marked this pull request as ready for review January 2, 2026 21:04

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 2, 2026

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 2, 2026

workingjubilee approved these changes Jan 6, 2026

View reviewed changes

workingjubilee added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 6, 2026

RalfJung reviewed Jan 7, 2026

View reviewed changes

folkertdev changed the title ~~c_variadic: use Clone instead of va_copy~~ c_variadic: use Clone instead of LLVM va_copy Jan 7, 2026

folkertdev force-pushed the va-list-copy branch from 8e09112 to 00673e0 Compare January 7, 2026 20:15

This comment has been minimized.

Sign in to view

folkertdev force-pushed the va-list-copy branch from 00673e0 to 19e7342 Compare January 7, 2026 20:22

This comment has been minimized.

Sign in to view

c_variadic: use Clone instead of LLVM va_copy

97e26cb

folkertdev force-pushed the va-list-copy branch from 19e7342 to 97e26cb Compare January 7, 2026 21:27

rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 8, 2026

rust-bors bot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 8, 2026

rustbot assigned RalfJung Jan 8, 2026

rust-bors bot removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jan 8, 2026

rustbot unassigned workingjubilee Jan 8, 2026

-    }
+        // The only target with a possibly-non-conformant C implementation
+        // was Pyramid Technology (usually `#ifdef pyr` in old C headers)
+        // which used `#define`s to replace `va_start` and `va_end` with `{ }`
+        // and thus needed to be called in the same scope.
+        // This should still be irrelevant for *binary* compatibility.
+        //
+        // Pyramid Technology was acquired by Siemens AG in 1995.
+        // Its proprietary arch was replaced by MIPS in 1991[^1][^2]
+        // and DCOS/X, its Unix implementation, was replaced by SINIX in 1995[^3].
+        //
+        // [^1]: ComputerWorld, Vol XXV, No. 16, 1991 April 22nd, from digitized text: https://archive.org/stream/computerworld2516unse/computerworld2516unse_djvu.txt
+        // [^2]: "Pyramid Server Uses Mips Chip" from digitized collection: https://archive.computerhistory.org/resources/access/text/2022/03/102739303-05-01-acc.pdf
+        // [^3]: https://en.wikipedia.org/wiki/DC/OSx
+    }

Uh oh!

c_variadic: use Clone instead of LLVM va_copy #150436

Are you sure you want to change the base?

c_variadic: use Clone instead of LLVM va_copy #150436

Conversation

folkertdev commented Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

folkertdev commented Jan 2, 2026

Uh oh!

rustbot commented Jan 2, 2026

Uh oh!

rustbot commented Jan 2, 2026

Uh oh!

RalfJung commented Jan 3, 2026

Uh oh!

folkertdev commented Jan 3, 2026

Uh oh!

workingjubilee left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung commented Jan 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Jan 7, 2026

Uh oh!

folkertdev commented Jan 7, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

workingjubilee commented Jan 7, 2026

Uh oh!

workingjubilee commented Jan 8, 2026

Uh oh!

rust-bors bot commented Jan 8, 2026

Uh oh!

workingjubilee commented Jan 8, 2026

Uh oh!

rust-bors bot commented Jan 8, 2026

Uh oh!

folkertdev commented Jan 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

`c_variadic`: use `Clone` instead of LLVM `va_copy` #150436

`c_variadic`: use `Clone` instead of LLVM `va_copy` #150436

folkertdev commented Dec 27, 2025 •

edited

Loading

workingjubilee left a comment •

edited by rustbot

Loading