c-variadic: fix implementation on `avr` by folkertdev · Pull Request #152980 · rust-lang/rust

folkertdev · 2026-02-22T17:28:52Z

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.

rust/library/core/src/ffi/primitives.rs

Lines 174 to 185 in 1500f0f

    
           mod c_int_definition { 
        
               crate::cfg_select! { 
        
                   any(target_arch = "avr", target_arch = "msp430") => { 
        
                       pub(super) type c_int = i16; 
        
                       pub(super) type c_uint = u16; 
        
                   } 
        
                   _ => { 
        
                       pub(super) type c_int = i32; 
        
                       pub(super) type c_uint = u32; 
        
                   } 
        
               } 
        
           }

Perhaps we should expose this like endianness and pointer width?

Next, it turned out that c_double should be f32 on this target. So for now I hardcoded that in core too, but doing this properly would require another field in the target config I guess?

Finally there are some changes to the test to make it compile with no_std.

workingjubilee · 2026-02-22T17:39:32Z

...a double-precision float is sometimes a single-precision float in C?

folkertdev · 2026-02-22T17:44:07Z

I don't make the rules... This is what the target does (it's a 16-bit target, so it sort of makes sense I guess)

folkertdev · 2026-02-22T18:04:54Z

@Patryk27 did the c_double difference just not come up before? and by extension, what other c_* types are incorrect?

Also, I have some local code that makes the test compile on avr, which I can then run in qemu. It looks like there is no qemu-avr like there is for many other targets, so I'm using qemu-system-avr which is less convenient. Do you have ideas on making this test easier to run on embedded targets?

folkertdev · 2026-02-22T18:57:38Z

compiler/rustc_codegen_llvm/src/intrinsic.rs

+                                assert!(
+                                    self.cx().size_of(result.layout.ty).bits()
+                                        >= u64::from(self.cx().sess().target.options.c_int_width)
+                                );


@workingjubilee how do you feel about the checks here. The c_int_width is not actually used by core at all, it has its own logic. We already have several layers of defence against an incorrect type ending up here, so maybe we can just remove the check here?

workingjubilee · 2026-02-22T20:20:33Z

I think it mostly bothers me that it isn't a symmetric halving. If the (almost always software, I assume) implementation of c_float used f16, then it would make sense.

Patryk27 · 2026-02-23T18:25:22Z

did the c_double difference just not come up before?

I don't recall anything - it's not every day one tries to use floating-point on an 8-bit micro!

We did have a tangentially related type-problem over at compiler-builtins - it turns out that on AVRs some floating-point intrinsics return i8 instead of i32:

https://github.com/rust-lang/compiler-builtins/pull/791/changes#diff-dbe76d574d284f795de0a76a6ab505c78ca91b59579778633a67503ecd6c22c4R6

... but I'm not sure if that type is exposed as any of those c_ thingies (it's not c_int).

and by extension, what other c_* types are incorrect?

You can see the sizes here:

https://gcc.gnu.org/wiki/avr-gcc#Type_Layout

Note that double and long double say 4 or 8, because you can change them via command line (-mdouble=32/64), but I think we don't have to worry about this.

It looks like there is no qemu-avr like there is for many other targets, [...]

There's simavr which I've managed to hook up with Rust, see: avr-tester - does that help? 👀

folkertdev · 2026-02-23T18:51:42Z

I don't recall anything - it's not every day one tries to use floating-point on an 8-bit micro!

Fair. I didn't even do any math though, just passed some bits around.

You can see the sizes here:

The rest looks good.

There's simavr which I've managed to hook up with Rust, see: avr-tester - does that help? 👀

So conceivably you could take the rmake test and turn it into a rust repo (using cc to compile the C)? Maybe it can actually be configured as the runner in the bootstrap.toml (and/or .cargo/config)

folkertdev

r?workingjubilee

View changes since this review

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

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

folkertdev · 2026-02-23T19:01:10Z

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

+                // targets, c_double is f32 and c_int/c_uing are i16/u16, and these types implement
+                // VaArgSafe there. On all other targets, these types do not implement VaArgSafe.
                let arg_ty = self.structurally_resolve_type(arg.span, arg_ty);
+                if let Some(trait_def_id) = tcx.lang_items().va_arg_safe()


just unwrap ing the lang item causes bootstrap issues.

rustbot · 2026-02-23T19:01:59Z

workingjubilee is currently at their maximum review capacity.
They may take a while to respond.

…eanup, r=jieyouxu Cleanup of c-variadic link test Some changes pulled out of rust-lang#152980 that are just cosmetic, but will help make the code run on embedded targets. r? jieyouxu

Rollup merge of #153309 - folkertdev:c-variadic-link-test-cleanup, r=jieyouxu Cleanup of c-variadic link test Some changes pulled out of #152980 that are just cosmetic, but will help make the code run on embedded targets. r? jieyouxu

so that we can check whether a type implements the trait

rustbot · 2026-03-03T10:52:13Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

folkertdev added the F-c_variadic `#![feature(c_variadic)]` label Feb 22, 2026

folkertdev commented Feb 22, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

folkertdev force-pushed the c-variadic-avr branch from f039e11 to 61b24ec Compare February 22, 2026 20:03

rustbot added the A-test-infra-minicore Area: `minicore` test auxiliary and `//@ add-core-stubs` label Feb 22, 2026

This comment has been minimized.

Sign in to view

folkertdev force-pushed the c-variadic-avr branch from 61b24ec to a88e5f9 Compare February 22, 2026 20:47

folkertdev force-pushed the c-variadic-avr branch from a88e5f9 to 0e7395a Compare February 23, 2026 18:54

folkertdev commented Feb 23, 2026

View reviewed changes

rustbot assigned workingjubilee Feb 23, 2026

folkertdev marked this pull request as ready for review February 23, 2026 19:02

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 23, 2026

folkertdev mentioned this pull request Mar 2, 2026

Cleanup of c-variadic link test #153309

Merged

c-variadic: handle c_int being i16 and c_double being f32 on avr

ae52c99

c-variadic: make VaArgSafe a lang item

dbdc731

so that we can check whether a type implements the trait

folkertdev force-pushed the c-variadic-avr branch from 0e7395a to dbdc731 Compare March 3, 2026 10:52

	mod c_int_definition {
	crate::cfg_select! {
	any(target_arch = "avr", target_arch = "msp430") => {
	pub(super) type c_int = i16;
	pub(super) type c_uint = u16;
	}
	_ => {
	pub(super) type c_int = i32;
	pub(super) type c_uint = u32;
	}
	}
	}

Uh oh!

Conversation

folkertdev commented Feb 22, 2026

Uh oh!

workingjubilee commented Feb 22, 2026

Uh oh!

folkertdev commented Feb 22, 2026

Uh oh!

folkertdev commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

folkertdev Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

workingjubilee commented Feb 22, 2026

Uh oh!

This comment has been minimized.

Patryk27 commented Feb 23, 2026

Uh oh!

folkertdev commented Feb 23, 2026

Uh oh!

folkertdev left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

folkertdev Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

rustbot commented Feb 23, 2026

Uh oh!

rustbot commented Mar 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

folkertdev commented Feb 22, 2026 •

edited

Loading

folkertdev left a comment •

edited by rustbot

Loading