ctest: add foreign fn tests #4594

mbyx · 2025-07-29T10:12:15Z

Description

Adds support for testing foreign functions. A lot of hard to track bugs had to be fixed to do this:

Translator was changed to use exactly the same parsing format as the original ctest. It was easier to do everything this way, but I believe it does cause it to become more specific in the types it works with.
Previously things like *const T1Bar was being translated as struct *const T1Bar, that has been fixed, although in a hacky way.

Sources

Checklist

Relevant tests in libc-test/semver have been updated
No placeholder or unstable values like *LAST or *MAX are
included (see #3131)
Tested locally (cd libc-test && cargo test --target mytarget);
especially relevant for platforms that may not be checked in CI

mbyx · 2025-07-29T10:16:29Z

While it does work, it's definitely very dirty, but I'm not sure how to improve it more.

ctest-next/src/ast/function.rs

ctest-next/src/generator.rs

tgross35 · 2025-08-01T10:34:06Z

ctest-next/src/template.rs

@@ -482,36 +583,97 @@ impl<'a> TranslateHelper<'a> {
    fn basic_c_type(&self, ty: &syn::Type) -> Result<String, TranslationError> {
        let type_name = match ty {
            syn::Type::Path(p) => p.path.segments.last().unwrap().ident.to_string(),
-            syn::Type::Ptr(p) => self.basic_c_type(&p.elem)?,
-            syn::Type::Reference(r) => self.basic_c_type(&r.elem)?,
+            // Using recursion here causes breakage.


What breaks here?

Recursing caused *const T1Bar to be translated as struct const struct T1Bar *, #4617 removes basic_c_type so it's not a problem.

tgross35

Sorry this has been waiting so long, busy week. I'll have more time to review this weekend.

tgross35 · 2025-08-09T12:22:20Z

ctest-next/src/template.rs

+    pub(crate) fn parse_signature_to_bare_fn(
+        &self,
+        signature: &str,
+        abi: &str,
+    ) -> Result<syn::TypeBareFn, TranslationError> {
+        let (_, s) = signature.split_once('(').unwrap();
+        let type_str = format!("type T = unsafe extern \"{abi}\" fn({s};");
+        let item: syn::ItemType = syn::parse_str(&type_str).map_err(|_| {
+            TranslationError::new(
+                TranslationErrorKind::UnsupportedType,
+                signature,
+                Span::call_site(),
+            )
+        })?;
+        if let syn::Type::BareFn(f) = item.ty.deref() {
+            return Ok(f.clone());
+        }
+        panic!("`parse_signature_to_bare_fn` returned a non function type.")
+    }


What are you trying to guard against here with the panic?

String manipulation is never very robust, and you have syn types for everything already. Why not construct a TypeBareFn from Signature? This also allows you to reject specific syntax that you don't want to support.

All the string manipulation is doing is removing the name of the function from the signature (since that isn't allowed in bare fn types). I can't do that directly from syn::Signature since it's not allowed to have an empty Ident.

The panic should actually be an unreachable though.

AST -> string -> modify -> parse to AST is just a definite code smell to avoid: remember that things like the below, while ugly, is completely valid Rust syntax that this crate may have to deal with in some places :)

fn foo<const N: (usize)>() {} fn bar<T: (Copy)>() {} static FOO: (fn()) = foo::<{ (10) }>;

Doing things at an AST level can be more complicated. But that's a good thing: it makes you think about what needs to get passed, what gets modified, and what gets rejected.

I can't do that directly from syn::Signature since it's not allowed to have an empty Ident.

Think about it a bit differently: both your input and your output are valid Rust code, so syn must have a representation for it. You just need to figure out what it is and how to translate between them.

In this case, your input is Signature and the output is TypeBareFn. Most things have a 1:1 mapping that you just copy over (you'll need to iterate the args). For anything that doesn't copy over, you should consider if it should be rejected (constness, async, generics, etc).

Another option here is to use quote, something like this:

let mut args = Vec::new(); for arg in sig.inputs { match arg { FnArg::Typed(pat) => args.push(pat), FnArg::Receiver(_) => panic!("`self` args are not supported: got `{arg}`), } } let ty = quote! { fn(#(#args),*) };

That's a bit easier than building a TypeBareFn from scratch, but still avoids string hazards.

Actually, with #4594 (comment) this all goes away right?

ctest-next/src/translator.rs

ctest-next/templates/test.rs

ctest-next/templates/test.c

tgross35

I think some more can be dropped here; if so, the rest lgtm with a couple remaining nits

ctest-next/templates/test.c

tgross35 · 2025-08-11T09:12:44Z

ctest-next/templates/test.rs

+        let actual = unsafe { ctest_foreign_fn__{{ item.id }}() };
+        let expected: unsafe extern "C" fn() = unsafe {
+            mem::transmute({{ item.id }} as *const ())
+        };


No transmute needed, you can compare function pointers of different types:

let a: unsafe extern "C" fn(usize) -> *mut c_void = libc::malloc; let b: unsafe extern "C" fn(*mut c_void) = libc::free; assert!(!std::ptr::fn_addr_eq(a, b));

the let binding might be needed so it gives you the right thing.

That doesn't work, it fails with FnPtr not implemented for *const (). It also doesn't work with {{ item.id }} directly.

Where is the *const coming from? Could you paste the code and error?

This does seem to work: https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=378ac3c0e7aa61a0a99911f305b230f2

That's because abort is a function that has no parameters. If it does then it fails with error (taken from ctest-test):

expected fn pointer `unsafe extern "C" fn() -> ()` found fn item `unsafe extern "C" fn(*mut c_void, *mut c_void)

snippet causing that:

let actual = unsafe { ctest_foreign_fn__{{ item.id }}() }; let expected: unsafe extern "C" fn() = {{ item.id }}; check_same(ptr::fn_addr_eq(expected, actual), true, "{{ item.id }} function pointer");

Dropping the type coercion of unsafe extern "C" fn() completely results in (taken from ctest-next)

error[E0277]: the trait bound `unsafe extern "C" fn(usize) -> *mut c_void` {foo::malloc}: FnPtr is not satisfied

Ah, darn, so we do need the type to do the coercion. Alright, let's just do the safe thing; skip the transmute (to avoid some unsafe), just as cast both function pointers to a u64 and compare those. Rust allows for this https://doc.rust-lang.org/std/primitive.fn.html#casting-to-and-from-integers (I'm not exactly clear whether or not C does).

That may not work on cheri, but it will be a while before we need to cross that bridge.

Done.

Also what is cheri?

An experimental arch that has 128-bit pointers :D that's not the address space though, the top half is a tag about what the pointer is allowed to do. (Pretty interesting to look into if you are interested in that sort of thing)

ctest-next/src/translator.rs

tgross35

Awesome, thank you!

rustbot added ctest Issues relating to the ctest crate S-waiting-on-review labels Jul 29, 2025

mbyx force-pushed the ctest-func-tests branch from aaacc5d to 2284e5e Compare July 29, 2025 10:14

mbyx force-pushed the ctest-func-tests branch from 2284e5e to 8ef3d6d Compare August 2, 2025 13:24

tgross35 reviewed Aug 5, 2025

View reviewed changes

mbyx force-pushed the ctest-func-tests branch from 8ef3d6d to 1e22015 Compare August 6, 2025 12:11

mbyx requested a review from tgross35 August 6, 2025 12:12

tgross35 requested changes Aug 9, 2025

View reviewed changes

This was referenced Aug 9, 2025

ctest: add foreign static test #4601

Merged

libc-test: port windows to use ctest-next #4600

Open

mbyx force-pushed the ctest-func-tests branch 2 times, most recently from ae8f57a to 17c3633 Compare August 11, 2025 08:02

mbyx requested a review from tgross35 August 11, 2025 08:04

mbyx force-pushed the ctest-func-tests branch 2 times, most recently from da92f22 to c6a7386 Compare August 11, 2025 08:59

tgross35 requested changes Aug 11, 2025

View reviewed changes

mbyx force-pushed the ctest-func-tests branch 2 times, most recently from eed6d85 to 6867007 Compare August 11, 2025 09:42

mbyx requested a review from tgross35 August 11, 2025 09:44

mbyx force-pushed the ctest-func-tests branch 2 times, most recently from bfb1cf9 to ac91046 Compare August 11, 2025 14:37

tgross35 approved these changes Aug 11, 2025

View reviewed changes

ctest: add foreign func tests

30831f5

tgross35 force-pushed the ctest-func-tests branch from ac91046 to 30831f5 Compare August 11, 2025 16:11

tgross35 enabled auto-merge August 11, 2025 16:12

tgross35 added this pull request to the merge queue Aug 11, 2025

Merged via the queue into rust-lang:main with commit 954727f Aug 11, 2025
48 of 52 checks passed

rustbot removed the S-waiting-on-review label Aug 11, 2025

ctest: add foreign fn tests #4594

ctest: add foreign fn tests #4594

Uh oh!

Conversation

mbyx commented Jul 29, 2025

Description

Sources

Checklist

Uh oh!

mbyx commented Jul 29, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross35 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tgross35 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbyx Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tgross35 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mbyx Aug 11, 2025 •

edited

Loading