diff --git a/c2rust-transpile/src/convert_type.rs b/c2rust-transpile/src/convert_type.rs index ec11b14059..e50985d269 100644 --- a/c2rust-transpile/src/convert_type.rs +++ b/c2rust-transpile/src/convert_type.rs @@ -250,7 +250,10 @@ impl TypeConverter { ) -> TranslationResult> { let barefn_inputs = params .iter() - .map(|x| mk().bare_arg(self.convert(ctxt, x.ctype).unwrap(), None::>)) + .map(|x| { + let ty = self.convert_function_param(ctxt, x.ctype).unwrap(); + mk().bare_arg(ty, None::>) + }) .collect::>(); let output = match ret { @@ -317,7 +320,7 @@ impl TypeConverter { ctype: CTypeId, ) -> TranslationResult> { 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> { + 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 index 0bbc1637c2..15732a8823 100644 --- 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 index 383a55a70b..ff914705ff 100644 --- a/c2rust-transpile/src/translator/mod.rs +++ b/c2rust-transpile/src/translator/mod.rs @@ -120,10 +120,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, } @@ -187,13 +183,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 { @@ -494,7 +483,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, }; @@ -1177,6 +1165,11 @@ struct ConvertedVariable { pub init: TranslationResult>>, } +struct ConvertedFunctionParam { + pub ty: Box, + pub mutbl: Mutability, +} + impl<'c> Translation<'c> { pub fn new( mut ast_context: TypedAstContext, @@ -2250,8 +2243,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() @@ -2968,6 +2960,25 @@ impl<'c> Translation<'c> { Ok(ConvertedVariable { ty, mutbl, init }) } + fn convert_function_param( + &self, + ctx: ExprContext, + typ: CQualTypeId, + ) -> TranslationResult { + 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> { if let Some(cur_file) = self.cur_file.get() { self.import_type(type_id, cur_file); @@ -3245,19 +3256,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>>> { + 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() } @@ -3417,12 +3440,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; @@ -3958,19 +3975,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 { @@ -4047,14 +4053,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::>::new()) - }); - } - // if the context wants a different type, add a cast if let Some(expected_ty) = override_ty { if expected_ty != qual_ty { @@ -4169,6 +4167,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, + ) -> TranslationResult>> { + 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 index e38019e086..9e10be21c7 100644 --- 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>> { 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 _>` from `VaList::arg`, so we detect function pointers diff --git a/tests/items/src/test_varargs.rs b/tests/items/src/test_varargs.rs index 8c9d23469d..d373bb9127 100644 --- a/tests/items/src/test_varargs.rs +++ b/tests/items/src/test_varargs.rs @@ -4,8 +4,9 @@ use crate::varargs::rust_call_printf; // See #1281. Varargs don't yet work on aarch64. #[cfg(not(target_arch = "aarch64"))] use crate::varargs::{ - rust_call_vprintf, rust_my_printf, rust_restart_valist, rust_sample_stddev, rust_simple_vacopy, - rust_valist_struct_member, rust_valist_struct_pointer_member, + rust_borrowed_valist, rust_call_vprintf, rust_my_printf, rust_restart_valist, + rust_sample_stddev, rust_simple_vacopy, rust_valist_struct_member, + rust_valist_struct_pointer_member, }; use std::ffi::c_char; @@ -32,6 +33,8 @@ extern "C" { fn restart_valist(_: *const c_char, ...); + fn borrowed_valist(_: usize, ...); + fn sample_stddev(count: i32, ...) -> f64; } @@ -108,6 +111,15 @@ pub fn test_restart_valist() { } } +#[cfg(not(target_arch = "aarch64"))] +#[test] +pub fn test_borrowed_valist() { + unsafe { + borrowed_valist(5, 1, 2, 3, 4, 5); + rust_borrowed_valist(5, 1, 2, 3, 4, 5); + } +} + #[cfg(not(target_arch = "aarch64"))] #[test] pub fn test_sample_stddev() { diff --git a/tests/items/src/varargs.c b/tests/items/src/varargs.c index f39111214b..6f0c99c86b 100644 --- a/tests/items/src/varargs.c +++ b/tests/items/src/varargs.c @@ -112,6 +112,22 @@ void restart_valist(const char *fmt, ...) { va_end(ap); } +void print_int(va_list *ap) { + printf("%d", va_arg(*ap, int)); +} + +void borrowed_valist(size_t count, ...) { + va_list ap; + va_start(ap, count); + + while (count > 0) { + print_int(&ap); + --count; + } + + va_end(ap); +} + // From: https://en.cppreference.com/w/c/variadic/va_copy (CC-BY-SA) double sample_stddev(int count, ...) {