🔧 Update CodeQL configuration (#75)

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 <burgholzer@me.com>
cda-tum · Sep 8, 2022 · 10dc664 · 10dc664
1 parent 58ee277
commit 10dc664
Show file tree

Hide file tree

Showing 20 changed files with 98 additions and 187 deletions.
diff --git a/.github/codeql-config.yml 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
@@ -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
@@ -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
diff --git 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
@@ -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
@@ -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
@@ -5,14 +5,21 @@
 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"]
 
 if os.environ.get("CI", None):
     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
@@ -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
@@ -167,7 +167,7 @@ namespace syrec {
 
                 return std::make_shared<number>(num_value);
             }
-            return std::make_shared<number>(num); //return std::make_shared<number>(lhs, op, rhs);
+            return std::make_shared<number>(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<numeric_expression*>(lhs.get())) {
-                if (auto* rhs_exp = dynamic_cast<numeric_expression*>(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<number>(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
@@ -60,5 +60,9 @@ add_custom_command(
           $<TARGET_FILE_DIR:${PROJECT_NAME}_test>/circuits
   COMMAND ${CMAKE_COMMAND} -E create_symlink $<TARGET_FILE_DIR:${PROJECT_NAME}_test>/circuits
           ${CMAKE_BINARY_DIR}/circuits
+  COMMAND ${CMAKE_COMMAND} -E copy_directory ${CMAKE_CURRENT_SOURCE_DIR}/configs
+          $<TARGET_FILE_DIR:${PROJECT_NAME}_test>/configs
+  COMMAND ${CMAKE_COMMAND} -E create_symlink $<TARGET_FILE_DIR:${PROJECT_NAME}_test>/configs
+          ${CMAKE_BINARY_DIR}/configs
   COMMENT "Copying circuits and creating symlinks for ${PROJECT_NAME}_test"
   VERBATIM)
diff --git a/test/circuits/circuits_simulation.json → test/configs/circuits_simulation.json b/test/circuits/circuits_simulation.json → test/configs/circuits_simulation.json
diff --git a/...rcuits/circuits_simulation_add_lines.json → ...onfigs/circuits_simulation_add_lines.json b/...rcuits/circuits_simulation_add_lines.json → ...onfigs/circuits_simulation_add_lines.json
diff --git a/test/circuits/circuits_synthesis.json → test/configs/circuits_synthesis.json b/test/circuits/circuits_synthesis.json → test/configs/circuits_synthesis.json
diff --git a/...ircuits/circuits_synthesis_add_lines.json → ...configs/circuits_synthesis_add_lines.json b/...ircuits/circuits_synthesis_add_lines.json → ...configs/circuits_synthesis_add_lines.json