transpile: Translate va_list with correct types

Rua · Rua · commit b03e54339364 · 2025-12-03T19:02:36.000+01:00
diff --git a/c2rust-transpile/src/convert_type.rs b/c2rust-transpile/src/convert_type.rs
@@ -250,7 +250,10 @@ impl TypeConverter {
     ) -> TranslationResult<Box<Type>> {
         let barefn_inputs = params
             .iter()
-            .map(|x| mk().bare_arg(self.convert(ctxt, x.ctype).unwrap(), None::<Box<Ident>>))
+            .map(|x| {
+                let ty = self.convert_function_param(ctxt, x.ctype).unwrap();
+                mk().bare_arg(ty, None::<Box<Ident>>)
+            })
             .collect::<Vec<_>>();
 
         let output = match ret {
@@ -317,7 +320,7 @@ impl TypeConverter {
         ctype: CTypeId,
     ) -> TranslationResult<Box<Type>> {
         if self.translate_valist && ctxt.is_va_list(ctype) {
-            let ty = mk().abs_path_ty(vec!["core", "ffi", "VaList"]);
+            let ty = mk().abs_path_ty(vec!["core", "ffi", "VaListImpl"]);
             return Ok(ty);
         }
 
@@ -442,6 +445,22 @@ impl TypeConverter {
         }
     }
 
+    // Variant of `convert` that handles types that need to be converted differently if they
+    // are the type of a function parameter.
+    pub fn convert_function_param(
+        &mut self,
+        ctxt: &TypedAstContext,
+        ctype: CTypeId,
+    ) -> TranslationResult<Box<Type>> {
+        if ctxt.is_va_list(ctype) {
+            // va_list parameters are translated as VaList rather than VaListImpl
+            let ty = mk().abs_path_ty(vec!["core", "ffi", "VaList"]);
+            return Ok(ty);
+        }
+
+        self.convert(ctxt, ctype)
+    }
+
     /// Add the given parameters to a K&R function pointer type,
     /// returning a full signature or `None` if the function isn't K&R.
     pub fn knr_function_type_with_parameters(
diff --git a/c2rust-transpile/src/translator/builtins.rs b/c2rust-transpile/src/translator/builtins.rs
@@ -319,8 +319,7 @@ impl<'c> Translation<'c> {
                 if ctx.is_unused() && args.len() == 2 {
                     if let Some(va_id) = self.match_vastart(args[0]) {
                         if self.ast_context.get_decl(&va_id).is_some() {
-                            let dst =
-                                self.convert_expr(ctx.expect_valistimpl().used(), args[0], None)?;
+                            let dst = self.convert_expr(ctx.used(), args[0], None)?;
                             let fn_ctx = self.function_context.borrow();
                             let src = fn_ctx.get_va_list_arg_name();
 
@@ -341,10 +340,8 @@ impl<'c> Translation<'c> {
             "__builtin_va_copy" => {
                 if ctx.is_unused() && args.len() == 2 {
                     if let Some((_dst_va_id, _src_va_id)) = self.match_vacopy(args[0], args[1]) {
-                        let dst =
-                            self.convert_expr(ctx.expect_valistimpl().used(), args[0], None)?;
-                        let src =
-                            self.convert_expr(ctx.expect_valistimpl().used(), args[1], None)?;
+                        let dst = self.convert_expr(ctx.used(), args[0], None)?;
+                        let src = self.convert_expr(ctx.used(), args[1], None)?;
 
                         let call_expr = mk().method_call_expr(src.to_expr(), "clone", vec![]);
                         let assign_expr = mk().assign_expr(dst.to_expr(), call_expr);
diff --git a/c2rust-transpile/src/translator/mod.rs b/c2rust-transpile/src/translator/mod.rs
@@ -126,10 +126,6 @@ pub struct ExprContext {
     // address in function pointer literals.
     needs_address: bool,
 
-    /// Set to false if we should decay VaListImpl to VaList or true if we are
-    /// expect a VaListImpl in this context.
-    expecting_valistimpl: bool,
-
     ternary_needs_parens: bool,
     expanding_macro: Option<CDeclId>,
 }
@@ -193,13 +189,6 @@ impl ExprContext {
         }
     }
 
-    pub fn expect_valistimpl(self) -> Self {
-        ExprContext {
-            expecting_valistimpl: true,
-            ..self
-        }
-    }
-
     /// Are we expanding the given macro in the current context?
     pub fn expanding_macro(&self, mac: &CDeclId) -> bool {
         match self.expanding_macro {
@@ -505,7 +494,6 @@ pub fn translate(
         decay_ref: DecayRef::Default,
         is_bitfield_write: false,
         needs_address: false,
-        expecting_valistimpl: false,
         ternary_needs_parens: false,
         expanding_macro: None,
     };
@@ -1272,6 +1260,11 @@ struct ConvertedVariable {
     pub init: TranslationResult<WithStmts<Box<Expr>>>,
 }
 
+struct ConvertedFunctionParam {
+    pub ty: Box<Type>,
+    pub mutbl: Mutability,
+}
+
 impl<'c> Translation<'c> {
     pub fn new(
         mut ast_context: TypedAstContext,
@@ -2345,8 +2338,7 @@ impl<'c> Translation<'c> {
 
             // handle regular (non-variadic) arguments
             for &(decl_id, ref var, typ) in arguments {
-                let ConvertedVariable { ty, mutbl, init: _ } =
-                    self.convert_variable(ctx, None, typ)?;
+                let ConvertedFunctionParam { ty, mutbl } = self.convert_function_param(ctx, typ)?;
 
                 let pat = if var.is_empty() {
                     mk().wild_pat()
@@ -3063,6 +3055,25 @@ impl<'c> Translation<'c> {
         Ok(ConvertedVariable { ty, mutbl, init })
     }
 
+    fn convert_function_param(
+        &self,
+        ctx: ExprContext,
+        typ: CQualTypeId,
+    ) -> TranslationResult<ConvertedFunctionParam> {
+        if self.ast_context.is_va_list(typ.ctype) {
+            let mutbl = if typ.qualifiers.is_const {
+                Mutability::Immutable
+            } else {
+                Mutability::Mutable
+            };
+            let ty = mk().abs_path_ty(vec!["core", "ffi", "VaList"]);
+            return Ok(ConvertedFunctionParam { mutbl, ty });
+        }
+
+        self.convert_variable(ctx, None, typ)
+            .map(|ConvertedVariable { ty, mutbl, .. }| ConvertedFunctionParam { ty, mutbl })
+    }
+
     fn convert_type(&self, type_id: CTypeId) -> TranslationResult<Box<Type>> {
         self.import_type(type_id);
 
@@ -3339,19 +3350,31 @@ impl<'c> Translation<'c> {
             .collect()
     }
 
-    /// Variant of `convert_exprs` for when only a prefix of the expression types are known due to
-    /// the relevant signature being varargs
+    /// Variant of `convert_exprs` for the arguments of a function call.
+    /// Accounts for differences in translation for arguments, and for varargs where only a prefix
+    /// of the expression types are known.
     #[allow(clippy::vec_box/*, reason = "not worth a substantial refactor"*/)]
-    fn convert_exprs_partial(
+    fn convert_call_args(
         &self,
         ctx: ExprContext,
         exprs: &[CExprId],
-        arg_tys: &[CQualTypeId],
+        arg_tys: Option<&[CQualTypeId]>,
+        is_variadic: bool,
     ) -> TranslationResult<WithStmts<Vec<Box<Expr>>>> {
+        let arg_tys = if let Some(arg_tys) = arg_tys {
+            if !is_variadic {
+                assert!(arg_tys.len() == exprs.len());
+            }
+
+            arg_tys
+        } else {
+            &[]
+        };
+
         exprs
             .iter()
             .enumerate()
-            .map(|(n, arg)| self.convert_expr(ctx, *arg, arg_tys.get(n).copied()))
+            .map(|(n, arg)| self.convert_call_arg(ctx, *arg, arg_tys.get(n).copied()))
             .collect()
     }
 
@@ -3509,12 +3532,6 @@ impl<'c> Translation<'c> {
                     val = mk().cast_expr(val, ty);
                 }
 
-                // Most references to the va_list should refer to the VaList
-                // type, not VaListImpl
-                if !ctx.expecting_valistimpl && self.ast_context.is_va_list(qual_ty.ctype) {
-                    val = mk().method_call_expr(val, "as_va_list", Vec::new());
-                }
-
                 // If we are referring to a function and need its address, we
                 // need to cast it to fn() to ensure that it has a real address.
                 let mut set_unsafe = false;
@@ -4057,19 +4074,8 @@ impl<'c> Translation<'c> {
                     // We want to decay refs only when function is variadic
                     ctx.decay_ref = DecayRef::from(is_variadic);
 
-                    let args = if is_variadic {
-                        let arg_tys = arg_tys.unwrap_or_default();
-                        self.convert_exprs_partial(ctx.used(), args, arg_tys.as_slice())?
-                    } else {
-                        let arg_tys = if let Some(ref arg_tys) = arg_tys {
-                            assert!(arg_tys.len() == args.len());
-                            Some(arg_tys.as_slice())
-                        } else {
-                            None
-                        };
-
-                        self.convert_exprs(ctx.used(), args, arg_tys)?
-                    };
+                    let args =
+                        self.convert_call_args(ctx.used(), args, arg_tys.as_deref(), is_variadic)?;
 
                     let mut call_expr = args.map(|args| mk().call_expr(func, args));
                     if let Some(expected_ty) = override_ty {
@@ -4146,14 +4152,6 @@ impl<'c> Translation<'c> {
                         val = val.map(|v| mk().field_expr(v, field_name));
                     };
 
-                    // Most references to the va_list should refer to the VaList
-                    // type, not VaListImpl
-                    if !ctx.expecting_valistimpl && self.ast_context.is_va_list(qual_ty.ctype) {
-                        val = val.map(|v| {
-                            mk().method_call_expr(v, "as_va_list", Vec::<Box<Expr>>::new())
-                        });
-                    }
-
                     // if the context wants a different type, add a cast
                     if let Some(expected_ty) = override_ty {
                         if expected_ty != qual_ty {
@@ -4268,6 +4266,28 @@ impl<'c> Translation<'c> {
         Ok(expr)
     }
 
+    /// Wrapper around `convert_expr` for the arguments of a function call.
+    pub fn convert_call_arg(
+        &self,
+        ctx: ExprContext,
+        expr_id: CExprId,
+        override_ty: Option<CQualTypeId>,
+    ) -> TranslationResult<WithStmts<Box<Expr>>> {
+        let mut val;
+
+        if (self.ast_context.index(expr_id).kind.get_qual_type())
+            .map_or(false, |qtype| self.ast_context.is_va_list(qtype.ctype))
+        {
+            // No `override_ty` to avoid unwanted casting.
+            val = self.convert_expr(ctx, expr_id, None)?;
+            val = val.map(|val| mk().method_call_expr(val, "as_va_list", Vec::new()));
+        } else {
+            val = self.convert_expr(ctx, expr_id, override_ty)?;
+        }
+
+        Ok(val)
+    }
+
     /// Convert the expansion of a const-like macro.
     ///
     /// See [`TranspilerConfig::translate_const_macros`].
diff --git a/c2rust-transpile/src/translator/variadic.rs b/c2rust-transpile/src/translator/variadic.rs
@@ -144,7 +144,7 @@ impl<'c> Translation<'c> {
         val_id: CExprId,
     ) -> TranslationResult<WithStmts<Box<Expr>>> {
         if self.tcfg.translate_valist {
-            let val = self.convert_expr(ctx.expect_valistimpl().used(), val_id, None)?;
+            let val = self.convert_expr(ctx.used(), val_id, None)?;
 
             // The current implementation of the C-variadics feature doesn't allow us to
             // return `Option<fn(...) -> _>` from `VaList::arg`, so we detect function pointers
diff --git a/c2rust-transpile/tests/snapshots/snapshots__transpile-x86_64-linux@varargs.c.snap b/c2rust-transpile/tests/snapshots/snapshots__transpile-x86_64-linux@varargs.c.snap
@@ -49,7 +49,7 @@ pub unsafe extern "C" fn my_vprintf(
     mut format: *const ::core::ffi::c_char,
     mut ap: ::core::ffi::VaList,
 ) {
-    vprintf(format, ap.as_va_list() as ::core::ffi::VaList);
+    vprintf(format, ap.as_va_list());
 }
 #[no_mangle]
 pub unsafe extern "C" fn call_vprintf(mut format: *const ::core::ffi::c_char, mut args: ...) {
@@ -145,7 +145,7 @@ pub unsafe extern "C" fn restart_valist(mut fmt: *const ::core::ffi::c_char, mut
     vprintf(fmt, ap.as_va_list());
 }
 #[no_mangle]
-pub unsafe extern "C" fn print_int(mut ap: *mut ::core::ffi::VaList) {
+pub unsafe extern "C" fn print_int(mut ap: *mut ::core::ffi::VaListImpl) {
     printf(
         b"%d\0" as *const u8 as *const ::core::ffi::c_char,
         (*ap).arg::<::core::ffi::c_int>(),
@@ -156,7 +156,7 @@ pub unsafe extern "C" fn borrowed_valist(mut count: size_t, mut args: ...) {
     let mut ap: ::core::ffi::VaListImpl;
     ap = args.clone();
     while count > 0 as size_t {
-        print_int(&mut ap.as_va_list());
+        print_int(&mut ap);
         count = count.wrapping_sub(1);
     }
 }
