Fix comments

- apply suggestions from code review
- include note about backends no overriding `va_end` and `va_copy` in
  the doc comment.
- link to where `va_copy` is lowered to `memcpy`

Co-authored-by: Ralf Jung <post@ralfj.de>

Author: folkertdev

library/core/src/ffi/va_list.rs

@@ -34,6 +34,10 @@ use crate::marker::PhantomCovariantLifetime;
 //
 // The Clang `BuiltinVaListKind` enumerates the `va_list` variations that Clang supports,
 // and we mirror these here.
+//
+// For all current LLVM targets, `va_copy` lowers to `memcpy`. Hence the inner structs below all
+// derive `Copy`. However, in the future we might want to support a target where `va_copy`
+// allocates, or otherwise violates the requirements of `Copy`. Therefore `VaList` is only `Clone`.
 crate::cfg_select! {
     all(
         target_arch = "aarch64",
@@ -45,6 +49,8 @@ crate::cfg_select! {
         ///
         /// See the [AArch64 Procedure Call Standard] for more details.
         ///
+        /// `va_copy` is `memcpy`: <https://github.com/llvm/llvm-project/blob/5aee01a3df011e660f26660bc30a8c94a1651d8e/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp#L12682-L12700>
+        ///
         /// [AArch64 Procedure Call Standard]:
         /// http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
         #[repr(C)]
@@ -62,6 +68,8 @@ crate::cfg_select! {
         ///
         /// See the [LLVM source] and [GCC header] for more details.
         ///
+        /// `va_copy` is `memcpy`: <https://github.com/llvm/llvm-project/blob/5aee01a3df011e660f26660bc30a8c94a1651d8e/llvm/lib/Target/PowerPC/PPCISelLowering.cpp#L3755-L3764>
+        ///
         /// [LLVM source]:
         /// https://github.com/llvm/llvm-project/blob/af9a4263a1a209953a1d339ef781a954e31268ff/llvm/lib/Target/PowerPC/PPCISelLowering.cpp#L4089-L4111
         /// [GCC header]: https://web.mit.edu/darwin/src/modules/gcc/gcc/ginclude/va-ppc.h
@@ -81,6 +89,8 @@ crate::cfg_select! {
         ///
         /// See the [S/390x ELF Application Binary Interface Supplement] for more details.
         ///
+        /// `va_copy` is `memcpy`: <https://github.com/llvm/llvm-project/blob/5aee01a3df011e660f26660bc30a8c94a1651d8e/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp#L4457-L4472>
+        ///
         /// [S/390x ELF Application Binary Interface Supplement]:
         /// https://docs.google.com/gview?embedded=true&url=https://github.com/IBM/s390x-abi/releases/download/v1.7/lzsabi_s390x.pdf
         #[repr(C)]
@@ -98,6 +108,9 @@ crate::cfg_select! {
         ///
         /// See the [System V AMD64 ABI] for more details.
         ///
+        /// `va_copy` is `memcpy`: <https://github.com/llvm/llvm-project/blob/5aee01a3df011e660f26660bc30a8c94a1651d8e/llvm/lib/Target/X86/X86ISelLowering.cpp#26319>
+        /// (github won't render that file, look for `SDValue LowerVACOPY`)
+        ///
         /// [System V AMD64 ABI]:
         /// https://refspecs.linuxbase.org/elf/x86_64-abi-0.99.pdf
         #[repr(C)]
@@ -115,6 +128,8 @@ crate::cfg_select! {
         ///
         /// See the [LLVM source] for more details.
         ///
+        /// `va_copy` is `memcpy`: <https://github.com/llvm/llvm-project/blob/5aee01a3df011e660f26660bc30a8c94a1651d8e/llvm/lib/Target/Xtensa/XtensaISelLowering.cpp#L1260>
+        ///
         /// [LLVM source]:
         /// https://github.com/llvm/llvm-project/blob/af9a4263a1a209953a1d339ef781a954e31268ff/llvm/lib/Target/Xtensa/XtensaISelLowering.cpp#L1211-L1215
         #[repr(C)]
@@ -132,6 +147,8 @@ crate::cfg_select! {
         ///
         /// See the [LLVM source] for more details. On bare metal Hexagon uses an opaque pointer.
         ///
+        /// `va_copy` is `memcpy`: <https://github.com/llvm/llvm-project/blob/5aee01a3df011e660f26660bc30a8c94a1651d8e/llvm/lib/Target/Hexagon/HexagonISelLowering.cpp#L1087-L1102>
+        ///
         /// [LLVM source]:
         /// https://github.com/llvm/llvm-project/blob/0cdc1b6dd4a870fc41d4b15ad97e0001882aba58/clang/lib/CodeGen/Targets/Hexagon.cpp#L407-L417
         #[repr(C)]
@@ -190,12 +207,9 @@ impl Clone for VaList<'_> {
     #[inline]
     fn clone(&self) -> Self {
         // We only implement Clone and not Copy because some future target might not be able to
-        // implement Copy (e.g. because it allocates).
-
-        // We still use a `va_copy` intrinsic to provide a hook for const evaluation. The hook is
-        // used to report UB when a variable argument list is duplicated with a manual `memcpy`.
-        // While that works in practice for all current targets, we want to be able to support
-        // targets in the future where that is not the case.
+        // implement Copy (e.g. because it allocates). For the same reason we use an intrinsic
+        // to do the copying: the fact that on all current targets, this is just `memcpy`, is an implementation
+        // detail. The intrinsic lets Miri catch UB from code incorrectly relying on that implementation detail.
         va_copy(self)
     }
 }

library/core/src/intrinsics/mod.rs

@@ -3471,12 +3471,15 @@ pub unsafe fn va_arg<T: VaArgSafe>(ap: &mut VaList<'_>) -> T;
 
 /// Duplicates a variable argument list. The returned list is initially at the same position as
 /// the one in `src`, but can be advanced independently.
+///
+/// Codegen backends should not have custom behavior for this intrinsic, they should always use
+/// this fallback implementation. This intrinsic *does not* map to the LLVM `va_copy` intrinsic.
+///
+/// This intrinsic exists only as a hook for Miri and constant evaluation, and is used to detect UB
+/// when a variable argument list is used incorrectly.
 #[rustc_intrinsic]
 #[rustc_nounwind]
 pub fn va_copy<'f>(src: &VaList<'f>) -> VaList<'f> {
-    // NOTE: this intrinsic exists only as a hook for constant evaluation, and is used to detect UB
-    // when `VaList` is used incorrectly. Codegen backends should not have custom behavior for this
-    // intrinsic, they should always use this fallback implementation.
     src.duplicate()
 }