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.

Sign up for GitHub

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

Jump to bottom

Make signing table fully static #988

Merged
merged 18 commits into from
Dec 15, 2021
Merged

Make signing table fully static #988

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
22dc2c0
ecmult_gen: Move table creation to new file and force static prec
real-or-random Sep 9, 2021
e43ba02
refactor: Decouple table generation and ecmult_gen context
real-or-random Nov 9, 2021
3b0c218
ecmult_gen: Simplify ecmult_gen context after making table static
real-or-random Nov 9, 2021
00d2fa1
ecmult_gen: Make code consistent with comment
real-or-random Oct 7, 2021
8ae18f1
refactor: Rename file that contains static ecmult_gen table
real-or-random Oct 7, 2021
9ad09f6
refactor: Rename program that generates static ecmult_gen table
real-or-random Oct 7, 2021
e1a7653
refactor: Make generator a parameter of ecmult_gen_create_prec_table
real-or-random Oct 6, 2021
5106226
exhaustive_tests: Fix with ecmult_gen table with custom generator
real-or-random Oct 7, 2021
4c94c55
doc: Remove obsolete hint for valgrind stack size
real-or-random Oct 20, 2021
a4875e3
refactor: Move default callbacks to util.h
real-or-random Nov 8, 2021
fdb33dd
refactor: Make PREC_BITS a parameter of ecmult_gen_build_prec_table
real-or-random Sep 8, 2021
5eba83f
ecmult_gen: Precompute tables for all values of ECMULT_GEN_PREC_BITS
real-or-random Nov 8, 2021
6573c08
ecmult_gen: Tidy precomputed file and save space
real-or-random Nov 8, 2021
ac49361
prealloc: Get rid of manual memory management for prealloc contexts
real-or-random Nov 9, 2021
ad63bb4
build: Prebuild and distribute ecmult_gen table
real-or-random Aug 27, 2021
d94a37a
build: Remove CC_FOR_BUILD stuff
real-or-random Nov 10, 2021
bb36fe9
ci: Test `make precomp`
real-or-random Nov 13, 2021
7dfcece
build: Remove #undef hack for ASM in the precomputation programs
real-or-random Dec 2, 2021
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
1 change: 0 additions & 1 deletion .cirrus.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,6 @@ task:
- name: "UBSan, ASan, LSan"
env:
CFLAGS: "-fsanitize=undefined,address -g"
CFLAGS_FOR_BUILD: "-fsanitize=undefined,address -g"
UBSAN_OPTIONS: "print_stacktrace=1:halt_on_error=1"
ASAN_OPTIONS: "strict_string_checks=1:detect_stack_use_after_return=1:detect_leaks=1"
LSAN_OPTIONS: "use_unaligned=1"
Expand Down
1 change: 1 addition & 0 deletions .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
src/ecmult_static_pre_g.h linguist-generated
src/ecmult_gen_static_prec_table.h linguist-generated
3 changes: 1 addition & 2 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ bench_ecmult
bench_internal
tests
exhaustive_tests
gen_context
gen_ecmult_gen_static_prec_table
Copy link
Contributor

@robot-dreams robot-dreams Nov 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

(No action needed) It's too bad that gen stands for both "generate" (the static table) and "generator" (of a group) 😅 , but it's probably not worth changing, especially not as part of this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this confused me a lot when starting to work on the library a few years back. And gen_context should have been called gen_gen_context at least, even though that's ugly.

I tried to avoid this discussion in this PR (only renaming thins I'm touching anyway) and it may be worth unifying the names in the follow-up PR that switches to linking instead of including (e.g., I intentionally used "create" in a function name instead of "generate") Then we anyway need to touch both "generate" things again, so this would be a good opportunity.

gen_ecmult_static_pre_g
valgrind_ctime_test
*.exe
Expand Down Expand Up @@ -41,7 +41,6 @@ coverage.*.html

src/libsecp256k1-config.h
src/libsecp256k1-config.h.in
src/ecmult_static_context.h
build-aux/config.guess
build-aux/config.sub
build-aux/depcomp
Expand Down
58 changes: 36 additions & 22 deletions Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
.PHONY: clean-precomp precomp

ACLOCAL_AMFLAGS = -I build-aux/m4

# AM_CFLAGS will be automatically prepended to CFLAGS by Automake when compiling some foo
Expand Down Expand Up @@ -28,6 +30,8 @@ noinst_HEADERS += src/ecmult_const.h
noinst_HEADERS += src/ecmult_const_impl.h
noinst_HEADERS += src/ecmult_gen.h
noinst_HEADERS += src/ecmult_gen_impl.h
noinst_HEADERS += src/ecmult_gen_prec.h
noinst_HEADERS += src/ecmult_gen_prec_impl.h
noinst_HEADERS += src/field_10x26.h
noinst_HEADERS += src/field_10x26_impl.h
noinst_HEADERS += src/field_5x52.h
Expand All @@ -50,6 +54,7 @@ noinst_HEADERS += src/hash_impl.h
noinst_HEADERS += src/field.h
noinst_HEADERS += src/field_impl.h
noinst_HEADERS += src/bench.h
noinst_HEADERS += src/basic-config.h
noinst_HEADERS += contrib/lax_der_parsing.h
noinst_HEADERS += contrib/lax_der_parsing.c
noinst_HEADERS += contrib/lax_der_privatekey_parsing.h
Expand Down Expand Up @@ -114,7 +119,7 @@ endif
if USE_EXHAUSTIVE_TESTS
noinst_PROGRAMS += exhaustive_tests
exhaustive_tests_SOURCES = src/tests_exhaustive.c
exhaustive_tests_CPPFLAGS = -I$(top_srcdir)/src $(SECP_INCLUDES)
exhaustive_tests_CPPFLAGS = $(SECP_INCLUDES)
if !ENABLE_COVERAGE
exhaustive_tests_CPPFLAGS += -DVERIFY
endif
Expand All @@ -123,36 +128,45 @@ exhaustive_tests_LDFLAGS = -static
TESTS += exhaustive_tests
endif

EXTRA_PROGRAMS = gen_ecmult_static_pre_g
### Precomputed tables
EXTRA_PROGRAMS = gen_ecmult_static_pre_g gen_ecmult_gen_static_prec_table
CLEANFILES = $(EXTRA_PROGRAMS)

gen_ecmult_static_pre_g_SOURCES = src/gen_ecmult_static_pre_g.c
# See Automake manual, Section "Errors with distclean"
gen_ecmult_static_pre_g_CPPFLAGS = $(SECP_INCLUDES)
gen_ecmult_static_pre_g_LDADD = $(SECP_LIBS) $(COMMON_LIB)

gen_ecmult_gen_static_prec_table_SOURCES = src/gen_ecmult_gen_static_prec_table.c
gen_ecmult_gen_static_prec_table_CPPFLAGS = $(SECP_INCLUDES)
gen_ecmult_gen_static_prec_table_LDADD = $(SECP_LIBS) $(COMMON_LIB)

# See Automake manual, Section "Errors with distclean".
# We don't list any dependencies for the prebuilt files here because
# otherwise make's decision whether to rebuild them (even in the first
# build by a normal user) depends on mtimes, and thus is very fragile.
# This means that rebuilds of the prebuilt files always need to be
# forced by deleting them, e.g., by invoking `make clean-precomp`.
src/ecmult_static_pre_g.h:
$(MAKE) $(AM_MAKEFLAGS) gen_ecmult_static_pre_g$(EXEEXT)
./gen_ecmult_static_pre_g$(EXEEXT)
src/ecmult_gen_static_prec_table.h:
$(MAKE) $(AM_MAKEFLAGS) gen_ecmult_gen_static_prec_table$(EXEEXT)
./gen_ecmult_gen_static_prec_table$(EXEEXT)

if USE_ECMULT_STATIC_PRECOMPUTATION
CPPFLAGS_FOR_BUILD +=-I$(top_srcdir) -I$(builddir)/src

gen_context_OBJECTS = gen_context.o
gen_context_BIN = gen_context$(BUILD_EXEEXT)
$(gen_context_OBJECTS): src/gen_context.c src/libsecp256k1-config.h
$(CC_FOR_BUILD) $(DEFS) $(CPPFLAGS_FOR_BUILD) $(SECP_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) -c $< -o $@
PRECOMP = src/ecmult_gen_static_prec_table.h src/ecmult_static_pre_g.h
noinst_HEADERS += $(PRECOMP)
precomp: $(PRECOMP)

$(gen_context_BIN): $(gen_context_OBJECTS)
$(CC_FOR_BUILD) $(SECP_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) $^ -o $@
# Ensure the prebuilt files will be build first (only if they don't exist,
# e.g., after `make maintainer-clean`).
BUILT_SOURCES = $(PRECOMP)

$(libsecp256k1_la_OBJECTS): src/ecmult_static_context.h
$(tests_OBJECTS): src/ecmult_static_context.h
$(bench_internal_OBJECTS): src/ecmult_static_context.h
$(bench_ecmult_OBJECTS): src/ecmult_static_context.h
maintainer-clean-local: clean-precomp

src/ecmult_static_context.h: $(gen_context_BIN)
./$(gen_context_BIN)

CLEANFILES = $(gen_context_BIN) src/ecmult_static_context.h
endif
clean-precomp:
rm -f $(PRECOMP)

EXTRA_DIST = autogen.sh src/gen_context.c src/ecmult_static_pre_g.h src/basic-config.h
EXTRA_DIST = autogen.sh SECURITY.md

if ENABLE_MODULE_ECDH
include src/modules/ecdh/Makefile.am.include
Expand Down
11 changes: 1 addition & 10 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,9 @@ libsecp256k1 is built using autotools:
$ ./autogen.sh
$ ./configure
$ make
$ make check
$ make check # run the test suite
$ sudo make install # optional

Exhaustive tests
-----------

$ ./exhaustive_tests

With valgrind, you might need to increase the max stack size:

$ valgrind --max-stackframe=2500000 ./exhaustive_tests

Test coverage
-----------

Expand Down
125 changes: 0 additions & 125 deletions build-aux/m4/ax_prog_cc_for_build.m4
View file
Open in desktop

This file was deleted.

12 changes: 12 additions & 0 deletions ci/cirrus.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,19 @@ then
$EXEC ./bench
} >> bench.log 2>&1
fi

if [ "$CTIMETEST" = "yes" ]
then
./libtool --mode=execute valgrind --error-exitcode=42 ./valgrind_ctime_test > valgrind_ctime_test.log 2>&1
fi

# Rebuild precomputed files (if not cross-compiling).
if [ -z "$HOST" ]
then
make clean-precomp
make precomp
fi

# Check that no repo files have been modified by the build.
# (This fails for example if the precomp files need to be updated in the repo.)
git diff --exit-code
Loading