diff --git a/.cirrus.yml b/.cirrus.yml index 6d63511e6d..fdd24e5953 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -43,6 +43,37 @@ merge_base_script_snippet: &MERGE_BASE - git config --global user.name "ci" - git merge FETCH_HEAD # Merge base to detect silent merge conflicts +task: + name: Code coverage + container: + dockerfile: ci/linux-debian.Dockerfile + cpu: 1 + memory: 1G + env: + EXTRAFLAGS: "--enable-coverage" + ASM: no + ECDH: yes + RECOVERY: yes + EXPERIMENTAL: yes + SCHNORRSIG: yes + CTIMETEST: no + BENCH: no + matrix: + - env: + WIDEMUL: int128 + STATICPRECOMPUTATION: yes + - env: + WIDEMUL: int64 + STATICPRECOMPUTATION: no + << : *MERGE_BASE + test_script: + - ./ci/cirrus.sh + << : *CAT_LOGS + gcovr_script: + - gcovr --print-summary --exclude 'src/bench*' --exclude 'src/valgrind_ctime_test.c' --html --html-title "${CIRRUS_CHANGE_IN_REPO:0:7}, STATICPRECOMPUTATION=$STATICPRECOMPUTATION, WIDEMUL=$WIDEMUL" --html-details -o index.html + coverage_report_artifacts: + path: "**.html" + task: name: "x86_64: Linux (Debian stable)" container: @@ -312,4 +343,3 @@ task: test_script: - ./ci/cirrus.sh << : *CAT_LOGS - diff --git a/README.md b/README.md index a7eb2b0e89..8e65528a33 100644 --- a/README.md +++ b/README.md @@ -92,12 +92,12 @@ Run the tests: To create a report, `gcovr` is recommended, as it includes branch coverage reporting: - $ gcovr --exclude 'src/bench*' --print-summary + $ gcovr --exclude 'src/bench*' --exclude 'src/valgrind_ctime_test.c' --print-summary To create a HTML report with coloured and annotated source code: $ mkdir -p coverage - $ gcovr --exclude 'src/bench*' --html --html-details -o coverage/coverage.html + $ gcovr --exclude 'src/bench*' --exclude 'src/valgrind_ctime_test.c' --html --html-details -o coverage/coverage.html Reporting a vulnerability ------------ diff --git a/ci/linux-debian.Dockerfile b/ci/linux-debian.Dockerfile index 6def91333d..421bd2723a 100644 --- a/ci/linux-debian.Dockerfile +++ b/ci/linux-debian.Dockerfile @@ -11,7 +11,7 @@ RUN apt-get update # llvm: for llvm-symbolizer, which is used by clang's UBSan for symbolized stack traces RUN apt-get install --no-install-recommends --no-upgrade -y \ git ca-certificates \ - make automake libtool pkg-config dpkg-dev valgrind qemu-user \ + make automake libtool pkg-config dpkg-dev valgrind qemu-user gcovr \ gcc clang llvm libc6-dbg \ gcc-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 libubsan1:i386 libasan5:i386 \ gcc-s390x-linux-gnu libc6-dev-s390x-cross libc6-dbg:s390x \ diff --git a/src/ecmult_impl.h b/src/ecmult_impl.h index 5c2edac68f..ab24b934cc 100644 --- a/src/ecmult_impl.h +++ b/src/ecmult_impl.h @@ -417,12 +417,11 @@ static int secp256k1_ecmult_wnaf(int *wnaf, int len, const secp256k1_scalar *a, bit += now; } -#ifdef VERIFY - CHECK(carry == 0); + VERIFY_CHECK(carry == 0); while (bit < 256) { - CHECK(secp256k1_scalar_get_bits(&s, bit++, 1) == 0); + VERIFY_CHECK(secp256k1_scalar_get_bits(&s, bit, 1) == 0); + bit++; } -#endif return last_set_bit + 1; } diff --git a/src/util.h b/src/util.h index f78846836c..37a651a698 100644 --- a/src/util.h +++ b/src/util.h @@ -25,7 +25,11 @@ static SECP256K1_INLINE void secp256k1_callback_call(const secp256k1_callback * cb->fn(text, (void*)cb->data); } -#ifdef DETERMINISTIC +#if defined(COVERAGE) +/* Do nothing in COVERAGE mode. This will make the compiler optimize away the actual branch, + and we get useful branch coverage within our test files. */ +#define TEST_FAILURE(msg) +#elif defined(DETERMINISTIC) #define TEST_FAILURE(msg) do { \ fprintf(stderr, "%s\n", msg); \ abort(); \ @@ -43,7 +47,17 @@ static SECP256K1_INLINE void secp256k1_callback_call(const secp256k1_callback * #define EXPECT(x,c) (x) #endif -#ifdef DETERMINISTIC +/* CHECK() is like assert(). We use it only in the tests. */ +#if defined(COVERAGE) +/* Don't abort in coverage mode. + This avoids branches which are not expected to be taken. + We still use cond as usual to avoid unused variable warnings. */ +#define CHECK(cond) do { \ + if (EXPECT(!(cond), 0)) { \ + ; \ + } \ +} while (0) +#elif defined(DETERMINISTIC) #define CHECK(cond) do { \ if (EXPECT(!(cond), 0)) { \ TEST_FAILURE("test condition failed"); \ @@ -59,7 +73,11 @@ static SECP256K1_INLINE void secp256k1_callback_call(const secp256k1_callback * /* Like assert(), but when VERIFY is defined, and side-effect safe. */ #if defined(COVERAGE) -#define VERIFY_CHECK(check) +/* Do nothing in coverage mode but try to stay syntactically correct. + This suppresses a lot of implicit branches introduced by shortcutting + operators at the cost of not being side-effect safe in coverage mode. + We rely on the compiler to eliminate the if (0) statement entirely. */ +#define VERIFY_CHECK(cond) do { if (0) (void)(cond); } while(0) #define VERIFY_SETUP(stmt) #elif defined(VERIFY) #define VERIFY_CHECK CHECK