Skip to content

Commit 4bb5903

Browse files
committed
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 godot-rust/godot4-prebuilt#2 or similar, will not work with current tip-of-master.
1 parent b125e1b commit 4bb5903

File tree

2 files changed

+175
-74
lines changed

2 files changed

+175
-74
lines changed

godot-bindings/src/header_gen.rs

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,31 @@
88
use std::env;
99
use std::path::Path;
1010

11+
/// Target pointer width for binding generation.
12+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
13+
pub enum TargetPointerWidth {
14+
/// 32-bit target (e.g., wasm32, i686)
15+
Bits32,
16+
/// 64-bit target (e.g., x86_64, aarch64)
17+
Bits64,
18+
}
19+
20+
impl TargetPointerWidth {
21+
/// Returns the clang target triple for this pointer width.
22+
fn clang_target(&self) -> &'static str {
23+
match self {
24+
// Use wasm32-unknown-emscripten as the 32-bit target since that's the primary
25+
// 32-bit platform supported by Godot/gdext.
26+
TargetPointerWidth::Bits32 => "wasm32-unknown-emscripten",
27+
TargetPointerWidth::Bits64 => "x86_64-unknown-linux-gnu",
28+
}
29+
}
30+
}
31+
32+
/// Generate Rust bindings from a C header file.
33+
///
34+
/// This is the standard function that determines whether to enable layout tests
35+
/// based on cross-compilation detection (host vs target pointer width).
1136
pub(crate) fn generate_rust_binding(in_h_path: &Path, out_rs_path: &Path) {
1237
let c_header_path = in_h_path.display().to_string();
1338

@@ -75,6 +100,68 @@ pub(crate) fn generate_rust_binding(in_h_path: &Path, out_rs_path: &Path) {
75100
});
76101
}
77102

103+
/// Generate Rust bindings from a C header file for a specific target pointer width.
104+
///
105+
/// This function is intended for use by the prebuilt artifact generator (godot4-prebuilt)
106+
/// to generate bindings for both 32-bit and 64-bit targets from a single host machine.
107+
///
108+
/// Unlike [`generate_rust_binding`], this function:
109+
/// - Explicitly targets a specific pointer width via clang's `--target` flag
110+
/// - Always enables layout tests (since the target is explicitly specified)
111+
///
112+
/// # Arguments
113+
/// * `in_h_path` - Path to the input C header file
114+
/// * `out_rs_path` - Path where the generated Rust bindings will be written
115+
/// * `target_width` - The target pointer width to generate bindings for
116+
pub fn generate_rust_binding_for_target(
117+
in_h_path: &Path,
118+
out_rs_path: &Path,
119+
target_width: TargetPointerWidth,
120+
) {
121+
let c_header_path = in_h_path.display().to_string();
122+
123+
// We don't need cargo rerun-if-changed since this is for prebuilt generation.
124+
let cargo_cfg = bindgen::CargoCallbacks::new().rerun_on_header_files(false);
125+
126+
let builder = bindgen::Builder::default()
127+
.header(c_header_path)
128+
.parse_callbacks(Box::new(cargo_cfg))
129+
.prepend_enum_name(false)
130+
// Enable layout tests - they will be valid for the specified target.
131+
.layout_tests(true)
132+
// Blocklist max_align_t due to bindgen issues.
133+
// See: https://github.com/rust-lang/rust-bindgen/issues/3295.
134+
.blocklist_type("max_align_t")
135+
// Target the specified architecture for correct pointer sizes.
136+
.clang_arg(format!("--target={}", target_width.clang_target()));
137+
138+
std::fs::create_dir_all(
139+
out_rs_path
140+
.parent()
141+
.expect("bindgen output file has parent dir"),
142+
)
143+
.expect("create bindgen output dir");
144+
145+
let bindings = builder.generate().unwrap_or_else(|err| {
146+
panic!(
147+
"bindgen generate failed\n c: {}\n rs: {}\n target: {:?}\n err: {}\n",
148+
in_h_path.display(),
149+
out_rs_path.display(),
150+
target_width,
151+
err
152+
)
153+
});
154+
155+
bindings.write_to_file(out_rs_path).unwrap_or_else(|err| {
156+
panic!(
157+
"bindgen write failed\n c: {}\n rs: {}\n err: {}\n",
158+
in_h_path.display(),
159+
out_rs_path.display(),
160+
err
161+
)
162+
});
163+
}
164+
78165
//#[cfg(target_os = "macos")]
79166
fn configure_platform_specific(builder: bindgen::Builder) -> bindgen::Builder {
80167
// On macOS arm64 architecture, we currently get the following error. Tried using different LLVM versions.
@@ -140,3 +227,52 @@ fn apple_include_path() -> Result<String, std::io::Error> {
140227
println!("cargo:rerun-if-changed={}", path.display());
141228
}
142229
}*/
230+
231+
#[cfg(test)]
232+
mod tests {
233+
use super::*;
234+
235+
#[test]
236+
fn test_target_pointer_width_clang_targets() {
237+
// Verify 32-bit target produces wasm32 triple (primary 32-bit target for Godot)
238+
assert_eq!(
239+
TargetPointerWidth::Bits32.clang_target(),
240+
"wasm32-unknown-emscripten"
241+
);
242+
243+
// Verify 64-bit target produces x86_64 triple
244+
assert_eq!(
245+
TargetPointerWidth::Bits64.clang_target(),
246+
"x86_64-unknown-linux-gnu"
247+
);
248+
}
249+
250+
#[test]
251+
fn test_target_pointer_width_equality() {
252+
// Test PartialEq derive
253+
assert_eq!(TargetPointerWidth::Bits32, TargetPointerWidth::Bits32);
254+
assert_eq!(TargetPointerWidth::Bits64, TargetPointerWidth::Bits64);
255+
assert_ne!(TargetPointerWidth::Bits32, TargetPointerWidth::Bits64);
256+
}
257+
258+
#[test]
259+
fn test_target_pointer_width_clone_copy() {
260+
// Test Clone and Copy derives
261+
let width = TargetPointerWidth::Bits64;
262+
let cloned = width.clone();
263+
let copied = width; // Copy
264+
265+
assert_eq!(width, cloned);
266+
assert_eq!(width, copied);
267+
}
268+
269+
#[test]
270+
fn test_target_pointer_width_debug() {
271+
// Test Debug derive produces meaningful output
272+
let debug_32 = format!("{:?}", TargetPointerWidth::Bits32);
273+
let debug_64 = format!("{:?}", TargetPointerWidth::Bits64);
274+
275+
assert!(debug_32.contains("Bits32") || debug_32.contains("32"));
276+
assert!(debug_64.contains("Bits64") || debug_64.contains("64"));
277+
}
278+
}

godot-bindings/src/lib.rs

Lines changed: 39 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,38 @@ mod depend_on_custom {
6969
godot_exe::write_gdextension_headers(h_path, rs_path, true, watch);
7070
}
7171

72+
// Re-export types for prebuilt artifact generation.
73+
#[cfg(feature = "api-custom-extheader")]
74+
pub use header_gen::TargetPointerWidth;
75+
76+
/// Generate Rust bindings for a specific target pointer width.
77+
///
78+
/// This function is intended for the prebuilt artifact generator (godot4-prebuilt)
79+
/// to produce bindings for both 32-bit and 64-bit architectures from a single host.
80+
///
81+
/// # Example (in godot4-prebuilt generator)
82+
/// ```ignore
83+
/// use godot_bindings::{write_gdextension_headers_for_target, TargetPointerWidth};
84+
///
85+
/// // Generate 64-bit bindings
86+
/// write_gdextension_headers_for_target(h_path, rs_64_path, TargetPointerWidth::Bits64);
87+
///
88+
/// // Generate 32-bit bindings
89+
/// write_gdextension_headers_for_target(h_path, rs_32_path, TargetPointerWidth::Bits32);
90+
/// ```
91+
#[cfg(feature = "api-custom-extheader")]
92+
pub fn write_gdextension_headers_for_target(
93+
h_path: &Path,
94+
rs_path: &Path,
95+
target_width: header_gen::TargetPointerWidth,
96+
) {
97+
// Patch the C header first (same as write_gdextension_headers_from_c).
98+
godot_exe::patch_c_header(h_path, h_path);
99+
100+
// Generate bindings for the specified target.
101+
header_gen::generate_rust_binding_for_target(h_path, rs_path, target_width);
102+
}
103+
72104
pub(crate) fn get_godot_version() -> GodotVersion {
73105
godot_exe::read_godot_version(&godot_exe::locate_godot_binary())
74106
}
@@ -123,94 +155,27 @@ mod depend_on_prebuilt {
123155
}
124156

125157
pub fn write_gdextension_headers(h_path: &Path, rs_path: &Path, watch: &mut StopWatch) {
126-
// Note: prebuilt artifacts just return a static str.
127158
let h_contents = prebuilt::load_gdextension_header_h();
128159
std::fs::write(h_path, h_contents.as_ref())
129160
.unwrap_or_else(|e| panic!("failed to write gdextension_interface.h: {e}"));
130161
watch.record("write_header_h");
131162

132-
let rs_contents = prebuilt::load_gdextension_header_rs();
163+
// Use CARGO_CFG_TARGET_POINTER_WIDTH to select the correct bindings for cross-compilation.
164+
// This is set by Cargo to the target's pointer width, not the host's.
165+
let target_pointer_width =
166+
std::env::var("CARGO_CFG_TARGET_POINTER_WIDTH").unwrap_or_else(|_| "64".to_string());
133167

134-
// Prebuilt bindings are generated for 64-bit. When targeting 32-bit, we must strip
135-
// the layout assertions since they validate 64-bit struct sizes which don't match.
136-
// For native 64-bit builds, we keep the safety checks intact.
137-
// See: https://github.com/godot-rust/gdext/issues/347
138-
let target_pointer_width = std::env::var("CARGO_CFG_TARGET_POINTER_WIDTH")
139-
.expect("CARGO_CFG_TARGET_POINTER_WIDTH not set");
140168
let rs_contents = if target_pointer_width == "32" {
141-
strip_bindgen_layout_tests(rs_contents.as_ref())
169+
prebuilt::load_gdextension_header_rs_32()
142170
} else {
143-
rs_contents.to_string()
171+
prebuilt::load_gdextension_header_rs_64()
144172
};
145173

146-
std::fs::write(rs_path, rs_contents)
174+
std::fs::write(rs_path, rs_contents.as_ref())
147175
.unwrap_or_else(|e| panic!("failed to write gdextension_interface.rs: {e}"));
148176
watch.record("write_header_rs");
149177
}
150178

151-
/// Removes bindgen-generated layout test assertion blocks from the source code.
152-
///
153-
/// Bindgen generates compile-time assertions in blocks like:
154-
/// ```ignore
155-
/// const _: () = {
156-
/// ["Size of SomeStruct"][::std::mem::size_of::<SomeStruct>() - 48usize];
157-
/// ["Alignment of SomeStruct"][::std::mem::align_of::<SomeStruct>() - 8usize];
158-
/// };
159-
/// ```
160-
///
161-
/// These cause compile-time errors when cross-compiling between 32-bit and 64-bit targets
162-
/// because struct sizes differ based on pointer width.
163-
/// See: https://github.com/godot-rust/gdext/issues/347
164-
fn strip_bindgen_layout_tests(source: &str) -> String {
165-
let mut result = String::with_capacity(source.len());
166-
let mut in_const_block = false;
167-
let mut brace_depth = 0;
168-
169-
for line in source.lines() {
170-
let trimmed = line.trim();
171-
172-
// Detect start of a const assertion block.
173-
// Pattern: `const _: () = {` or with #[allow(...)] attributes before.
174-
if !in_const_block && trimmed.starts_with("const _: () = {") {
175-
in_const_block = true;
176-
brace_depth = 1;
177-
continue;
178-
}
179-
180-
// Skip lines with #[allow(...)] that precede const blocks.
181-
if !in_const_block
182-
&& trimmed.starts_with("#[allow(")
183-
&& trimmed.contains("clippy::unnecessary_operation")
184-
{
185-
continue;
186-
}
187-
188-
if in_const_block {
189-
// Track brace depth to find the end of the block.
190-
for ch in trimmed.chars() {
191-
match ch {
192-
'{' => brace_depth += 1,
193-
'}' => {
194-
brace_depth -= 1;
195-
if brace_depth == 0 {
196-
in_const_block = false;
197-
break;
198-
}
199-
}
200-
_ => {}
201-
}
202-
}
203-
continue; // Skip all lines inside the const block.
204-
}
205-
206-
// Keep all other lines.
207-
result.push_str(line);
208-
result.push('\n');
209-
}
210-
211-
result
212-
}
213-
214179
pub(crate) fn get_godot_version() -> GodotVersion {
215180
let version: Vec<&str> = prebuilt::GODOT_VERSION_STRING
216181
.split('.')

0 commit comments

Comments
 (0)