Rollup merge of #155486 - folkertdev:c-variadic-roundtrip, r=RalfJung

c-variadic: add roundtrip test

tracking issue: https://github.com/rust-lang/rust/issues/44930

Test that our `va_arg` implementation matches (as in, can decode) how LLVM passes c-variadic arguments.

And some comment followup to https://github.com/rust-lang/rust/pull/152980 (cc @RalfJung, feel free to review this PR too btw).

r? tgross35

compiler/rustc_codegen_llvm/src/intrinsic.rs

@@ -3,7 +3,8 @@
 use std::{assert_matches, iter, ptr};
 
 use rustc_abi::{
-    Align, BackendRepr, Float, HasDataLayout, NumScalableVectors, Primitive, Size, WrappingRange,
+    Align, BackendRepr, Float, HasDataLayout, Integer, NumScalableVectors, Primitive, Size,
+    WrappingRange,
 };
 use rustc_codegen_ssa::base::{compare_simd_types, wants_msvc_seh, wants_wasm_eh};
 use rustc_codegen_ssa::common::{IntPredicate, TypeKind};
@@ -288,10 +289,17 @@ fn codegen_intrinsic_call(
                     bug!("the va_arg intrinsic does not support non-scalar types")
                 };
 
+                // We reject types that would never be passed as varargs in C because
+                // they get promoted to a larger type, specifically integers smaller than
+                // c_int and float type smaller than c_double.
                 match scalar.primitive() {
                     Primitive::Pointer(_) => {
                         // Pointers are always OK.
-                        emit_va_arg(self, args[0], result.layout.ty)
+                    }
+                    Primitive::Int(Integer::I128, _) => {
+                        // FIXME: maybe we should support these? At least on 32-bit powerpc
+                        // the logic in LLVM does not handle i128 correctly though.
+                        bug!("the va_arg intrinsic does not support `i128`/`u128`")
                     }
                     Primitive::Int(..) => {
                         let int_width = self.cx().size_of(result.layout.ty).bits();
@@ -305,27 +313,26 @@ fn codegen_intrinsic_call(
                                 target_c_int_width
                             );
                         }
-                        emit_va_arg(self, args[0], result.layout.ty)
                     }
                     Primitive::Float(Float::F16) => {
                         bug!("the va_arg intrinsic does not support `f16`")
                     }
                     Primitive::Float(Float::F32) => {
-                        if self.cx().sess().target.arch == Arch::Avr {
-                            // c_double is actually f32 on avr.
-                            emit_va_arg(self, args[0], result.layout.ty)
-                        } else {
+                        // c_double is actually f32 on avr.
+                        if self.cx().sess().target.arch != Arch::Avr {
                             bug!("the va_arg intrinsic does not support `f32` on this target")
                         }
                     }
                     Primitive::Float(Float::F64) => {
                         // 64-bit floats are always OK.
-                        emit_va_arg(self, args[0], result.layout.ty)
                     }
                     Primitive::Float(Float::F128) => {
+                        // FIXME(f128) figure out whether we should support this.
                         bug!("the va_arg intrinsic does not support `f128`")
                     }
                 }
+
+                emit_va_arg(self, args[0], result.layout.ty)
             }
 
             sym::volatile_load | sym::unaligned_volatile_load => {

tests/ui/c-variadic/roundtrip.rs

@@ -0,0 +1,78 @@
+//@ run-pass
+//@ ignore-backends: gcc
+#![feature(c_variadic, const_c_variadic, const_destruct, const_raw_ptr_comparison)]
+
+use std::ffi::*;
+
+// In rustc we implement `va_arg` for the callee reading from a VaList, but still rely on LLVM
+// for exactly how to pass c-variadic arguments and for constructing the VaList. Here we test
+// that the rustc implementation works with what LLVM gives us.
+
+#[allow(improper_ctypes_definitions)]
+const unsafe extern "C" fn variadic<T: VaArgSafe>(mut ap: ...) -> (T, T) {
+    let x = ap.arg::<T>();
+    // Intersperse a small type to test alignment logic. A `u32` (i.e. `c_uint`) is the smallest
+    // type that implements `VaArgSafe`: smaller types would automatically be promoted.
+    assert!(ap.arg::<u32>() == 0xAAAA_AAAA);
+    let y = ap.arg::<T>();
+
+    (x, y)
+}
+
+macro_rules! roundtrip {
+    ($ty:ty, $a:expr, $b:expr) => {
+        const {
+            let a: $ty = $a;
+            let b: $ty = $b;
+            let (x, y) = variadic::<$ty>(a, 0xAAAA_AAAAu32, b);
+            assert!(a == x);
+            assert!(b == y);
+        }
+
+        let a: $ty = $a;
+        let b: $ty = $b;
+        assert_eq!(variadic::<$ty>(a, 0xAAAA_AAAAu32, b), (a, b))
+    };
+}
+
+macro_rules! roundtrip_ptr {
+    ($ty:ty, $a:expr, $b:expr) => {
+        const {
+            let a: $ty = $a;
+            let b: $ty = $b;
+            let (x, y) = variadic::<$ty>(a, 0xAAAA_AAAAu32, b);
+            assert!(a.guaranteed_eq(x).unwrap());
+            assert!(b.guaranteed_eq(y).unwrap());
+        }
+
+        let a: $ty = $a;
+        let b: $ty = $b;
+        assert_eq!(variadic::<$ty>(a, 0xAAAA_AAAAu32, b), (a, b))
+    };
+}
+
+fn main() {
+    unsafe {
+        roundtrip!(i32, -1, -2);
+        roundtrip!(i64, -1, -2);
+        roundtrip!(isize, -1, -2);
+        roundtrip!(c_int, -1, -2);
+        roundtrip!(c_long, -1, -2);
+        roundtrip!(c_longlong, -1, -2);
+
+        roundtrip!(u32, 1, 2);
+        roundtrip!(u64, 1, 2);
+        roundtrip!(usize, 1, 2);
+        roundtrip!(c_uint, 1, 2);
+        roundtrip!(c_ulong, 1, 2);
+        roundtrip!(c_ulonglong, 1, 2);
+
+        roundtrip!(f64, 3.14, 6.28);
+        roundtrip!(c_double, 3.14, 6.28);
+
+        static mut A: u32 = 1u32;
+        static mut B: u32 = 2u32;
+        roundtrip_ptr!(*const u32, &raw const A, &raw const B);
+        roundtrip_ptr!(*mut u32, &raw mut A, &raw mut B);
+    }
+}