Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: Various improvements #1047

Merged
merged 6 commits into from
Dec 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 27 additions & 43 deletions .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ env:
# Specific warnings can be disabled with -Wno-error=foo.
# -pedantic-errors is not equivalent to -Werror=pedantic and thus not implied by -Werror according to the GCC manual.
WERROR_CFLAGS: -Werror -pedantic-errors
MAKEFLAGS: -j2
MAKEFLAGS: -j4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the CI server actually have 4 CPUs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We set cpu: 1 or cpu: 2 depending on the task. But the greedy flag will occasionally give us more CPUs when available. Increasing this to -j4 makes sure we actually can make use of this.

BUILD: check
### secp256k1 config
STATICPRECOMPUTATION: yes
ECMULTWINDOW: auto
ECMULTGENPRECISION: auto
ASM: no
WIDEMUL: auto
Expand Down Expand Up @@ -50,14 +50,19 @@ 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: "x86_64: Linux (Debian stable)"
linux_container_snippet: &LINUX_CONTAINER
container:
dockerfile: ci/linux-debian.Dockerfile
# Reduce number of CPUs to be able to do more builds in parallel.
cpu: 1
# Gives us more CPUs for free if they're available.
greedy: true
# More than enough for our scripts.
memory: 1G

task:
name: "x86_64: Linux (Debian stable)"
<< : *LINUX_CONTAINER
matrix: &ENV_MATRIX
- env: {WIDEMUL: int64, RECOVERY: yes}
- env: {WIDEMUL: int64, ECDH: yes, EXPERIMENTAL: yes, SCHNORRSIG: yes}
Expand All @@ -66,12 +71,11 @@ task:
- env: {WIDEMUL: int128, ECDH: yes, EXPERIMENTAL: yes, SCHNORRSIG: yes}
- env: {WIDEMUL: int128, ASM: x86_64}
- env: { RECOVERY: yes, EXPERIMENTAL: yes, SCHNORRSIG: yes}
- env: { STATICPRECOMPUTATION: no}
- env: {BUILD: distcheck, WITH_VALGRIND: no, CTIMETEST: no, BENCH: no}
- env: {CPPFLAGS: -DDETERMINISTIC}
- env: {CFLAGS: -O0, CTIMETEST: no}
- env: { ECMULTGENPRECISION: 2 }
- env: { ECMULTGENPRECISION: 8 }
- env: { ECMULTGENPRECISION: 2, ECMULTWINDOW: 2 }
- env: { ECMULTGENPRECISION: 8, ECMULTWINDOW: 4 }
matrix:
- env:
CC: gcc
Expand All @@ -84,10 +88,7 @@ task:

task:
name: "i686: Linux (Debian stable)"
container:
dockerfile: ci/linux-debian.Dockerfile
cpu: 1
memory: 1G
<< : *LINUX_CONTAINER
env:
HOST: i686-linux-gnu
ECDH: yes
Expand Down Expand Up @@ -134,8 +135,9 @@ task:
## - rm /tmp/.com.apple.dt.CommandLineTools.installondemand.in-progress
##
brew_valgrind_pre_script:
- brew update
- brew config
- brew tap --shallow LouisBrunner/valgrind
- brew tap LouisBrunner/valgrind
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not shallow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shallow doesn't exist anymore in recent brew versions, see the commit message.

# Fetch valgrind source but don't build it yet.
- brew fetch --HEAD LouisBrunner/valgrind/valgrind
brew_valgrind_cache:
Expand Down Expand Up @@ -165,10 +167,7 @@ task:

task:
name: "s390x (big-endian): Linux (Debian stable, QEMU)"
container:
dockerfile: ci/linux-debian.Dockerfile
cpu: 1
memory: 1G
<< : *LINUX_CONTAINER
env:
WRAPPER_CMD: qemu-s390x
SECP256K1_TEST_ITERS: 16
Expand All @@ -188,10 +187,7 @@ task:

task:
name: "ARM32: Linux (Debian stable, QEMU)"
container:
dockerfile: ci/linux-debian.Dockerfile
cpu: 1
memory: 1G
<< : *LINUX_CONTAINER
env:
WRAPPER_CMD: qemu-arm
SECP256K1_TEST_ITERS: 16
Expand All @@ -212,10 +208,7 @@ task:

task:
name: "ARM64: Linux (Debian stable, QEMU)"
container:
dockerfile: ci/linux-debian.Dockerfile
cpu: 1
memory: 1G
<< : *LINUX_CONTAINER
env:
WRAPPER_CMD: qemu-aarch64
SECP256K1_TEST_ITERS: 16
Expand All @@ -233,10 +226,7 @@ task:

task:
name: "ppc64le: Linux (Debian stable, QEMU)"
container:
dockerfile: ci/linux-debian.Dockerfile
cpu: 1
memory: 1G
<< : *LINUX_CONTAINER
env:
WRAPPER_CMD: qemu-ppc64le
SECP256K1_TEST_ITERS: 16
Expand All @@ -254,10 +244,7 @@ task:

task:
name: "x86_64 (mingw32-w64): Windows (Debian stable, Wine)"
container:
dockerfile: ci/linux-debian.Dockerfile
cpu: 1
memory: 1G
<< : *LINUX_CONTAINER
env:
WRAPPER_CMD: wine64-stable
SECP256K1_TEST_ITERS: 16
Expand All @@ -275,10 +262,7 @@ task:

# Sanitizers
task:
container:
dockerfile: ci/linux-debian.Dockerfile
cpu: 1
memory: 2G
<< : *LINUX_CONTAINER
env:
ECDH: yes
RECOVERY: yes
Expand All @@ -287,11 +271,15 @@ task:
CTIMETEST: no
matrix:
- name: "Valgrind (memcheck)"
container:
cpu: 2
env:
# The `--error-exitcode` is required to make the test fail if valgrind found errors, otherwise it'll return 0 (https://www.valgrind.org/docs/manual/manual-core.html)
WRAPPER_CMD: "valgrind --error-exitcode=42"
SECP256K1_TEST_ITERS: 2
- name: "UBSan, ASan, LSan"
container:
memory: 2G
env:
CFLAGS: "-fsanitize=undefined,address -g"
UBSAN_OPTIONS: "print_stacktrace=1:halt_on_error=1"
Expand All @@ -302,11 +290,10 @@ task:
matrix:
- env:
ASM: auto
STATICPRECOMPUTATION: yes
- env:
ASM: no
STATICPRECOMPUTATION: no
ECMULTGENPRECISION: 2
ECMULTWINDOW: 2
matrix:
- env:
CC: clang
Expand All @@ -320,15 +307,12 @@ task:

task:
name: "C++ -fpermissive"
container:
dockerfile: ci/linux-debian.Dockerfile
cpu: 1
memory: 1G
<< : *LINUX_CONTAINER
env:
# ./configure correctly errors out when given CC=g++.
# We hack around this by passing CC=g++ only to make.
CC: gcc
MAKEFLAGS: -j2 CC=g++ CFLAGS=-fpermissive\ -g
MAKEFLAGS: -j4 CC=g++ CFLAGS=-fpermissive\ -g
WERROR_CFLAGS:
EXPERIMENTAL: yes
ECDH: yes
Expand Down
3 changes: 2 additions & 1 deletion ci/cirrus.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ valgrind --version || true
./configure \
--enable-experimental="$EXPERIMENTAL" \
--with-test-override-wide-multiply="$WIDEMUL" --with-asm="$ASM" \
--enable-ecmult-static-precomputation="$STATICPRECOMPUTATION" --with-ecmult-gen-precision="$ECMULTGENPRECISION" \
--with-ecmult-window="$ECMULTWINDOW" \
--with-ecmult-gen-precision="$ECMULTGENPRECISION" \
--enable-module-ecdh="$ECDH" --enable-module-recovery="$RECOVERY" \
--enable-module-schnorrsig="$SCHNORRSIG" \
--with-valgrind="$WITH_VALGRIND" \
Expand Down
8 changes: 5 additions & 3 deletions src/precompute_ecmult.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ static void print_table(FILE *fp, const char *name, int window_g, const secp256k

j = 1;
for(i = 3; i <= window_g; ++i) {
fprintf(fp, "#if ECMULT_TABLE_SIZE(WINDOW_G) > %ld\n", ECMULT_TABLE_SIZE(i-1));
fprintf(fp, "#if WINDOW_G > %d\n", i-1);
for(;j < ECMULT_TABLE_SIZE(i); ++j) {
fprintf(fp, ",S(%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32
",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32")\n",
Expand All @@ -57,6 +57,8 @@ static void print_two_tables(FILE *fp, int window_g) {
}

int main(void) {
/* Always compute all tables for window sizes up to 15. */
int window_g = (ECMULT_WINDOW_SIZE < 15) ? 15 : ECMULT_WINDOW_SIZE;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

15_

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FILE* fp;

fp = fopen("src/precomputed_ecmult.c","w");
Expand All @@ -77,15 +79,15 @@ int main(void) {
fprintf(fp, "#include \"ecmult.h\"\n");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ecmult.h is no longer needed if you remove ECMULT_TABLE_SIZE.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's correct to use ECMULT_TABLE_SIZE.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right sorry. For some reason I thought all the users of ECMULT_TABLE_SIZE were eliminated, but that is not true.

fprintf(fp, "#include \"precomputed_ecmult.h\"\n");
fprintf(fp, "#define S(a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p) SECP256K1_GE_STORAGE_CONST(0x##a##u,0x##b##u,0x##c##u,0x##d##u,0x##e##u,0x##f##u,0x##g##u,0x##h##u,0x##i##u,0x##j##u,0x##k##u,0x##l##u,0x##m##u,0x##n##u,0x##o##u,0x##p##u)\n");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


fprintf(fp, "#if ECMULT_TABLE_SIZE(ECMULT_WINDOW_SIZE) > %ld\n", ECMULT_TABLE_SIZE(ECMULT_WINDOW_SIZE));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


fprintf(fp, "#if ECMULT_WINDOW_SIZE > %d\n", window_g);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__

fprintf(fp, " #error configuration mismatch, invalid ECMULT_WINDOW_SIZE. Try deleting precomputed_ecmult.c before the build.\n");
fprintf(fp, "#endif\n");
fprintf(fp, "#ifdef EXHAUSTIVE_TEST_ORDER\n");
fprintf(fp, "# error Cannot compile precomputed_ecmult.c in exhaustive test mode\n");
fprintf(fp, "#endif /* EXHAUSTIVE_TEST_ORDER */\n");
fprintf(fp, "#define WINDOW_G ECMULT_WINDOW_SIZE\n");

print_two_tables(fp, ECMULT_WINDOW_SIZE);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • [ ]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

00

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@

print_two_tables(fp, window_g);

fprintf(fp, "#undef S\n");
fclose(fp);
Expand Down
Loading