From 73197c6516566ed1b5cc717d88af5501aec33ad1 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Wed, 27 Feb 2019 12:44:21 +0100 Subject: [PATCH 1/7] Add sanitizers to functests --- test/Makefile | 10 ++++++++++ test/Makefile.Microsoft_nmake | 6 ++++++ test/helpers.py | 18 ++++++++++++++++++ test/test_functest.py | 27 +++++++++++++++++++++++++++ 4 files changed, 61 insertions(+) diff --git a/test/Makefile b/test/Makefile index 31967c2f..a048d7a6 100644 --- a/test/Makefile +++ b/test/Makefile @@ -22,6 +22,16 @@ all: $(DEST_DIR)/functest_$(SCHEME)_$(IMPLEMENTATION) $(DEST_DIR)/testvectors_$( build-scheme: cd $(SCHEME_DIR) && $(MAKE) +.PHONY: clean-scheme +clean-scheme: + cd $(SCHEME_DIR) && $(MAKE) clean + +.PHONY: functest +functest: $(DEST_DIR)/functest_$(SCHEME)_$(IMPLEMENTATION) + +.PHONY: testvectors +testvectors: $(DEST_DIR)/testvectors_$(SCHEME)_$(IMPLEMENTATION) + $(DEST_DIR)/functest_$(SCHEME)_$(IMPLEMENTATION): build-scheme crypto_$(TYPE)/functest.c $(COMMON_FILES) $(COMMON_DIR)/randombytes.c $(COMMON_HEADERS) mkdir -p $(DEST_DIR) $(CC) $(CFLAGS) -DPQCLEAN_NAMESPACE=PQCLEAN_$(SCHEME_UPPERCASE)_$(IMPLEMENTATION_UPPERCASE) -I$(SCHEME_DIR) crypto_$(TYPE)/functest.c $(COMMON_FILES) $(COMMON_DIR)/notrandombytes.c -o $@ -L$(SCHEME_DIR) -l$(SCHEME)_$(IMPLEMENTATION) diff --git a/test/Makefile.Microsoft_nmake b/test/Makefile.Microsoft_nmake index 60e0682f..c37a727e 100644 --- a/test/Makefile.Microsoft_nmake +++ b/test/Makefile.Microsoft_nmake @@ -2,6 +2,7 @@ # nmake /f Makefile.Microsoft_nmake # override as desired +# vim: set ts=4 sw=4 et: TYPE=kem SCHEME=kyber768 SCHEME_UPPERCASE=KYBER768 @@ -25,6 +26,11 @@ build-scheme: nmake /f Makefile.Microsoft_nmake cd ..\..\..\test +clean-scheme: + cd $(SCHEME_DIR) + nmake /f Makefile.Microsoft_nmake clean + cd ..\..\..\test + $(DEST_DIR)\functest_$(SCHEME)_$(IMPLEMENTATION).EXE: build-scheme $(COMMON_OBJECTS) $(COMMON_DIR)\randombytes.obj -MKDIR $(DEST_DIR) $(CC) /c crypto_$(TYPE)\functest.c $(CFLAGS) /I $(SCHEME_DIR) /DPQCLEAN_NAMESPACE=PQCLEAN_$(SCHEME_UPPERCASE)_$(IMPLEMENTATION_UPPERCASE) diff --git a/test/helpers.py b/test/helpers.py index 6b558e7e..0d8537ee 100644 --- a/test/helpers.py +++ b/test/helpers.py @@ -19,3 +19,21 @@ def run_subprocess(command, working_dir='.', expected_returncode=0): print(result.stdout.decode('utf-8')) assert(result.returncode == expected_returncode) return result.stdout.decode('utf-8') + + +def make(*args, working_dir='.', **kwargs): + """ + Runs a make target in the specified working directory + + Usage: + make('clean', 'targetb', SCHEME='bla') + """ + make_command = 'make' + return run_subprocess( + [ + make_command, + *args, + *['{}={}'.format(k, v) for k, v in kwargs.items()], + ], + working_dir=working_dir, + ) diff --git a/test/test_functest.py b/test/test_functest.py index c4d0fbbe..5cf1df6a 100644 --- a/test/test_functest.py +++ b/test/test_functest.py @@ -4,6 +4,9 @@ and executed for every scheme/implementation. """ import os +import platform +import unittest + import pqclean import helpers @@ -14,6 +17,12 @@ def test_functest(): yield check_functest, scheme.name, implementation.name +def test_functest_sanitizers(): + for scheme in pqclean.Scheme.all_schemes(): + for implementation in scheme.implementations: + yield check_functest_sanitizers, scheme.name, implementation.name + + def check_functest(scheme_name, implementation_name): implementation = pqclean.Implementation.by_name( scheme_name, implementation_name) @@ -30,6 +39,24 @@ def check_functest(scheme_name, implementation_name): ) +def check_functest_sanitizers(scheme_name, implementation_name): + if platform.machine() not in ['i386', 'x86_64']: + raise unittest.SkipTest() + implementation = pqclean.Implementation.by_name( + scheme_name, implementation_name) + helpers.make('clean-scheme', 'functest', + TYPE=implementation.scheme.type, + SCHEME=scheme_name, + IMPLEMENTATION=implementation_name, + working_dir=os.path.join('..', 'test'), + EXTRAFLAGS='-fsanitize=address,undefined') + helpers.run_subprocess( + ['./functest_{}_{}'.format(scheme_name, implementation_name)], + os.path.join('..', 'bin'), + ) + return check_functest(scheme_name, implementation_name) + + if __name__ == '__main__': try: import nose2 From 59792522aefe3aaa0331bcf48a406c4d84aac6d5 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Wed, 27 Feb 2019 12:48:55 +0100 Subject: [PATCH 2/7] Update Makefile to only include tests that are not in Python yet --- Makefile | 47 +---------------------------------------------- 1 file changed, 1 insertion(+), 46 deletions(-) diff --git a/Makefile b/Makefile index 44d53d92..f0b2eb5b 100644 --- a/Makefile +++ b/Makefile @@ -41,21 +41,6 @@ else @echo "Valgrind not supported on this platform." endif -bin/sanitizer_$(subst /,_,$(SCHEME)): test/$(dir $(SCHEME))functest.c $(wildcard $(SCHEME)/clean/*.c) $(wildcard $(SCHEME)/clean/*.h) | require_scheme - mkdir -p bin - $(CC) $(CFLAGS) -fsanitize=address,undefined \ - -DPQCLEAN_NAMESPACE=$(shell echo PQCLEAN_$(subst -,,$(notdir $(SCHEME)))_CLEAN | tr a-z A-Z) \ - -iquote "./common/" \ - -iquote "$(SCHEME)/clean/" \ - -o bin/sanitizer_$(subst /,_,$(SCHEME)) \ - $(COMMON_FILES) \ - $(RANDOM_IMPL) \ - $(SCHEME)/clean/*.c \ - $< - -.PHONY: sanitizer -sanitizer: bin/sanitizer_$(subst /,_,$(SCHEME)) - bin/shared_$(subst /,_,$(SCHEME))_clean.so: $(wildcard $(SCHEME)/clean/*.c) | require_scheme mkdir -p bin $(CC) $(CFLAGS) \ @@ -100,10 +85,7 @@ apply-tidy: help: @echo "make test-all Run all tests" @echo "make functest SCHEME=scheme Build functional tests for SCHEME" - @echo "make functest-all Build functional tests for all schemes" @echo "make run-functest SCHEME=scheme Run functional tests for SCHEME" - @echo "make run-functest-all Run all functests" - @echo "make run-sanitizer-all Run address sanitizer for all schemes" @echo "make run-valgrind SCHEME=scheme Run valgrind checks for SCHEME" @echo "make run-valgrind-all Run valgrind checks all schemes" @echo "make clean Clean up the bin/ folder" @@ -114,17 +96,6 @@ help: @echo "make apply-tidy-all Tidy up all schemes" @echo "make help Displays this message" -.PHONY: functest-all -functest-all: - @for scheme in $(ALL_SCHEMES); do \ - $(MAKE) functest SCHEME=$$scheme || exit 1; \ - done - -.PHONY: sanitizer-all -sanitizer-all: - @for scheme in $(ALL_SCHEMES); do \ - $(MAKE) sanitizer SCHEME=$$scheme || exit 1; \ - done .PHONY: run-valgrind-all run-valgrind-all: @@ -132,24 +103,8 @@ run-valgrind-all: $(MAKE) run-valgrind SCHEME=$$scheme || exit 1; \ done -.PHONY: run-functest-all -run-functest-all: functest-all - @for functest in $$(find bin/ -maxdepth 1 -name 'functest_*' -not -type d) ; do \ - echo ./$$functest ; \ - ./$$functest || exit 1 ;\ - done - @echo Tests completed - -.PHONY: run-sanitizer-all -run-sanitizer-all: sanitizer-all - @for sanitizer in $$(find bin/ -maxdepth 1 -name 'sanitizer_*' -not -type d) ; do \ - echo ./$$sanitizer ; \ - ./$$sanitizer || exit 1 ;\ - done - @echo Tests completed - .PHONY: test-all -test-all: run-functest-all run-valgrind-all run-sanitizer-all +test-all: run-valgrind-all .PHONY: tidy-all tidy-all: From 1180de5d306c599e50ffc8599538ebd7bdce6f2c Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Wed, 27 Feb 2019 12:58:00 +0100 Subject: [PATCH 3/7] Add environment to sanitizers on ARM --- test/helpers.py | 14 +++++++++++--- test/test_functest.py | 18 +++++++++++++++--- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/test/helpers.py b/test/helpers.py index 0d8537ee..09a20985 100644 --- a/test/helpers.py +++ b/test/helpers.py @@ -1,11 +1,17 @@ +import os import subprocess -def run_subprocess(command, working_dir='.', expected_returncode=0): +def run_subprocess(command, working_dir='.', env=None, expected_returncode=0): """ Helper function to run a shell command and report success/failure depending on the exit status of the shell command. """ + if env is not None: + env_ = os.environ.copy() + env_.update(env) + env = env_ + # Note we need to capture stdout/stderr from the subprocess, # then print it, which nose/unittest will then capture and # buffer appropriately @@ -13,7 +19,8 @@ def run_subprocess(command, working_dir='.', expected_returncode=0): command, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - cwd=working_dir + cwd=working_dir, + env=env, ) print(working_dir + " > " + " ".join(command)) print(result.stdout.decode('utf-8')) @@ -21,7 +28,7 @@ def run_subprocess(command, working_dir='.', expected_returncode=0): return result.stdout.decode('utf-8') -def make(*args, working_dir='.', **kwargs): +def make(*args, working_dir='.', env=None, **kwargs): """ Runs a make target in the specified working directory @@ -36,4 +43,5 @@ def make(*args, working_dir='.', **kwargs): *['{}={}'.format(k, v) for k, v in kwargs.items()], ], working_dir=working_dir, + env=env, ) diff --git a/test/test_functest.py b/test/test_functest.py index 5cf1df6a..9cffd265 100644 --- a/test/test_functest.py +++ b/test/test_functest.py @@ -40,21 +40,33 @@ def check_functest(scheme_name, implementation_name): def check_functest_sanitizers(scheme_name, implementation_name): - if platform.machine() not in ['i386', 'x86_64']: + env = None + if platform.machine() == 'powerpc': raise unittest.SkipTest() + elif platform.machine() in ['arm32', 'aarch64']: + env = {'ASAN_OPTIONS': 'detect_leaks=0'} + else: + print("Supported platform: {}".format(platform.machine)) implementation = pqclean.Implementation.by_name( scheme_name, implementation_name) helpers.make('clean-scheme', 'functest', TYPE=implementation.scheme.type, SCHEME=scheme_name, IMPLEMENTATION=implementation_name, + EXTRAFLAGS='-fsanitize=address,undefined', working_dir=os.path.join('..', 'test'), - EXTRAFLAGS='-fsanitize=address,undefined') + env=env) helpers.run_subprocess( ['./functest_{}_{}'.format(scheme_name, implementation_name)], os.path.join('..', 'bin'), + env=env, ) - return check_functest(scheme_name, implementation_name) + # Remove files with ASAN library compiled in + helpers.make('clean-scheme', + TYPE=implementation.scheme.type, + SCHEME=scheme_name, + IMPLEMENTATION=implementation_name, + working_dir=os.path.join('..', 'test')) if __name__ == '__main__': From ea47ab3dadee5939411dbe36a94c7ff0db25401a Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Wed, 27 Feb 2019 13:11:36 +0100 Subject: [PATCH 4/7] Fix arm platform names --- test/test_functest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_functest.py b/test/test_functest.py index 9cffd265..16e1cbb8 100644 --- a/test/test_functest.py +++ b/test/test_functest.py @@ -43,7 +43,7 @@ def check_functest_sanitizers(scheme_name, implementation_name): env = None if platform.machine() == 'powerpc': raise unittest.SkipTest() - elif platform.machine() in ['arm32', 'aarch64']: + elif platform.machine() in ['armv7l', 'aarch64']: env = {'ASAN_OPTIONS': 'detect_leaks=0'} else: print("Supported platform: {}".format(platform.machine)) From 53591961c95970d84e7a5af922bfd954418f8065 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Thu, 28 Feb 2019 16:32:24 +0100 Subject: [PATCH 5/7] Fix supported platform debug print --- test/test_functest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_functest.py b/test/test_functest.py index 16e1cbb8..21f66163 100644 --- a/test/test_functest.py +++ b/test/test_functest.py @@ -46,7 +46,7 @@ def check_functest_sanitizers(scheme_name, implementation_name): elif platform.machine() in ['armv7l', 'aarch64']: env = {'ASAN_OPTIONS': 'detect_leaks=0'} else: - print("Supported platform: {}".format(platform.machine)) + print("Supported platform: {}".format(platform.machine())) implementation = pqclean.Implementation.by_name( scheme_name, implementation_name) helpers.make('clean-scheme', 'functest', From dac723564339934556fe12cee03dec18a44af8a4 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Thu, 28 Feb 2019 16:38:19 +0100 Subject: [PATCH 6/7] Only skip ASAN on ppc with Clang --- test/test_functest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_functest.py b/test/test_functest.py index 21f66163..77846618 100644 --- a/test/test_functest.py +++ b/test/test_functest.py @@ -41,7 +41,7 @@ def check_functest(scheme_name, implementation_name): def check_functest_sanitizers(scheme_name, implementation_name): env = None - if platform.machine() == 'powerpc': + if platform.machine() == 'ppc' and os.environ.get('CC', 'gcc') == 'clang': raise unittest.SkipTest() elif platform.machine() in ['armv7l', 'aarch64']: env = {'ASAN_OPTIONS': 'detect_leaks=0'} From 17c9840b57ad2b2324cd05bf231fd3aa11bb4d29 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Thu, 28 Feb 2019 14:05:55 +0100 Subject: [PATCH 7/7] Move valgrind tests to python-based testing framework --- Makefile | 58 ------------------------------------------- test/Makefile | 1 + test/test_valgrind.py | 40 +++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 58 deletions(-) create mode 100644 test/test_valgrind.py diff --git a/Makefile b/Makefile index f0b2eb5b..1abfdec7 100644 --- a/Makefile +++ b/Makefile @@ -1,9 +1,4 @@ -# This -Wall was supported by the European Commission through the ERC Starting Grant 805031 (EPOQUE) -CFLAGS=-Wall -Wextra -Wpedantic -Werror -std=c99 -g $(EXTRAFLAGS) - ALL_SCHEMES=$(filter-out crypto_%.c, $(wildcard crypto_*/*)) -COMMON_FILES = common/fips202.c common/sha2.c -RANDOM_IMPL = common/randombytes.c default: help @@ -14,45 +9,6 @@ ifndef SCHEME $(error The SCHEME variable is not set. Example: SCHEME=crypto_kem/kyber768) endif -bin/functest_$(subst /,_,$(SCHEME)): test/$(dir $(SCHEME))functest.c $(wildcard $(SCHEME)/clean/*.c) $(wildcard $(SCHEME)/clean/*.h) | require_scheme - mkdir -p bin - $(CC) $(CFLAGS) \ - -DPQCLEAN_NAMESPACE=$(shell echo PQCLEAN_$(subst -,,$(notdir $(SCHEME)))_CLEAN | tr a-z A-Z) \ - -iquote "./common/" \ - -iquote "$(SCHEME)/clean/" \ - -o bin/functest_$(subst /,_,$(SCHEME)) \ - $(SCHEME)/clean/*.c \ - $(COMMON_FILES) \ - $(RANDOM_IMPL) \ - $< - -.PHONY: functest -functest: bin/functest_$(subst /,_,$(SCHEME)) - -.PHONY: run-functest -run-functest: bin/functest_$(subst /,_,$(SCHEME)) - ./$< - -.PHONY: run-valgrind -run-valgrind: bin/functest_$(subst /,_,$(SCHEME)) -ifeq ($(shell uname -s),Linux) - valgrind --leak-check=full --error-exitcode=1 $< -else - @echo "Valgrind not supported on this platform." -endif - -bin/shared_$(subst /,_,$(SCHEME))_clean.so: $(wildcard $(SCHEME)/clean/*.c) | require_scheme - mkdir -p bin - $(CC) $(CFLAGS) \ - -DPQCLEAN_NAMESPACE=$(shell echo PQCLEAN_$(subst -,,$(notdir $(SCHEME)))_CLEAN | tr a-z A-Z) \ - -nostdlib \ - -shared \ - -fPIC \ - -iquote "./common/" \ - -iquote "$(SCHEME)/clean/" \ - -o $@ \ - $^ - .PHONY: clean clean: rm -rf bin @@ -83,11 +39,6 @@ apply-tidy: # The below should be outlined with ts=8 .PHONY: help help: - @echo "make test-all Run all tests" - @echo "make functest SCHEME=scheme Build functional tests for SCHEME" - @echo "make run-functest SCHEME=scheme Run functional tests for SCHEME" - @echo "make run-valgrind SCHEME=scheme Run valgrind checks for SCHEME" - @echo "make run-valgrind-all Run valgrind checks all schemes" @echo "make clean Clean up the bin/ folder" @echo "make format Automatically formats all the source code" @echo "make tidy SCHEME=scheme Runs the clang-tidy linter against SCHEME" @@ -97,15 +48,6 @@ help: @echo "make help Displays this message" -.PHONY: run-valgrind-all -run-valgrind-all: - @for scheme in $(ALL_SCHEMES); do \ - $(MAKE) run-valgrind SCHEME=$$scheme || exit 1; \ - done - -.PHONY: test-all -test-all: run-valgrind-all - .PHONY: tidy-all tidy-all: @for scheme in $(ALL_SCHEMES); do \ diff --git a/test/Makefile b/test/Makefile index a048d7a6..5d184dff 100644 --- a/test/Makefile +++ b/test/Makefile @@ -14,6 +14,7 @@ COMMON_FILES=$(COMMON_DIR)/fips202.c $(COMMON_DIR)/sha2.c COMMON_HEADERS=$(COMMON_DIR)/fips202.h $(COMMON_DIR)/randombytes.h $(COMMON_DIR)/sha2.h DEST_DIR=../bin +# This -Wall was supported by the European Commission through the ERC Starting Grant 805031 (EPOQUE) CFLAGS=-Wall -Wextra -Wpedantic -Werror -std=c99 -I$(COMMON_DIR) $(EXTRAFLAGS) all: $(DEST_DIR)/functest_$(SCHEME)_$(IMPLEMENTATION) $(DEST_DIR)/testvectors_$(SCHEME)_$(IMPLEMENTATION) diff --git a/test/test_valgrind.py b/test/test_valgrind.py new file mode 100644 index 00000000..219791cd --- /dev/null +++ b/test/test_valgrind.py @@ -0,0 +1,40 @@ +""" +Runs the test files under valgrind to detect memory problems +""" + +import os +import platform +import unittest + +import pqclean +import helpers + + +def test_functest(): + for scheme in pqclean.Scheme.all_schemes(): + for implementation in scheme.implementations: + yield check_valgrind, implementation + + +def check_valgrind(implementation: pqclean.Implementation): + if (platform.machine() not in ('i386', 'x86_64') or + platform.system() != 'Linux'): + raise unittest.SkipTest() + + helpers.make(TYPE=implementation.scheme.type, + SCHEME=implementation.scheme.name, + IMPLEMENTATION=implementation.name, + working_dir=os.path.join('..', 'test')) + functest_name = './functest_{}_{}'.format(implementation.scheme.name, + implementation.name) + helpers.run_subprocess(['valgrind', functest_name], + os.path.join('..', 'bin')) + + +if __name__ == '__main__': + try: + import nose2 + nose2.main() + except ImportError: + import nose + nose.runmodule()