Rollup merge of #59625 - immunant:copy_variadics_typealias, r=eddyb

Refactor C FFI variadics to more closely match their C counterparts, and add Clone implementation

We had to make some changes to expose `va_copy` and `va_end` directly to users (mainly for C2Rust, but not exclusively):
- redefine the Rust variadic structures to more closely correspond to C: `VaList` now matches `va_list`, and `VaListImpl` matches `__va_list_tag`
- add `Clone` for `VaListImpl`
- add explicit `as_va_list()` conversion function from `VaListImpl` to `VaList`
- add deref coercion from `VaList` to `VaListImpl`

All these changes were needed for use cases like:
```Rust
let mut ap2 = va_copy(ap);
vprintf(fmt, ap2);
va_end(&mut ap2);
```

Author: Centril

src/libcore/ffi.rs

@@ -5,6 +5,8 @@
 //! Utilities related to FFI bindings.
 
 use crate::fmt;
+use crate::marker::PhantomData;
+use crate::ops::{Deref, DerefMut};
 
 /// Equivalent to C's `void` type when used as a [pointer].
 ///
@@ -45,25 +47,33 @@ impl fmt::Debug for c_void {
 }
 
 /// Basic implementation of a `va_list`.
+// The name is WIP, using `VaListImpl` for now.
 #[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
               not(target_arch = "x86_64")),
           all(target_arch = "aarch64", target_os = "ios"),
           windows))]
+#[repr(transparent)]
 #[unstable(feature = "c_variadic",
            reason = "the `c_variadic` feature has not been properly tested on \
                      all supported platforms",
            issue = "44930")]
-extern {
-    type VaListImpl;
+#[lang = "va_list"]
+pub struct VaListImpl<'f> {
+    ptr: *mut c_void,
+    _marker: PhantomData<&'f c_void>,
 }
 
 #[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
               not(target_arch = "x86_64")),
           all(target_arch = "aarch64", target_os = "ios"),
           windows))]
-impl fmt::Debug for VaListImpl {
+#[unstable(feature = "c_variadic",
+           reason = "the `c_variadic` feature has not been properly tested on \
+                     all supported platforms",
+           issue = "44930")]
+impl<'f> fmt::Debug for VaListImpl<'f> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "va_list* {:p}", self)
+        write!(f, "va_list* {:p}", self.ptr)
     }
 }
 
@@ -79,12 +89,14 @@ impl fmt::Debug for VaListImpl {
            reason = "the `c_variadic` feature has not been properly tested on \
                      all supported platforms",
            issue = "44930")]
-struct VaListImpl {
+#[lang = "va_list"]
+pub struct VaListImpl<'f> {
     stack: *mut c_void,
     gr_top: *mut c_void,
     vr_top: *mut c_void,
     gr_offs: i32,
     vr_offs: i32,
+    _marker: PhantomData<&'f c_void>,
 }
 
 /// PowerPC ABI implementation of a `va_list`.
@@ -95,12 +107,14 @@ struct VaListImpl {
            reason = "the `c_variadic` feature has not been properly tested on \
                      all supported platforms",
            issue = "44930")]
-struct VaListImpl {
+#[lang = "va_list"]
+pub struct VaListImpl<'f> {
     gpr: u8,
     fpr: u8,
     reserved: u16,
     overflow_arg_area: *mut c_void,
     reg_save_area: *mut c_void,
+    _marker: PhantomData<&'f c_void>,
 }
 
 /// x86_64 ABI implementation of a `va_list`.
@@ -111,22 +125,99 @@ struct VaListImpl {
            reason = "the `c_variadic` feature has not been properly tested on \
                      all supported platforms",
            issue = "44930")]
-struct VaListImpl {
+#[lang = "va_list"]
+pub struct VaListImpl<'f> {
     gp_offset: i32,
     fp_offset: i32,
     overflow_arg_area: *mut c_void,
     reg_save_area: *mut c_void,
+    _marker: PhantomData<&'f c_void>,
 }
 
 /// A wrapper for a `va_list`
-#[lang = "va_list"]
+#[repr(transparent)]
 #[derive(Debug)]
 #[unstable(feature = "c_variadic",
            reason = "the `c_variadic` feature has not been properly tested on \
                      all supported platforms",
            issue = "44930")]
-#[repr(transparent)]
-pub struct VaList<'a>(&'a mut VaListImpl);
+pub struct VaList<'a, 'f: 'a> {
+    #[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
+                  not(target_arch = "x86_64")),
+              all(target_arch = "aarch64", target_os = "ios"),
+              windows))]
+    inner: VaListImpl<'f>,
+
+    #[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc",
+                  target_arch = "x86_64"),
+              any(not(target_arch = "aarch64"), not(target_os = "ios")),
+              not(windows)))]
+    inner: &'a mut VaListImpl<'f>,
+
+    _marker: PhantomData<&'a mut VaListImpl<'f>>,
+}
+
+#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
+              not(target_arch = "x86_64")),
+          all(target_arch = "aarch64", target_os = "ios"),
+          windows))]
+#[unstable(feature = "c_variadic",
+           reason = "the `c_variadic` feature has not been properly tested on \
+                     all supported platforms",
+           issue = "44930")]
+impl<'f> VaListImpl<'f> {
+    /// Convert a `VaListImpl` into a `VaList` that is binary-compatible with C's `va_list`.
+    #[inline]
+    pub fn as_va_list<'a>(&'a mut self) -> VaList<'a, 'f> {
+        VaList {
+            inner: VaListImpl { ..*self },
+            _marker: PhantomData,
+        }
+    }
+}
+
+#[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc",
+              target_arch = "x86_64"),
+          any(not(target_arch = "aarch64"), not(target_os = "ios")),
+          not(windows)))]
+#[unstable(feature = "c_variadic",
+           reason = "the `c_variadic` feature has not been properly tested on \
+                     all supported platforms",
+           issue = "44930")]
+impl<'f> VaListImpl<'f> {
+    /// Convert a `VaListImpl` into a `VaList` that is binary-compatible with C's `va_list`.
+    #[inline]
+    pub fn as_va_list<'a>(&'a mut self) -> VaList<'a, 'f> {
+        VaList {
+            inner: self,
+            _marker: PhantomData,
+        }
+    }
+}
+
+#[unstable(feature = "c_variadic",
+           reason = "the `c_variadic` feature has not been properly tested on \
+                     all supported platforms",
+           issue = "44930")]
+impl<'a, 'f: 'a> Deref for VaList<'a, 'f> {
+    type Target = VaListImpl<'f>;
+
+    #[inline]
+    fn deref(&self) -> &VaListImpl<'f> {
+        &self.inner
+    }
+}
+
+#[unstable(feature = "c_variadic",
+           reason = "the `c_variadic` feature has not been properly tested on \
+                     all supported platforms",
+           issue = "44930")]
+impl<'a, 'f: 'a> DerefMut for VaList<'a, 'f> {
+    #[inline]
+    fn deref_mut(&mut self) -> &mut VaListImpl<'f> {
+        &mut self.inner
+    }
+}
 
 // The VaArgSafe trait needs to be used in public interfaces, however, the trait
 // itself must not be allowed to be used outside this module. Allowing users to
@@ -175,56 +266,76 @@ impl<T> sealed_trait::VaArgSafe for *mut T {}
            issue = "44930")]
 impl<T> sealed_trait::VaArgSafe for *const T {}
 
-impl<'a> VaList<'a> {
+#[unstable(feature = "c_variadic",
+           reason = "the `c_variadic` feature has not been properly tested on \
+                     all supported platforms",
+           issue = "44930")]
+#[cfg(not(stage0))]
+impl<'f> VaListImpl<'f> {
     /// Advance to the next arg.
-    #[unstable(feature = "c_variadic",
-               reason = "the `c_variadic` feature has not been properly tested on \
-                         all supported platforms",
-               issue = "44930")]
+    #[inline]
     pub unsafe fn arg<T: sealed_trait::VaArgSafe>(&mut self) -> T {
         va_arg(self)
     }
 
     /// Copies the `va_list` at the current location.
-    #[unstable(feature = "c_variadic",
-               reason = "the `c_variadic` feature has not been properly tested on \
-                         all supported platforms",
-               issue = "44930")]
     pub unsafe fn with_copy<F, R>(&self, f: F) -> R
-            where F: for<'copy> FnOnce(VaList<'copy>) -> R {
-        #[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
-                      not(target_arch = "x86_64")),
-                  all(target_arch = "aarch64", target_os = "ios"),
-                  windows))]
-        let mut ap = va_copy(self);
-        #[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc", target_arch = "x86_64"),
-                  not(windows), not(all(target_arch = "aarch64", target_os = "ios"))))]
-        let mut ap_inner = va_copy(self);
-        #[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc", target_arch = "x86_64"),
-                  not(windows), not(all(target_arch = "aarch64", target_os = "ios"))))]
-        let mut ap = VaList(&mut ap_inner);
-        let ret = f(VaList(ap.0));
+            where F: for<'copy> FnOnce(VaList<'copy, 'f>) -> R {
+        let mut ap = self.clone();
+        let ret = f(ap.as_va_list());
         va_end(&mut ap);
         ret
     }
 }
 
+#[unstable(feature = "c_variadic",
+           reason = "the `c_variadic` feature has not been properly tested on \
+                     all supported platforms",
+           issue = "44930")]
+#[cfg(not(stage0))]
+impl<'f> Clone for VaListImpl<'f> {
+    #[inline]
+    fn clone(&self) -> Self {
+        let mut dest = crate::mem::MaybeUninit::uninit();
+        unsafe {
+            va_copy(dest.as_mut_ptr(), self);
+            dest.assume_init()
+        }
+    }
+}
+
+#[unstable(feature = "c_variadic",
+           reason = "the `c_variadic` feature has not been properly tested on \
+                     all supported platforms",
+           issue = "44930")]
+#[cfg(not(stage0))]
+impl<'f> Drop for VaListImpl<'f> {
+    fn drop(&mut self) {
+        // FIXME: this should call `va_end`, but there's no clean way to
+        // guarantee that `drop` always gets inlined into its caller,
+        // so the `va_end` would get directly called from the same function as
+        // the corresponding `va_copy`. `man va_end` states that C requires this,
+        // and LLVM basically follows the C semantics, so we need to make sure
+        // that `va_end` is always called from the same function as `va_copy`.
+        // For more details, see https://github.com/rust-lang/rust/pull/59625
+        // and https://llvm.org/docs/LangRef.html#llvm-va-end-intrinsic.
+        //
+        // This works for now, since `va_end` is a no-op on all current LLVM targets.
+    }
+}
+
 extern "rust-intrinsic" {
     /// Destroy the arglist `ap` after initialization with `va_start` or
     /// `va_copy`.
-    fn va_end(ap: &mut VaList<'_>);
+    #[cfg(not(stage0))]
+    fn va_end(ap: &mut VaListImpl<'_>);
 
     /// Copies the current location of arglist `src` to the arglist `dst`.
-    #[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
-                  not(target_arch = "x86_64")),
-              all(target_arch = "aarch64", target_os = "ios"),
-              windows))]
-    fn va_copy<'a>(src: &VaList<'a>) -> VaList<'a>;
-    #[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc", target_arch = "x86_64"),
-              not(windows), not(all(target_arch = "aarch64", target_os = "ios"))))]
-    fn va_copy(src: &VaList<'_>) -> VaListImpl;
+    #[cfg(not(stage0))]
+    fn va_copy<'f>(dest: *mut VaListImpl<'f>, src: &VaListImpl<'f>);
 
     /// Loads an argument of type `T` from the `va_list` `ap` and increment the
     /// argument `ap` points to.
-    fn va_arg<T: sealed_trait::VaArgSafe>(ap: &mut VaList<'_>) -> T;
+    #[cfg(not(stage0))]
+    fn va_arg<T: sealed_trait::VaArgSafe>(ap: &mut VaListImpl<'_>) -> T;
 }

src/librustc/hir/lowering.rs

@@ -1523,7 +1523,7 @@ impl<'a> LoweringContext<'a> {
             }
             TyKind::Mac(_) => panic!("TyMac should have been expanded by now."),
             TyKind::CVarArgs => {
-                // Create the implicit lifetime of the "spoofed" `VaList`.
+                // Create the implicit lifetime of the "spoofed" `VaListImpl`.
                 let span = self.sess.source_map().next_point(t.span.shrink_to_lo());
                 let lt = self.new_implicit_lifetime(span);
                 hir::TyKind::CVarArgs(lt)

src/librustc/hir/mod.rs

@@ -1882,7 +1882,7 @@ pub enum TyKind {
     Infer,
     /// Placeholder for a type that has failed to be defined.
     Err,
-    /// Placeholder for C-variadic arguments. We "spoof" the `VaList` created
+    /// Placeholder for C-variadic arguments. We "spoof" the `VaListImpl` created
     /// from the variadic arguments. This type is only valid up to typeck.
     CVarArgs(Lifetime),
 }

src/librustc/ty/layout.rs

@@ -2501,7 +2501,7 @@ where
             }
 
             // If this is a C-variadic function, this is not the return value,
-            // and there is one or more fixed arguments; ensure that the `VaList`
+            // and there is one or more fixed arguments; ensure that the `VaListImpl`
             // is ignored as an argument.
             if sig.c_variadic {
                 match (last_arg_idx, arg_idx) {
@@ -2512,7 +2512,7 @@ where
                         };
                         match ty.sty {
                             ty::Adt(def, _) if def.did == va_list_did => {
-                                // This is the "spoofed" `VaList`. Set the arguments mode
+                                // This is the "spoofed" `VaListImpl`. Set the arguments mode
                                 // so that it will be ignored.
                                 arg.mode = PassMode::Ignore(IgnoreMode::CVarArgs);
                             }

src/librustc_codegen_llvm/intrinsic.rs

@@ -143,15 +143,8 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> {
                 self.va_end(args[0].immediate())
             }
             "va_copy" => {
-                let va_list = match (tcx.lang_items().va_list(), &result.layout.ty.sty) {
-                    (Some(did), ty::Adt(def, _)) if def.did == did => args[0].immediate(),
-                    (Some(_), _)  => self.load(args[0].immediate(),
-                                               tcx.data_layout.pointer_align.abi),
-                    (None, _) => bug!("`va_list` language item must be defined")
-                };
                 let intrinsic = self.cx().get_intrinsic(&("llvm.va_copy"));
-                self.call(intrinsic, &[llresult, va_list], None);
-                return;
+                self.call(intrinsic, &[args[0].immediate(), args[1].immediate()], None)
             }
             "va_arg" => {
                 match fn_ty.ret.layout.abi {
@@ -718,37 +711,12 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> {
         self.call(expect, &[cond, self.const_bool(expected)], None)
     }
 
-    fn va_start(&mut self, list: &'ll Value) -> &'ll Value {
-        let target = &self.cx.tcx.sess.target.target;
-        let arch = &target.arch;
-        // A pointer to the architecture specific structure is passed to this
-        // function. For pointer variants (i686, RISC-V, Windows, etc), we
-        // should do do nothing, as the address to the pointer is needed. For
-        // architectures with a architecture specific structure (`Aarch64`,
-        // `X86_64`, etc), this function should load the structure from the
-        // address provided.
-        let va_list = match &**arch {
-            _ if target.options.is_like_windows => list,
-            "aarch64" if target.target_os == "ios" => list,
-            "aarch64" | "x86_64" | "powerpc" =>
-                self.load(list, self.tcx().data_layout.pointer_align.abi),
-            _ => list,
-        };
+    fn va_start(&mut self, va_list: &'ll Value) -> &'ll Value {
         let intrinsic = self.cx().get_intrinsic("llvm.va_start");
         self.call(intrinsic, &[va_list], None)
     }
 
-    fn va_end(&mut self, list: &'ll Value) -> &'ll Value {
-        let target = &self.cx.tcx.sess.target.target;
-        let arch = &target.arch;
-        // See the comment in `va_start` for the purpose of the following.
-        let va_list = match &**arch {
-            _ if target.options.is_like_windows => list,
-            "aarch64" if target.target_os == "ios" => list,
-            "aarch64" | "x86_64" | "powerpc" =>
-                self.load(list, self.tcx().data_layout.pointer_align.abi),
-            _ => list,
-        };
+    fn va_end(&mut self, va_list: &'ll Value) -> &'ll Value {
         let intrinsic = self.cx().get_intrinsic("llvm.va_end");
         self.call(intrinsic, &[va_list], None)
     }