From 5f9950eac4e5b31c26b814f1ca60598786cdc143 Mon Sep 17 00:00:00 2001 From: Niklas Dusenlund Date: Wed, 3 Jun 2026 11:22:07 +0200 Subject: [PATCH] Centralize Rust bindgen clang flags Route CMake cargo invocations through scripts/cargo-wrapper so bindgen gets target-specific clang arguments from one place. The wrapper supplies --target for explicit cargo targets, the ARM sysroot and enum ABI flags for thumb targets, and cortex-m4 soft-float flags for thumbv7 builds. Add a clang wrapper with the same ARM sysroot discovery for non-GCC compiler use. Remove hard-coded bindgen sysroot, target, and ABI flags from Rust build scripts while keeping crate-specific defines there. Update AGENTS.md to document wrapper usage. --- AGENTS.md | 24 ++-- scripts/cargo-wrapper | 54 ++++++++ scripts/clang-wrapper | 16 +++ src/CMakeLists.txt | 12 +- src/rust/bitbox-lvgl-sys/build.rs | 3 +- src/rust/bitbox-securechip-sys/build.rs | 159 +++++++++++------------- src/rust/bitbox02-sys/build.rs | 75 +++++------ 7 files changed, 193 insertions(+), 150 deletions(-) create mode 100755 scripts/cargo-wrapper create mode 100755 scripts/clang-wrapper diff --git a/AGENTS.md b/AGENTS.md index 7c5e5a5d6b..83aee21c3f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,8 +27,10 @@ for the current scope. Run regular Unix commands such as `git`, `rg`, `grep`, `ls`, `find`, `sed`, and `cat` directly on the host. -Cargo commands for the Rust workspace, such as `cargo test`, `cargo check`, and `cargo clippy`, may -also be run directly on the host by passing `--manifest-path src/rust/Cargo.toml`. +Cargo commands for the Rust workspace must be invoked through `./scripts/cargo-wrapper`, e.g. +`./scripts/cargo-wrapper test`, `./scripts/cargo-wrapper check`, or +`./scripts/cargo-wrapper clippy`. They may be run directly on the host by passing +`--manifest-path src/rust/Cargo.toml`. Use `./scripts/dev_exec.sh ` only for project-specific commands that depend on the project toolchain or compiler environment. @@ -46,13 +48,15 @@ an explicit shell as the command, e.g. `./scripts/dev_exec.sh bash -lc 'cat vers - `make run-rust-clippy`: lint Rust code with the workspace configuration. - When invoking the above `make` targets from the host, prefer `./scripts/dev_exec.sh make `. -- Rust workspace commands may also be run directly with `cargo`, without `./scripts/dev_exec.sh`: +- Rust workspace commands may also be run directly with `./scripts/cargo-wrapper`, without + `./scripts/dev_exec.sh`: - From the repository root on the host, use - `cargo test --manifest-path src/rust/Cargo.toml [ -p ] --all-features -- --test-threads 1`. + `./scripts/cargo-wrapper test --manifest-path src/rust/Cargo.toml [ -p ] --all-features -- --test-threads 1`. - For checks, use - `cargo check --manifest-path src/rust/Cargo.toml [ -p ] --all-features`. - - If you modify `messages/*.proto`, run `make generate-protobufs` before direct Rust `cargo` - commands. Plain `cargo test`/`cargo check` does not regenerate the protobuf outputs. + `./scripts/cargo-wrapper check --manifest-path src/rust/Cargo.toml [ -p ] --all-features`. + - If you modify `messages/*.proto`, run `make generate-protobufs` before direct Rust workspace + commands. Plain `./scripts/cargo-wrapper test`/`./scripts/cargo-wrapper check` does not + regenerate the protobuf outputs. You may use `make -j$(nproc)` to speed up compilation. Do not use `make -j` without specfiying the number of processing units. @@ -72,13 +76,15 @@ bindings (`cbindgen`, protobuf) when interfaces change. When changing protobuf i * For C code changes, run `./scripts/dev_exec.sh ./scripts/format` to format the code. * For Python changes, run `./scripts/dev_exec.sh ./scripts/format-python` to format the code. * For Rust code changes, run - `./scripts/dev_exec.sh cargo fmt --manifest-path src/rust/Cargo.toml --all` to format the code. + `./scripts/dev_exec.sh ./scripts/cargo-wrapper fmt --manifest-path src/rust/Cargo.toml --all` + to format the code. ## Testing Guidelines Place new C specs in `test/unit-test` and add doubles to `test/hardware-fakes` when hardware behavior is mocked; follow the `test_.c` naming pattern and update CMake lists. Rust crates use standard `tests/` modules or `#[cfg(test)]` blocks. Before opening a PR, run both `make -run-unit-tests` and `cargo test --manifest-path src/rust/Cargo.toml --all-features -- --test-threads 1`, +run-unit-tests` and +`./scripts/cargo-wrapper test --manifest-path src/rust/Cargo.toml --all-features -- --test-threads 1`, and refresh `make coverage` for cryptography or security-sensitive areas. - in Rust unit tests, prefer .unwrap() over .expect(). diff --git a/scripts/cargo-wrapper b/scripts/cargo-wrapper new file mode 100755 index 0000000000..688a89032a --- /dev/null +++ b/scripts/cargo-wrapper @@ -0,0 +1,54 @@ +#!/usr/bin/env bash +set -euo pipefail + +target="${CARGO_BUILD_TARGET:-${TARGET:-}}" +args=("$@") + +for ((i = 0; i < ${#args[@]}; i++)); do + case "${args[i]}" in + --target=*) target="${args[i]#--target=}" ;; + --target) + if ((i + 1 < ${#args[@]})); then + target="${args[i + 1]}" + fi + ;; + esac +done + +bindgen_extra_args=() + +if [[ -n "${target}" ]]; then + bindgen_extra_args+=("--target=${target}") +fi + +if [[ "${target}" == thumb* ]]; then + bindgen_extra_args+=("--sysroot=$(arm-none-eabi-gcc -print-sysroot)" "-fshort-enums" "-mthumb") + + if [[ "${target}" == thumbv7* ]]; then + bindgen_extra_args+=("-mcpu=cortex-m4" "-mfloat-abi=soft") + fi +fi + +if ((${#bindgen_extra_args[@]} > 0)); then + bindgen_env_name="BINDGEN_EXTRA_CLANG_ARGS_${target}" + bindgen_env_name_normalized="BINDGEN_EXTRA_CLANG_ARGS_${target//-/_}" + existing_bindgen_args="$(printenv "${bindgen_env_name}" 2>/dev/null || true)" + if [[ -z "${existing_bindgen_args}" ]]; then + existing_bindgen_args="$(printenv "${bindgen_env_name_normalized}" 2>/dev/null || true)" + fi + if [[ -z "${existing_bindgen_args}" ]]; then + existing_bindgen_args="${BINDGEN_EXTRA_CLANG_ARGS:-}" + fi + + bindgen_args="${bindgen_extra_args[*]}" + if [[ -n "${existing_bindgen_args}" ]]; then + bindgen_args="${existing_bindgen_args} ${bindgen_args}" + fi + + exec env \ + "${bindgen_env_name}=${bindgen_args}" \ + "${bindgen_env_name_normalized}=${bindgen_args}" \ + cargo "$@" +fi + +exec cargo "$@" diff --git a/scripts/clang-wrapper b/scripts/clang-wrapper new file mode 100755 index 0000000000..358b732f67 --- /dev/null +++ b/scripts/clang-wrapper @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +set -euo pipefail + +cc="${CC:-clang-21}" + +resolved_cc="$(command -v -- "$cc" 2>/dev/null || printf '%s\n' "$cc")" +resolved_cc="$(readlink -f -- "$resolved_cc" 2>/dev/null || printf '%s\n' "$resolved_cc")" + +# GCC-based ARM toolchains already know their sysroot. Clang needs it explicitly +# so it can find the ARM C headers while keeping the path host-independent. +if [[ "$(basename -- "$resolved_cc")" == *gcc* ]]; then + exec "$cc" "$@" +fi + +sysroot="$(arm-none-eabi-gcc -print-sysroot)" +exec "$cc" "--sysroot=${sysroot}" "$@" diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index deb4e45a54..fc8185099b 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -205,6 +205,7 @@ set(BOOTLOADER-SOURCES find_program(CBINDGEN cbindgen) # cargo is the rust build system and dependency manager find_program(CARGO cargo) +set(CARGO_WRAPPER ${CMAKE_CURRENT_SOURCE_DIR}/../scripts/cargo-wrapper) if(CMAKE_BUILD_TYPE STREQUAL "DEBUG") set(RUST_PROFILE "debug") @@ -233,7 +234,7 @@ set(LIBBITBOX02_RUST ${LIBBITBOX02_RUST} PARENT_SCOPE) add_custom_target(rust-cbindgen COMMAND ${CMAKE_COMMAND} -E env - CARGO_BIN=${CARGO} + CARGO_BIN=${CARGO_WRAPPER} CBINDGEN_BIN=${CBINDGEN} bash ${CMAKE_CURRENT_SOURCE_DIR}/../scripts/generate_rust_header.sh @@ -251,9 +252,8 @@ if(NOT CMAKE_CROSSCOMPILING) add_custom_target(rust-clippy COMMAND ${CMAKE_COMMAND} -E env - CMAKE_SYSROOT=${CMAKE_SYSROOT} CMAKE_CURRENT_BINARY_DIR=${CMAKE_CURRENT_BINARY_DIR} - ${CARGO} clippy + ${CARGO_WRAPPER} clippy $<$:-v> --all-features --manifest-path ${CMAKE_CURRENT_SOURCE_DIR}/rust/Cargo.toml @@ -346,7 +346,6 @@ foreach(type ${RUST_LIBS}) OUTPUT ${lib} ${CMAKE_ARCHIVE_OUTPUT_DIRECTORY}/lib${type}_rust_c.a dummy COMMAND ${CMAKE_COMMAND} -E env - CMAKE_SYSROOT=${CMAKE_SYSROOT} CMAKE_CURRENT_BINARY_DIR=${CMAKE_CURRENT_BINARY_DIR} RUSTFLAGS=${RUSTFLAGS} CFLAGS=${CARGO_C_FLAGS} @@ -354,7 +353,7 @@ foreach(type ${RUST_LIBS}) RUSTC_BOOTSTRAP=1 MACOSX_DEPLOYMENT_TARGET=${CMAKE_OSX_DEPLOYMENT_TARGET} LIB_TYPE=${type} - ${CARGO} + ${CARGO_WRAPPER} rustc $<$:-vv> --offline @@ -400,9 +399,8 @@ endforeach() if(CMAKE_CROSSCOMPILING) add_custom_target(rust-docs COMMAND - CMAKE_SYSROOT=${CMAKE_SYSROOT} ${CMAKE_COMMAND} -E env - ${CARGO} doc --document-private-items --target-dir ${CMAKE_BINARY_DIR}/docs-rust --target thumbv7em-none-eabi + ${CARGO_WRAPPER} doc --document-private-items --target-dir ${CMAKE_BINARY_DIR}/docs-rust --target thumbv7em-none-eabi COMMAND ${CMAKE_COMMAND} -E echo "See docs at file://${CMAKE_BINARY_DIR}/docs-rust/thumbv7em-none-eabi/doc/bitbox02_rust/index.html" WORKING_DIRECTORY diff --git a/src/rust/bitbox-lvgl-sys/build.rs b/src/rust/bitbox-lvgl-sys/build.rs index e515c7c790..bb56c1e2b2 100644 --- a/src/rust/bitbox-lvgl-sys/build.rs +++ b/src/rust/bitbox-lvgl-sys/build.rs @@ -245,6 +245,7 @@ fn main() -> Result<(), &'static str> { format!("-DLV_CONF_PATH=\"{}\"", lv_conf.display()), "-DLV_CONF_INCLUDE_SIMPLE".to_owned(), ]; + let bindgen_clang_args = cflags.to_vec(); let out_path = PathBuf::from(env::var("OUT_DIR").expect("OUT_DIR not set")).join("bindings.rs"); @@ -269,5 +270,5 @@ fn main() -> Result<(), &'static str> { fonts.flag(flag); } fonts.compile("lvgl_fonts"); - run_bindgen(&wrapper, &out_path, &cflags) + run_bindgen(&wrapper, &out_path, &bindgen_clang_args) } diff --git a/src/rust/bitbox-securechip-sys/build.rs b/src/rust/bitbox-securechip-sys/build.rs index cd77f2cd9f..f32f434027 100644 --- a/src/rust/bitbox-securechip-sys/build.rs +++ b/src/rust/bitbox-securechip-sys/build.rs @@ -122,28 +122,16 @@ pub fn main() -> BuildResult<()> { emit_rerun_if_changed(external_dir.join("optiga-trust-m/config")); emit_rerun_if_changed(&optiga_include_dir); - let target = env::var("TARGET").expect("TARGET not set"); - let cross_compiling = target == "thumbv7em-none-eabi"; - let arm_sysroot = env::var("CMAKE_SYSROOT").unwrap_or("/usr/local/arm-none-eabi".to_string()); - let arm_sysroot = format!("--sysroot={arm_sysroot}"); - - let mut extra_flags = if cross_compiling { - vec![ - // Generate bindings for the firmware target ABI, not the host ABI. - "--target=thumbv7em-none-eabi", - &arm_sysroot, - // The firmware C code is compiled with arm-none-eabi-gcc, which uses - // -fshort-enums by default. Bindgen must match those enum sizes. - "-fshort-enums", - ] - } else { - vec![] - }; + let mut definitions = vec![ + // Expose the U2F counter declarations guarded by APP_U2F in atecc.h/optiga.h. + "-DAPP_U2F=1", + "-DOPTIGA_LIB_EXTERNAL=\"optiga_config.h\"", + ]; if let Ok(rustflags) = std::env::var("CARGO_ENCODED_RUSTFLAGS") { for flag in rustflags.split('\x1f') { if flag == "-Dwarnings" { - extra_flags.push("-Werror"); + definitions.push("-Werror"); } } } @@ -151,78 +139,71 @@ pub fn main() -> BuildResult<()> { let out_path = out_dir.join("bindings.rs"); let out_path = out_path.into_os_string().into_string().unwrap(); - let mut definitions = vec![ - // Expose the U2F counter declarations guarded by APP_U2F in atecc.h/optiga.h. - "-DAPP_U2F=1", - "-DOPTIGA_LIB_EXTERNAL=\"optiga_config.h\"", - ]; - definitions.extend(&extra_flags); + let mut bindgen = Command::new("bindgen"); + bindgen + .args(["--output", &out_path]) + .arg("--use-core") + .arg("--with-derive-default") + .args( + ALLOWLIST_FNS + .iter() + .flat_map(|item| ["--allowlist-function", item]), + ) + .args( + ALLOWLIST_TYPES + .iter() + .flat_map(|item| ["--allowlist-type", item]), + ) + .args( + ALLOWLIST_VARS + .iter() + .flat_map(|item| ["--allowlist-var", item]), + ) + .args( + RUSTIFIED_ENUMS + .iter() + .flat_map(|item| ["--rustified-enum", item]), + ) + .arg(&wrapper) + .arg("--") + .args(&definitions) + .arg(format!("-I{}", src_dir.display())) + .arg(format!("-I{}", external_dir.display())) + .arg(format!( + "-I{}", + external_dir.join("optiga-trust-m/config").display() + )) + .arg(format!("-I{}", optiga_include_dir.display())) + .arg(format!( + "-I{}", + external_dir.join("optiga-trust-m/include/cmd").display() + )) + .arg(format!( + "-I{}", + external_dir.join("optiga-trust-m/include/common").display() + )) + .arg(format!( + "-I{}", + external_dir + .join("optiga-trust-m/include/ifx_i2c") + .display() + )) + .arg(format!( + "-I{}", + external_dir.join("optiga-trust-m/include/pal").display() + )) + .arg(format!( + "-I{}", + external_dir.join("optiga-trust-m/include/comms").display() + )) + .arg(format!( + "-I{}", + external_dir + .join("optiga-trust-m/external/mbedtls/include") + .display() + )); - run_command( - Command::new("bindgen") - .args(["--output", &out_path]) - .arg("--use-core") - .arg("--with-derive-default") - .args( - ALLOWLIST_FNS - .iter() - .flat_map(|item| ["--allowlist-function", item]), - ) - .args( - ALLOWLIST_TYPES - .iter() - .flat_map(|item| ["--allowlist-type", item]), - ) - .args( - ALLOWLIST_VARS - .iter() - .flat_map(|item| ["--allowlist-var", item]), - ) - .args( - RUSTIFIED_ENUMS - .iter() - .flat_map(|item| ["--rustified-enum", item]), - ) - .arg(&wrapper) - .arg("--") - .args(&definitions) - .arg(format!("-I{}", src_dir.display())) - .arg(format!("-I{}", external_dir.display())) - .arg(format!( - "-I{}", - external_dir.join("optiga-trust-m/config").display() - )) - .arg(format!("-I{}", optiga_include_dir.display())) - .arg(format!( - "-I{}", - external_dir.join("optiga-trust-m/include/cmd").display() - )) - .arg(format!( - "-I{}", - external_dir.join("optiga-trust-m/include/common").display() - )) - .arg(format!( - "-I{}", - external_dir - .join("optiga-trust-m/include/ifx_i2c") - .display() - )) - .arg(format!( - "-I{}", - external_dir.join("optiga-trust-m/include/pal").display() - )) - .arg(format!( - "-I{}", - external_dir.join("optiga-trust-m/include/comms").display() - )) - .arg(format!( - "-I{}", - external_dir - .join("optiga-trust-m/external/mbedtls/include") - .display() - )), - "run bindgen", - )?; + run_command(&mut bindgen, "run bindgen")?; Ok(()) } diff --git a/src/rust/bitbox02-sys/build.rs b/src/rust/bitbox02-sys/build.rs index c74fc6871e..4ffffc54be 100644 --- a/src/rust/bitbox02-sys/build.rs +++ b/src/rust/bitbox02-sys/build.rs @@ -328,33 +328,17 @@ pub fn main() -> BuildResult<()> { ensure_command_exists("bindgen")?; let target = env::var("TARGET").expect("TARGET not set"); - let cross_compiling = target == "thumbv7em-none-eabi"; - - let arm_sysroot = env::var("CMAKE_SYSROOT").unwrap_or("/usr/local/arm-none-eabi".to_string()); - let arm_sysroot = format!("--sysroot={arm_sysroot}"); - - let mut extra_flags = if cross_compiling { - vec![ - "-D__SAMD51J20A__", - "--target=thumbv7em-none-eabi", - "-mcpu=cortex-m4", - "-mthumb", - "-mfloat-abi=soft", - &arm_sysroot, - "-fshort-enums", - ] + let cross_compiling = target.starts_with("thumb"); + + let target_definitions = if cross_compiling { + vec!["-D__SAMD51J20A__"] } else { vec!["-DTESTING", "-D_UNIT_TEST_", "-DPRODUCT_BITBOX_MULTI=1"] }; - // If user enables -Dwarnings for rust we also want to enable -Werror for C. - if let Ok(rustflags) = std::env::var("CARGO_ENCODED_RUSTFLAGS") { - for flag in rustflags.split('\x1f') { - if flag == "-Dwarnings" { - extra_flags.push("-Werror"); - } - } - } + let warnings_as_errors = std::env::var("CARGO_ENCODED_RUSTFLAGS") + .map(|rustflags| rustflags.split('\x1f').any(|flag| flag == "-Dwarnings")) + .unwrap_or(false); let out_dir = PathBuf::from(env::var("OUT_DIR").expect("OUT_DIR not set")); @@ -425,28 +409,31 @@ pub fn main() -> BuildResult<()> { // Needs to match the definitions in `CMakeList.txt' files (unit tests, hardware fakes and // simulator) let mut definitions = vec!["-DAPP_U2F=1"]; - definitions.extend(&extra_flags); + definitions.extend(target_definitions); + if warnings_as_errors { + definitions.push("-Werror"); + } - run_command( - Command::new("bindgen") - .args(["--output", &out_path]) - .arg("--use-core") - .arg("--with-derive-default") - .args( - ALLOWLIST_FNS - .iter() - .flat_map(|s| ["--allowlist-function", s]), - ) - .args(ALLOWLIST_TYPES.iter().flat_map(|s| ["--allowlist-type", s])) - .args(ALLOWLIST_VARS.iter().flat_map(|s| ["--allowlist-var", s])) - .args(RUSTIFIED_ENUMS.iter().flat_map(|s| ["--rustified-enum", s])) - .args(OPAQUE_TYPES.iter().flat_map(|s| ["--opaque-type", s])) - .arg("wrapper.h") - .arg("--") - .args(&definitions) - .args(includes.iter().map(|s| format!("-I{s}"))), - "run bindgen", - )?; + let mut bindgen = Command::new("bindgen"); + bindgen + .args(["--output", &out_path]) + .arg("--use-core") + .arg("--with-derive-default") + .args( + ALLOWLIST_FNS + .iter() + .flat_map(|s| ["--allowlist-function", s]), + ) + .args(ALLOWLIST_TYPES.iter().flat_map(|s| ["--allowlist-type", s])) + .args(ALLOWLIST_VARS.iter().flat_map(|s| ["--allowlist-var", s])) + .args(RUSTIFIED_ENUMS.iter().flat_map(|s| ["--rustified-enum", s])) + .args(OPAQUE_TYPES.iter().flat_map(|s| ["--opaque-type", s])) + .arg("wrapper.h") + .arg("--") + .args(&definitions) + .args(includes.iter().map(|s| format!("-I{s}"))); + + run_command(&mut bindgen, "run bindgen")?; let excludes = if let Ok(libtype) = env::var("LIB_TYPE") { match libtype.as_str() {