Initial pass of generated C layout tests

ctru-sys/Cargo.toml

@@ -16,13 +16,15 @@ edition = "2021"
 libc = { version = "0.2.121", default-features = false }
 
 [build-dependencies]
-bindgen = { version = "0.65.1", features = ["experimental"] }
+bindgen = { version = "0.66.1", features = ["experimental"] }
 cc = "1.0"
+cpp_build = "0.5.9"
 doxygen-rs = "0.4.2"
 itertools = "0.11.0"
 which = "4.4.0"
 
 [dev-dependencies]
+cpp = "0.5.9"
 shim-3ds = { git = "https://github.com/rust3ds/shim-3ds.git" }
 test-runner = { git = "https://github.com/rust3ds/test-runner.git" }
 

ctru-sys/build.rs

@@ -1,19 +1,47 @@
-use bindgen::callbacks::ParseCallbacks;
-use bindgen::{Builder, RustTarget};
+use bindgen::callbacks::{DeriveInfo, FieldInfo, ParseCallbacks};
+use bindgen::{Builder, FieldVisibilityKind, RustTarget};
 use itertools::Itertools;
+use std::io::{self, Write};
 
+use std::cell::RefCell;
+use std::collections::{HashMap, HashSet};
 use std::env;
 use std::error::Error;
 use std::path::{Path, PathBuf};
 use std::process::{Command, Output, Stdio};
+use std::rc::Rc;
+
+#[derive(Debug, Default)]
+struct StructInfo {
+    fields: HashMap<String, HashSet<String>>,
+    names: HashSet<String>,
+}
 
 #[derive(Debug)]
-struct CustomCallbacks;
+struct CustomCallbacks(Rc<RefCell<StructInfo>>);
 
 impl ParseCallbacks for CustomCallbacks {
     fn process_comment(&self, comment: &str) -> Option<String> {
         Some(doxygen_rs::transform(comment))
     }
+
+    fn add_derives(&self, info: &DeriveInfo<'_>) -> Vec<String> {
+        self.0.borrow_mut().names.insert(info.name.to_string());
+        Vec::new()
+    }
+
+    // We don't actually ever change visibility, but this allows us to keep track
+    // of all the fields in the structs bindgen processes.
+    fn field_visibility(&self, info: FieldInfo<'_>) -> Option<FieldVisibilityKind> {
+        self.0
+            .borrow_mut()
+            .fields
+            .entry(info.type_name.to_string())
+            .or_default()
+            .insert(info.field_name.to_string());
+
+        None
+    }
 }
 
 fn main() {
@@ -65,6 +93,9 @@ fn main() {
     ));
     let errno_header = system_include.join("errno.h");
 
+    let struct_info = Rc::default();
+    let callbacks = CustomCallbacks(Rc::clone(&struct_info));
+
     // Build libctru bindings
     let bindings = Builder::default()
         .header(ctru_header.to_str().unwrap())
@@ -109,7 +140,7 @@ fn main() {
             // gcc, so bindgen will generate enums with the proper sizes.
             "-fshort-enums",
         ])
-        .parse_callbacks(Box::new(CustomCallbacks))
+        .parse_callbacks(Box::new(callbacks))
         .generate()
         .expect("unable to generate bindings");
 
@@ -122,8 +153,8 @@ fn main() {
     let ar = Path::new(devkitarm.as_str()).join("bin/arm-none-eabi-ar");
 
     cc::Build::new()
-        .compiler(cc)
-        .archiver(ar)
+        .compiler(&cc)
+        .archiver(&ar)
         .include(&include_path)
         .file(out_dir.join("libctru_statics_wrapper.c"))
         .flag("-march=armv6k")
@@ -133,6 +164,21 @@ fn main() {
         .flag("-mtp=soft")
         .flag("-Wno-deprecated-declarations")
         .compile("ctru_statics_wrapper");
+
+    let struct_info = struct_info.borrow();
+    let generated_test_file = struct_info.build_layout_tests().unwrap();
+
+    cpp_build::Config::default()
+        .compiler(cc)
+        .archiver(ar)
+        .include(include_path)
+        .flag("-march=armv6k")
+        .flag("-mtune=mpcore")
+        .flag("-mfloat-abi=hard")
+        .flag("-mfpu=vfp")
+        .flag("-mtp=soft")
+        .flag("-Wno-deprecated-declarations")
+        .build(generated_test_file);
 }
 
 fn get_gcc_version(path_to_gcc: PathBuf) -> String {
@@ -223,7 +269,7 @@ fn track_libctru_files(pacman: &Path) -> Result<(), String> {
         }
     };
 
-    for line in String::from_utf8_lossy(&stdout).trim().split('\n') {
+    for line in String::from_utf8_lossy(&stdout).lines() {
         let Some((_pkg, file)) = line.split_once(char::is_whitespace) else {
             println!("cargo:warning=unexpected line from pacman query: {line:?}");
             continue;
@@ -234,3 +280,95 @@ fn track_libctru_files(pacman: &Path) -> Result<(), String> {
 
     Ok(())
 }
+
+impl StructInfo {
+    fn build_layout_tests(&self) -> io::Result<PathBuf> {
+        let output_file = PathBuf::from(env::var("OUT_DIR").unwrap()).join("layout_test.rs");
+        let mut file = std::fs::File::create(&output_file)?;
+
+        writeln!(
+            file,
+            r#"
+use cpp::cpp;
+use ctru_sys::*;
+
+cpp! {{{{
+    #include <3ds.h>
+}}}}
+"#,
+        )?;
+
+        for (strukt, fields) in &self.fields {
+            if strukt.contains("bindgen") || !self.names.contains(strukt) {
+                // We don't have an easy way to map the mangled name back to the original name,
+                // so we will just skip testing any structs with names generated by bindgen.
+                //
+                // If we needed to we could maybe hardcode a map for the ones we care about.
+                continue;
+            }
+
+            if let "sigevent"
+            | "siginfo_t"
+            | "sigval"
+            | "bintime"
+            | "fd_set"
+            | "pthread_rwlock_t"
+            | "ExHeader_Arm11CoreInfo"
+            | "ExHeader_Arm11StorageInfo"
+            | "ExHeader_SystemInfoFlags"
+            | "FS_ExtSaveDataInfo"
+            | "FS_SystemSaveDataInfo"
+            | "FS_ProgramInfo"
+            | "Y2RU_ConversionParams" = strukt.as_str()
+            {
+                // Some of these are only forward declared (in stdlibs), or have bitfields, etc.
+                // If we wanted to be more precise we could check specific fields in
+                // these instead of just skipping the whole struct.
+                continue;
+            }
+
+            // TODO: May need to mangle rust names to match what bindgen spits out...
+            writeln!(
+                file,
+                r#"
+#[test]
+fn {strukt}_layout() {{
+    assert_eq!(
+        std::mem::size_of::<{strukt}>(),
+        cpp!(unsafe [] -> usize as "size_t" {{ return sizeof({strukt}); }}),
+    );
+
+    assert_eq!(
+        std::mem::align_of::<{strukt}>(),
+        cpp!(unsafe [] -> usize as "size_t" {{ return alignof({strukt}); }}),
+    );
+"#,
+            )?;
+
+            for field in fields {
+                if field.contains("bindgen") {
+                    // Similar to struct names, just skip these ones
+                    continue;
+                }
+
+                // HACK: This will break if some struct actually has a field called `type_`
+                let c_field = if field == "type_" { "type" } else { field };
+
+                // TODO: also check field size + align if reasonably feasible
+                writeln!(
+                    file,
+                    r#"
+    assert_eq!(
+        std::mem::offset_of!({strukt}, {field}),
+        cpp!(unsafe [] -> usize as "size_t" {{ return offsetof({strukt}, {c_field}); }}),
+    );
+"#,
+                )?;
+            }
+
+            writeln!(file, "}}")?;
+        }
+
+        Ok(output_file)
+    }
+}

ctru-sys/tests/layout_test.rs

@@ -0,0 +1,18 @@
+//! This is a stub for the generated layout test. We use bindgen callbacks along
+//! with the [`cpp`] crate to compile actual `sizeof` and `alignof` calls in C,
+//! as opposed to bindgen's generated layout tests which use hardcoded size literals.
+//!
+//! This should help ensure that the generated bindings are correct for the actual
+//! ABI used by libctru and the devkitARM toolchain, instead of just what libclang
+//! thinks they should be at bindgen time.
+
+#![allow(non_snake_case)]
+#![feature(custom_test_frameworks)]
+#![test_runner(test_runner::run_gdb)]
+
+extern crate shim_3ds;
+
+// TODO: might want to move this into a test crate so we can avoid compiling it
+// for non-test builds? Idk if there's a reasonable way to do it though.
+
+include!(concat!(env!("OUT_DIR"), "/layout_test.rs"));