From 973c0a68bbef12634b111d6ebf0be1c8687ada6f Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Wed, 21 Aug 2024 21:20:47 -0400 Subject: [PATCH 1/4] Remove riscv64gc-specific CI logic --- .github/workflows/cross.yml | 16 +++++++++------- Cross.toml | 2 +- aws-lc-sys/builder/cc_builder.rs | 8 +++++++- aws-lc-sys/builder/cmake_builder.rs | 6 +++++- aws-lc-sys/builder/main.rs | 8 ++++++++ docker/linux-cross/Dockerfile | 2 +- 6 files changed, 31 insertions(+), 11 deletions(-) diff --git a/.github/workflows/cross.yml b/.github/workflows/cross.yml index 2fd645a37c3..7b60e637c8f 100644 --- a/.github/workflows/cross.yml +++ b/.github/workflows/cross.yml @@ -21,12 +21,11 @@ env: jobs: aws-lc-rs-cross-test: if: github.repository_owner == 'aws' - name: aws-lc-rs cross tests - runs-on: ${{ matrix.os }} + name: cross tests ${{ matrix.target }} + runs-on: ubuntu-22.04 strategy: fail-fast: false matrix: - os: [ ubuntu-22.04 ] target: - aarch64-linux-android - aarch64-unknown-linux-gnu @@ -51,18 +50,21 @@ jobs: with: toolchain: 'stable' - name: Install cross - run: cargo install cross ${{ (matrix.target == 'riscv64gc-unknown-linux-gnu' && '--locked') || '' }} --git https://github.com/cross-rs/cross + run: cargo install cross --git https://github.com/cross-rs/cross - uses: dtolnay/rust-toolchain@master id: toolchain with: - toolchain: ${{ (matrix.target == 'riscv64gc-unknown-linux-gnu' && '1.72.1') || 'stable' }} + toolchain: 'stable' target: ${{ matrix.target }} - name: Set Rust toolchain override run: rustup override set ${{ steps.toolchain.outputs.name }} + # The flag below is set to avoid the following error with GCC 11.4.0 on the riscv64 platform: + # /home/runner/work/aws-lc-rs/aws-lc-rs/aws-lc-sys/aws-lc/crypto/pem/pem_lib.c:707:11: error: 'strncmp' of strings of length 1 and 9 and bound of 9 evaluates to nonzero [-Werror=string-compare] + # 707 | if (strncmp(buf, "-----END ", 9) == 0) { + # | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ - if: ${{ matrix.target == 'riscv64gc-unknown-linux-gnu' }} run: | - cargo update - cargo update -p clap --precise 4.4.18 + echo 'AWS_LC_SYS_CFLAGS="-Wno-string-compare"' >> "$GITHUB_ENV" - name: Cross-compilation (build debug) run: cross build -p aws-lc-rs --features unstable --target ${{ matrix.target }} - name: Cross-compilation (test release) diff --git a/Cross.toml b/Cross.toml index 027a609beea..6b2c7012f34 100644 --- a/Cross.toml +++ b/Cross.toml @@ -27,5 +27,5 @@ passthrough = [ "AWS_LC_SYS_EXTERNAL_BINDGEN", "AWS_LC_FIPS_SYS_EXTERNAL_BINDGEN", "AWS_LC_SYS_STATIC", "AWS_LC_FIPS_SYS_STATIC", "AWS_LC_SYS_CMAKE_BUILDER", "AWS_LC_SYS_CC_SRC_COLLECTOR", - "GOPROXY", + "GOPROXY", "AWS_LC_SYS_CFLAGS" ] diff --git a/aws-lc-sys/builder/cc_builder.rs b/aws-lc-sys/builder/cc_builder.rs index 854558b9cad..809af268250 100644 --- a/aws-lc-sys/builder/cc_builder.rs +++ b/aws-lc-sys/builder/cc_builder.rs @@ -14,7 +14,7 @@ mod x86_64_unknown_linux_gnu; mod x86_64_unknown_linux_musl; use crate::{ - cargo_env, env_var_to_bool, execute_command, out_dir, requested_c_std, target, target_arch, + cargo_env, env_var_to_bool, execute_command, get_cflags, out_dir, requested_c_std, target, target_arch, target_os, target_vendor, CStdRequested, OutputLibType, }; use std::path::PathBuf; @@ -132,6 +132,12 @@ impl CcBuilder { } } + if !get_cflags().is_empty() { + get_cflags().split(' ').for_each(|flag| { + cc_build.flag_if_supported(flag); + }); + } + self.add_includes(&mut cc_build); cc_build } diff --git a/aws-lc-sys/builder/cmake_builder.rs b/aws-lc-sys/builder/cmake_builder.rs index 35f429c9b1c..43d491c93bd 100644 --- a/aws-lc-sys/builder/cmake_builder.rs +++ b/aws-lc-sys/builder/cmake_builder.rs @@ -3,7 +3,7 @@ use crate::OutputLib::{Crypto, RustWrapper, Ssl}; use crate::{ - allow_prebuilt_nasm, cargo_env, emit_warning, execute_command, is_crt_static, is_no_asm, + allow_prebuilt_nasm, cargo_env, emit_warning, execute_command, get_cflags, is_crt_static, is_no_asm, option_env, requested_c_std, target, target_arch, target_env, target_family, target_os, target_underscored, target_vendor, test_nasm_command, CStdRequested, OutputLibType, }; @@ -145,6 +145,10 @@ impl CmakeBuilder { CStdRequested::None => {} } + if !get_cflags().is_empty() { + cmake_cfg.cflag(get_cflags()); + } + // Allow environment to specify CMake toolchain. if let Some(toolchain) = option_env("CMAKE_TOOLCHAIN_FILE").or(option_env(format!( "CMAKE_TOOLCHAIN_FILE_{}", diff --git a/aws-lc-sys/builder/main.rs b/aws-lc-sys/builder/main.rs index 59c7f35447b..863f79eba48 100644 --- a/aws-lc-sys/builder/main.rs +++ b/aws-lc-sys/builder/main.rs @@ -218,6 +218,8 @@ fn generate_src_bindings(manifest_dir: &Path, prefix: &Option, src_bindi ) .write_to_file(src_bindings_path.join(format!("{}.rs", target_platform_prefix("crypto")))) .expect("write bindings"); + + } fn emit_rustc_cfg(cfg: &str) { @@ -343,6 +345,7 @@ static mut AWS_LC_SYS_NO_PREFIX: bool = false; static mut AWS_LC_SYS_INTERNAL_BINDGEN: bool = false; static mut AWS_LC_SYS_EXTERNAL_BINDGEN: bool = false; static mut AWS_LC_SYS_NO_ASM: bool = false; +static mut AWS_LC_SYS_CFLAGS: String = String::new(); static mut AWS_LC_SYS_PREBUILT_NASM: Option = None; static mut AWS_LC_SYS_C_STD: CStdRequested = CStdRequested::None; @@ -355,6 +358,7 @@ fn initialize() { AWS_LC_SYS_EXTERNAL_BINDGEN = env_var_to_bool("AWS_LC_SYS_EXTERNAL_BINDGEN").unwrap_or(false); AWS_LC_SYS_NO_ASM = env_var_to_bool("AWS_LC_SYS_NO_ASM").unwrap_or(false); + AWS_LC_SYS_CFLAGS = option_env("AWS_LC_SYS_CFLAGS").unwrap_or(String::new()); AWS_LC_SYS_PREBUILT_NASM = env_var_to_bool("AWS_LC_SYS_PREBUILT_NASM"); AWS_LC_SYS_C_STD = CStdRequested::from_env(); } @@ -415,6 +419,10 @@ fn is_no_asm() -> bool { unsafe { AWS_LC_SYS_NO_ASM } } +fn get_cflags() -> &'static str { + unsafe { AWS_LC_SYS_CFLAGS.as_str() } +} + fn allow_prebuilt_nasm() -> Option { unsafe { AWS_LC_SYS_PREBUILT_NASM } } diff --git a/docker/linux-cross/Dockerfile b/docker/linux-cross/Dockerfile index d25fc4eefa2..31e4e5e52c8 100644 --- a/docker/linux-cross/Dockerfile +++ b/docker/linux-cross/Dockerfile @@ -4,7 +4,7 @@ FROM $CROSS_BASE_IMAGE ARG DEBIAN_FRONTEND=noninteractive RUN apt-get update && \ - apt-get install --assume-yes --no-install-recommends software-properties-common && \ + apt-get install --assume-yes --no-install-recommends gpg-agent software-properties-common && \ add-apt-repository --yes ppa:longsleep/golang-backports && \ apt-get update && \ apt-get install --assume-yes --no-install-recommends build-essential cmake golang-go clang && \ From 9c8ff242b265486f0183dcb01230cd70b6cc336a Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Thu, 22 Aug 2024 08:51:30 -0400 Subject: [PATCH 2/4] Set CFLAGS; Emit warning --- aws-lc-sys/builder/cc_builder.rs | 14 ++++++++------ aws-lc-sys/builder/cmake_builder.rs | 6 +++++- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/aws-lc-sys/builder/cc_builder.rs b/aws-lc-sys/builder/cc_builder.rs index 809af268250..5d5b0336fe9 100644 --- a/aws-lc-sys/builder/cc_builder.rs +++ b/aws-lc-sys/builder/cc_builder.rs @@ -14,8 +14,8 @@ mod x86_64_unknown_linux_gnu; mod x86_64_unknown_linux_musl; use crate::{ - cargo_env, env_var_to_bool, execute_command, get_cflags, out_dir, requested_c_std, target, target_arch, - target_os, target_vendor, CStdRequested, OutputLibType, + cargo_env, emit_warning, env_var_to_bool, execute_command, get_cflags, out_dir, requested_c_std, target, + target_arch, target_os, target_vendor, CStdRequested, OutputLibType, }; use std::path::PathBuf; @@ -26,7 +26,7 @@ pub(crate) struct CcBuilder { output_lib_type: OutputLibType, } -use std::fs; +use std::{env, fs}; pub(crate) struct Library { name: &'static str, @@ -133,9 +133,11 @@ impl CcBuilder { } if !get_cflags().is_empty() { - get_cflags().split(' ').for_each(|flag| { - cc_build.flag_if_supported(flag); - }); + let cflags = get_cflags(); + emit_warning(&format!( + "AWS_LC_SYS_CFLAGS found. Setting CFLAGS: '{cflags}'" + )); + env::set_var("CFLAGS", cflags); } self.add_includes(&mut cc_build); diff --git a/aws-lc-sys/builder/cmake_builder.rs b/aws-lc-sys/builder/cmake_builder.rs index 43d491c93bd..0d916951d91 100644 --- a/aws-lc-sys/builder/cmake_builder.rs +++ b/aws-lc-sys/builder/cmake_builder.rs @@ -146,7 +146,11 @@ impl CmakeBuilder { } if !get_cflags().is_empty() { - cmake_cfg.cflag(get_cflags()); + let cflags = get_cflags(); + emit_warning(&format!( + "AWS_LC_SYS_CFLAGS found. Setting CFLAGS: '{cflags}'" + )); + env::set_var("CFLAGS", cflags); } // Allow environment to specify CMake toolchain. From bf91762652ad70d005098980a50817f8834b63c0 Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Thu, 22 Aug 2024 12:14:25 -0400 Subject: [PATCH 3/4] Fix clippy lint --- aws-lc-sys/builder/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws-lc-sys/builder/main.rs b/aws-lc-sys/builder/main.rs index 863f79eba48..0fd4e736347 100644 --- a/aws-lc-sys/builder/main.rs +++ b/aws-lc-sys/builder/main.rs @@ -358,7 +358,7 @@ fn initialize() { AWS_LC_SYS_EXTERNAL_BINDGEN = env_var_to_bool("AWS_LC_SYS_EXTERNAL_BINDGEN").unwrap_or(false); AWS_LC_SYS_NO_ASM = env_var_to_bool("AWS_LC_SYS_NO_ASM").unwrap_or(false); - AWS_LC_SYS_CFLAGS = option_env("AWS_LC_SYS_CFLAGS").unwrap_or(String::new()); + AWS_LC_SYS_CFLAGS = option_env("AWS_LC_SYS_CFLAGS").unwrap_or_default(); AWS_LC_SYS_PREBUILT_NASM = env_var_to_bool("AWS_LC_SYS_PREBUILT_NASM"); AWS_LC_SYS_C_STD = CStdRequested::from_env(); } From 4d0f99ef72614483c8e5c49a96c70075b05f6c86 Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Wed, 28 Aug 2024 08:34:19 -0400 Subject: [PATCH 4/4] Satisfy clippy --- aws-lc-rs/src/aead/nonce_sequence/counter32.rs | 1 + aws-lc-rs/src/aead/nonce_sequence/counter64.rs | 1 + aws-lc-rs/src/constant_time.rs | 1 + aws-lc-rs/src/unstable/kdf/kbkdf.rs | 1 + aws-lc-rs/src/unstable/kdf/sskdf.rs | 2 ++ aws-lc-sys/builder/cc_builder.rs | 4 ++-- aws-lc-sys/builder/cmake_builder.rs | 6 +++--- aws-lc-sys/builder/main.rs | 2 -- 8 files changed, 11 insertions(+), 7 deletions(-) diff --git a/aws-lc-rs/src/aead/nonce_sequence/counter32.rs b/aws-lc-rs/src/aead/nonce_sequence/counter32.rs index ca66ff9e5e6..e7d33e22ee1 100644 --- a/aws-lc-rs/src/aead/nonce_sequence/counter32.rs +++ b/aws-lc-rs/src/aead/nonce_sequence/counter32.rs @@ -6,6 +6,7 @@ use crate::error::Unspecified; use crate::iv::FixedLength; /// `Counter32` is an implementation of the `NonceSequence` trait. +/// /// The internal state of a `Counter32` is a 32-bit unsigned counter that /// increments on each call to `advance` and an optional 8-byte identifier. Counter and identifier /// values are used to construct each nonce. diff --git a/aws-lc-rs/src/aead/nonce_sequence/counter64.rs b/aws-lc-rs/src/aead/nonce_sequence/counter64.rs index 0ebcb74dfe3..8c6af9cbce9 100644 --- a/aws-lc-rs/src/aead/nonce_sequence/counter64.rs +++ b/aws-lc-rs/src/aead/nonce_sequence/counter64.rs @@ -6,6 +6,7 @@ use crate::error::Unspecified; use crate::iv::FixedLength; /// `Counter64` is an implementation of the `NonceSequence` trait. +/// /// The internal state of a `Counter64` is a 64-bit unsigned counter that /// increments on each call to `advance` and an optional 4-byte identifier. Counter and identifier /// values are used to construct each nonce. diff --git a/aws-lc-rs/src/constant_time.rs b/aws-lc-rs/src/constant_time.rs index 7d0ab9dc277..8850d8dfaa6 100644 --- a/aws-lc-rs/src/constant_time.rs +++ b/aws-lc-rs/src/constant_time.rs @@ -9,6 +9,7 @@ use crate::error; use aws_lc::CRYPTO_memcmp; /// Returns `Ok(())` if `a == b` and `Err(error::Unspecified)` otherwise. +/// /// The comparison of `a` and `b` is done in constant time with respect to the /// contents of each, but NOT in constant time with respect to the lengths of /// `a` and `b`. diff --git a/aws-lc-rs/src/unstable/kdf/kbkdf.rs b/aws-lc-rs/src/unstable/kdf/kbkdf.rs index 5b901195def..ef75c093ad4 100644 --- a/aws-lc-rs/src/unstable/kdf/kbkdf.rs +++ b/aws-lc-rs/src/unstable/kdf/kbkdf.rs @@ -57,6 +57,7 @@ const KBKDF_CTR_HMAC_SHA512: KbkdfCtrHmacAlgorithm = KbkdfCtrHmacAlgorithm { }; /// Retrieve an unstable [`KbkdfCtrHmacAlgorithm`] using the [`KbkdfCtrHmacAlgorithmId`] specified by `id`. +/// /// May return [`None`] if the algorithm is not usable with the configured crate feature set (i.e. `fips`). #[must_use] pub const fn get_kbkdf_ctr_hmac_algorithm( diff --git a/aws-lc-rs/src/unstable/kdf/sskdf.rs b/aws-lc-rs/src/unstable/kdf/sskdf.rs index 9d26a891fcd..35df79eadf9 100644 --- a/aws-lc-rs/src/unstable/kdf/sskdf.rs +++ b/aws-lc-rs/src/unstable/kdf/sskdf.rs @@ -96,6 +96,7 @@ const SSKDF_DIGEST_SHA512: SskdfDigestAlgorithm = SskdfDigestAlgorithm { }; /// Retrieve an unstable [`SskdfHmacAlgorithm`] using the [`SskdfHmacAlgorithmId`] specified by `id`. +/// /// May return [`None`] if the algorithm is not usable with the configured crate feature set (i.e. `fips`). #[must_use] pub const fn get_sskdf_hmac_algorithm( @@ -118,6 +119,7 @@ pub const fn get_sskdf_hmac_algorithm( } /// Retrieve an unstable [`SskdfDigestAlgorithm`] using the [`SskdfDigestAlgorithmId`] specified by `id`. +/// /// May return [`None`] if the algorithm is not usable with the configured crate feature set (i.e. `fips`). #[must_use] pub const fn get_sskdf_digest_algorithm( diff --git a/aws-lc-sys/builder/cc_builder.rs b/aws-lc-sys/builder/cc_builder.rs index 5d5b0336fe9..c7dfe9285ae 100644 --- a/aws-lc-sys/builder/cc_builder.rs +++ b/aws-lc-sys/builder/cc_builder.rs @@ -14,8 +14,8 @@ mod x86_64_unknown_linux_gnu; mod x86_64_unknown_linux_musl; use crate::{ - cargo_env, emit_warning, env_var_to_bool, execute_command, get_cflags, out_dir, requested_c_std, target, - target_arch, target_os, target_vendor, CStdRequested, OutputLibType, + cargo_env, emit_warning, env_var_to_bool, execute_command, get_cflags, out_dir, + requested_c_std, target, target_arch, target_os, target_vendor, CStdRequested, OutputLibType, }; use std::path::PathBuf; diff --git a/aws-lc-sys/builder/cmake_builder.rs b/aws-lc-sys/builder/cmake_builder.rs index 0d916951d91..d377c20515c 100644 --- a/aws-lc-sys/builder/cmake_builder.rs +++ b/aws-lc-sys/builder/cmake_builder.rs @@ -3,9 +3,9 @@ use crate::OutputLib::{Crypto, RustWrapper, Ssl}; use crate::{ - allow_prebuilt_nasm, cargo_env, emit_warning, execute_command, get_cflags, is_crt_static, is_no_asm, - option_env, requested_c_std, target, target_arch, target_env, target_family, target_os, - target_underscored, target_vendor, test_nasm_command, CStdRequested, OutputLibType, + allow_prebuilt_nasm, cargo_env, emit_warning, execute_command, get_cflags, is_crt_static, + is_no_asm, option_env, requested_c_std, target, target_arch, target_env, target_family, + target_os, target_underscored, target_vendor, test_nasm_command, CStdRequested, OutputLibType, }; use std::env; use std::ffi::OsString; diff --git a/aws-lc-sys/builder/main.rs b/aws-lc-sys/builder/main.rs index 0fd4e736347..c12110c83f5 100644 --- a/aws-lc-sys/builder/main.rs +++ b/aws-lc-sys/builder/main.rs @@ -218,8 +218,6 @@ fn generate_src_bindings(manifest_dir: &Path, prefix: &Option, src_bindi ) .write_to_file(src_bindings_path.join(format!("{}.rs", target_platform_prefix("crypto")))) .expect("write bindings"); - - } fn emit_rustc_cfg(cfg: &str) {