From c3475afef552b1d4d6ae339581a180a06c536018 Mon Sep 17 00:00:00 2001 From: "Christian Mauduit (ufoot)" Date: Mon, 15 Dec 2025 07:14:32 +0100 Subject: [PATCH 1/4] [lint] add clippy override to tolerate suspicious formatting --- itest/rust/build.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/itest/rust/build.rs b/itest/rust/build.rs index 40a5f6268..c1dd2e9b6 100644 --- a/itest/rust/build.rs +++ b/itest/rust/build.rs @@ -369,7 +369,7 @@ fn generate_rust_methods(inputs: &[Input]) -> Vec { .collect::>(); let manual_methods = quote! { - #[allow(clippy::suspicious_else_formatting)] // `quote!` might output whole file as one big line. + #[allow(clippy::suspicious_else_formatting, clippy::possible_missing_else)] // `quote!` might output whole file as one big line. #[func] fn check_last_notrace(last_method_name: String, expected_callconv: String) -> bool { let last = godot::private::trace::pop(); From 99730697a529d2ea8a7214b6680c7bb5530d90c2 Mon Sep 17 00:00:00 2001 From: "Christian Mauduit (ufoot)" Date: Mon, 15 Dec 2025 09:24:22 +0100 Subject: [PATCH 2/4] Add 32-bit architecture support Enable cross-compilation to 32-bit targets (e.g., wasm) by addressing bindgen layout test incompatibilities. Problem: Bindgen generates compile-time struct size/alignment assertions based on the host architecture (64-bit). When cross-compiling to 32-bit, these assertions fail because pointer sizes differ (8 bytes vs 4 bytes). Solution: - For `api-custom` mode: disable layout_tests in bindgen configuration - For prebuilt mode: strip layout assertion blocks only when targeting 32-bit (CARGO_CFG_TARGET_POINTER_WIDTH == "32") Native 64-bit builds retain all safety checks intact. The assertions are only removed for 32-bit targets where they would be incorrect anyway (validating 64-bit sizes against 32-bit structs). --- godot-bindings/src/header_gen.rs | 12 +++++ godot-bindings/src/lib.rs | 78 +++++++++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/godot-bindings/src/header_gen.rs b/godot-bindings/src/header_gen.rs index d999d383e..68eafbada 100644 --- a/godot-bindings/src/header_gen.rs +++ b/godot-bindings/src/header_gen.rs @@ -23,10 +23,22 @@ pub(crate) fn generate_rust_binding(in_h_path: &Path, out_rs_path: &Path) { // If you have an idea to address this without too invasive changes, please comment on that issue. let cargo_cfg = bindgen::CargoCallbacks::new().rerun_on_header_files(false); + // Only disable layout tests when cross-compiling between different pointer widths. + // Layout tests are generated based on the host architecture but validated on the target, + // which causes failures when cross-compiling (e.g., from 64-bit host to 32-bit target) + // because struct sizes differ. See: https://github.com/godot-rust/gdext/issues/347. + let host_pointer_width = std::mem::size_of::() * 8; + let target_pointer_width: usize = env::var("CARGO_CFG_TARGET_POINTER_WIDTH") + .ok() + .and_then(|s| s.parse().ok()) + .unwrap_or(host_pointer_width); + let enable_layout_tests = host_pointer_width == target_pointer_width; + let builder = bindgen::Builder::default() .header(c_header_path) .parse_callbacks(Box::new(cargo_cfg)) .prepend_enum_name(false) + .layout_tests(enable_layout_tests) // Bindgen can generate wrong size checks for types defined as `__attribute__((aligned(__alignof__(struct {...}))))`, // which is how clang defines max_align_t: https://clang.llvm.org/doxygen/____stddef__max__align__t_8h_source.html. // Size checks seems to be fine on all the targets but `wasm32-unknown-emscripten`, disallowing web builds. diff --git a/godot-bindings/src/lib.rs b/godot-bindings/src/lib.rs index 464fd3983..9325a36a2 100644 --- a/godot-bindings/src/lib.rs +++ b/godot-bindings/src/lib.rs @@ -130,11 +130,87 @@ mod depend_on_prebuilt { watch.record("write_header_h"); let rs_contents = prebuilt::load_gdextension_header_rs(); - std::fs::write(rs_path, rs_contents.as_ref()) + + // Prebuilt bindings are generated for 64-bit. When targeting 32-bit, we must strip + // the layout assertions since they validate 64-bit struct sizes which don't match. + // For native 64-bit builds, we keep the safety checks intact. + // See: https://github.com/godot-rust/gdext/issues/347 + let target_pointer_width = std::env::var("CARGO_CFG_TARGET_POINTER_WIDTH") + .expect("CARGO_CFG_TARGET_POINTER_WIDTH not set"); + let rs_contents = if target_pointer_width == "32" { + strip_bindgen_layout_tests(rs_contents.as_ref()) + } else { + rs_contents.to_string() + }; + + std::fs::write(rs_path, rs_contents) .unwrap_or_else(|e| panic!("failed to write gdextension_interface.rs: {e}")); watch.record("write_header_rs"); } + /// Removes bindgen-generated layout test assertion blocks from the source code. + /// + /// Bindgen generates compile-time assertions in blocks like: + /// ```ignore + /// const _: () = { + /// ["Size of SomeStruct"][::std::mem::size_of::() - 48usize]; + /// ["Alignment of SomeStruct"][::std::mem::align_of::() - 8usize]; + /// }; + /// ``` + /// + /// These cause compile-time errors when cross-compiling between 32-bit and 64-bit targets + /// because struct sizes differ based on pointer width. + /// See: https://github.com/godot-rust/gdext/issues/347 + fn strip_bindgen_layout_tests(source: &str) -> String { + let mut result = String::with_capacity(source.len()); + let mut in_const_block = false; + let mut brace_depth = 0; + + for line in source.lines() { + let trimmed = line.trim(); + + // Detect start of a const assertion block. + // Pattern: `const _: () = {` or with #[allow(...)] attributes before. + if !in_const_block && trimmed.starts_with("const _: () = {") { + in_const_block = true; + brace_depth = 1; + continue; + } + + // Skip lines with #[allow(...)] that precede const blocks. + if !in_const_block + && trimmed.starts_with("#[allow(") + && trimmed.contains("clippy::unnecessary_operation") + { + continue; + } + + if in_const_block { + // Track brace depth to find the end of the block. + for ch in trimmed.chars() { + match ch { + '{' => brace_depth += 1, + '}' => { + brace_depth -= 1; + if brace_depth == 0 { + in_const_block = false; + break; + } + } + _ => {} + } + } + continue; // Skip all lines inside the const block. + } + + // Keep all other lines. + result.push_str(line); + result.push('\n'); + } + + result + } + pub(crate) fn get_godot_version() -> GodotVersion { let version: Vec<&str> = prebuilt::GODOT_VERSION_STRING .split('.') From b125e1bbbfbf3743f11c067dfe1b3658e05f0538 Mon Sep 17 00:00:00 2001 From: "Christian Mauduit (ufoot)" Date: Mon, 15 Dec 2025 09:24:27 +0100 Subject: [PATCH 3/4] Add wasm32 compilation check to CI Add a `wasm-check` job that verifies compilation for wasm32-unknown-emscripten. Since wasm32 is a 32-bit target, this catches pointer width regressions before merge. Related: https://github.com/godot-rust/gdext/issues/347 --- .github/workflows/full-ci.yml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/.github/workflows/full-ci.yml b/.github/workflows/full-ci.yml index 71aa01eea..436dd1f8c 100644 --- a/.github/workflows/full-ci.yml +++ b/.github/workflows/full-ci.yml @@ -213,6 +213,24 @@ jobs: - name: "Test" run: cargo test -p godot-cell --features="proptest" + # Verifies that the library compiles for wasm32, which is a 32-bit target. + # This catches regressions in 32-bit support (e.g., pointer width assumptions). + # See https://github.com/godot-rust/gdext/issues/347. + wasm-check: + name: wasm-check + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v4 + + - name: "Install Rust" + uses: ./.github/composite/rust + + - name: "Add wasm target" + run: rustup target add wasm32-unknown-emscripten + + - name: "Check wasm32 compilation (32-bit target)" + run: cargo check --target wasm32-unknown-emscripten -p godot --features experimental-wasm,experimental-wasm-nothreads + # For complex matrix workflow, see https://stackoverflow.com/a/65434401 godot-itest: name: godot-itest (${{ matrix.name }}) @@ -493,6 +511,7 @@ jobs: - unit-test - miri-test - proptest + - wasm-check - godot-itest - cargo-deny-machete - license-guard From 4bb5903864b092e8a37d5ad31c6fbe0f68e22187 Mon Sep 17 00:00:00 2001 From: "Christian Mauduit (ufoot)" Date: Tue, 16 Dec 2025 08:32:34 +0100 Subject: [PATCH 4/4] Refine cross-compilation support for 32-bit targets (wasm32) Prebuilt bindings previously used a runtime workaround that stripped layout tests when cross-compiling to 32-bit. This was fragile and didn't validate struct layouts on 32-bit targets. Changes: - Add TargetPointerWidth enum and generate_rust_binding_for_target() for generating bindings for a specific architecture via clang's --target flag - Export write_gdextension_headers_for_target() for godot4-prebuilt to generate both 32-bit and 64-bit bindings from a single host - Update prebuilt mode to select bindings using CARGO_CFG_TARGET_POINTER_WIDTH (the actual cross-compile target, not the host) - Remove strip_bindgen_layout_tests() workaround - no longer needed since prebuilt artifacts now include proper architecture-specific bindings. This should be squashed with previous commits, but until this is validated let's keep full history of what's there. Requirements: - this requires https://github.com/godot-rust/godot4-prebuilt/pull/2 or similar, will not work with current tip-of-master. --- godot-bindings/src/header_gen.rs | 136 +++++++++++++++++++++++++++++++ godot-bindings/src/lib.rs | 113 +++++++++---------------- 2 files changed, 175 insertions(+), 74 deletions(-) diff --git a/godot-bindings/src/header_gen.rs b/godot-bindings/src/header_gen.rs index 68eafbada..3b639c6a0 100644 --- a/godot-bindings/src/header_gen.rs +++ b/godot-bindings/src/header_gen.rs @@ -8,6 +8,31 @@ use std::env; use std::path::Path; +/// Target pointer width for binding generation. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum TargetPointerWidth { + /// 32-bit target (e.g., wasm32, i686) + Bits32, + /// 64-bit target (e.g., x86_64, aarch64) + Bits64, +} + +impl TargetPointerWidth { + /// Returns the clang target triple for this pointer width. + fn clang_target(&self) -> &'static str { + match self { + // Use wasm32-unknown-emscripten as the 32-bit target since that's the primary + // 32-bit platform supported by Godot/gdext. + TargetPointerWidth::Bits32 => "wasm32-unknown-emscripten", + TargetPointerWidth::Bits64 => "x86_64-unknown-linux-gnu", + } + } +} + +/// Generate Rust bindings from a C header file. +/// +/// This is the standard function that determines whether to enable layout tests +/// based on cross-compilation detection (host vs target pointer width). pub(crate) fn generate_rust_binding(in_h_path: &Path, out_rs_path: &Path) { let c_header_path = in_h_path.display().to_string(); @@ -75,6 +100,68 @@ pub(crate) fn generate_rust_binding(in_h_path: &Path, out_rs_path: &Path) { }); } +/// Generate Rust bindings from a C header file for a specific target pointer width. +/// +/// This function is intended for use by the prebuilt artifact generator (godot4-prebuilt) +/// to generate bindings for both 32-bit and 64-bit targets from a single host machine. +/// +/// Unlike [`generate_rust_binding`], this function: +/// - Explicitly targets a specific pointer width via clang's `--target` flag +/// - Always enables layout tests (since the target is explicitly specified) +/// +/// # Arguments +/// * `in_h_path` - Path to the input C header file +/// * `out_rs_path` - Path where the generated Rust bindings will be written +/// * `target_width` - The target pointer width to generate bindings for +pub fn generate_rust_binding_for_target( + in_h_path: &Path, + out_rs_path: &Path, + target_width: TargetPointerWidth, +) { + let c_header_path = in_h_path.display().to_string(); + + // We don't need cargo rerun-if-changed since this is for prebuilt generation. + let cargo_cfg = bindgen::CargoCallbacks::new().rerun_on_header_files(false); + + let builder = bindgen::Builder::default() + .header(c_header_path) + .parse_callbacks(Box::new(cargo_cfg)) + .prepend_enum_name(false) + // Enable layout tests - they will be valid for the specified target. + .layout_tests(true) + // Blocklist max_align_t due to bindgen issues. + // See: https://github.com/rust-lang/rust-bindgen/issues/3295. + .blocklist_type("max_align_t") + // Target the specified architecture for correct pointer sizes. + .clang_arg(format!("--target={}", target_width.clang_target())); + + std::fs::create_dir_all( + out_rs_path + .parent() + .expect("bindgen output file has parent dir"), + ) + .expect("create bindgen output dir"); + + let bindings = builder.generate().unwrap_or_else(|err| { + panic!( + "bindgen generate failed\n c: {}\n rs: {}\n target: {:?}\n err: {}\n", + in_h_path.display(), + out_rs_path.display(), + target_width, + err + ) + }); + + bindings.write_to_file(out_rs_path).unwrap_or_else(|err| { + panic!( + "bindgen write failed\n c: {}\n rs: {}\n err: {}\n", + in_h_path.display(), + out_rs_path.display(), + err + ) + }); +} + //#[cfg(target_os = "macos")] fn configure_platform_specific(builder: bindgen::Builder) -> bindgen::Builder { // On macOS arm64 architecture, we currently get the following error. Tried using different LLVM versions. @@ -140,3 +227,52 @@ fn apple_include_path() -> Result { println!("cargo:rerun-if-changed={}", path.display()); } }*/ + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_target_pointer_width_clang_targets() { + // Verify 32-bit target produces wasm32 triple (primary 32-bit target for Godot) + assert_eq!( + TargetPointerWidth::Bits32.clang_target(), + "wasm32-unknown-emscripten" + ); + + // Verify 64-bit target produces x86_64 triple + assert_eq!( + TargetPointerWidth::Bits64.clang_target(), + "x86_64-unknown-linux-gnu" + ); + } + + #[test] + fn test_target_pointer_width_equality() { + // Test PartialEq derive + assert_eq!(TargetPointerWidth::Bits32, TargetPointerWidth::Bits32); + assert_eq!(TargetPointerWidth::Bits64, TargetPointerWidth::Bits64); + assert_ne!(TargetPointerWidth::Bits32, TargetPointerWidth::Bits64); + } + + #[test] + fn test_target_pointer_width_clone_copy() { + // Test Clone and Copy derives + let width = TargetPointerWidth::Bits64; + let cloned = width.clone(); + let copied = width; // Copy + + assert_eq!(width, cloned); + assert_eq!(width, copied); + } + + #[test] + fn test_target_pointer_width_debug() { + // Test Debug derive produces meaningful output + let debug_32 = format!("{:?}", TargetPointerWidth::Bits32); + let debug_64 = format!("{:?}", TargetPointerWidth::Bits64); + + assert!(debug_32.contains("Bits32") || debug_32.contains("32")); + assert!(debug_64.contains("Bits64") || debug_64.contains("64")); + } +} diff --git a/godot-bindings/src/lib.rs b/godot-bindings/src/lib.rs index 9325a36a2..04b187bdb 100644 --- a/godot-bindings/src/lib.rs +++ b/godot-bindings/src/lib.rs @@ -69,6 +69,38 @@ mod depend_on_custom { godot_exe::write_gdextension_headers(h_path, rs_path, true, watch); } + // Re-export types for prebuilt artifact generation. + #[cfg(feature = "api-custom-extheader")] + pub use header_gen::TargetPointerWidth; + + /// Generate Rust bindings for a specific target pointer width. + /// + /// This function is intended for the prebuilt artifact generator (godot4-prebuilt) + /// to produce bindings for both 32-bit and 64-bit architectures from a single host. + /// + /// # Example (in godot4-prebuilt generator) + /// ```ignore + /// use godot_bindings::{write_gdextension_headers_for_target, TargetPointerWidth}; + /// + /// // Generate 64-bit bindings + /// write_gdextension_headers_for_target(h_path, rs_64_path, TargetPointerWidth::Bits64); + /// + /// // Generate 32-bit bindings + /// write_gdextension_headers_for_target(h_path, rs_32_path, TargetPointerWidth::Bits32); + /// ``` + #[cfg(feature = "api-custom-extheader")] + pub fn write_gdextension_headers_for_target( + h_path: &Path, + rs_path: &Path, + target_width: header_gen::TargetPointerWidth, + ) { + // Patch the C header first (same as write_gdextension_headers_from_c). + godot_exe::patch_c_header(h_path, h_path); + + // Generate bindings for the specified target. + header_gen::generate_rust_binding_for_target(h_path, rs_path, target_width); + } + pub(crate) fn get_godot_version() -> GodotVersion { godot_exe::read_godot_version(&godot_exe::locate_godot_binary()) } @@ -123,94 +155,27 @@ mod depend_on_prebuilt { } pub fn write_gdextension_headers(h_path: &Path, rs_path: &Path, watch: &mut StopWatch) { - // Note: prebuilt artifacts just return a static str. let h_contents = prebuilt::load_gdextension_header_h(); std::fs::write(h_path, h_contents.as_ref()) .unwrap_or_else(|e| panic!("failed to write gdextension_interface.h: {e}")); watch.record("write_header_h"); - let rs_contents = prebuilt::load_gdextension_header_rs(); + // Use CARGO_CFG_TARGET_POINTER_WIDTH to select the correct bindings for cross-compilation. + // This is set by Cargo to the target's pointer width, not the host's. + let target_pointer_width = + std::env::var("CARGO_CFG_TARGET_POINTER_WIDTH").unwrap_or_else(|_| "64".to_string()); - // Prebuilt bindings are generated for 64-bit. When targeting 32-bit, we must strip - // the layout assertions since they validate 64-bit struct sizes which don't match. - // For native 64-bit builds, we keep the safety checks intact. - // See: https://github.com/godot-rust/gdext/issues/347 - let target_pointer_width = std::env::var("CARGO_CFG_TARGET_POINTER_WIDTH") - .expect("CARGO_CFG_TARGET_POINTER_WIDTH not set"); let rs_contents = if target_pointer_width == "32" { - strip_bindgen_layout_tests(rs_contents.as_ref()) + prebuilt::load_gdextension_header_rs_32() } else { - rs_contents.to_string() + prebuilt::load_gdextension_header_rs_64() }; - std::fs::write(rs_path, rs_contents) + std::fs::write(rs_path, rs_contents.as_ref()) .unwrap_or_else(|e| panic!("failed to write gdextension_interface.rs: {e}")); watch.record("write_header_rs"); } - /// Removes bindgen-generated layout test assertion blocks from the source code. - /// - /// Bindgen generates compile-time assertions in blocks like: - /// ```ignore - /// const _: () = { - /// ["Size of SomeStruct"][::std::mem::size_of::() - 48usize]; - /// ["Alignment of SomeStruct"][::std::mem::align_of::() - 8usize]; - /// }; - /// ``` - /// - /// These cause compile-time errors when cross-compiling between 32-bit and 64-bit targets - /// because struct sizes differ based on pointer width. - /// See: https://github.com/godot-rust/gdext/issues/347 - fn strip_bindgen_layout_tests(source: &str) -> String { - let mut result = String::with_capacity(source.len()); - let mut in_const_block = false; - let mut brace_depth = 0; - - for line in source.lines() { - let trimmed = line.trim(); - - // Detect start of a const assertion block. - // Pattern: `const _: () = {` or with #[allow(...)] attributes before. - if !in_const_block && trimmed.starts_with("const _: () = {") { - in_const_block = true; - brace_depth = 1; - continue; - } - - // Skip lines with #[allow(...)] that precede const blocks. - if !in_const_block - && trimmed.starts_with("#[allow(") - && trimmed.contains("clippy::unnecessary_operation") - { - continue; - } - - if in_const_block { - // Track brace depth to find the end of the block. - for ch in trimmed.chars() { - match ch { - '{' => brace_depth += 1, - '}' => { - brace_depth -= 1; - if brace_depth == 0 { - in_const_block = false; - break; - } - } - _ => {} - } - } - continue; // Skip all lines inside the const block. - } - - // Keep all other lines. - result.push_str(line); - result.push('\n'); - } - - result - } - pub(crate) fn get_godot_version() -> GodotVersion { let version: Vec<&str> = prebuilt::GODOT_VERSION_STRING .split('.')