From 10dc664e8b52b64a2e5917fbec05f21dc8ff63a3 Mon Sep 17 00:00:00 2001 From: Lukas Burgholzer Date: Thu, 8 Sep 2022 02:34:36 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A7=20Update=20CodeQL=20configuration?= =?UTF-8?q?=20(#75)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR updates the GitHub CodeQL configuration in anticipation of the deprecation of LGTM. Most importantly, it adds further checks and a custom build step for C++ to also cover the Python bindings. These additional checks have revealed a couple of easy to fix warnings that have subsequently been fixed. As a pleasant side effect, the Python tests now work irrespective of the directory they are executed in. A corresponding `nox` session `tests` (similar to `lint`, and `docs`) is added to the project. Conveniently run the Python tests via ```bash nox -rs tests ``` Use `nox -rs tests-3.9` to run the tests on Python 3.9 only. Signed-off-by: burgholzer --- .github/codeql-config.yml | 4 + .github/workflows/ci.yml | 2 +- .github/workflows/codeql-analysis.yml | 32 ++++- .lgtm.yml | 22 ---- include/core/syrec/grammar.hpp | 1 - include/core/syrec/module.hpp | 2 +- include/core/syrec/statement.hpp | 2 +- noxfile.py | 9 +- setup.py | 16 ++- src/core/syrec/parser.cpp | 120 +----------------- test/CMakeLists.txt | 4 + .../circuits_simulation.json | 0 .../circuits_simulation_add_lines.json | 0 .../circuits_synthesis.json | 0 .../circuits_synthesis_add_lines.json | 0 test/python/test_syrec.py | 59 +++++---- test/unittests/test_add_lines_simulation.cpp | 3 +- test/unittests/test_add_lines_synthesis.cpp | 3 +- test/unittests/test_simulation.cpp | 3 +- test/unittests/test_synthesis.cpp | 3 +- 20 files changed, 98 insertions(+), 187 deletions(-) create mode 100644 .github/codeql-config.yml delete mode 100644 .lgtm.yml rename test/{circuits => configs}/circuits_simulation.json (100%) rename test/{circuits => configs}/circuits_simulation_add_lines.json (100%) rename test/{circuits => configs}/circuits_synthesis.json (100%) rename test/{circuits => configs}/circuits_synthesis_add_lines.json (100%) diff --git a/.github/codeql-config.yml b/.github/codeql-config.yml new file mode 100644 index 00000000..a9899683 --- /dev/null +++ b/.github/codeql-config.yml @@ -0,0 +1,4 @@ +name: GitHub CodeQL Config + +queries: + - uses: security-and-quality diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bcb1a6b5..ac3031c1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,7 +13,7 @@ concurrency: env: BUILD_TYPE: Release - MAKEFLAGS: "-j2" + MAKEFLAGS: "-j3" BOOST_VERSION_MAJOR: 1 BOOST_VERSION_MINOR: 76 BOOST_VERSION_PATCH: 0 diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index d0add124..9e3b5cf2 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -15,10 +15,8 @@ concurrency: jobs: analyze: name: Analyze - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 permissions: - actions: read - contents: read security-events: write strategy: @@ -31,6 +29,7 @@ jobs: uses: actions/checkout@v3 with: submodules: recursive + fetch-depth: 0 - name: Install boost run: sudo apt-get update && sudo apt-get -y install libboost-all-dev @@ -40,10 +39,31 @@ jobs: uses: github/codeql-action/init@v2 with: languages: ${{ matrix.language }} + config-file: .github/codeql-config.yml - # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). - - name: Autobuild - uses: github/codeql-action/autobuild@v2 + - if: matrix.language == 'cpp' + name: Configure CMake + run: cmake -S . -B build -DBINDINGS=ON + + - if: matrix.language == 'cpp' + name: Build + run: cmake --build build --parallel 3 - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@v2 + with: + upload: False + output: sarif-results + + - name: filter-sarif + uses: advanced-security/filter-sarif@main + with: + patterns: | + -**/extern/** + input: sarif-results/${{ matrix.language }}.sarif + output: sarif-results/${{ matrix.language }}.sarif + + - name: Upload SARIF + uses: github/codeql-action/upload-sarif@v2 + with: + sarif_file: sarif-results/${{ matrix.language }}.sarif diff --git a/.lgtm.yml b/.lgtm.yml deleted file mode 100644 index b0963ee6..00000000 --- a/.lgtm.yml +++ /dev/null @@ -1,22 +0,0 @@ -path_classifiers: - test: - - test - - scripts - library: - - extern/qfr - -extraction: - cpp: - prepare: - packages: - - python-pip - after_prepare: - - export PATH=~/.local/bin:$PATH - - pip3 install cmake --user - - cmake --version - configure: - command: - - cmake -S . -B build -DBUILD_SYREC_TESTS=ON -DBUILD_QFR_TESTS=ON -DBUILD_DD_PACKAGE_TESTS=ON -DBINDINGS=ON - index: - build_command: - - cmake --build build --parallel 4 diff --git a/include/core/syrec/grammar.hpp b/include/core/syrec/grammar.hpp index d2676be7..8f7d8e96 100644 --- a/include/core/syrec/grammar.hpp +++ b/include/core/syrec/grammar.hpp @@ -248,7 +248,6 @@ namespace syrec { statement_rule.name("statement"); expression_rule.name("expression"); binary_expression_rule.name("binary_expression"); - //unary_expression_rule.name("unary_expression"); shift_expression_rule.name("shift_expression"); variable_rule.name("variable"); number_rule.name("number"); diff --git a/include/core/syrec/module.hpp b/include/core/syrec/module.hpp index 85f5fde7..bd3c0318 100644 --- a/include/core/syrec/module.hpp +++ b/include/core/syrec/module.hpp @@ -10,7 +10,7 @@ namespace syrec { - class statement; + struct statement; /** * @brief SyReC module data type diff --git a/include/core/syrec/statement.hpp b/include/core/syrec/statement.hpp index a82f4390..19c485d7 100644 --- a/include/core/syrec/statement.hpp +++ b/include/core/syrec/statement.hpp @@ -11,7 +11,7 @@ namespace syrec { - class module; + struct module; /** * @brief Abstract base class for all SyReC statements diff --git a/noxfile.py b/noxfile.py index 61b6c97b..5c365558 100644 --- a/noxfile.py +++ b/noxfile.py @@ -5,7 +5,7 @@ import nox from nox.sessions import Session -nox.options.sessions = ["lint"] +nox.options.sessions = ["lint", "tests"] PYTHON_ALL_VERSIONS = ["3.7", "3.8", "3.9", "3.10"] @@ -13,6 +13,13 @@ nox.options.error_on_missing_interpreters = True +@nox.session(python=PYTHON_ALL_VERSIONS) +def tests(session: Session) -> None: + """Run the test suite.""" + session.install("-e", ".[test]") + session.run("pytest", *session.posargs) + + @nox.session def lint(session: Session) -> None: """Lint the Python part of the codebase.""" diff --git a/setup.py b/setup.py index da52c928..e4eb7441 100644 --- a/setup.py +++ b/setup.py @@ -2,6 +2,7 @@ import re import subprocess import sys +from pathlib import Path from setuptools import Extension, find_namespace_packages, setup from setuptools.command.build_ext import build_ext @@ -75,13 +76,14 @@ def build_extension(self, ext): if sys.platform == "win32": cmake_args += ["-T", "ClangCl"] - if not os.path.exists(self.build_temp): - os.makedirs(self.build_temp) - else: - try: - os.remove(os.path.join(self.build_temp, "CMakeCache.txt")) - except OSError: - pass + build_dir = Path(self.build_temp) + build_dir.mkdir(parents=True, exist_ok=True) + try: + Path(build_dir / "CMakeCache.txt").unlink() + except FileNotFoundError: + # if the file doesn't exist, it's probably a fresh build + pass + subprocess.check_call(["cmake", ext.sourcedir] + cmake_args, cwd=self.build_temp) subprocess.check_call( ["cmake", "--build", ".", "--target", ext.name.split(".")[-1]] + build_args, diff --git a/src/core/syrec/parser.cpp b/src/core/syrec/parser.cpp index 72f04ebd..190e0b47 100644 --- a/src/core/syrec/parser.cpp +++ b/src/core/syrec/parser.cpp @@ -167,7 +167,7 @@ namespace syrec { return std::make_shared(num_value); } - return std::make_shared(num); //return std::make_shared(lhs, op, rhs); + return std::make_shared(num); } private: @@ -310,113 +310,6 @@ namespace syrec { expression::ptr rhs = parse_expression(ast_exp2, proc, lhs->bitwidth(), context); if (!rhs) return nullptr; - /*if (auto* lhs_exp = dynamic_cast(lhs.get())) { - if (auto* rhs_exp = dynamic_cast(rhs.get())) { - if (lhs_exp->value()->is_constant() && rhs_exp->value()->is_constant()) { - unsigned lhs_value = lhs_exp->value()->evaluate(number::loop_variable_mapping()); - unsigned rhs_value = rhs_exp->value()->evaluate(number::loop_variable_mapping()); - unsigned num_value = 0; - - switch (op) { - case binary_expression::add: // + - { - num_value = lhs_value + rhs_value; - } break; - - case binary_expression::subtract: // - - { - num_value = lhs_value - rhs_value; - } break; - - case binary_expression::exor: // ^ - { - num_value = lhs_value ^ rhs_value; - } break; - - case binary_expression::multiply: // * - { - num_value = lhs_value * rhs_value; - } break; - - case binary_expression::divide: // / - { - num_value = lhs_value / rhs_value; - } break; - - case binary_expression::modulo: // % - { - num_value = lhs_value % rhs_value; - } break; - - case binary_expression::frac_divide: // *> - { - std::cerr << "Operator *> is undefined for numbers w/o specified bit width: ( " + std::to_string(lhs_value) + " *> " + std::to_string(rhs_value) + " )" << std::endl; - assert(false); - return nullptr; - } break; - - case binary_expression::logical_and: // && - { - num_value = lhs_value && rhs_value; - } break; - - case binary_expression::logical_or: // || - { - num_value = lhs_value || rhs_value; - } break; - - case binary_expression::bitwise_and: // & - { - num_value = lhs_value & rhs_value; - } break; - - case binary_expression::bitwise_or: // | - { - num_value = lhs_value | rhs_value; - } break; - - case binary_expression::less_than: // < - { - num_value = lhs_value < rhs_value; - } break; - - case binary_expression::greater_than: // > - { - num_value = lhs_value > rhs_value; - } break; - - case binary_expression::equals: // = - { - num_value = lhs_value == rhs_value; - } break; - - case binary_expression::not_equals: // != - { - num_value = lhs_value != rhs_value; - } break; - - case binary_expression::less_equals: // <= - { - num_value = lhs_value <= rhs_value; - } break; - - case binary_expression::greater_equals: // >= - { - num_value = lhs_value >= rhs_value; - } break; - - default: - std::cerr << "Invalid operator in binary expression" << std::endl; - assert(false); - } - - return new numeric_expression(std::make_shared(num_value), lhs->bitwidth()); - } - } else { - lhs_exp = new numeric_expression(lhs_exp->value(), rhs->bitwidth()); - lhs = expression::ptr(lhs_exp); - } - }*/ return new binary_expression(lhs, op, rhs); } @@ -599,17 +492,6 @@ namespace syrec { for_stat->range = std::make_pair(from, to); // step - /*if (ast_for_stat.step) { - number::ptr step = parse_number(bf::at_c<1>(*ast_for_stat.step), proc, context); - if (!step) return nullptr; - - for_stat->set_step(step); - - if (bf::at_c<0>(*ast_for_stat.step)) { - for_stat->set_negative_step(true); - } - }*/ - for (const ast_statement& ast_stat: ast_for_stat.do_statement) { statement::ptr stat = parse_statement(ast_stat, prog, proc, context); if (!stat) return nullptr; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 73420d3c..dd8ebcee 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -60,5 +60,9 @@ add_custom_command( $/circuits COMMAND ${CMAKE_COMMAND} -E create_symlink $/circuits ${CMAKE_BINARY_DIR}/circuits + COMMAND ${CMAKE_COMMAND} -E copy_directory ${CMAKE_CURRENT_SOURCE_DIR}/configs + $/configs + COMMAND ${CMAKE_COMMAND} -E create_symlink $/configs + ${CMAKE_BINARY_DIR}/configs COMMENT "Copying circuits and creating symlinks for ${PROJECT_NAME}_test" VERBATIM) diff --git a/test/circuits/circuits_simulation.json b/test/configs/circuits_simulation.json similarity index 100% rename from test/circuits/circuits_simulation.json rename to test/configs/circuits_simulation.json diff --git a/test/circuits/circuits_simulation_add_lines.json b/test/configs/circuits_simulation_add_lines.json similarity index 100% rename from test/circuits/circuits_simulation_add_lines.json rename to test/configs/circuits_simulation_add_lines.json diff --git a/test/circuits/circuits_synthesis.json b/test/configs/circuits_synthesis.json similarity index 100% rename from test/circuits/circuits_synthesis.json rename to test/configs/circuits_synthesis.json diff --git a/test/circuits/circuits_synthesis_add_lines.json b/test/configs/circuits_synthesis_add_lines.json similarity index 100% rename from test/circuits/circuits_synthesis_add_lines.json rename to test/configs/circuits_synthesis_add_lines.json diff --git a/test/python/test_syrec.py b/test/python/test_syrec.py index 47b3da95..b2fc1672 100644 --- a/test/python/test_syrec.py +++ b/test/python/test_syrec.py @@ -1,40 +1,51 @@ import json +from pathlib import Path +import pytest from mqt import syrec -f_synthesis_no_lines = open("../circuits/circuits_synthesis.json") -data_synthesis_no_lines = json.load(f_synthesis_no_lines) -f_synthesis_no_lines.close() +test_dir = Path(__file__).resolve().parent.parent +configs_dir = test_dir / "configs" +circuit_dir = test_dir / "circuits" -f_synthesis_add_lines = open("../circuits/circuits_synthesis_add_lines.json") -data_synthesis_add_lines = json.load(f_synthesis_add_lines) -f_synthesis_add_lines.close() -f_simulation_no_lines = open("../circuits/circuits_simulation.json") -data_simulation_no_lines = json.load(f_simulation_no_lines) -f_simulation_no_lines.close() +@pytest.fixture +def data_synthesis_no_lines(): + with open(configs_dir / "circuits_synthesis.json") as f: + return json.load(f) -f_simulation_add_lines = open("../circuits/circuits_simulation_add_lines.json") -data_simulation_add_lines = json.load(f_simulation_add_lines) -f_simulation_add_lines.close() -test_circuit_dir = "../circuits/" -string_src = ".src" +@pytest.fixture +def data_synthesis_add_lines(): + with open(configs_dir / "circuits_synthesis_add_lines.json") as f: + return json.load(f) -def test_parser(): +@pytest.fixture +def data_simulation_no_lines(): + with open(configs_dir / "circuits_simulation.json") as f: + return json.load(f) + + +@pytest.fixture +def data_simulation_add_lines(): + with open(configs_dir / "circuits_simulation_add_lines.json") as f: + return json.load(f) + + +def test_parser(data_synthesis_no_lines): for file_name in data_synthesis_no_lines: prog = syrec.program() - error = prog.read(test_circuit_dir + file_name + string_src) + error = prog.read(str(circuit_dir / (file_name + ".src"))) assert error == "" -def test_synthesis_no_lines(): +def test_synthesis_no_lines(data_synthesis_no_lines): for file_name in data_synthesis_no_lines: circ = syrec.circuit() prog = syrec.program() - error = prog.read(test_circuit_dir + file_name + string_src) + error = prog.read(str(circuit_dir / (file_name + ".src"))) assert error == "" assert syrec.syrec_synthesis_no_additional_lines(circ, prog) @@ -44,11 +55,11 @@ def test_synthesis_no_lines(): assert data_synthesis_no_lines[file_name]["transistor_costs"] == circ.transistor_cost() -def test_synthesis_add_lines(): +def test_synthesis_add_lines(data_synthesis_add_lines): for file_name in data_synthesis_add_lines: circ = syrec.circuit() prog = syrec.program() - error = prog.read(test_circuit_dir + file_name + string_src) + error = prog.read(str(circuit_dir / (file_name + ".src"))) assert error == "" assert syrec.syrec_synthesis_additional_lines(circ, prog) @@ -58,11 +69,11 @@ def test_synthesis_add_lines(): assert data_synthesis_add_lines[file_name]["transistor_costs"] == circ.transistor_cost() -def test_simulation_no_lines(): +def test_simulation_no_lines(data_simulation_no_lines): for file_name in data_simulation_no_lines: circ = syrec.circuit() prog = syrec.program() - error = prog.read(test_circuit_dir + file_name + string_src) + error = prog.read(str(circuit_dir / (file_name + ".src"))) assert error == "" assert syrec.syrec_synthesis_no_additional_lines(circ, prog) @@ -78,11 +89,11 @@ def test_simulation_no_lines(): assert data_simulation_no_lines[file_name]["sim_out"] == str(my_out_bitset) -def test_simulation_add_lines(): +def test_simulation_add_lines(data_simulation_add_lines): for file_name in data_simulation_add_lines: circ = syrec.circuit() prog = syrec.program() - error = prog.read(test_circuit_dir + file_name + string_src) + error = prog.read(str(circuit_dir / (file_name + ".src"))) assert error == "" assert syrec.syrec_synthesis_additional_lines(circ, prog) diff --git a/test/unittests/test_add_lines_simulation.cpp b/test/unittests/test_add_lines_simulation.cpp index 17eda341..bd7ddda8 100644 --- a/test/unittests/test_add_lines_simulation.cpp +++ b/test/unittests/test_add_lines_simulation.cpp @@ -17,6 +17,7 @@ using namespace syrec; class SyrecAddLinesSimulationTest: public testing::TestWithParam { protected: + std::string test_configs_dir = "./configs/"; std::string test_circuits_dir = "./circuits/"; std::string file_name; boost::dynamic_bitset<> input; @@ -28,7 +29,7 @@ class SyrecAddLinesSimulationTest: public testing::TestWithParam { void SetUp() override { std::string synthesis_param = GetParam(); file_name = test_circuits_dir + GetParam() + ".src"; - std::ifstream i(test_circuits_dir + "circuits_simulation_add_lines.json"); + std::ifstream i(test_configs_dir + "circuits_simulation_add_lines.json"); json j = json::parse(i); expected_sim_out = j[synthesis_param]["sim_out"]; set_lines = j[synthesis_param]["set_lines"].get>(); diff --git a/test/unittests/test_add_lines_synthesis.cpp b/test/unittests/test_add_lines_synthesis.cpp index 98bcf6f1..caa2313f 100644 --- a/test/unittests/test_add_lines_synthesis.cpp +++ b/test/unittests/test_add_lines_synthesis.cpp @@ -14,6 +14,7 @@ using namespace syrec; class SyrecAddLinesSynthesisTest: public testing::TestWithParam { protected: + std::string test_configs_dir = "./configs/"; std::string test_circuits_dir = "./circuits/"; std::string file_name; gate::cost_t qc = 0; @@ -26,7 +27,7 @@ class SyrecAddLinesSynthesisTest: public testing::TestWithParam { void SetUp() override { std::string synthesis_param = GetParam(); file_name = test_circuits_dir + GetParam() + ".src"; - std::ifstream i(test_circuits_dir + "circuits_synthesis_add_lines.json"); + std::ifstream i(test_configs_dir + "circuits_synthesis_add_lines.json"); json j = json::parse(i); expected_num_gates = j[synthesis_param]["num_gates"]; expected_lines = j[synthesis_param]["lines"]; diff --git a/test/unittests/test_simulation.cpp b/test/unittests/test_simulation.cpp index f0244332..5b9cf718 100644 --- a/test/unittests/test_simulation.cpp +++ b/test/unittests/test_simulation.cpp @@ -17,6 +17,7 @@ using namespace syrec; class SyrecSimulationTest: public testing::TestWithParam { protected: + std::string test_configs_dir = "./configs/"; std::string test_circuits_dir = "./circuits/"; std::string file_name; boost::dynamic_bitset<> input; @@ -28,7 +29,7 @@ class SyrecSimulationTest: public testing::TestWithParam { void SetUp() override { std::string synthesis_param = GetParam(); file_name = test_circuits_dir + GetParam() + ".src"; - std::ifstream i(test_circuits_dir + "circuits_simulation.json"); + std::ifstream i(test_configs_dir + "circuits_simulation.json"); json j = json::parse(i); expected_sim_out = j[synthesis_param]["sim_out"]; set_lines = j[synthesis_param]["set_lines"].get>(); diff --git a/test/unittests/test_synthesis.cpp b/test/unittests/test_synthesis.cpp index c5bf75d4..2e2fa6e3 100644 --- a/test/unittests/test_synthesis.cpp +++ b/test/unittests/test_synthesis.cpp @@ -14,6 +14,7 @@ using namespace syrec; class SyrecSynthesisTest: public testing::TestWithParam { protected: + std::string test_configs_dir = "./configs/"; std::string test_circuits_dir = "./circuits/"; std::string file_name; gate::cost_t qc = 0; @@ -26,7 +27,7 @@ class SyrecSynthesisTest: public testing::TestWithParam { void SetUp() override { std::string synthesis_param = GetParam(); file_name = test_circuits_dir + GetParam() + ".src"; - std::ifstream i(test_circuits_dir + "circuits_synthesis.json"); + std::ifstream i(test_configs_dir + "circuits_synthesis.json"); json j = json::parse(i); expected_num_gates = j[synthesis_param]["num_gates"]; expected_lines = j[synthesis_param]["lines"];