From 17fc0da52e48979ea36d39cbe84476975e259a85 Mon Sep 17 00:00:00 2001 From: Joost Rijneveld Date: Thu, 4 Apr 2019 13:42:03 +0200 Subject: [PATCH 01/13] Prohibit using char without explicit sign modifier Related to #79 --- .gitmodules | 3 +++ requirements.txt | 1 + test/pycparser | 1 + test/test_char.py | 62 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+) create mode 100644 .gitmodules create mode 160000 test/pycparser create mode 100644 test/test_char.py diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 00000000..b547037a --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "test/pycparser"] + path = test/pycparser + url = https://github.com/eliben/pycparser.git diff --git a/requirements.txt b/requirements.txt index 01b892fa..b3f4ea5d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,4 @@ PyYAML nose rednose +pycparser diff --git a/test/pycparser b/test/pycparser new file mode 160000 index 00000000..e1a1d737 --- /dev/null +++ b/test/pycparser @@ -0,0 +1 @@ +Subproject commit e1a1d737be66308b633215fa26ac5ed30e890103 diff --git a/test/test_char.py b/test/test_char.py new file mode 100644 index 00000000..32ad99a0 --- /dev/null +++ b/test/test_char.py @@ -0,0 +1,62 @@ + +""" +Checks that the implementation does not make use of the `char` type. +This is ambiguous; compilers can freely choose `signed` or `unsigned` char. +""" + +import pqclean +import pycparser +import os + + +def test_char(): + for scheme in pqclean.Scheme.all_schemes(): + for implementation in scheme.implementations: + yield check_char, implementation + + +def walk_tree(ast): + if type(ast) is pycparser.c_ast.IdentifierType: + if ast.names == ['char']: + yield ast + + for (_, child) in ast.children(): + yield from walk_tree(child) # recursively yield prohibited nodes + + +def check_char(implementation): + errors = [] + for fname in os.listdir(implementation.path()): + if not fname.endswith(".c"): + continue + tdir, _ = os.path.split(os.path.realpath(__file__)) + ast = pycparser.parse_file( + os.path.join(implementation.path(), fname), + use_cpp=True, + cpp_args=[ + '-nostdinc', # pycparser cannot deal with e.g. __attribute__ + '-I{}'.format(os.path.join(tdir, "../common")), + # necessary to mock e.g. + '-I{}'.format( + os.path.join(tdir, 'pycparser/utils/fake_libc_include')), + ] + ) + for node in walk_tree(ast): + # flatten nodes to a string to easily enforce uniqueness + err = "\n at {c.file}:{c.line}:{c.column}".format(c=node.coord) + if err not in errors: + errors.append(err) + if errors: + raise AssertionError( + "Prohibited use of char without explicit signed/unsigned" + + "".join(errors) + ) + + +if __name__ == '__main__': + try: + import nose2 + nose2.main() + except ImportError: + import nose + nose.runmodule() From 97e428a0b7e925b6ea408ce8fe57f16e5659326e Mon Sep 17 00:00:00 2001 From: Joost Rijneveld Date: Thu, 4 Apr 2019 13:52:40 +0200 Subject: [PATCH 02/13] Skip preprocessing when cpp unavailable --- test/test_char.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/test_char.py b/test/test_char.py index 32ad99a0..703e1b17 100644 --- a/test/test_char.py +++ b/test/test_char.py @@ -7,6 +7,8 @@ This is ambiguous; compilers can freely choose `signed` or `unsigned` char. import pqclean import pycparser import os +import unittest +import shutil def test_char(): @@ -25,6 +27,8 @@ def walk_tree(ast): def check_char(implementation): + if not shutil.which('cpp'): + raise unittest.SkipTest("C pre-processor (cpp) was not found.") errors = [] for fname in os.listdir(implementation.path()): if not fname.endswith(".c"): @@ -34,6 +38,7 @@ def check_char(implementation): os.path.join(implementation.path(), fname), use_cpp=True, cpp_args=[ + '-std=c99', '-nostdinc', # pycparser cannot deal with e.g. __attribute__ '-I{}'.format(os.path.join(tdir, "../common")), # necessary to mock e.g. From 8067df4aa98433f8d29cc9d7d9949a059e7ef9d6 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Fri, 5 Apr 2019 09:20:50 +0200 Subject: [PATCH 03/13] install pycparser with pip3 --- .circleci/config.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 17f13e49..09a6e8d2 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -24,6 +24,7 @@ version: 2 name: Run tests command: | export CC=${CC} + pip3 install -r requirements.txt cd test && python3 -m nose --rednose --verbose From 9a6787c87250ca9b3db5712cf557909d58ad6e70 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Fri, 5 Apr 2019 09:28:22 +0200 Subject: [PATCH 04/13] Properly set up GCC on OS X --- .travis.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 66a4893b..1de9ed98 100644 --- a/.travis.yml +++ b/.travis.yml @@ -20,9 +20,12 @@ matrix: homebrew: packages: - astyle - - gcc@8 + - gcc before_install: - pip3 install -r requirements.txt + - export PATH="/usr/local/bin:$PATH" + - ln -s /usr/local/Cellar/gcc/8.3.0/bin/gcc-8 /usr/local/bin/gcc + - ln -s /usr/local/Cellar/gcc/8.3.0/bin/cpp-8 /usr/local/bin/cpp - gcc --version script: - "cd test && python3 -m nose --rednose --verbose" From bfa8589f968eec5add0b9f8286cf3872773781d8 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Fri, 5 Apr 2019 10:31:31 +0200 Subject: [PATCH 05/13] Use cc -E instead of cpp --- test/test_char.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/test_char.py b/test/test_char.py index 703e1b17..29a0b857 100644 --- a/test/test_char.py +++ b/test/test_char.py @@ -37,7 +37,9 @@ def check_char(implementation): ast = pycparser.parse_file( os.path.join(implementation.path(), fname), use_cpp=True, + cpp_path='cc', cpp_args=[ + '-E', '-std=c99', '-nostdinc', # pycparser cannot deal with e.g. __attribute__ '-I{}'.format(os.path.join(tdir, "../common")), From 4e47a0b513eff75cea9b884289f9fcdfb6a5b086 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Fri, 5 Apr 2019 10:39:08 +0200 Subject: [PATCH 06/13] Skip the prohibit-char test on Windows due to lack of cc --- test/helpers.py | 1 - test/test_char.py | 6 ++---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/test/helpers.py b/test/helpers.py index 238e234c..e6c3adc1 100644 --- a/test/helpers.py +++ b/test/helpers.py @@ -86,7 +86,6 @@ def ensure_available(executable): """ path = shutil.which(executable) if path: - print("Found", path) return path # Installing clang-tidy on LLVM will be too much of a mess. diff --git a/test/test_char.py b/test/test_char.py index 29a0b857..8f823241 100644 --- a/test/test_char.py +++ b/test/test_char.py @@ -7,8 +7,7 @@ This is ambiguous; compilers can freely choose `signed` or `unsigned` char. import pqclean import pycparser import os -import unittest -import shutil +import helpers def test_char(): @@ -26,9 +25,8 @@ def walk_tree(ast): yield from walk_tree(child) # recursively yield prohibited nodes +@helpers.skip_windows() def check_char(implementation): - if not shutil.which('cpp'): - raise unittest.SkipTest("C pre-processor (cpp) was not found.") errors = [] for fname in os.listdir(implementation.path()): if not fname.endswith(".c"): From dac0d96904f088d8c256a63d3434d534d13ab801 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Fri, 5 Apr 2019 10:40:16 +0200 Subject: [PATCH 07/13] We don't use cpp anymore --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 1de9ed98..1f54de06 100644 --- a/.travis.yml +++ b/.travis.yml @@ -25,7 +25,6 @@ matrix: - pip3 install -r requirements.txt - export PATH="/usr/local/bin:$PATH" - ln -s /usr/local/Cellar/gcc/8.3.0/bin/gcc-8 /usr/local/bin/gcc - - ln -s /usr/local/Cellar/gcc/8.3.0/bin/cpp-8 /usr/local/bin/cpp - gcc --version script: - "cd test && python3 -m nose --rednose --verbose" From 347217ba137929f51127ea8e427f11d18c02e81b Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Fri, 5 Apr 2019 10:42:14 +0200 Subject: [PATCH 08/13] Mention submodules in README --- README.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 6936d629..c055e9f4 100644 --- a/README.md +++ b/README.md @@ -139,10 +139,16 @@ To do this, make sure the following is installed: * Python 3.5+ * `nosetests` or `nose2` (either for Python 3) +You will also need to make sure the submodules are initialized by running: + +``` +git submodule update --init +``` + Run the Python-based tests by going into the `test` directory and running `nosetests -v` or `nose2 -B -v`, depending on what you installed. If you have the `rednose` plugin for `nosetests` installed, run `nosetests --rednose` to get colored output. -You may also run `python ` where `` is any of the files starting with `test_` in the `test/` folder. +You may also run `python3 ` where `` is any of the files starting with `test_` in the `test/` folder. [circleci-pqc]: https://circleci.com/gh/PQClean/PQClean/ [travis-pqc]: https://travis-ci.com/PQClean/PQClean/ From 13e84aec00d8edf7f962eafafc2932898bd73457 Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Fri, 5 Apr 2019 10:45:06 +0200 Subject: [PATCH 09/13] Clone submodules in CircleCI --- .circleci/config.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 09a6e8d2..e51c2ab3 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -4,6 +4,11 @@ version: 2 machine: true steps: - checkout + - run: + name: Pull submodules + command: | + git submodule init + git submodule update - run: name: Install the emulation handlers command: docker run --rm --privileged multiarch/qemu-user-static:register --reset @@ -20,6 +25,11 @@ version: 2 - image: pqclean/ci-container:$ARCH steps: - checkout + - run: + name: Pull submodules + command: | + git submodule init + git submodule update - run: name: Run tests command: | From 2ffdc863c932a095aa3b5e7af0677ddb0b3498ed Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Fri, 5 Apr 2019 11:51:19 +0200 Subject: [PATCH 10/13] Also install requirements in native versions --- .circleci/config.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index e51c2ab3..c3dc74ec 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -18,6 +18,7 @@ version: 2 docker run -e CI=true --rm -v `pwd`:`pwd` -w `pwd` "pqclean/ci-container:$ARCH" /bin/bash -c " uname -a && export CC=${CC} && + pip3 install -r requirements.txt cd test && python3 -m nose --rednose --verbose" .native_job: &nativejob From 3d8faae4836011c5ae513f16de5dda83cb7ebdf0 Mon Sep 17 00:00:00 2001 From: Joost Rijneveld Date: Fri, 5 Apr 2019 12:51:35 +0200 Subject: [PATCH 11/13] Fix missing && separator in docker command string It seems to have worked in CircleCI without this, though; it is unclear to me why. --- .circleci/config.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index c3dc74ec..7db6185c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -18,7 +18,7 @@ version: 2 docker run -e CI=true --rm -v `pwd`:`pwd` -w `pwd` "pqclean/ci-container:$ARCH" /bin/bash -c " uname -a && export CC=${CC} && - pip3 install -r requirements.txt + pip3 install -r requirements.txt && cd test && python3 -m nose --rednose --verbose" .native_job: &nativejob @@ -35,7 +35,7 @@ version: 2 name: Run tests command: | export CC=${CC} - pip3 install -r requirements.txt + pip3 install -r requirements.txt && cd test && python3 -m nose --rednose --verbose From 41edb79c0a234094d477065228f782c36979b226 Mon Sep 17 00:00:00 2001 From: Joost Rijneveld Date: Fri, 5 Apr 2019 13:04:32 +0200 Subject: [PATCH 12/13] Clarify cc vs cpp --- test/test_char.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_char.py b/test/test_char.py index 8f823241..85bb47ac 100644 --- a/test/test_char.py +++ b/test/test_char.py @@ -35,7 +35,7 @@ def check_char(implementation): ast = pycparser.parse_file( os.path.join(implementation.path(), fname), use_cpp=True, - cpp_path='cc', + cpp_path='cc', # not all platforms link cpp correctly; cc -E works cpp_args=[ '-E', '-std=c99', From eb08730d277f61e0344c35c9e450fb86fb54503a Mon Sep 17 00:00:00 2001 From: Thom Wiggers Date: Fri, 5 Apr 2019 13:38:02 +0200 Subject: [PATCH 13/13] Use brew link to install gcc in a more predictable place --- .travis.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1f54de06..6d7d3780 100644 --- a/.travis.yml +++ b/.travis.yml @@ -20,11 +20,12 @@ matrix: homebrew: packages: - astyle - - gcc + - gcc@8 before_install: - pip3 install -r requirements.txt + - brew link gcc - export PATH="/usr/local/bin:$PATH" - - ln -s /usr/local/Cellar/gcc/8.3.0/bin/gcc-8 /usr/local/bin/gcc + - ln -s /usr/local/bin/gcc-8 /usr/local/bin/gcc - gcc --version script: - "cd test && python3 -m nose --rednose --verbose"