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?

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?

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

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

folkertdev

@rustbot ready

View changes since this review

folkertdev · 2026-04-15T22:37:23Z

 // types with an alignment larger than 8, or with a non-scalar layout. Inline assembly can be used
 // to accept unsupported types in the meantime.
+#[lang = "va_arg_safe"]
 pub unsafe trait VaArgSafe: sealed::Sealed {}


Seems like a bunch of work for little upside? But maybe

folkertdev · 2026-04-15T22:37:39Z

 unsafe impl VaArgSafe for usize {}

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


Thanks for checking!

tgross35

One last thought, r=me either way

View changes since this review

tgross35 · 2026-04-16T02:26:51Z

+// Check that relevant `core::ffi` types implement `VaArgSafe`.
+const _: () = {
+    const fn va_arg_safe_check<T: VaArgSafe>() {}
+
+    va_arg_safe_check::<crate::ffi::c_int>();
+    va_arg_safe_check::<crate::ffi::c_uint>();
+    va_arg_safe_check::<crate::ffi::c_double>();
+};


Maybe worth adding long/ulong and longlong/ulonglong?

so that we can check whether a type implements the trait

folkertdev · 2026-04-16T11:24:15Z

@bors r=tgross35

rust-bors · 2026-04-16T11:24:19Z

📌 Commit a875e14 has been approved by tgross35

It is now in the queue for this repository.

RalfJung · 2026-04-16T11:50:16Z

+                };
+
+                match scalar.primitive() {
+                    Primitive::Pointer(_) => {


Suggested change

Primitive::Pointer(_) => {

// We reject types that would never be passed as varargs in C because

// they get promoted to a larger type: small integer types, and all

// float types except `f64`.

Primitive::Pointer(_) => {

View changes since the review

RalfJung · 2026-04-16T11:51:30Z

+                    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)


This line is always the same, right?

So maybe refactor this so the match just determines whether the type is valid (let valid = ...), and then always continue the same way?

View changes since the review

@Patryk27

c-variadic: fix implementation on `avr` tracking issue: rust-lang#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`.

@Patryk27

c-variadic: fix implementation on `avr` tracking issue: rust-lang#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`.

@Patryk27

c-variadic: fix implementation on `avr` tracking issue: rust-lang#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`.

Rollup of 13 pull requests Successful merges: - #152980 (c-variadic: fix implementation on `avr`) - #154491 (Extend `core::char`'s documentation of casing issues (and fix a rustdoc bug)) - #155354 (Remove AttributeSafety from BUILTIN_ATTRIBUTES) - #154970 (rustdoc: preserve `doc(cfg)` on locally re-exported type aliases) - #155095 (changed the information provided by (mut x) to mut x (Fix 155030)) - #155358 (ImproperCTypes: Move erasing_region_normalisation into helper function) - #155377 (tests/debuginfo/basic-stepping.rs: Remove FIXME related to ZSTs) - #155383 (Rearrange `rustc_ast_pretty`) - #155384 (triagebot: notify on diagnostic attribute changes) - #155386 (Use `box_new` diagnostic item for Box::new suggestions) - #155391 (Small refactor of `QueryJob::latch` method) - #155395 (Tweak how the "copy path" rustdoc button works to allow some accessibility tool to work with rustdoc) - #155396 (`as_ref_unchecked` docs link fix)

Rollup of 19 pull requests Successful merges: - #141633 (Suggest to bind `self.x` to `x` when field `x` may be in format string) - #152980 (c-variadic: fix implementation on `avr`) - #154491 (Extend `core::char`'s documentation of casing issues (and fix a rustdoc bug)) - #155318 (Use mutable pointers for Unix path buffers) - #155335 (Bump bootstrap to 1.96 beta) - #155354 (Remove AttributeSafety from BUILTIN_ATTRIBUTES) - #154970 (rustdoc: preserve `doc(cfg)` on locally re-exported type aliases) - #155095 (changed the information provided by (mut x) to mut x (Fix 155030)) - #155305 (Make `convert_while_ascii` unsafe) - #155358 (ImproperCTypes: Move erasing_region_normalisation into helper function) - #155377 (tests/debuginfo/basic-stepping.rs: Remove FIXME related to ZSTs) - #155383 (Rearrange `rustc_ast_pretty`) - #155384 (triagebot: notify on diagnostic attribute changes) - #155386 (Use `box_new` diagnostic item for Box::new suggestions) - #155391 (Small refactor of `QueryJob::latch` method) - #155395 (Tweak how the "copy path" rustdoc button works to allow some accessibility tool to work with rustdoc) - #155396 (`as_ref_unchecked` docs link fix) - #155411 (compiletest: Remove the `//@ should-ice` directive) - #155413 (fix: typo in `std::fs::hard_link` documentation)

@Patryk27

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`.

@RalfJung

…=RalfJung c-variadic: add roundtrip test tracking issue: rust-lang#44930 Test that our `va_arg` implementation matches (as in, can decode) how LLVM passes c-variadic arguments. And some comment followup to rust-lang#152980 (cc @RalfJung, feel free to review this PR too btw). r? tgross35

@RalfJung

Rollup merge of #155486 - folkertdev:c-variadic-roundtrip, r=RalfJung c-variadic: add roundtrip test tracking issue: #44930 Test that our `va_arg` implementation matches (as in, can decode) how LLVM passes c-variadic arguments. And some comment followup to #152980 (cc @RalfJung, feel free to review this PR too btw). r? tgross35

Rollup of 19 pull requests Successful merges: - rust-lang/rust#141633 (Suggest to bind `self.x` to `x` when field `x` may be in format string) - rust-lang/rust#152980 (c-variadic: fix implementation on `avr`) - rust-lang/rust#154491 (Extend `core::char`'s documentation of casing issues (and fix a rustdoc bug)) - rust-lang/rust#155318 (Use mutable pointers for Unix path buffers) - rust-lang/rust#155335 (Bump bootstrap to 1.96 beta) - rust-lang/rust#155354 (Remove AttributeSafety from BUILTIN_ATTRIBUTES) - rust-lang/rust#154970 (rustdoc: preserve `doc(cfg)` on locally re-exported type aliases) - rust-lang/rust#155095 (changed the information provided by (mut x) to mut x (Fix 155030)) - rust-lang/rust#155305 (Make `convert_while_ascii` unsafe) - rust-lang/rust#155358 (ImproperCTypes: Move erasing_region_normalisation into helper function) - rust-lang/rust#155377 (tests/debuginfo/basic-stepping.rs: Remove FIXME related to ZSTs) - rust-lang/rust#155383 (Rearrange `rustc_ast_pretty`) - rust-lang/rust#155384 (triagebot: notify on diagnostic attribute changes) - rust-lang/rust#155386 (Use `box_new` diagnostic item for Box::new suggestions) - rust-lang/rust#155391 (Small refactor of `QueryJob::latch` method) - rust-lang/rust#155395 (Tweak how the "copy path" rustdoc button works to allow some accessibility tool to work with rustdoc) - rust-lang/rust#155396 (`as_ref_unchecked` docs link fix) - rust-lang/rust#155411 (compiletest: Remove the `//@ should-ice` directive) - rust-lang/rust#155413 (fix: typo in `std::fs::hard_link` documentation)

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

folkertdev commented Feb 22, 2026

View reviewed changes

Comment thread compiler/rustc_codegen_llvm/src/intrinsic.rs Outdated

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

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

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

Comment thread compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

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

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

folkertdev force-pushed the c-variadic-avr branch from 0712ba6 to ebeb9a0 Compare April 15, 2026 10:41

folkertdev commented Apr 15, 2026

View reviewed changes

tgross35 approved these changes Apr 16, 2026

View reviewed changes

folkertdev added 2 commits April 16, 2026 11:48

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

b8ba400

c-variadic: make VaArgSafe a lang item

a875e14

so that we can check whether a type implements the trait

folkertdev force-pushed the c-variadic-avr branch from ebeb9a0 to a875e14 Compare April 16, 2026 09:48

folkertdev mentioned this pull request Apr 16, 2026

va_arg handling: remove integer promotion work-around #155387

Closed

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 16, 2026

RalfJung reviewed Apr 16, 2026

View reviewed changes

Comment thread compiler/rustc_codegen_llvm/src/intrinsic.rs

RalfJung reviewed Apr 16, 2026

View reviewed changes

JonathanBrouwer mentioned this pull request Apr 16, 2026

Rollup of 15 pull requests #155398

Closed

JonathanBrouwer mentioned this pull request Apr 16, 2026

Rollup of 14 pull requests #155401

Closed

GuillaumeGomez mentioned this pull request Apr 16, 2026

Rollup of 13 pull requests #155407

Closed

Zalathar mentioned this pull request Apr 17, 2026

Rollup of 19 pull requests #155416

Merged

rust-bors Bot merged commit 91a7d1b into rust-lang:main Apr 17, 2026
11 checks passed

rustbot added this to the 1.97.0 milestone Apr 17, 2026

folkertdev mentioned this pull request Apr 18, 2026

c-variadic: add roundtrip test #155486

Merged

	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;
	}
	}
	}

-                    Primitive::Pointer(_) => {
+                    // We reject types that would never be passed as varargs in C because
+                    // they get promoted to a larger type: small integer types, and all
+                    // float types except `f64`.
+                    Primitive::Pointer(_) => {

Uh oh!

Conversation

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

Uh oh!

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!

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!

Uh oh!

rustbot 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!

folkertdev Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

folkertdev Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

tgross35 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!

tgross35 Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

folkertdev commented Apr 16, 2026

Uh oh!

rust-bors Bot commented Apr 16, 2026

Uh oh!

RalfJung Apr 16, 2026 • 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!

RalfJung Apr 16, 2026 • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

folkertdev commented Feb 22, 2026 •

edited by rustbot

Loading

folkertdev commented Feb 22, 2026 •

edited

Loading

folkertdev left a comment •

edited by rustbot

Loading

folkertdev left a comment •

edited by rustbot

Loading

tgross35 left a comment •

edited by rustbot

Loading

RalfJung Apr 16, 2026 •

edited by rustbot

Loading

RalfJung Apr 16, 2026 •

edited by rustbot

Loading