From 70c526983a279810dc7f0b5d50aadc344d577af7 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Sat, 19 Mar 2022 10:16:52 +1100 Subject: [PATCH 01/11] Remove trailing whitespace Remove single character of trailing whitespace. --- .github/workflows/rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 145bdacfe..516a5e91f 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -11,7 +11,7 @@ jobs: rust: - nightly steps: - - name: Checkout Crate + - name: Checkout Crate uses: actions/checkout@v2 - name: Checkout Toolchain uses: actions-rs/toolchain@v1 From 03401d4f25acf6a23f26e03f5191d0428fe0427a Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Sat, 19 Mar 2022 10:52:14 +1100 Subject: [PATCH 02/11] Move panic test to top of script The test that checks for a panic uses `cargo test --exact`, it makes sense to put it at the top of the script right after we run `cargo test` so we can run the tests without triggering a re-build. --- contrib/test.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/test.sh b/contrib/test.sh index a892a78d2..95d9afc6d 100755 --- a/contrib/test.sh +++ b/contrib/test.sh @@ -15,6 +15,9 @@ fi cargo --version rustc --version +# Test if panic in C code aborts the process (either with a real panic or with SIGILL) +cargo test -- --ignored --exact 'tests::test_panic_raw_ctx_should_terminate_abnormally' 2>&1 | tee /dev/stderr | grep "SIGILL\\|panicked at '\[libsecp256k1\]" + # Make all cargo invocations verbose export CARGO_TERM_VERBOSE=true @@ -86,9 +89,6 @@ if [ "$DO_ASAN" = true ]; then cargo run --release --features=alloc --manifest-path=./no_std_test/Cargo.toml | grep -q "Verified alloc Successfully" fi -# Test if panic in C code aborts the process (either with a real panic or with SIGILL) -cargo test -- --ignored --exact 'tests::test_panic_raw_ctx_should_terminate_abnormally' 2>&1 | tee /dev/stderr | grep "SIGILL\\|panicked at '\[libsecp256k1\]" - # Bench if [ "$DO_BENCH" = true ]; then cargo bench --all --features="unstable" From a5f7cfecce79f79341cc0ede675555b0ce5ef83f Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Sat, 19 Mar 2022 10:22:28 +1100 Subject: [PATCH 03/11] Re-name nightly CI job to Nightly In line with the `Tests` job and for the facot that this job does stuff with the nightly toolchain other than bench. Re-name nightly CI job from `bench_nightly`to `Nightly`. --- .github/workflows/rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 516a5e91f..c9930e3a1 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -3,7 +3,7 @@ on: [push, pull_request] name: Continuous integration jobs: - bench_nightly: + Nightly: name: Nightly - ASan + Bench runs-on: ubuntu-latest strategy: From 21531c7191eb8ff30ee0a1d90bd24cd060145e63 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Sat, 19 Mar 2022 10:24:00 +1100 Subject: [PATCH 04/11] Remove unnecessary matrix We use a matrix with a single element, this is unnecessary. --- .github/workflows/rust.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index c9930e3a1..9060001e6 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -6,10 +6,6 @@ jobs: Nightly: name: Nightly - ASan + Bench runs-on: ubuntu-latest - strategy: - matrix: - rust: - - nightly steps: - name: Checkout Crate uses: actions/checkout@v2 @@ -17,7 +13,7 @@ jobs: uses: actions-rs/toolchain@v1 with: profile: minimal - toolchain: ${{ matrix.rust }} + toolchain: nightly override: true components: rust-src - name: Running address sanitizer From d5202f9edfd099fe1132baebe6a5350ea9e13507 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Sat, 19 Mar 2022 10:24:57 +1100 Subject: [PATCH 05/11] Install clang to run adress sanitizer The address sanitizer job is silently failing at the moment because we do not install clang. Install clang so the address sanitizer job can run. Do not fix the silent failure, that will be done later on. --- .github/workflows/rust.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 9060001e6..685582968 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -9,6 +9,8 @@ jobs: steps: - name: Checkout Crate uses: actions/checkout@v2 + - name: Install clang # Needed for ASan. + run: sudo apt-get install -y clang-9 - name: Checkout Toolchain uses: actions-rs/toolchain@v1 with: From a188c7144ab0d99e1de5d131064ff71271d28049 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Sat, 19 Mar 2022 10:27:48 +1100 Subject: [PATCH 06/11] Do docs build in Nightly job We have a separate CI job for things that require a nightly toolchain. Building the docs requires a nightly toolchain (because of `--cfg docsrc` flag). It makes more sense to run the docs build in the `Nightly` job instead of hidden in the `Tests` job. Do the docs build in the `Nightly` job instead of in the `Tests` job. --- .github/workflows/rust.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 685582968..821532be2 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -4,7 +4,7 @@ name: Continuous integration jobs: Nightly: - name: Nightly - ASan + Bench + name: Nightly - ASan + Bench + Docs runs-on: ubuntu-latest steps: - name: Checkout Crate @@ -26,6 +26,10 @@ jobs: env: DO_BENCH: true run: ./contrib/test.sh + - name: Building docs + env: + DO_DOCS: true + run: ./contrib/test.sh Tests: name: Tests @@ -43,7 +47,6 @@ jobs: - rust: nightly env: DO_FEATURE_MATRIX: true - DO_DOCS: true - rust: 1.29.0 env: DO_FEATURE_MATRIX: true From a574bf02e6f2978b6a9b627ea48808b5260c8377 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Sat, 19 Mar 2022 10:40:19 +1100 Subject: [PATCH 07/11] Run WASM for multiple toolchains WASM is supported by Rust 1.30. We can therefore run the WASM tests on any all the toolchains except MSRV (1.29.0). This has benefit of catching nightly/beta issues before they get to stable. Done as a separate CI job since it is conceptually different to the `Tests` job. Run WASM for nightly, beta, and stable toolchains. --- .github/workflows/rust.yml | 40 ++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 821532be2..b8a338019 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -36,20 +36,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - include: - - rust: stable - env: - DO_FEATURE_MATRIX: true - DO_WASM: true - - rust: beta - env: - DO_FEATURE_MATRIX: true - - rust: nightly - env: - DO_FEATURE_MATRIX: true - - rust: 1.29.0 - env: - DO_FEATURE_MATRIX: true + rust: [stable, beta, nightly, 1.29.0] steps: - name: Checkout Crate uses: actions/checkout@v2 @@ -63,6 +50,29 @@ jobs: if: matrix.rust == '1.29.0' run: cargo generate-lockfile --verbose && cargo update -p cc --precise "1.0.41" --verbose - name: Running cargo - env: ${{ matrix.env }} + env: + DO_FEATURE_MATRIX: true run: ./contrib/test.sh + WASM: + name: WASM + runs-on: ubuntu-latest + strategy: + matrix: + rust: [stable, beta, nightly] # WASM requires Rust 1.30 + steps: + - name: Checkout Crate + uses: actions/checkout@v2 + - name: Install clang + run: sudo apt-get install -y clang-9 + - name: Checkout Toolchain + uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: nightly + override: true + components: rust-src + - name: Running WASM tests + env: + DO_WASM: true + run: ./contrib/test.sh From 68bedbf4695d0f9d98bb5b10fc1b8fda6d71d7b7 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Sat, 19 Mar 2022 09:43:40 +1100 Subject: [PATCH 08/11] Remove WASM32_* const definitions The const definitions in `stdio.h` are causing the `wasm-pack` build to fail due to 'duplicate symbol' errors. This can be solved by just removing the definitions. --- secp256k1-sys/src/types.rs | 38 ------------------------------ secp256k1-sys/wasm-sysroot/stdio.h | 17 ------------- src/context.rs | 6 ----- 3 files changed, 61 deletions(-) diff --git a/secp256k1-sys/src/types.rs b/secp256k1-sys/src/types.rs index e257d437b..54cdb0ba1 100644 --- a/secp256k1-sys/src/types.rs +++ b/secp256k1-sys/src/types.rs @@ -66,41 +66,3 @@ mod tests { assert!(mem::align_of::() >= mem::align_of::()); } } - -#[doc(hidden)] -#[cfg(target_arch = "wasm32")] -pub fn sanity_checks_for_wasm() { - use core::mem::{align_of, size_of}; - extern "C" { - pub static WASM32_INT_SIZE: c_uchar; - pub static WASM32_INT_ALIGN: c_uchar; - - pub static WASM32_UNSIGNED_INT_SIZE: c_uchar; - pub static WASM32_UNSIGNED_INT_ALIGN: c_uchar; - - pub static WASM32_SIZE_T_SIZE: c_uchar; - pub static WASM32_SIZE_T_ALIGN: c_uchar; - - pub static WASM32_UNSIGNED_CHAR_SIZE: c_uchar; - pub static WASM32_UNSIGNED_CHAR_ALIGN: c_uchar; - - pub static WASM32_PTR_SIZE: c_uchar; - pub static WASM32_PTR_ALIGN: c_uchar; - } - unsafe { - assert_eq!(size_of::(), WASM32_INT_SIZE as usize); - assert_eq!(align_of::(), WASM32_INT_ALIGN as usize); - - assert_eq!(size_of::(), WASM32_UNSIGNED_INT_SIZE as usize); - assert_eq!(align_of::(), WASM32_UNSIGNED_INT_ALIGN as usize); - - assert_eq!(size_of::(), WASM32_SIZE_T_SIZE as usize); - assert_eq!(align_of::(), WASM32_SIZE_T_ALIGN as usize); - - assert_eq!(size_of::(), WASM32_UNSIGNED_CHAR_SIZE as usize); - assert_eq!(align_of::(), WASM32_UNSIGNED_CHAR_ALIGN as usize); - - assert_eq!(size_of::<*const ()>(), WASM32_PTR_SIZE as usize); - assert_eq!(align_of::<*const ()>(), WASM32_PTR_ALIGN as usize); - } -} diff --git a/secp256k1-sys/wasm-sysroot/stdio.h b/secp256k1-sys/wasm-sysroot/stdio.h index 36cc90052..e69de29bb 100644 --- a/secp256k1-sys/wasm-sysroot/stdio.h +++ b/secp256k1-sys/wasm-sysroot/stdio.h @@ -1,17 +0,0 @@ -#include -#define alignof(type) offsetof (struct { char c; type member; }, member) - -extern const unsigned char WASM32_INT_SIZE = sizeof(int); -extern const unsigned char WASM32_INT_ALIGN = alignof(int); - -extern const unsigned char WASM32_UNSIGNED_INT_SIZE = sizeof(unsigned int); -extern const unsigned char WASM32_UNSIGNED_INT_ALIGN = alignof(unsigned int); - -extern const unsigned char WASM32_SIZE_T_SIZE = sizeof(size_t); -extern const unsigned char WASM32_SIZE_T_ALIGN = alignof(size_t); - -extern const unsigned char WASM32_UNSIGNED_CHAR_SIZE = sizeof(unsigned char); -extern const unsigned char WASM32_UNSIGNED_CHAR_ALIGN = alignof(unsigned char); - -extern const unsigned char WASM32_PTR_SIZE = sizeof(void*); -extern const unsigned char WASM32_PTR_ALIGN = alignof(void*); \ No newline at end of file diff --git a/src/context.rs b/src/context.rs index b7568778a..f9f308914 100644 --- a/src/context.rs +++ b/src/context.rs @@ -192,9 +192,6 @@ mod alloc_only { /// ``` #[allow(unused_mut)] // Unused when `rand-std` is not enabled. pub fn gen_new() -> Secp256k1 { - #[cfg(target_arch = "wasm32")] - ffi::types::sanity_checks_for_wasm(); - let size = unsafe { ffi::secp256k1_context_preallocated_size(C::FLAGS) }; let layout = alloc::Layout::from_size_align(size, ALIGN_TO).unwrap(); let ptr = unsafe {alloc::alloc(layout)}; @@ -302,9 +299,6 @@ unsafe impl<'buf> Context for AllPreallocated<'buf> { impl<'buf, C: Context + 'buf> Secp256k1 { /// Lets you create a context with preallocated buffer in a generic manner(sign/verify/all) pub fn preallocated_gen_new(buf: &'buf mut [AlignedType]) -> Result, Error> { - #[cfg(target_arch = "wasm32")] - ffi::types::sanity_checks_for_wasm(); - if buf.len() < Self::preallocate_size_gen() { return Err(Error::NotEnoughMemory); } From ceda800741fcc1fd8260df96414ed840ca426d68 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Thu, 17 Mar 2022 10:30:36 +1100 Subject: [PATCH 09/11] test.sh: explicitly return 0 As per UNIX convention a Bash script should exit with status code 0 if it completes successfully. --- contrib/test.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/test.sh b/contrib/test.sh index 95d9afc6d..06d2d980e 100755 --- a/contrib/test.sh +++ b/contrib/test.sh @@ -94,3 +94,4 @@ if [ "$DO_BENCH" = true ]; then cargo bench --all --features="unstable" fi +exit 0 From e750f82a0278665e7b7d4a1ffc89e45611c5f956 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Thu, 17 Mar 2022 10:57:30 +1100 Subject: [PATCH 10/11] test.sh: Use set -e to exit on failure Currently the `test.sh` script is silently failing because we do not exit if a command fails. We can achieve this by using the Bash builtin `set -e`. For some reason I cannot explain a chain of commands that fails does not fail the script. Instead of working out _why_ just remove the chain and run each command on its own. This is functionally the same and, I hazard a guess, is what the original author hoped to achieve with the chaining. --- contrib/test.sh | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/contrib/test.sh b/contrib/test.sh index 06d2d980e..271f6032b 100755 --- a/contrib/test.sh +++ b/contrib/test.sh @@ -1,5 +1,7 @@ #!/bin/sh -ex +set -e + # TODO: Add "alloc" once we bump MSRV to past 1.29 FEATURES="bitcoin_hashes global-context lowmemory rand recovery serde std" # These features are typically enabled along with the 'std' feature, so we test @@ -67,11 +69,11 @@ fi # Webassembly stuff if [ "$DO_WASM" = true ]; then - clang --version && - CARGO_TARGET_DIR=wasm cargo install --force wasm-pack && - printf '\n[lib]\ncrate-type = ["cdylib", "rlib"]\n' >> Cargo.toml && - CC=clang-9 wasm-pack build && - CC=clang-9 wasm-pack test --node; + clang --version + CARGO_TARGET_DIR=wasm cargo install --force wasm-pack + printf '\n[lib]\ncrate-type = ["cdylib", "rlib"]\n' >> Cargo.toml + CC=clang-9 wasm-pack build + CC=clang-9 wasm-pack test --node fi # Address Sanitizer @@ -80,11 +82,11 @@ if [ "$DO_ASAN" = true ]; then CC='clang -fsanitize=address -fno-omit-frame-pointer' \ RUSTFLAGS='-Zsanitizer=address -Clinker=clang -Cforce-frame-pointers=yes' \ ASAN_OPTIONS='detect_leaks=1 detect_invalid_pointer_pairs=1 detect_stack_use_after_return=1' \ - cargo test --lib --all --features="$FEATURES" -Zbuild-std --target x86_64-unknown-linux-gnu && - cargo clean && + cargo test --lib --all --features="$FEATURES" -Zbuild-std --target x86_64-unknown-linux-gnu + cargo clean CC='clang -fsanitize=memory -fno-omit-frame-pointer' \ RUSTFLAGS='-Zsanitizer=memory -Zsanitizer-memory-track-origins -Cforce-frame-pointers=yes' \ - cargo test --lib --all --features="$FEATURES" -Zbuild-std --target x86_64-unknown-linux-gnu && + cargo test --lib --all --features="$FEATURES" -Zbuild-std --target x86_64-unknown-linux-gnu cargo run --release --manifest-path=./no_std_test/Cargo.toml | grep -q "Verified Successfully" cargo run --release --features=alloc --manifest-path=./no_std_test/Cargo.toml | grep -q "Verified alloc Successfully" fi From d4d35d131559217b9219d2a29469efb2338576cb Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Fri, 25 Mar 2022 12:17:22 +1100 Subject: [PATCH 11/11] Use clang-10 for WASM build A user reported that WASM only used to work with `clang-10`, I cannot verify this but lets use v10 anyways. --- .github/workflows/rust.yml | 2 +- contrib/test.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index b8a338019..1e8edede4 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -64,7 +64,7 @@ jobs: - name: Checkout Crate uses: actions/checkout@v2 - name: Install clang - run: sudo apt-get install -y clang-9 + run: sudo apt-get install -y clang-10 - name: Checkout Toolchain uses: actions-rs/toolchain@v1 with: diff --git a/contrib/test.sh b/contrib/test.sh index 271f6032b..0d9bb5e85 100755 --- a/contrib/test.sh +++ b/contrib/test.sh @@ -72,8 +72,8 @@ if [ "$DO_WASM" = true ]; then clang --version CARGO_TARGET_DIR=wasm cargo install --force wasm-pack printf '\n[lib]\ncrate-type = ["cdylib", "rlib"]\n' >> Cargo.toml - CC=clang-9 wasm-pack build - CC=clang-9 wasm-pack test --node + CC=clang wasm-pack build + CC=clang wasm-pack test --node fi # Address Sanitizer