Clean up backend features and vendor curve25519_dalek_derive (dalek-c…

…ryptography#531) * Vendor import unsafe_target_features as curve25519-dalek-derive Co-authored-by: Jan Bujak <[email protected]> * Remove feature gates from avx2/ifma * Add buildtime compile diagnostics about backend selection * Add build script tests * Documentation changes * Disable simd related features unless simd was determined via build * Add note and test about the override warning when unsuccesful * Reduce complexity in build gating via compile_error --------- Co-authored-by: Jan Bujak <[email protected]> Co-authored-by: Michael Rosenberg <[email protected]>
gear-tech · Jun 22, 2023 · e429bde · e429bde
1 parent e111b5d
commit e429bde
Show file tree

Hide file tree

Showing 24 changed files with 1,123 additions and 192 deletions.
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -11,7 +11,7 @@ env:
   RUSTFLAGS: '-D warnings'
 
 jobs:
-  test:
+  test-auto:
     runs-on: ubuntu-latest
     strategy:
       matrix:
@@ -38,10 +38,58 @@ jobs:
       - run: cargo test --target ${{ matrix.target }} --features digest
       - run: cargo test --target ${{ matrix.target }} --features rand_core
       - run: cargo test --target ${{ matrix.target }} --features serde
+
+  test-fiat:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        include:
+          # 32-bit target
+          - target: i686-unknown-linux-gnu
+            deps: sudo apt update && sudo apt install gcc-multilib
+
+          # 64-bit target
+          - target: x86_64-unknown-linux-gnu
+    steps:
+      - uses: actions/checkout@v3
+      - uses: dtolnay/rust-toolchain@stable
+      - run: rustup target add ${{ matrix.target }}
+      - run: ${{ matrix.deps }}
       - env:
           RUSTFLAGS: '--cfg curve25519_dalek_backend="fiat"'
         run: cargo test --target ${{ matrix.target }}
 
+  test-serial:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        include:
+          # 32-bit target
+          - target: i686-unknown-linux-gnu
+            deps: sudo apt update && sudo apt install gcc-multilib
+
+          # 64-bit target
+          - target: x86_64-unknown-linux-gnu
+    steps:
+      - uses: actions/checkout@v3
+      - uses: dtolnay/rust-toolchain@stable
+      - run: rustup target add ${{ matrix.target }}
+      - run: ${{ matrix.deps }}
+      - env:
+          RUSTFLAGS: '--cfg curve25519_dalek_backend="serial"'
+        run: cargo test --target ${{ matrix.target }}
+
+  build-script:
+    name: Test Build Script
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: stable
+          targets: wasm32-unknown-unknown,x86_64-unknown-linux-gnu,i686-unknown-linux-gnu
+      - run: bash tests/build_tests.sh
+
   build-nostd:
     name: Build on no_std target (thumbv7em-none-eabi)
     runs-on: ubuntu-latest
@@ -55,8 +103,8 @@ jobs:
       - run: cargo build --target thumbv7em-none-eabi --release
       - run: cargo build --target thumbv7em-none-eabi --release --features serde
 
-  test-simd-native:
-    name: Test simd backend (native)
+  test-simd-nightly:
+    name: Test simd backend (nightly)
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v3
@@ -66,20 +114,23 @@ jobs:
         #   1) build all of the x86_64 SIMD code,
         #   2) run all of the SIMD-specific tests that the test runner supports,
         #   3) run all of the normal tests using the best available SIMD backend.
+        # This should automatically pick up the simd backend in a x84_64 runner
         RUSTFLAGS: '-C target_cpu=native'
-      run: cargo test --features simd --target x86_64-unknown-linux-gnu
+      run: cargo test --target x86_64-unknown-linux-gnu
 
-  test-simd-avx2:
-    name: Test simd backend (avx2)
+  test-simd-stable:
+    name: Test simd backend (stable)
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v3
     - uses: dtolnay/rust-toolchain@stable
     - env:
         # This will run AVX2-specific tests and run all of the normal tests
         # with the AVX2 backend, even if the runner supports AVX512.
+        # This should automatically pick up the simd backend in a x86_64 runner
+        # It should pick AVX2 due to stable toolchain used since AVX512 requires nigthly
         RUSTFLAGS: '-C target_feature=+avx2'
-      run: cargo test --no-default-features --features alloc,precomputed-tables,zeroize,simd_avx2 --target x86_64-unknown-linux-gnu
+      run: cargo test --no-default-features --features alloc,precomputed-tables,zeroize --target x86_64-unknown-linux-gnu
 
   build-docs:
     name: Build docs

diff --git a/.gitignore b/.gitignore
@@ -1,6 +1,8 @@
+*/target/*
 target
 Cargo.lock
-
+*/Cargo.lock
+build*.txt
 *~
 \#*
 .\#*

diff --git a/Cargo.toml b/Cargo.toml
@@ -53,7 +53,6 @@ digest = { version = "0.10", default-features = false, optional = true }
 subtle = { version = "2.3.0", default-features = false }
 serde = { version = "1.0", default-features = false, optional = true, features = ["derive"] }
 zeroize = { version = "1", default-features = false, optional = true }
-unsafe_target_feature = { version = "= 0.1.1", optional = true }
 
 [target.'cfg(target_arch = "x86_64")'.dependencies]
 cpufeatures = "0.2.6"
@@ -62,20 +61,13 @@ cpufeatures = "0.2.6"
 fiat-crypto = "0.1.19"
 
 [features]
-default = ["alloc", "precomputed-tables", "zeroize", "simd"]
+default = ["alloc", "precomputed-tables", "zeroize"]
 alloc = ["zeroize?/alloc"]
 precomputed-tables = []
 legacy_compatibility = []
 
-# Whether to allow the use of the AVX2 SIMD backend.
-simd_avx2 = ["unsafe_target_feature"]
-
-# Whether to allow the use of the AVX512 SIMD backend.
-# (Note: This requires Rust nightly; on Rust stable this feature will be ignored.)
-simd_avx512 = ["unsafe_target_feature"]
-
-# A meta-feature to allow all SIMD backends to be used.
-simd = ["simd_avx2", "simd_avx512"]
+[target.'cfg(all(not(curve25519_dalek_backend = "fiat"), not(curve25519_dalek_backend = "serial"), target_arch = "x86_64"))'.dependencies]
+curve25519-dalek-derive = { version = "0.1", path = "curve25519-dalek-derive" }
 
 [profile.dev]
 opt-level = 2
diff --git a/README.md b/README.md
@@ -53,9 +53,6 @@ curve25519-dalek = "4.0.0-rc.2"
 | `alloc`            |    ✓     | Enables Edwards and Ristretto multiscalar multiplication, batch scalar inversion, and batch Ristretto double-and-compress. Also enables `zeroize`. |
 | `zeroize`          |    ✓     | Enables [`Zeroize`][zeroize-trait] for all scalar and curve point types. |
 | `precomputed-tables` |    ✓     | Includes precomputed basepoint multiplication tables. This speeds up `EdwardsPoint::mul_base` and `RistrettoPoint::mul_base` by ~4x, at the cost of ~30KB added to the code size. |
-| `simd_avx2`        |    ✓     | Allows the AVX2 SIMD backend to be used, if available. |
-| `simd_avx512`      |    ✓     | Allows the AVX512 SIMD backend to be used, if available. |
-| `simd`             |    ✓     | Allows every SIMD backend to be used, if available. |
 | `rand_core`        |          | Enables `Scalar::random` and `RistrettoPoint::random`. This is an optional dependency whose version is not subject to SemVer. See [below](#public-api-semver-exemptions) for more details. |
 | `digest`           |          | Enables `RistrettoPoint::{from_hash, hash_from_bytes}` and `Scalar::{from_hash, hash_from_bytes}`. This is an optional dependency whose version is not subject to SemVer. See [below](#public-api-semver-exemptions) for more details. |
 | `serde`            |          | Enables `serde` serialization/deserialization for all the point and scalar types. |
@@ -90,25 +87,27 @@ latest breaking changes in high level are below:
 
 This release also does a lot of dependency updates and relaxations to unblock upstream build issues.
 
-### 4.0.0 - Open Breaking Changes
+# Backends
 
-See tracking issue: [curve25519-dalek/issues/521](https://github.com/dalek-cryptography/curve25519-dalek/issues/521)
+Curve arithmetic is implemented and used by one of the following backends:
 
-# Backends
+| Backend  | Selection | Implementation                                                | Bits / Word sizes |
+| :---     | :---      | :---                                                          | :---              |
+| `serial` | Automatic | An optimized, non-parllel implementation                      | `32` and `64`     |
+| `fiat`   | Manual    | Formally verified field arithmetic from [fiat-crypto]         | `32` and `64`     |
+| `simd`   | Automatic | Intel AVX2 / AVX512 IFMA accelerated backend                  | `64` only         |
 
-Curve arithmetic is implemented and used by selecting one of the following backends:
+At runtime, `curve25519-dalek` selects an arithmetic backend from the set of backends it was compiled to support. For Intel x86-64 targets, unless otherwise specified, it will build itself with `simd` support, and default to `serial` at runtime if the appropriate CPU features aren't detected. See [SIMD backend] for more details.
 
-| Backend            | Implementation                                                | Target backends             |
-| :---               | :---                                                          | :---                        |
-| `[default]`        | Automatic runtime backend selection (either serial or SIMD)   | `u32` <br/> `u64` <br/> `avx2` <br/> `avx512` |
-| `fiat`             | Formally verified field arithmetic from [fiat-crypto]         | `fiat_u32` <br/> `fiat_u64` |
+In the future, `simd` backend may be extended to cover more instruction sets. This change will be non-breaking as this is considered as implementation detail.
 
-To choose a backend other than the `[default]` backend, set the
-environment variable:
+## Manual Backend Override
+
+You can force the crate to compile with specific backend support, e.g., `serial` for x86-64 targets to save code size, or `fiat` to force the runtime to use verified code. To do this, set the environment variable:
 ```sh
 RUSTFLAGS='--cfg curve25519_dalek_backend="BACKEND"'
 ```
-where `BACKEND` is `fiat`. Equivalently, you can write to
+Equivalently, you can write to
 `~/.cargo/config`:
 ```toml
 [build]
@@ -117,53 +116,49 @@ rustflags = ['--cfg=curve25519_dalek_backend="BACKEND"']
 More info [here](https://doc.rust-lang.org/cargo/reference/config.html#buildrustflags).
 
 Note for contributors: The target backends are not entirely independent of each
-other. The SIMD backend directly depends on parts of the the `u64` backend to
+other. The [SIMD backend] directly depends on parts of the serial backend to
 function.
 
-## Word size for serial backends
+## Bits / Word size
 
-`curve25519-dalek` will automatically choose the word size for the `[default]`
-and `fiat` serial backends, based on the build target. For example, building
-for a 64-bit machine, the default `u64` target backend is automatically chosen
-when the `[default]` backend is selected, and `fiat_u64` is chosen when the
-`fiat backend is selected.
+`curve25519-dalek` will automatically choose the word size for the `fiat` and
+`serial` backends, based on the build target.
+For example, building for a 64-bit machine, the default 64 bit word size is
+automatically chosen when either the `serial` or `fiat` backend is selected.
 
-Backend word size can be overridden for `[default]` and `fiat` by setting the
+In some targets it might be required to override the word size for better
+performance.
+Backend word size can be overridden for `serial` and `fiat` by setting the
 environment variable:
 ```sh
 RUSTFLAGS='--cfg curve25519_dalek_bits="SIZE"'
 ```
-where `SIZE` is `32` or `64`. As in the above section, this can also be placed
+`SIZE` is `32` or `64`. As in the above section, this can also be placed
 in `~/.cargo/config`.
 
-**NOTE:** Using a word size of 32 will automatically disable SIMD support.
+Note: The [SIMD backend] requires a word size of 64 bits. Attempting to set bits=32 and backend=`simd` will yield a compile error.
 
 ### Cross-compilation
 
-Because backend selection is done by target, cross-compiling will select the
-correct word size automatically. For example, on an x86-64 Linux machine,
-`curve25519-dalek` will use the `u32` target backend if the following is run:
+Because backend selection is done by target, cross-compiling will select the correct word size automatically. For example, if a x86-64 Linux machine runs the following commands, `curve25519-dalek` will be compiled with the 32-bit `serial` backend.
 ```console
 $ sudo apt install gcc-multilib # (or whatever package manager you use)
 $ rustup target add i686-unknown-linux-gnu
 $ cargo build --target i686-unknown-linux-gnu
 ```
 
-## SIMD target backends
+## SIMD backend
 
-The SIMD target backend selection is done automatically at runtime depending
-on the available CPU features, provided the appropriate feature flag is enabled.
+The specific SIMD backend (AVX512 / AVX2 / `serial` default) is selected automatically at runtime, depending on the currently available CPU features, and whether Rust nightly is being used for compilation. The precise conditions are specified below.
 
-You can also specify an appropriate `-C target_feature` to build a binary
-which assumes the required SIMD instructions are always available.
+For a given CPU feature, you can also specify an appropriate `-C target_feature` to build a binary which assumes the required SIMD instructions are always available. Don't do this if you don't have a good reason.
 
-| Backend | Feature flag  | `RUSTFLAGS`                               | Requires nightly? |
-| :---    | :---          | :---                                      | :---              |
-| avx2    | `simd_avx2`   | `-C target_feature=+avx2`                 | no                |
-| avx512  | `simd_avx512` | `-C target_feature=+avx512ifma,+avx512vl` | yes               |
+| Backend | `RUSTFLAGS`                               | Requires nightly? |
+| :---    | :---                                      | :---              |
+| avx2    | `-C target_feature=+avx2`                 | no                |
+| avx512  | `-C target_feature=+avx512ifma,+avx512vl` | yes               |
 
-The AVX512 backend requires Rust nightly. When compiled on a non-nightly
-compiler it will always be disabled.
+If compiled on a non-nightly compiler, `curve25519-dalek` will not include AVX512 code, and therefore will never select it at runtime.
 
 # Documentation
 
@@ -326,3 +321,4 @@ contributions.
 [semver]: https://semver.org/spec/v2.0.0.html
 [rngcorestd]: https://github.com/rust-random/rand/tree/7aa25d577e2df84a5156f824077bb7f6bdf28d97/rand_core#crate-features
 [zeroize-trait]: https://docs.rs/zeroize/latest/zeroize/trait.Zeroize.html
+[SIMD backend]: #simd-backend
diff --git a/build.rs b/build.rs
@@ -3,6 +3,7 @@
 #![deny(clippy::unwrap_used, dead_code)]
 
 #[allow(non_camel_case_types)]
+#[derive(PartialEq, Debug)]
 enum DalekBits {
     Dalek32,
     Dalek64,
@@ -34,6 +35,38 @@ fn main() {
         // so for those we want to apply the `#[allow(unused_unsafe)]` attribute to get rid of that warning.
         println!("cargo:rustc-cfg=allow_unused_unsafe");
     }
+
+    let target_arch = match std::env::var("CARGO_CFG_TARGET_ARCH") {
+        Ok(arch) => arch,
+        _ => "".to_string(),
+    };
+
+    // Backend overrides / defaults
+    let curve25519_dalek_backend =
+        match std::env::var("CARGO_CFG_CURVE25519_DALEK_BACKEND").as_deref() {
+            Ok("fiat") => "fiat",
+            Ok("serial") => "serial",
+            Ok("simd") => {
+                // simd can only be enabled on x86_64 & 64bit target_pointer_width
+                match is_capable_simd(&target_arch, curve25519_dalek_bits) {
+                    true => "simd",
+                    // If override is not possible this must result to compile error
+                    // See: issues/532
+                    false => panic!("Could not override curve25519_dalek_backend to simd"),
+                }
+            }
+            // default between serial / simd (if potentially capable)
+            _ => match is_capable_simd(&target_arch, curve25519_dalek_bits) {
+                true => "simd",
+                false => "serial",
+            },
+        };
+    println!("cargo:rustc-cfg=curve25519_dalek_backend=\"{curve25519_dalek_backend}\"");
+}
+
+// Is the target arch & curve25519_dalek_bits potentially simd capable ?
+fn is_capable_simd(arch: &str, bits: DalekBits) -> bool {
+    arch == "x86_64" && bits == DalekBits::Dalek64
 }
 
 // Deterministic cfg(curve25519_dalek_bits) when this is not explicitly set.

diff --git a/curve25519-dalek-derive/Cargo.toml b/curve25519-dalek-derive/Cargo.toml
@@ -0,0 +1,19 @@
+[package]
+name = "curve25519-dalek-derive"
+version = "0.1.0"
+edition = "2021"
+
+repository = "https://github.com/dalek-cryptography/curve25519-dalek"
+homepage = "https://github.com/dalek-cryptography/curve25519-dalek"
+documentation = "https://docs.rs/curve25519-dalek-derive"
+license = "MIT/Apache-2.0"
+readme = "README.md"
+description = "curve25519-dalek Derives"
+
+[lib]
+proc-macro = true
+
+[dependencies]
+proc-macro2 = "1.0.53"
+quote = "1.0.26"
+syn = { version = "2.0.8", features = ["full"] }