Redesign VaList

Author: folkertdev

compiler/rustc_codegen_ssa/src/mir/block.rs

@@ -503,7 +503,22 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             let va_list_arg_idx = self.fn_abi.args.len();
             match self.locals[mir::Local::from_usize(1 + va_list_arg_idx)] {
                 LocalRef::Place(va_list) => {
-                    bx.va_end(va_list.val.llval);
+                    if super::is_va_list_struct_on_stack(bx) {
+                        // Call `va_end` on the `&mut VaListTag` that is stored in `va_list`.
+                        let inner_field = va_list.project_field(bx, 0);
+
+                        let tag_ptr = bx.load(
+                            bx.backend_type(inner_field.layout),
+                            inner_field.val.llval,
+                            inner_field.layout.align.abi,
+                        );
+
+                        bx.va_end(tag_ptr);
+                    } else {
+                        // Call `va_end` on the `VaListTag` that is stored in `va_list`.
+                        let tag_ptr = va_list.project_field(bx, 0);
+                        bx.va_end(tag_ptr.val.llval);
+                    }
                 }
                 _ => bug!("C-variadic function must have a `VaList` place"),
             }

compiler/rustc_codegen_ssa/src/mir/mod.rs

@@ -1,5 +1,7 @@
 use std::iter;
 
+use rustc_ast::Mutability;
+use rustc_hir as hir;
 use rustc_index::IndexVec;
 use rustc_index::bit_set::DenseBitSet;
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
@@ -370,6 +372,41 @@ fn optimize_use_clone<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     mir
 }
 
+/// Whether on this target, the `VaList` is a reference to a struct on the stack.
+///
+/// If this function returns `true`, the `core` crate defines a `VaListTag` struct
+/// matching the varargs ABI for this target.
+///
+/// In other cases, the `VaList` is an opaque pointer. Generally this is just a pointer to the
+/// caller's stack where the variadic arguments are stored sequentially.
+fn is_va_list_struct_on_stack<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(bx: &mut Bx) -> bool {
+    let did = bx.tcx().require_lang_item(hir::LangItem::VaList, None);
+    let ty = bx.tcx().type_of(did).instantiate_identity();
+
+    // Check how `VaList` is implemented for the current target. It can be either:
+    //
+    // - a reference to a value on the stack
+    // - a struct wrapping a pointer
+    let Some(adt_def) = ty.ty_adt_def() else { bug!("invalid `VaList`") };
+    let variant = adt_def.non_enum_variant();
+
+    if variant.fields.len() != 1 {
+        bug!("`VaList` must have exactly 1 field")
+    }
+    let field = variant.fields.iter().next().unwrap();
+    let field_ty = bx.tcx().type_of(field.did).instantiate_identity();
+
+    if field_ty.is_adt() {
+        return false;
+    }
+
+    if let Some(Mutability::Mut) = field_ty.ref_mutability() {
+        return true;
+    }
+
+    bug!("invalid `VaList` field type")
+}
+
 /// Produces, for each argument, a `Value` pointing at the
 /// argument's value. As arguments are places, these are always
 /// indirect.
@@ -436,10 +473,34 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
             }
 
             if fx.fn_abi.c_variadic && arg_index == fx.fn_abi.args.len() {
-                let va_list = PlaceRef::alloca(bx, bx.layout_of(arg_ty));
-                bx.va_start(va_list.val.llval);
+                use rustc_hir::LangItem;
+
+                let va_list_tag_ty = {
+                    let did = bx.tcx().require_lang_item(LangItem::VaListTag, None);
+                    let ty = bx.tcx().type_of(did).instantiate_identity();
+                    bx.tcx().normalize_erasing_regions(fx.cx.typing_env(), ty)
+                };
+                let va_list_tag_layout = bx.layout_of(va_list_tag_ty);
+
+                // Construct the `VaListTag` on the stack.
+                let va_list_tag = PlaceRef::alloca(bx, va_list_tag_layout);
+
+                // Initialize the alloca.
+                bx.va_start(va_list_tag.val.llval);
+
+                let va_list_tag = if is_va_list_struct_on_stack(bx) {
+                    va_list_tag.val.llval
+                } else {
+                    bx.load(
+                        bx.backend_type(va_list_tag_layout),
+                        va_list_tag.val.llval,
+                        va_list_tag.layout.align.abi,
+                    )
+                };
 
-                return LocalRef::Place(va_list);
+                let tmp = PlaceRef::alloca(bx, bx.layout_of(arg_ty));
+                bx.store_to_place(va_list_tag, tmp.val);
+                return LocalRef::Place(tmp);
             }
 
             let arg = &fx.fn_abi.args[idx];

compiler/rustc_hir/src/lang_items.rs

@@ -228,6 +228,7 @@ language_item_table! {
     UnsafePinned,            sym::unsafe_pinned,       unsafe_pinned_type,         Target::Struct,         GenericRequirement::None;
 
     VaList,                  sym::va_list,             va_list,                    Target::Struct,         GenericRequirement::None;
+    VaListTag,               sym::va_list_tag,         va_list_tag,                Target::Struct,         GenericRequirement::None;
 
     Deref,                   sym::deref,               deref_trait,                Target::Trait,          GenericRequirement::Exact(0);
     DerefMut,                sym::deref_mut,           deref_mut_trait,            Target::Trait,          GenericRequirement::Exact(0);

compiler/rustc_hir_analysis/src/check/intrinsic.rs

@@ -179,8 +179,8 @@ pub(crate) fn check_intrinsic_type(
         ty::BoundVariableKind::Region(ty::BoundRegionKind::Anon),
         ty::BoundVariableKind::Region(ty::BoundRegionKind::ClosureEnv),
     ]);
-    let mk_va_list_ty = |mutbl| {
-        let did = tcx.require_lang_item(LangItem::VaList, Some(span));
+    let mk_va_list_tag_ty = |mutbl| {
+        let did = tcx.require_lang_item(LangItem::VaListTag, Some(span));
         let region = ty::Region::new_bound(
             tcx,
             ty::INNERMOST,
@@ -566,16 +566,16 @@ pub(crate) fn check_intrinsic_type(
             }
 
             sym::va_start | sym::va_end => {
-                (0, 0, vec![mk_va_list_ty(hir::Mutability::Mut).0], tcx.types.unit)
+                (0, 0, vec![mk_va_list_tag_ty(hir::Mutability::Mut).0], tcx.types.unit)
             }
 
             sym::va_copy => {
-                let (va_list_ref_ty, va_list_ty) = mk_va_list_ty(hir::Mutability::Not);
+                let (va_list_ref_ty, va_list_ty) = mk_va_list_tag_ty(hir::Mutability::Not);
                 let va_list_ptr_ty = Ty::new_mut_ptr(tcx, va_list_ty);
                 (0, 0, vec![va_list_ptr_ty, va_list_ref_ty], tcx.types.unit)
             }
 
-            sym::va_arg => (1, 0, vec![mk_va_list_ty(hir::Mutability::Mut).0], param(0)),
+            sym::va_arg => (1, 0, vec![mk_va_list_tag_ty(hir::Mutability::Mut).0], param(0)),
 
             sym::nontemporal_store => {
                 (1, 0, vec![Ty::new_mut_ptr(tcx, param(0)), param(0)], tcx.types.unit)

compiler/rustc_span/src/symbol.rs

@@ -2274,6 +2274,7 @@ symbols! {
         va_copy,
         va_end,
         va_list,
+        va_list_tag,
         va_start,
         val,
         validity,

library/core/src/ffi/mod.rs

@@ -28,7 +28,7 @@ pub mod c_str;
     issue = "44930",
     reason = "the `c_variadic` feature has not been properly tested on all supported platforms"
 )]
-pub use self::va_list::{VaArgSafe, VaList, VaListImpl};
+pub use self::va_list::{VaArgSafe, VaList, VaListTag, va_copy};
 
 #[unstable(
     feature = "c_variadic",