Skip to content

Commit 44a47b8

Browse files
rust-timerrust-timer
authored
Unrolled build for #152980
Rollup merge of #152980 - folkertdev:c-variadic-avr, r=tgross35 c-variadic: fix implementation on `avr` tracking issue: #44930 cc target maintainer @Patryk27 I ran into multiple issues, and although with this PR and a little harness I can run the test with qemu on avr, the implementation is perhaps not ideal. The problem we found is that on `avr` the `c_int/c_uint` types are `i16/u16`, and this was not handled in the c-variadic checks. Luckily there is a field in the target configuration that contains the targets `c_int_width`. However, this field is not actually used in `core` at all, there the 16-bit targets are just hardcoded. https://github.com/rust-lang/rust/blob/1500f0f47f5fe8ddcd6528f6c6c031b210b4eac5/library/core/src/ffi/primitives.rs#L174-L185 Perhaps we should expose this like endianness and pointer width? --- Finally there are some changes to the test to make it compile with `no_std`.
2 parents 1b8f2e4 + a875e14 commit 44a47b8

7 files changed

Lines changed: 108 additions & 39 deletions

File tree

‎compiler/rustc_codegen_llvm/src/intrinsic.rs‎

Lines changed: 39 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -285,37 +285,47 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {
285285
}
286286
sym::breakpoint => self.call_intrinsic("llvm.debugtrap", &[], &[]),
287287
sym::va_arg => {
288-
match result.layout.backend_repr {
289-
BackendRepr::Scalar(scalar) => {
290-
match scalar.primitive() {
291-
Primitive::Int(..) => {
292-
if self.cx().size_of(result.layout.ty).bytes() < 4 {
293-
// `va_arg` should not be called on an integer type
294-
// less than 4 bytes in length. If it is, promote
295-
// the integer to an `i32` and truncate the result
296-
// back to the smaller type.
297-
let promoted_result = emit_va_arg(self, args[0], tcx.types.i32);
298-
self.trunc(promoted_result, result.layout.llvm_type(self))
299-
} else {
300-
emit_va_arg(self, args[0], result.layout.ty)
301-
}
302-
}
303-
Primitive::Float(Float::F16) => {
304-
bug!("the va_arg intrinsic does not work with `f16`")
305-
}
306-
Primitive::Float(Float::F64) | Primitive::Pointer(_) => {
307-
emit_va_arg(self, args[0], result.layout.ty)
308-
}
309-
// `va_arg` should never be used with the return type f32.
310-
Primitive::Float(Float::F32) => {
311-
bug!("the va_arg intrinsic does not work with `f32`")
312-
}
313-
Primitive::Float(Float::F128) => {
314-
bug!("the va_arg intrinsic does not work with `f128`")
315-
}
288+
let BackendRepr::Scalar(scalar) = result.layout.backend_repr else {
289+
bug!("the va_arg intrinsic does not support non-scalar types")
290+
};
291+
292+
match scalar.primitive() {
293+
Primitive::Pointer(_) => {
294+
// Pointers are always OK.
295+
emit_va_arg(self, args[0], result.layout.ty)
296+
}
297+
Primitive::Int(..) => {
298+
let int_width = self.cx().size_of(result.layout.ty).bits();
299+
let target_c_int_width = self.cx().sess().target.options.c_int_width;
300+
if int_width < u64::from(target_c_int_width) {
301+
// Smaller integer types are automatically promototed and `va_arg`
302+
// should not be called on them.
303+
bug!(
304+
"va_arg got i{} but needs at least c_int (an i{})",
305+
int_width,
306+
target_c_int_width
307+
);
316308
}
309+
emit_va_arg(self, args[0], result.layout.ty)
310+
}
311+
Primitive::Float(Float::F16) => {
312+
bug!("the va_arg intrinsic does not support `f16`")
313+
}
314+
Primitive::Float(Float::F32) => {
315+
if self.cx().sess().target.arch == Arch::Avr {
316+
// c_double is actually f32 on avr.
317+
emit_va_arg(self, args[0], result.layout.ty)
318+
} else {
319+
bug!("the va_arg intrinsic does not support `f32` on this target")
320+
}
321+
}
322+
Primitive::Float(Float::F64) => {
323+
// 64-bit floats are always OK.
324+
emit_va_arg(self, args[0], result.layout.ty)
325+
}
326+
Primitive::Float(Float::F128) => {
327+
bug!("the va_arg intrinsic does not support `f128`")
317328
}
318-
_ => bug!("the va_arg intrinsic does not work with non-scalar types"),
319329
}
320330
}
321331

‎compiler/rustc_hir/src/lang_items.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ language_item_table! {
221221
UnsafeCell, sym::unsafe_cell, unsafe_cell_type, Target::Struct, GenericRequirement::None;
222222
UnsafePinned, sym::unsafe_pinned, unsafe_pinned_type, Target::Struct, GenericRequirement::None;
223223

224+
VaArgSafe, sym::va_arg_safe, va_arg_safe, Target::Trait, GenericRequirement::None;
224225
VaList, sym::va_list, va_list, Target::Struct, GenericRequirement::None;
225226

226227
Deref, sym::deref, deref_trait, Target::Trait, GenericRequirement::Exact(0);

‎compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
494494

495495
// There are a few types which get autopromoted when passed via varargs
496496
// in C but we just error out instead and require explicit casts.
497+
//
498+
// We use implementations of VaArgSafe as the source of truth. On some embedded
499+
// targets, c_double is f32 and c_int/c_uing are i16/u16, and these types implement
500+
// VaArgSafe there. On all other targets, these types do not implement VaArgSafe.
501+
//
502+
// cfg(bootstrap): change the if let to an unwrap.
497503
let arg_ty = self.structurally_resolve_type(arg.span, arg_ty);
504+
if let Some(trait_def_id) = tcx.lang_items().va_arg_safe()
505+
&& self
506+
.type_implements_trait(trait_def_id, [arg_ty], self.param_env)
507+
.must_apply_modulo_regions()
508+
{
509+
continue;
510+
}
511+
498512
match arg_ty.kind() {
499513
ty::Float(ty::FloatTy::F32) => {
500514
variadic_error(tcx.sess, arg.span, arg_ty, "c_double");

‎compiler/rustc_span/src/symbol.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2211,6 +2211,7 @@ symbols! {
22112211
v1,
22122212
v8plus,
22132213
va_arg,
2214+
va_arg_safe,
22142215
va_copy,
22152216
va_end,
22162217
va_list,

‎library/core/src/ffi/va_list.rs‎

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -266,14 +266,17 @@ impl<'f> const Drop for VaList<'f> {
266266
mod sealed {
267267
pub trait Sealed {}
268268

269+
impl Sealed for i16 {}
269270
impl Sealed for i32 {}
270271
impl Sealed for i64 {}
271272
impl Sealed for isize {}
272273

274+
impl Sealed for u16 {}
273275
impl Sealed for u32 {}
274276
impl Sealed for u64 {}
275277
impl Sealed for usize {}
276278

279+
impl Sealed for f32 {}
277280
impl Sealed for f64 {}
278281

279282
impl<T> Sealed for *mut T {}
@@ -297,24 +300,64 @@ mod sealed {
297300
// We may unseal this trait in the future, but currently our `va_arg` implementations don't support
298301
// types with an alignment larger than 8, or with a non-scalar layout. Inline assembly can be used
299302
// to accept unsupported types in the meantime.
303+
#[lang = "va_arg_safe"]
300304
pub unsafe trait VaArgSafe: sealed::Sealed {}
301305

302-
// i8 and i16 are implicitly promoted to c_int in C, and cannot implement `VaArgSafe`.
306+
crate::cfg_select! {
307+
any(target_arch = "avr", target_arch = "msp430") => {
308+
// c_int/c_uint are i16/u16 on these targets.
309+
//
310+
// - i8 is implicitly promoted to c_int in C, and cannot implement `VaArgSafe`.
311+
// - u8 is implicitly promoted to c_uint in C, and cannot implement `VaArgSafe`.
312+
unsafe impl VaArgSafe for i16 {}
313+
unsafe impl VaArgSafe for u16 {}
314+
}
315+
_ => {
316+
// c_int/c_uint are i32/u32 on this target.
317+
//
318+
// - i8 and i16 are implicitly promoted to c_int in C, and cannot implement `VaArgSafe`.
319+
// - u8 and u16 are implicitly promoted to c_uint in C, and cannot implement `VaArgSafe`.
320+
}
321+
}
322+
323+
crate::cfg_select! {
324+
target_arch = "avr" => {
325+
// c_double is f32 on this target.
326+
unsafe impl VaArgSafe for f32 {}
327+
}
328+
_ => {
329+
// c_double is f64 on this target.
330+
//
331+
// - f32 is implicitly promoted to c_double in C, and cannot implement `VaArgSafe`.
332+
}
333+
}
334+
303335
unsafe impl VaArgSafe for i32 {}
304336
unsafe impl VaArgSafe for i64 {}
305337
unsafe impl VaArgSafe for isize {}
306338

307-
// u8 and u16 are implicitly promoted to c_int in C, and cannot implement `VaArgSafe`.
308339
unsafe impl VaArgSafe for u32 {}
309340
unsafe impl VaArgSafe for u64 {}
310341
unsafe impl VaArgSafe for usize {}
311342

312-
// f32 is implicitly promoted to c_double in C, and cannot implement `VaArgSafe`.
313343
unsafe impl VaArgSafe for f64 {}
314344

315345
unsafe impl<T> VaArgSafe for *mut T {}
316346
unsafe impl<T> VaArgSafe for *const T {}
317347

348+
// Check that relevant `core::ffi` types implement `VaArgSafe`.
349+
const _: () = {
350+
const fn va_arg_safe_check<T: VaArgSafe>() {}
351+
352+
va_arg_safe_check::<crate::ffi::c_int>();
353+
va_arg_safe_check::<crate::ffi::c_uint>();
354+
va_arg_safe_check::<crate::ffi::c_long>();
355+
va_arg_safe_check::<crate::ffi::c_ulong>();
356+
va_arg_safe_check::<crate::ffi::c_longlong>();
357+
va_arg_safe_check::<crate::ffi::c_ulonglong>();
358+
va_arg_safe_check::<crate::ffi::c_double>();
359+
};
360+
318361
impl<'f> VaList<'f> {
319362
/// Read an argument from the variable argument list, and advance to the next argument.
320363
///

‎tests/run-make/c-link-to-rust-va-list-fn/checkrust.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,17 @@ pub unsafe extern "C" fn check_list_1(mut ap: VaList) -> usize {
3030
continue_if!(ap.arg::<c_int>() == '4' as c_int);
3131
continue_if!(ap.arg::<c_int>() == ';' as c_int);
3232
continue_if!(ap.arg::<c_int>() == 0x32);
33-
continue_if!(ap.arg::<c_int>() == 0x10000001);
33+
continue_if!(ap.arg::<i32>() == 0x10000001);
3434
continue_if!(compare_c_str(ap.arg::<*const c_char>(), c"Valid!"));
3535
0
3636
}
3737

3838
#[unsafe(no_mangle)]
3939
pub unsafe extern "C" fn check_list_2(mut ap: VaList) -> usize {
40-
continue_if!(ap.arg::<c_double>() == 3.14f64);
40+
continue_if!(ap.arg::<c_double>() == 3.14);
4141
continue_if!(ap.arg::<c_long>() == 12);
4242
continue_if!(ap.arg::<c_int>() == 'a' as c_int);
43-
continue_if!(ap.arg::<c_double>() == 6.28f64);
43+
continue_if!(ap.arg::<c_double>() == 6.28);
4444
continue_if!(compare_c_str(ap.arg::<*const c_char>(), c"Hello"));
4545
continue_if!(ap.arg::<c_int>() == 42);
4646
continue_if!(compare_c_str(ap.arg::<*const c_char>(), c"World"));
@@ -49,7 +49,7 @@ pub unsafe extern "C" fn check_list_2(mut ap: VaList) -> usize {
4949

5050
#[unsafe(no_mangle)]
5151
pub unsafe extern "C" fn check_list_copy_0(mut ap: VaList) -> usize {
52-
continue_if!(ap.arg::<c_double>() == 6.28f64);
52+
continue_if!(ap.arg::<c_double>() == 6.28);
5353
continue_if!(ap.arg::<c_int>() == 16);
5454
continue_if!(ap.arg::<c_int>() == 'A' as c_int);
5555
continue_if!(compare_c_str(ap.arg::<*const c_char>(), c"Skip Me!"));
@@ -66,7 +66,7 @@ pub unsafe extern "C" fn check_varargs_0(_: c_int, mut ap: ...) -> usize {
6666

6767
#[unsafe(no_mangle)]
6868
pub unsafe extern "C" fn check_varargs_1(_: c_int, mut ap: ...) -> usize {
69-
continue_if!(ap.arg::<c_double>() == 3.14f64);
69+
continue_if!(ap.arg::<c_double>() == 3.14);
7070
continue_if!(ap.arg::<c_long>() == 12);
7171
continue_if!(ap.arg::<c_int>() == 'A' as c_int);
7272
continue_if!(ap.arg::<c_longlong>() == 1);
@@ -156,7 +156,7 @@ extern "C" fn run_test_variadic() -> usize {
156156

157157
#[unsafe(no_mangle)]
158158
extern "C" fn run_test_va_list_by_value() -> usize {
159-
unsafe extern "C" fn helper(mut ap: ...) -> usize {
159+
unsafe extern "C" fn helper(ap: ...) -> usize {
160160
unsafe { test_va_list_by_value(ap) }
161161
}
162162

‎tests/run-make/c-link-to-rust-va-list-fn/test.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ int test_rust(size_t (*fn)(va_list), ...) {
3232
int main(int argc, char* argv[]) {
3333
assert(test_rust(check_list_0, 0x01LL, 0x02, 0x03LL) == 0);
3434

35-
assert(test_rust(check_list_1, -1, 'A', '4', ';', 0x32, 0x10000001, "Valid!") == 0);
35+
assert(test_rust(check_list_1, -1, 'A', '4', ';', 0x32, (int32_t)0x10000001, "Valid!") == 0);
3636

3737
assert(test_rust(check_list_2, 3.14, 12l, 'a', 6.28, "Hello", 42, "World") == 0);
3838

0 commit comments

Comments
 (0)