From e1b6c7fbcb21ef929aea795a1904decceb4296f0 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Tue, 6 Aug 2024 08:26:18 +0200 Subject: [PATCH 01/33] Run tests on Python 3.12 as well in CI --- .gitlab-ci.yml | 11 +++++++++++ test/Dockerfile.python-3.12 | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 test/Dockerfile.python-3.12 diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f391b3c49..00dcf3611 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -81,6 +81,11 @@ unit-tests:python-3.11: variables: PYTHON_VERSION: '3.11' +unit-tests:python-3.12: + <<: *unit-tests + variables: + PYTHON_VERSION: '3.12' + # Static checks check-format: stage: test @@ -228,3 +233,9 @@ build-docker:test:python-3.11: variables: DOCKERFILE: test/Dockerfile.python-3.11 IMAGE: test:python-3.11 + +build-docker:test:python-3.12: + extends: .build-docker + variables: + DOCKERFILE: test/Dockerfile.python-3.12 + IMAGE: test:python-3.12 diff --git a/test/Dockerfile.python-3.12 b/test/Dockerfile.python-3.12 new file mode 100644 index 000000000..3f2dfa721 --- /dev/null +++ b/test/Dockerfile.python-3.12 @@ -0,0 +1,33 @@ +# This file is part of BenchExec, a framework for reliable benchmarking: +# https://github.com/sosy-lab/benchexec +# +# SPDX-FileCopyrightText: 2007-2024 Dirk Beyer +# +# SPDX-License-Identifier: Apache-2.0 + +# This is a Docker image for running the tests. +# It should be pushed to registry.gitlab.com/sosy-lab/software/benchexec/test +# and will be used by CI as declared in .gitlab-ci.yml. +# +# Commands for updating the image: +# docker build --pull -t registry.gitlab.com/sosy-lab/software/benchexec/test:python-3.12 - < test/Dockerfile.python-3.12 +# docker push registry.gitlab.com/sosy-lab/software/benchexec/test + +FROM python:3.12 + +# Cannot use apt package python3-pystemd here +# because these images do not use the Python installed via apt. + +RUN apt-get update && apt-get install -y \ + libsystemd-dev \ + lxcfs \ + sudo \ + && rm -rf /var/lib/apt/lists/* + +RUN pip install \ + coloredlogs \ + "coverage[toml] >= 5.0" \ + lxml \ + pystemd \ + pytest \ + pyyaml From 913a36016aec006731f59dd74ca2d9cc20c06778 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Tue, 6 Aug 2024 08:27:43 +0200 Subject: [PATCH 02/33] Update CI on AppVeyor: use oldest and latest supported Python We do not want to test all Python versions on AppVeyor because it takes too much time, but testing oldest and latest is at least better than testing two old versions. For this we need to change the base image. --- .appveyor.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.appveyor.yml b/.appveyor.yml index 242702aba..370c1e3c6 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -1,21 +1,21 @@ # This file is part of BenchExec, a framework for reliable benchmarking: # https://github.com/sosy-lab/benchexec # -# SPDX-FileCopyrightText: 2007-2020 Dirk Beyer +# SPDX-FileCopyrightText: 2007-2024 Dirk Beyer # # SPDX-License-Identifier: Apache-2.0 +image: "Visual Studio 2022" + environment: matrix: - PYTHON: "C:\\Python38" - - PYTHON: "C:\\Python39" + - PYTHON: "C:\\Python312" build: off install: - set PATH=%PYTHON%;%PYTHON%\\Scripts;%PATH% - # The package installation via our setup.py file uses easy_install, which fails to correctly install lxml. - # As some Python environments don't have lxml preinstalled, install it here to avoid errors during the execution in those cases. - python -m pip install --user ".[dev]" test_script: From 98f031a716497792a2fc946b2522695edc8b3a97 Mon Sep 17 00:00:00 2001 From: Po-Chun Chien Date: Fri, 9 Aug 2024 10:20:40 +0200 Subject: [PATCH 03/33] Add tool-info module for super_prove --- benchexec/tools/super_prove.py | 57 ++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 benchexec/tools/super_prove.py diff --git a/benchexec/tools/super_prove.py b/benchexec/tools/super_prove.py new file mode 100644 index 000000000..ea6be6a37 --- /dev/null +++ b/benchexec/tools/super_prove.py @@ -0,0 +1,57 @@ +# This file is part of BenchExec, a framework for reliable benchmarking: +# https://github.com/sosy-lab/benchexec +# +# SPDX-FileCopyrightText: 2007-2020 Dirk Beyer +# +# SPDX-License-Identifier: Apache-2.0 + +import benchexec.result as result +import benchexec.tools.template +from pathlib import Path + + +class Tool(benchexec.tools.template.BaseTool2): + """ + Tool info for super_prove: A portfolio model checker based on ABC + - project URL: https://github.com/berkeley-abc/super_prove + - build repository: https://github.com/sterin/super-prove-build + - tool archive: https://gitlab.com/sosy-lab/benchmarking/coveriteam-archives/hardware-verifiers/-/blob/main/super_prove.zip + """ + + REQUIRED_PATHS = ["bin/", "lib/"] + + def executable(self, tool_locator): + return tool_locator.find_executable("super_prove.sh", subdir="bin") + + def name(self): + return "super_prove" + + def project_url(self): + return "https://github.com/berkeley-abc/super_prove" + + def version(self, executable): + version_file = Path(executable).parent.parent / "VERSION.txt" + if not version_file.is_file(): + return "" + with version_file.open() as f: + version = f.readline().strip() + return version + + def program_files(self, executable): + return self._program_files_from_executable( + executable, self.REQUIRED_PATHS, parent_dir=True + ) + + def cmdline(self, executable, options, task, rlimits): + return [executable, *options, task.single_input_file] + + def determine_result(self, run): + """ + @return: status of super_prove after executing a run + """ + if len(run.output) > 0: + if run.output[0] == "0": + return result.RESULT_TRUE_PROP + elif run.output[0] == "1": + return result.RESULT_FALSE_PROP + return result.RESULT_UNKNOWN From 62269e873d90f31faefedb4cc00965b3e23a9890 Mon Sep 17 00:00:00 2001 From: Po-Chun Chien Date: Fri, 9 Aug 2024 16:54:32 +0200 Subject: [PATCH 04/33] Simplify code Co-authored-by: Philipp Wendler <2545335+PhilippWendler@users.noreply.github.com> --- benchexec/tools/super_prove.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchexec/tools/super_prove.py b/benchexec/tools/super_prove.py index ea6be6a37..72ff9a014 100644 --- a/benchexec/tools/super_prove.py +++ b/benchexec/tools/super_prove.py @@ -49,7 +49,7 @@ def determine_result(self, run): """ @return: status of super_prove after executing a run """ - if len(run.output) > 0: + if run.output: if run.output[0] == "0": return result.RESULT_TRUE_PROP elif run.output[0] == "1": From 9aca1f6a7c2d4fd9f67779d20740ff3eefe6e480 Mon Sep 17 00:00:00 2001 From: Po-Chun Chien Date: Fri, 9 Aug 2024 16:52:27 +0200 Subject: [PATCH 05/33] Remove a redundant archive URL for super_prove --- benchexec/tools/super_prove.py | 1 - 1 file changed, 1 deletion(-) diff --git a/benchexec/tools/super_prove.py b/benchexec/tools/super_prove.py index 72ff9a014..ea192f3d5 100644 --- a/benchexec/tools/super_prove.py +++ b/benchexec/tools/super_prove.py @@ -15,7 +15,6 @@ class Tool(benchexec.tools.template.BaseTool2): Tool info for super_prove: A portfolio model checker based on ABC - project URL: https://github.com/berkeley-abc/super_prove - build repository: https://github.com/sterin/super-prove-build - - tool archive: https://gitlab.com/sosy-lab/benchmarking/coveriteam-archives/hardware-verifiers/-/blob/main/super_prove.zip """ REQUIRED_PATHS = ["bin/", "lib/"] From a0bff5e155dc10e460a0e4e9845a1831d26906dd Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Thu, 8 Aug 2024 09:28:06 +0200 Subject: [PATCH 06/33] Refactoring: rename variable --- benchexec/containerexecutor.py | 8 ++++---- benchexec/runexecutor.py | 16 +++++++++------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/benchexec/containerexecutor.py b/benchexec/containerexecutor.py index 1954729a7..a783ed864 100644 --- a/benchexec/containerexecutor.py +++ b/benchexec/containerexecutor.py @@ -457,13 +457,13 @@ def execute_run( cgroups = self._cgroups.create_fresh_child_cgroup( self._cgroups.subsystems.keys() ) - pid = None + tool_pid = None returnvalue = 0 logging.debug("Starting process.") try: - pid, result_fn = self._start_execution( + tool_pid, result_fn = self._start_execution( args=args, stdin=None, stdout=None, @@ -481,7 +481,7 @@ def execute_run( ) with self.SUB_PROCESS_PIDS_LOCK: - self.SUB_PROCESS_PIDS.add(pid) + self.SUB_PROCESS_PIDS.add(tool_pid) # wait until process has terminated returnvalue, unused_ru_child, unused = result_fn() @@ -491,7 +491,7 @@ def execute_run( logging.debug("Process terminated, exit code %s.", returnvalue) with self.SUB_PROCESS_PIDS_LOCK: - self.SUB_PROCESS_PIDS.discard(pid) + self.SUB_PROCESS_PIDS.discard(tool_pid) if temp_dir is not None: logging.debug("Cleaning up temporary directory.") diff --git a/benchexec/runexecutor.py b/benchexec/runexecutor.py index c3cf77d3d..aba059078 100644 --- a/benchexec/runexecutor.py +++ b/benchexec/runexecutor.py @@ -882,7 +882,7 @@ def preSubprocess(): error_filename, args, write_header=write_header ) - pid = None + tool_pid = None returnvalue = 0 ru_child = None self._termination_reason = None @@ -894,7 +894,7 @@ def preSubprocess(): logging.debug("Starting process.") try: - pid, result_fn = self._start_execution( + tool_pid, result_fn = self._start_execution( args=args, stdin=stdin, stdout=outputFile, @@ -912,14 +912,16 @@ def preSubprocess(): ) with self.SUB_PROCESS_PIDS_LOCK: - self.SUB_PROCESS_PIDS.add(pid) + self.SUB_PROCESS_PIDS.add(tool_pid) timelimitThread = self._setup_cgroup_time_limit( - hardtimelimit, softtimelimit, walltimelimit, cgroups, cores, pid + hardtimelimit, softtimelimit, walltimelimit, cgroups, cores, tool_pid + ) + oomThread = self._setup_cgroup_memory_limit_thread( + memlimit, cgroups, tool_pid ) - oomThread = self._setup_cgroup_memory_limit_thread(memlimit, cgroups, pid) file_hierarchy_limit_thread = self._setup_file_hierarchy_limit( - files_count_limit, files_size_limit, temp_dir, cgroups, pid + files_count_limit, files_size_limit, temp_dir, cgroups, tool_pid ) # wait until process has terminated @@ -932,7 +934,7 @@ def preSubprocess(): logging.debug("Process terminated, exit code %s.", returnvalue) with self.SUB_PROCESS_PIDS_LOCK: - self.SUB_PROCESS_PIDS.discard(pid) + self.SUB_PROCESS_PIDS.discard(tool_pid) if timelimitThread: timelimitThread.cancel() From bcf45157f88efb1da9c10cb1970aec4077f74108 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Thu, 8 Aug 2024 09:30:48 +0200 Subject: [PATCH 07/33] Refactoring: allow to pass back a cgroups instance from executor Currently RunExecutor creates a cgroup instance and uses that. But sometimes the underlying executor needs to create a nested cgroup and put the tool into that. Now we pass it back to RunExecutor such that it can make use of it for the measurements. The effect is that we can use different cgroups for limits and for measurements (and killing processes). But in this commit we still pass only the original cgroups instance back, so there is no behavior change. --- benchexec/baseexecutor.py | 9 +++++---- benchexec/containerexecutor.py | 16 +++++++++++----- benchexec/runexecutor.py | 28 +++++++++++++++++++--------- 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/benchexec/baseexecutor.py b/benchexec/baseexecutor.py index 64fc1cdca..d62ff24c0 100644 --- a/benchexec/baseexecutor.py +++ b/benchexec/baseexecutor.py @@ -80,9 +80,10 @@ def _start_execution( @param child_setup_fn a function without parameters that is called in the child process before the tool is started @param parent_cleanup_fn a function that is called in the parent process - immediately after the tool terminated, with three parameters: + immediately after the tool terminated, with four parameters: the result of parent_setup_fn, the result of the executed process as ProcessExitCode, - and the base path for looking up files as parameter values + the base path for looking up files as parameter values, + and the cgroup of the tool @return: a tuple of PID of process and a blocking function, which waits for the process and a triple of the exit code and the resource usage of the process and the result of parent_cleanup_fn (do not use os.wait) @@ -125,11 +126,11 @@ def wait_and_get_result(): exitcode, ru_child = self._wait_for_process(p.pid, args[0]) parent_cleanup = parent_cleanup_fn( - parent_setup, util.ProcessExitCode.from_raw(exitcode), "" + parent_setup, util.ProcessExitCode.from_raw(exitcode), "", cgroups ) return exitcode, ru_child, parent_cleanup - return p.pid, wait_and_get_result + return p.pid, cgroups, wait_and_get_result def _wait_for_process(self, pid, name): """Wait for the given process to terminate. diff --git a/benchexec/containerexecutor.py b/benchexec/containerexecutor.py index a783ed864..9b8151c9c 100644 --- a/benchexec/containerexecutor.py +++ b/benchexec/containerexecutor.py @@ -458,12 +458,13 @@ def execute_run( self._cgroups.subsystems.keys() ) tool_pid = None + tool_cgroups = None returnvalue = 0 logging.debug("Starting process.") try: - tool_pid, result_fn = self._start_execution( + tool_pid, tool_cgroups, result_fn = self._start_execution( args=args, stdin=None, stdout=None, @@ -974,10 +975,12 @@ def check_child_exit_code(): # of the cgroup ns, such that the limit settings are not accessible in the # container and cannot be changed. if use_cgroup_ns: - cgroups = cgroups.create_fresh_child_cgroup_for_delegation() + grandchild_cgroups = cgroups.create_fresh_child_cgroup_for_delegation() + else: + grandchild_cgroups = cgroups # start measurements - cgroups.add_task(grandchild_pid) + grandchild_cgroups.add_task(grandchild_pid) parent_setup = parent_setup_fn() # Signal grandchild that setup is finished @@ -1015,7 +1018,10 @@ def wait_for_grandchild(): base_path = f"/proc/{child_pid}/root" parent_cleanup = parent_cleanup_fn( - parent_setup, util.ProcessExitCode.from_raw(exitcode), base_path + parent_setup, + util.ProcessExitCode.from_raw(exitcode), + base_path, + cgroups, ) if result_files_patterns: @@ -1032,7 +1038,7 @@ def wait_for_grandchild(): return exitcode, ru_child, parent_cleanup - return grandchild_pid, wait_for_grandchild + return grandchild_pid, cgroups, wait_for_grandchild def _setup_container_filesystem(self, temp_dir, output_dir, memlimit, memory_nodes): """Setup the filesystem layout in the container. diff --git a/benchexec/runexecutor.py b/benchexec/runexecutor.py index aba059078..882e21569 100644 --- a/benchexec/runexecutor.py +++ b/benchexec/runexecutor.py @@ -831,7 +831,7 @@ def preParent(): walltime_before = time.monotonic() return starttime, walltime_before - def postParent(preParent_result, exit_code, base_path): + def postParent(preParent_result, exit_code, base_path, tool_cgroups): """Cleanup that is executed in the parent process immediately after the actual tool terminated.""" # finish measurements starttime, walltime_before = preParent_result @@ -846,8 +846,8 @@ def postParent(preParent_result, exit_code, base_path): # process existed, and killing via cgroups prevents this. # But if we do not have freezer, it is safer to just let all processes run # until the container is killed. - if cgroups.FREEZE in cgroups: - cgroups.kill_all_tasks() + if tool_cgroups.FREEZE in tool_cgroups: + tool_cgroups.kill_all_tasks() # For a similar reason, we cancel all limits. Otherwise a run could have # terminationreason=walltime because copying output files took a long time. @@ -883,6 +883,7 @@ def preSubprocess(): ) tool_pid = None + tool_cgroups = None returnvalue = 0 ru_child = None self._termination_reason = None @@ -894,7 +895,7 @@ def preSubprocess(): logging.debug("Starting process.") try: - tool_pid, result_fn = self._start_execution( + tool_pid, tool_cgroups, result_fn = self._start_execution( args=args, stdin=stdin, stdout=outputFile, @@ -910,18 +911,24 @@ def preSubprocess(): parent_cleanup_fn=postParent, **kwargs, ) + assert cgroups == tool_cgroups # temporarily with self.SUB_PROCESS_PIDS_LOCK: self.SUB_PROCESS_PIDS.add(tool_pid) timelimitThread = self._setup_cgroup_time_limit( - hardtimelimit, softtimelimit, walltimelimit, cgroups, cores, tool_pid + hardtimelimit, + softtimelimit, + walltimelimit, + tool_cgroups, + cores, + tool_pid, ) oomThread = self._setup_cgroup_memory_limit_thread( - memlimit, cgroups, tool_pid + memlimit, tool_cgroups, tool_pid ) file_hierarchy_limit_thread = self._setup_file_hierarchy_limit( - files_count_limit, files_size_limit, temp_dir, cgroups, tool_pid + files_count_limit, files_size_limit, temp_dir, tool_cgroups, tool_pid ) # wait until process has terminated @@ -947,7 +954,8 @@ def preSubprocess(): # Make sure to kill all processes if there are still some # (needs to come early to avoid accumulating more CPU time) - cgroups.kill_all_tasks() + if tool_cgroups: + tool_cgroups.kill_all_tasks() # normally subprocess closes file, we do this again after all tasks terminated outputFile.close() @@ -955,8 +963,10 @@ def preSubprocess(): errorFile.close() # measurements are not relevant in case of failure, but need to come before cgroup cleanup - self._get_cgroup_measurements(cgroups, ru_child, result) + if tool_cgroups: + self._get_cgroup_measurements(tool_cgroups, ru_child, result) logging.debug("Cleaning up cgroups.") + cgroups.kill_all_tasks() # currently necessary for removing child cgroups cgroups.remove() self._cleanup_temp_dir(temp_dir) From 22b8787610cac6ca90621167ecf1b575a3570ba8 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Thu, 8 Aug 2024 11:47:02 +0200 Subject: [PATCH 08/33] Move the benchmarked process one cgroup deeper in the cgroups v1 case We already do that for cgroups v2, such that the cgroup of the benchmarked tool is a child cgroup of the cgroup where we configure the limits. For cgroups v1 it is not necessary to do this so far, but it might be good for consistency and it is required for better integration of LXCFS. --- benchexec/cgroups.py | 4 ++++ benchexec/cgroupsv1.py | 16 ++++++++++++++-- benchexec/containerexecutor.py | 6 ++---- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/benchexec/cgroups.py b/benchexec/cgroups.py index d27e483d3..971fdf9ed 100644 --- a/benchexec/cgroups.py +++ b/benchexec/cgroups.py @@ -170,6 +170,10 @@ def handle_errors(self, critical_cgroups): def create_fresh_child_cgroup(self, subsystems): pass + @abstractmethod + def create_fresh_child_cgroup_for_delegation(self): + pass + @abstractmethod def add_task(self, pid): pass diff --git a/benchexec/cgroupsv1.py b/benchexec/cgroupsv1.py index 505e95c72..e77bbacd0 100644 --- a/benchexec/cgroupsv1.py +++ b/benchexec/cgroupsv1.py @@ -357,7 +357,7 @@ def get_group_name(gid): else: sys.exit(_ERROR_MSG_OTHER) # e.g., subsystem not mounted - def create_fresh_child_cgroup(self, subsystems): + def create_fresh_child_cgroup(self, subsystems, prefix=CGROUP_NAME_PREFIX): """ Create child cgroups of the current cgroup for at least the given subsystems. @return: A Cgroup instance representing the new child cgroup(s). @@ -374,7 +374,7 @@ def create_fresh_child_cgroup(self, subsystems): ] continue - cgroup = tempfile.mkdtemp(prefix=CGROUP_NAME_PREFIX, dir=parentCgroup) + cgroup = tempfile.mkdtemp(prefix=prefix, dir=parentCgroup) createdCgroupsPerSubsystem[subsystem] = cgroup createdCgroupsPerParent[parentCgroup] = cgroup @@ -395,6 +395,18 @@ def copy_parent_to_child(name): return CgroupsV1(createdCgroupsPerSubsystem) + def create_fresh_child_cgroup_for_delegation(self, prefix="delegate_"): + """ + Create a child cgroup with all controllers. + On cgroupsv1 there is no difference to a regular child cgroup. + """ + child_cgroup = self.create_fresh_child_cgroup(self.subsystems.keys(), prefix) + assert ( + self.subsystems.keys() == child_cgroup.subsystems.keys() + ), "delegation failed for at least one controller" + + return child_cgroup + def add_task(self, pid): """ Add a process to the cgroups represented by this instance. diff --git a/benchexec/containerexecutor.py b/benchexec/containerexecutor.py index 9b8151c9c..e90fec88b 100644 --- a/benchexec/containerexecutor.py +++ b/benchexec/containerexecutor.py @@ -974,10 +974,8 @@ def check_child_exit_code(): # So for isolation, we need to create a child cgroup that becomes the root # of the cgroup ns, such that the limit settings are not accessible in the # container and cannot be changed. - if use_cgroup_ns: - grandchild_cgroups = cgroups.create_fresh_child_cgroup_for_delegation() - else: - grandchild_cgroups = cgroups + # We also do this for cgroups v1 for consistency. + grandchild_cgroups = cgroups.create_fresh_child_cgroup_for_delegation() # start measurements grandchild_cgroups.add_task(grandchild_pid) From cd2c6a8acf622ab43f0931487590eaacdad71dee Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Thu, 8 Aug 2024 09:34:51 +0200 Subject: [PATCH 09/33] Pass back actual tool cgroup from ContainerExecutor to RunExecutor Building on the last commit, we now change the behavior and pass back the cgroup where the actual tool is in, instead of the parent cgroup (at least on cgroups v2, no change for cgroups v1). This should still not result in any visible changes, because measurements and killing processes should be the same for the parent cgroup and the tool cgroup - there is nothing else in the parent cgroup. --- benchexec/containerexecutor.py | 4 ++-- benchexec/runexecutor.py | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/benchexec/containerexecutor.py b/benchexec/containerexecutor.py index e90fec88b..bac723598 100644 --- a/benchexec/containerexecutor.py +++ b/benchexec/containerexecutor.py @@ -1019,7 +1019,7 @@ def wait_for_grandchild(): parent_setup, util.ProcessExitCode.from_raw(exitcode), base_path, - cgroups, + grandchild_cgroups, ) if result_files_patterns: @@ -1036,7 +1036,7 @@ def wait_for_grandchild(): return exitcode, ru_child, parent_cleanup - return grandchild_pid, cgroups, wait_for_grandchild + return grandchild_pid, grandchild_cgroups, wait_for_grandchild def _setup_container_filesystem(self, temp_dir, output_dir, memlimit, memory_nodes): """Setup the filesystem layout in the container. diff --git a/benchexec/runexecutor.py b/benchexec/runexecutor.py index 882e21569..e1107378c 100644 --- a/benchexec/runexecutor.py +++ b/benchexec/runexecutor.py @@ -911,7 +911,6 @@ def preSubprocess(): parent_cleanup_fn=postParent, **kwargs, ) - assert cgroups == tool_cgroups # temporarily with self.SUB_PROCESS_PIDS_LOCK: self.SUB_PROCESS_PIDS.add(tool_pid) From 1049c395db17b5fc9ff8f2f4e45ebc71eba4ddbe Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Thu, 8 Aug 2024 09:39:04 +0200 Subject: [PATCH 10/33] Improve integration of LXCFS and virtualize /proc/cpuinfo We recommend to install LXCFS together with BenchExec, because we use that to virtualize for example /proc/uptime in the container. However, a main use case of LXCFS is to virtualize files that contain information about the system such as the available CPU cores in /proc/cpuinfo. We never advertised this, but I assumed this was working all the time. I found out that it never worked, though. The reason is that LXCFS is using the limits configured for the init process of the container, but our init process has no limits, it is not part of the same cgroup as the other processes in the container (on purpose, because we do not want to measure its resource consumption). So now we create yet another cgroup for the init process that is below the one with the limits but outside of the one that is used for measurements. Note: A single runexec execution will now create up to 5 cgroups. This is made possible due to the separation between the cgroups for limits and for measurements in the last commits. With this change, /proc/cpuinfo now shows only the cores available in the container if LXCFS is running. This helps processes in the container to see how many CPU cores they are allowed to use and for example to decide how many threads to spawn. Fixes #1070 --- benchexec/containerexecutor.py | 23 ++++++++++++++++------- benchexec/test_runexecutor.py | 12 ++++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/benchexec/containerexecutor.py b/benchexec/containerexecutor.py index bac723598..53eaa443a 100644 --- a/benchexec/containerexecutor.py +++ b/benchexec/containerexecutor.py @@ -388,8 +388,8 @@ def is_accessible(path): # container, but we do not want a warning per run. if not is_accessible(container.LXCFS_PROC_DIR): logging.info( - "LXCFS is not available," - " some host information like the uptime leaks into the container." + "LXCFS is not available, some host information like the uptime" + " and the total number of CPU cores leaks into the container." ) if not NATIVE_CLONE_CALLBACK_SUPPORTED: @@ -970,11 +970,20 @@ def check_child_exit_code(): child_pid, ) - # cgroups is the cgroups where we configure limits. - # So for isolation, we need to create a child cgroup that becomes the root - # of the cgroup ns, such that the limit settings are not accessible in the - # container and cannot be changed. - # We also do this for cgroups v1 for consistency. + # cgroups is the cgroup where we configure limits. + # We add another layer of cgroups below it, for two reasons: + # - We want to move our child process (the init process of the container) + # into a cgroups where the limits apply because LXCFS reports the limits + # of the init process of the container. + # - On cgroupsv2 we want to move the grandchild process (the started tool) + # into a cgroup that becomes the root of the cgroup ns, + # such that no other cgroup (in particular the one with the limits) + # is accessible in the container and the limits cannot be changed + # from within the container. + child_cgroup = cgroups.create_fresh_child_cgroup( + cgroups.subsystems.keys(), prefix="init_" + ) + child_cgroup.add_task(child_pid) grandchild_cgroups = cgroups.create_fresh_child_cgroup_for_delegation() # start measurements diff --git a/benchexec/test_runexecutor.py b/benchexec/test_runexecutor.py index 75cf1886a..d41d539c6 100644 --- a/benchexec/test_runexecutor.py +++ b/benchexec/test_runexecutor.py @@ -43,6 +43,7 @@ def setUpClass(cls): cls.echo = shutil.which("echo") or "/bin/echo" cls.sleep = shutil.which("sleep") or "/bin/sleep" cls.cat = shutil.which("cat") or "/bin/cat" + cls.grep = shutil.which("grep") or "/bin/grep" def setUp(self, *args, **kwargs): with self.skip_if_logs( @@ -1151,6 +1152,17 @@ def test_path_with_space(self): finally: shutil.rmtree(temp_dir) + def test_cpuinfo_with_lxcfs(self): + if not os.path.exists("/var/lib/lxcfs/proc"): + self.skipTest("missing lxcfs") + result, output = self.execute_run( + self.grep, "^processor", "/proc/cpuinfo", cores=[0] + ) + self.check_result_keys(result) + self.check_exitcode(result, 0, "exit code for reading cpuinfo is not zero") + cpus = [int(line.split()[2]) for line in output if line.startswith("processor")] + self.assertListEqual(cpus, [0], "Unexpected CPU cores visible in container") + def test_uptime_with_lxcfs(self): if not os.path.exists("/var/lib/lxcfs/proc"): self.skipTest("missing lxcfs") From 58d4e88cd2da3729a9e051337d306621e1697e14 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Thu, 8 Aug 2024 12:34:07 +0200 Subject: [PATCH 11/33] Exclude large directory from pytest scan There are no Python tests in this directory anyway, but searching through it can take a surprisingly large time. --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 6be7e209c..ff9f14937 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -73,4 +73,4 @@ ignore = [ [tool.pytest.ini_options] python_files = ["test_*.py", "test_integration/__init__.py", "test.py"] -norecursedirs = ["contrib/p4/docker_files", "build"] +norecursedirs = ["contrib/p4/docker_files", "build", "benchexec/tablegenerator/react-table"] From 365bc283c8b989ba311761efed0ab318aa01e915 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Thu, 8 Aug 2024 13:06:51 +0200 Subject: [PATCH 12/33] Also virtualize /sys/devices/system/cpu with LXCFS if available Like for /proc, LXCFS provides a virtualized /sys/devices/system/cpu that only shows the allowed cores. Of course we want to mount that in the container as well, at least if the user has not requested /sys to be hidden or have full access to the host directory. Fixes #1069 --- benchexec/container.py | 13 ++++++++++++- benchexec/test_runexecutor.py | 11 +++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/benchexec/container.py b/benchexec/container.py index 75ecdb507..ba6dcfd95 100644 --- a/benchexec/container.py +++ b/benchexec/container.py @@ -107,7 +107,9 @@ DIR_MODES = [DIR_HIDDEN, DIR_READ_ONLY, DIR_OVERLAY, DIR_FULL_ACCESS] """modes how a directory can be mounted in the container""" -LXCFS_PROC_DIR = b"/var/lib/lxcfs/proc" +LXCFS_BASE_DIR = b"/var/lib/lxcfs" +LXCFS_PROC_DIR = LXCFS_BASE_DIR + b"/proc" +SYS_CPU_DIR = b"/sys/devices/system/cpu" _CLONE_NESTED_CALLBACK = ctypes.CFUNCTYPE(ctypes.c_int) """Type for callback of execute_in_namespace, nested in our primary callback.""" @@ -968,6 +970,15 @@ def setup_container_system_config(basedir, mountdir, dir_modes): {"h": CONTAINER_HOME, "p": os.path.dirname(CONTAINER_HOME)}, ) + # Virtualize CPU info with LXCFS if directory is not hidden nor full-access + if ( + os.access(LXCFS_BASE_DIR + SYS_CPU_DIR, os.R_OK) + and determine_directory_mode(dir_modes, SYS_CPU_DIR) == DIR_READ_ONLY + ): + make_bind_mount( + LXCFS_BASE_DIR + SYS_CPU_DIR, mountdir + SYS_CPU_DIR, private=True + ) + def is_container_system_config_file(file): """Determine whether a given file is one of the files created by diff --git a/benchexec/test_runexecutor.py b/benchexec/test_runexecutor.py index d41d539c6..ca84c1b95 100644 --- a/benchexec/test_runexecutor.py +++ b/benchexec/test_runexecutor.py @@ -1163,6 +1163,17 @@ def test_cpuinfo_with_lxcfs(self): cpus = [int(line.split()[2]) for line in output if line.startswith("processor")] self.assertListEqual(cpus, [0], "Unexpected CPU cores visible in container") + def test_sys_cpu_with_lxcfs(self): + if not os.path.exists("/var/lib/lxcfs/proc"): + self.skipTest("missing lxcfs") + result, output = self.execute_run( + self.cat, "/sys/devices/system/cpu/online", cores=[0] + ) + self.check_result_keys(result) + self.check_exitcode(result, 0, "exit code for reading online CPUs is not zero") + cpus = util.parse_int_list(output[-1]) + self.assertListEqual(cpus, [0], "Unexpected CPU cores online in container") + def test_uptime_with_lxcfs(self): if not os.path.exists("/var/lib/lxcfs/proc"): self.skipTest("missing lxcfs") From ab235f3b924a0855af0664c4d62ad9e555b794fb Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Fri, 16 Aug 2024 15:22:22 +0200 Subject: [PATCH 13/33] Fix relative link in documentation. --- doc/INSTALL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/INSTALL.md b/doc/INSTALL.md index 522f1592a..fe1f1788d 100644 --- a/doc/INSTALL.md +++ b/doc/INSTALL.md @@ -119,7 +119,7 @@ and install [cpu-energy-meter], [libseccomp2], [LXCFS], and [pqos_wrapper] if de ### Containerized Environments -Please refer to the [dedicated guide](doc/benchexec-in-container.md) for the +Please refer to the [dedicated guide](benchexec-in-container.md) for the necessary steps to install BenchExec inside a container. **IMPORTANT**: In any case, cgroups with all relevant controllers need to be From ea9abbcb8bafd4d54bf196746fca2b5c6196f470 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Mon, 26 Aug 2024 08:26:15 +0200 Subject: [PATCH 14/33] Add missing parameter for cgroups in command for podman run This was already documented in other examples, but missing in one example. --- doc/benchexec-in-container.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/benchexec-in-container.md b/doc/benchexec-in-container.md index 59401a48d..05449a7df 100644 --- a/doc/benchexec-in-container.md +++ b/doc/benchexec-in-container.md @@ -121,7 +121,7 @@ Dockerfile is located to build the container. Start the image with ``` -podman run --cgroups=split \ +podman run --security-opt unmask=/sys/fs/cgroup --cgroups=split \ --security-opt unmask=/proc/* \ --security-opt seccomp=unconfined \ -it From d36cd838916e9f95ffcb8c2020b250aba0b69cf2 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Tue, 20 Aug 2024 14:15:22 +0200 Subject: [PATCH 15/33] Add some testing help to our documentation Also include a script for preparing cgroups in testing containers. Based on our benchexec-in-container.md documentation. --- doc/DEVELOPMENT.md | 14 +++++++++++++ test/setup_cgroupsv2_in_container.sh | 31 ++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100755 test/setup_cgroupsv2_in_container.sh diff --git a/doc/DEVELOPMENT.md b/doc/DEVELOPMENT.md index 35d27ee0f..e1d46a543 100644 --- a/doc/DEVELOPMENT.md +++ b/doc/DEVELOPMENT.md @@ -58,6 +58,20 @@ To run the test suite of BenchExec, use the following command: python3 -m pytest +Specific tests can be selected with `-k TEST_NAME_SUBSTRING` +and debug logs can be captured with `--log-level DEBUG`. + +A container with the necessary environment and permissions +for executing the tests can be started with: + + podman run --rm -it \ + --security-opt unmask=/sys/fs/cgroup --cgroups=split \ + --security-opt unmask=/proc/* --security-opt seccomp=unconfined \ + -v /var/lib/lxcfs:/var/lib/lxcfs:ro --device /dev/fuse \ + -v $(pwd):$(pwd):rw -w $(pwd) \ + registry.gitlab.com/sosy-lab/software/benchexec/test:python-3.12 \ + test/setup_cgroupsv2_in_container.sh bash + We also check our code using the static-analysis tools [flake8](http://flake8.pycqa.org) and [ruff](https://github.com/astral-sh/ruff/). If you find a rule that should not be enforced in your opinion, diff --git a/test/setup_cgroupsv2_in_container.sh b/test/setup_cgroupsv2_in_container.sh new file mode 100755 index 000000000..1dc0bc389 --- /dev/null +++ b/test/setup_cgroupsv2_in_container.sh @@ -0,0 +1,31 @@ +#!/bin/sh + +# This file is part of BenchExec, a framework for reliable benchmarking: +# https://github.com/sosy-lab/benchexec +# +# SPDX-FileCopyrightText: 2007-2024 Dirk Beyer +# +# SPDX-License-Identifier: Apache-2.0 + +# This script can be run inside a container to prepare cgroups for BenchExec. +# If parameters are passed, it will execute them afterwards. + +set -eu + +# Create new sub-cgroups +# Note: While "init" can be renamed, the name "benchexec" is important +mkdir -p /sys/fs/cgroup/init /sys/fs/cgroup/benchexec +# Move all processes to that cgroup +while read pid; do + echo $pid > /sys/fs/cgroup/init/cgroup.procs +done < /sys/fs/cgroup/cgroup.procs + +# Enable controllers in subtrees for benchexec to use +for controller in $(cat /sys/fs/cgroup/cgroup.controllers); do + echo "+$controller" > /sys/fs/cgroup/cgroup.subtree_control + echo "+$controller" > /sys/fs/cgroup/benchexec/cgroup.subtree_control +done + +if [ ! -z "${1:-}" ]; then + exec "$@" +fi From 01985e36904392716bfbdff472ab049957c9e983 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Mon, 26 Aug 2024 09:29:41 +0200 Subject: [PATCH 16/33] Rename jobs for tests in GitLab CI We want to mention cgroupsv1 in the name to prepare for tests with cgroupsv2. Also, these are all our tests, we do not have separated unit and integration tests. --- .gitlab-ci.yml | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 00dcf3611..22913ec4f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -33,7 +33,7 @@ stages: - images - test -.unit-tests: &unit-tests +.tests:cgroupsv1: &tests-cgroupsv1 stage: test before_script: # Create user, we do not want to test as root @@ -61,28 +61,28 @@ stages: paths: - coverage.xml -unit-tests:python-3.8: - <<: *unit-tests +tests:cgroups1:python-3.8: + <<: *tests-cgroupsv1 variables: PYTHON_VERSION: '3.8' -unit-tests:python-3.9: - <<: *unit-tests +tests:cgroupsv1:python-3.9: + <<: *tests-cgroupsv1 variables: PYTHON_VERSION: '3.9' -unit-tests:python-3.10: - <<: *unit-tests +tests:cgroupsv1:python-3.10: + <<: *tests-cgroupsv1 variables: PYTHON_VERSION: '3.10' -unit-tests:python-3.11: - <<: *unit-tests +tests:cgroupsv1:python-3.11: + <<: *tests-cgroupsv1 variables: PYTHON_VERSION: '3.11' -unit-tests:python-3.12: - <<: *unit-tests +tests:cgroupsv1:python-3.12: + <<: *tests-cgroupsv1 variables: PYTHON_VERSION: '3.12' From 8282ed1044456177ec1183416b05cf1c5747f38e Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Mon, 26 Aug 2024 09:33:39 +0200 Subject: [PATCH 17/33] Add tests on GitLab CI for runner with cgroupsv2 --- .gitlab-ci.yml | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 22913ec4f..ae181be5a 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -86,6 +86,50 @@ tests:cgroupsv1:python-3.12: variables: PYTHON_VERSION: '3.12' +.tests:cgroupsv2: &tests-cgroupsv2 + stage: test + before_script: + # Activate coverage for subprocesses + - printf 'import coverage\ncoverage.process_startup()\n' > "/usr/local/lib/python${PYTHON_VERSION}/site-packages/sitecustomize.py" + # Install BenchExec with `dev` dependencies + - pip install --user ".[dev]" + script: + - COVERAGE_PROCESS_START=.coveragerc coverage run -m pytest + after_script: + - coverage combine + - coverage report + - coverage xml + tags: + - cgroupsv2 + artifacts: + paths: + - coverage.xml + +tests:cgroupsv2:python-3.8: + <<: *tests-cgroupsv2 + variables: + PYTHON_VERSION: '3.8' + +tests:cgroupsv2:python-3.9: + <<: *tests-cgroupsv2 + variables: + PYTHON_VERSION: '3.9' + +tests:cgroupsv2:python-3.10: + <<: *tests-cgroupsv2 + variables: + PYTHON_VERSION: '3.10' + +tests:cgroupsv2:python-3.11: + <<: *tests-cgroupsv2 + variables: + PYTHON_VERSION: '3.11' + +tests:cgroupsv2:python-3.12: + <<: *tests-cgroupsv2 + variables: + PYTHON_VERSION: '3.12' + # Static checks check-format: stage: test From 6079895fe1dd8c088b9c9280842d789d1db5f1a0 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Mon, 26 Aug 2024 09:51:27 +0200 Subject: [PATCH 18/33] Add forgotten command for cgroup preparation in GitLab CI --- .gitlab-ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ae181be5a..7dc392362 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -91,6 +91,8 @@ tests:cgroupsv1:python-3.12: before_script: # Activate coverage for subprocesses - printf 'import coverage\ncoverage.process_startup()\n' > "/usr/local/lib/python${PYTHON_VERSION}/site-packages/sitecustomize.py" + # Prepare cgroups + - test/setup_cgroupsv2_in_container.sh # Install BenchExec with `dev` dependencies - pip install --user ".[dev]" script: From b983c89ce12222af5c0cda4c54cdba2770f1b466 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Mon, 26 Aug 2024 10:00:26 +0200 Subject: [PATCH 19/33] Show skipped tests and their reason in CI Otherwise we do not see if tests are unexpectedly omitted due to an incomplete setup of the CI environment. --- .gitlab-ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 7dc392362..55008d5f3 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -50,7 +50,7 @@ stages: script: - sudo -u $PRIMARY_USER COVERAGE_PROCESS_START=.coveragerc - coverage run -m pytest + coverage run -m pytest -ra after_script: - sudo -u $PRIMARY_USER coverage combine - sudo -u $PRIMARY_USER coverage report @@ -96,7 +96,7 @@ tests:cgroupsv1:python-3.12: # Install BenchExec with `dev` dependencies - pip install --user ".[dev]" script: - - COVERAGE_PROCESS_START=.coveragerc coverage run -m pytest + - COVERAGE_PROCESS_START=.coveragerc coverage run -m pytest -ra after_script: - coverage combine - coverage report From 91d43a0d4bdb6b8f0d63895af2ec055ed1e43ac2 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Mon, 26 Aug 2024 10:21:53 +0200 Subject: [PATCH 20/33] Cleanup old reference to .coveragerc left over from 528a3ad0 We need to pass the value of the config file for coverage, and we moved from .coveragerc to pyproject.toml in that commit, but forgot to update the CI job. It continued to work because coverage hard-codes the value ".coveragerc" and uses the standard search for config files instead of requiring this specific file, but using the name of the actual config file is still cleaner and less confusing. --- .gitlab-ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 55008d5f3..1dc638071 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -49,7 +49,7 @@ stages: - lxcfs /var/lib/lxcfs & script: - sudo -u $PRIMARY_USER - COVERAGE_PROCESS_START=.coveragerc + COVERAGE_PROCESS_START=pyproject.toml coverage run -m pytest -ra after_script: - sudo -u $PRIMARY_USER coverage combine @@ -96,7 +96,7 @@ tests:cgroupsv1:python-3.12: # Install BenchExec with `dev` dependencies - pip install --user ".[dev]" script: - - COVERAGE_PROCESS_START=.coveragerc coverage run -m pytest -ra + - COVERAGE_PROCESS_START=pyproject.toml coverage run -m pytest -ra after_script: - coverage combine - coverage report From 0257f52171dea0a2163591a40bbe3ae318c2ae23 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Mon, 26 Aug 2024 10:24:21 +0200 Subject: [PATCH 21/33] Make coverage combine quiet It prints out a line for every coverage file it processes, and there are hundreds of them. This clutters our CI log. --- .gitlab-ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 1dc638071..41bdf4a9a 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -52,7 +52,7 @@ stages: COVERAGE_PROCESS_START=pyproject.toml coverage run -m pytest -ra after_script: - - sudo -u $PRIMARY_USER coverage combine + - sudo -u $PRIMARY_USER coverage combine -q - sudo -u $PRIMARY_USER coverage report - sudo -u $PRIMARY_USER coverage xml tags: @@ -98,7 +98,7 @@ tests:cgroupsv1:python-3.12: script: - COVERAGE_PROCESS_START=pyproject.toml coverage run -m pytest -ra after_script: - - coverage combine + - coverage combine -q - coverage report - coverage xml tags: From 82ee9a35e5a1e340974111d4192f3f171c2b549a Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Mon, 26 Aug 2024 10:49:10 +0200 Subject: [PATCH 22/33] Update ruff config They moved a few config options. --- pyproject.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index ff9f14937..acd0770cb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -57,7 +57,7 @@ exclude = [ '**/test_*/**.py', ] -[tool.ruff] +[tool.ruff.lint] # TODO: Enable more checks. #select = ["ALL"] ignore = [ @@ -65,7 +65,7 @@ ignore = [ 'E501', 'I001', ] -[tool.ruff.per-file-ignores] +[tool.ruff.lint.per-file-ignores] 'benchexec/test*.py' = [ # wildcard imports significantly shorten test code, 'F405', From 3c412b0b0f1d2d3133abfbe809c05786bcfb31f2 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Mon, 26 Aug 2024 11:14:29 +0200 Subject: [PATCH 23/33] Use pytest-cov for handling coverage in tests This pytest plugin does a lot that we otherwise would have to do manually. --- .gitlab-ci.yml | 18 ++++++------------ test/Dockerfile.python-3.10 | 1 + test/Dockerfile.python-3.11 | 1 + test/Dockerfile.python-3.12 | 1 + test/Dockerfile.python-3.8 | 1 + test/Dockerfile.python-3.9 | 1 + 6 files changed, 11 insertions(+), 12 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 41bdf4a9a..bd70a6a4b 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -38,22 +38,18 @@ stages: before_script: # Create user, we do not want to test as root - adduser --disabled-login --gecos "" $PRIMARY_USER - # Activate coverage for subprocesses - - printf 'import coverage\ncoverage.process_startup()\n' > "/usr/local/lib/python${PYTHON_VERSION}/site-packages/sitecustomize.py" # Give $PRIMARY_USER permission to create cgroups - test/for_each_of_my_cgroups.sh chgrp $PRIMARY_USER - test/for_each_of_my_cgroups.sh chmod g+w $PRIMARY_USER + # Install pytest-cov (does not work with --user) + - pip install pytest-cov # Install BenchExec with `dev` dependencies - sudo -u $PRIMARY_USER pip install --user ".[dev]" # Start lxcfs - lxcfs /var/lib/lxcfs & script: - - sudo -u $PRIMARY_USER - COVERAGE_PROCESS_START=pyproject.toml - coverage run -m pytest -ra + - sudo -u $PRIMARY_USER python -m pytest -ra --cov after_script: - - sudo -u $PRIMARY_USER coverage combine -q - - sudo -u $PRIMARY_USER coverage report - sudo -u $PRIMARY_USER coverage xml tags: - privileged @@ -89,17 +85,15 @@ tests:cgroupsv1:python-3.12: .tests:cgroupsv2: &tests-cgroupsv2 stage: test before_script: - # Activate coverage for subprocesses - - printf 'import coverage\ncoverage.process_startup()\n' > "/usr/local/lib/python${PYTHON_VERSION}/site-packages/sitecustomize.py" # Prepare cgroups - test/setup_cgroupsv2_in_container.sh + # Install pytest-cov (does not work with --user) + - pip install pytest-cov # Install BenchExec with `dev` dependencies - pip install --user ".[dev]" script: - - COVERAGE_PROCESS_START=pyproject.toml coverage run -m pytest -ra + - python -m pytest -ra --cov after_script: - - coverage combine -q - - coverage report - coverage xml tags: - cgroupsv2 diff --git a/test/Dockerfile.python-3.10 b/test/Dockerfile.python-3.10 index 825aa9e5e..ce1d5027b 100644 --- a/test/Dockerfile.python-3.10 +++ b/test/Dockerfile.python-3.10 @@ -30,4 +30,5 @@ RUN pip install \ lxml \ pystemd \ pytest \ + pytest-cov \ pyyaml diff --git a/test/Dockerfile.python-3.11 b/test/Dockerfile.python-3.11 index 825b67d09..212c1b9f4 100644 --- a/test/Dockerfile.python-3.11 +++ b/test/Dockerfile.python-3.11 @@ -30,4 +30,5 @@ RUN pip install \ lxml \ pystemd \ pytest \ + pytest-cov \ pyyaml diff --git a/test/Dockerfile.python-3.12 b/test/Dockerfile.python-3.12 index 3f2dfa721..2bcc3b071 100644 --- a/test/Dockerfile.python-3.12 +++ b/test/Dockerfile.python-3.12 @@ -30,4 +30,5 @@ RUN pip install \ lxml \ pystemd \ pytest \ + pytest-cov \ pyyaml diff --git a/test/Dockerfile.python-3.8 b/test/Dockerfile.python-3.8 index 276b79774..7fd39b16b 100644 --- a/test/Dockerfile.python-3.8 +++ b/test/Dockerfile.python-3.8 @@ -30,4 +30,5 @@ RUN pip install \ lxml \ pystemd \ pytest \ + pytest-cov \ pyyaml diff --git a/test/Dockerfile.python-3.9 b/test/Dockerfile.python-3.9 index ad464d9ff..7bb1148ad 100644 --- a/test/Dockerfile.python-3.9 +++ b/test/Dockerfile.python-3.9 @@ -30,4 +30,5 @@ RUN pip install \ lxml \ pystemd \ pytest \ + pytest-cov \ pyyaml From 80909bdc8d45004606066b1f2daa9e10653ca4b5 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Mon, 26 Aug 2024 11:22:38 +0200 Subject: [PATCH 24/33] Install fuse-overlayfs in test images This will allow testing #1062. --- test/Dockerfile.python-3.10 | 1 + test/Dockerfile.python-3.11 | 1 + test/Dockerfile.python-3.12 | 1 + test/Dockerfile.python-3.8 | 1 + test/Dockerfile.python-3.9 | 1 + 5 files changed, 5 insertions(+) diff --git a/test/Dockerfile.python-3.10 b/test/Dockerfile.python-3.10 index ce1d5027b..c881c0c02 100644 --- a/test/Dockerfile.python-3.10 +++ b/test/Dockerfile.python-3.10 @@ -19,6 +19,7 @@ FROM python:3.10 # because these images do not use the Python installed via apt. RUN apt-get update && apt-get install -y \ + fuse-overlayfs \ libsystemd-dev \ lxcfs \ sudo \ diff --git a/test/Dockerfile.python-3.11 b/test/Dockerfile.python-3.11 index 212c1b9f4..06cadd9bb 100644 --- a/test/Dockerfile.python-3.11 +++ b/test/Dockerfile.python-3.11 @@ -19,6 +19,7 @@ FROM python:3.11 # because these images do not use the Python installed via apt. RUN apt-get update && apt-get install -y \ + fuse-overlayfs \ libsystemd-dev \ lxcfs \ sudo \ diff --git a/test/Dockerfile.python-3.12 b/test/Dockerfile.python-3.12 index 2bcc3b071..87e99b6da 100644 --- a/test/Dockerfile.python-3.12 +++ b/test/Dockerfile.python-3.12 @@ -19,6 +19,7 @@ FROM python:3.12 # because these images do not use the Python installed via apt. RUN apt-get update && apt-get install -y \ + fuse-overlayfs \ libsystemd-dev \ lxcfs \ sudo \ diff --git a/test/Dockerfile.python-3.8 b/test/Dockerfile.python-3.8 index 7fd39b16b..5a0616ef8 100644 --- a/test/Dockerfile.python-3.8 +++ b/test/Dockerfile.python-3.8 @@ -19,6 +19,7 @@ FROM python:3.8 # because these images do not use the Python installed via apt. RUN apt-get update && apt-get install -y \ + fuse-overlayfs \ libsystemd-dev \ lxcfs \ sudo \ diff --git a/test/Dockerfile.python-3.9 b/test/Dockerfile.python-3.9 index 7bb1148ad..21c656572 100644 --- a/test/Dockerfile.python-3.9 +++ b/test/Dockerfile.python-3.9 @@ -19,6 +19,7 @@ FROM python:3.9 # because these images do not use the Python installed via apt. RUN apt-get update && apt-get install -y \ + fuse-overlayfs \ libsystemd-dev \ lxcfs \ sudo \ From 45949a8171f157a0527f93e33ffcbec15b904ce9 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Mon, 26 Aug 2024 13:00:39 +0200 Subject: [PATCH 25/33] Fix reporting of test coverage to GitLab --- .gitlab-ci.yml | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index bd70a6a4b..52da6fded 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -53,9 +53,12 @@ stages: - sudo -u $PRIMARY_USER coverage xml tags: - privileged + coverage: '/TOTAL.*? (100(?:\.0+)?\%|[1-9]?\d(?:\.\d+)?\%)$/' artifacts: - paths: - - coverage.xml + reports: + coverage_report: + coverage_format: cobertura + path: coverage.xml tests:cgroups1:python-3.8: <<: *tests-cgroupsv1 @@ -97,9 +100,12 @@ tests:cgroupsv1:python-3.12: - coverage xml tags: - cgroupsv2 + coverage: '/TOTAL.*? (100(?:\.0+)?\%|[1-9]?\d(?:\.\d+)?\%)$/' artifacts: - paths: - - coverage.xml + reports: + coverage_report: + coverage_format: cobertura + path: coverage.xml tests:cgroupsv2:python-3.8: <<: *tests-cgroupsv2 From 80e0af728d7e554310ad8ed6be2583b1cf73fefa Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Mon, 26 Aug 2024 13:26:01 +0200 Subject: [PATCH 26/33] fix typo in CI job name --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 52da6fded..adc1d7a5a 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -60,7 +60,7 @@ stages: coverage_format: cobertura path: coverage.xml -tests:cgroups1:python-3.8: +tests:cgroupsv1:python-3.8: <<: *tests-cgroupsv1 variables: PYTHON_VERSION: '3.8' From af027a1a8661003b90a84ed5b4aea1b9b2addd0d Mon Sep 17 00:00:00 2001 From: Test Date: Wed, 28 Aug 2024 07:16:24 +0200 Subject: [PATCH 27/33] Fix bug from 1049c395d: containerexec crashes since then --- benchexec/cgroups.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/benchexec/cgroups.py b/benchexec/cgroups.py index 971fdf9ed..3b16e23e5 100644 --- a/benchexec/cgroups.py +++ b/benchexec/cgroups.py @@ -167,7 +167,7 @@ def handle_errors(self, critical_cgroups): pass @abstractmethod - def create_fresh_child_cgroup(self, subsystems): + def create_fresh_child_cgroup(self, subsystems, prefix=None): pass @abstractmethod @@ -356,7 +356,7 @@ def add_task(self, pid): def kill_all_tasks(self): pass - def create_fresh_child_cgroup(self, subsystems): + def create_fresh_child_cgroup(self, subsystems, prefix=None): return self def create_fresh_child_cgroup_for_delegation(self): From ddb57f7ffc145642d501f771d9910949bf8d05a1 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Fri, 30 Aug 2024 10:57:13 +0200 Subject: [PATCH 28/33] Add some comments and documentations about kernel versions and features --- benchexec/cgroupsv2.py | 8 +++++++- doc/INSTALL.md | 7 ++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/benchexec/cgroupsv2.py b/benchexec/cgroupsv2.py index 4f2f337ce..70107b8cb 100644 --- a/benchexec/cgroupsv2.py +++ b/benchexec/cgroupsv2.py @@ -118,7 +118,7 @@ def initialize(): else: # No usable cgroup. We might still be able to continue if we actually # do not require cgroups for benchmarking. So we do not fail here - # but return an instance that will on produce an error later. + # but return an instance that will only produce an error later. return CgroupsV2({}) # Now we are the only process in this cgroup. In order to make it usable for @@ -562,6 +562,12 @@ def raise_error(e): # On cgroupsv2, frozen processes can still be killed, so this is all we need to # do. util.write_file("1", self.path, "cgroup.freeze", force=True) + # According to Lennart Poettering, this is not enough: + # https://lwn.net/Articles/855312/ + # But we never encountered any problems, and new kernels have cgroup.kill + # anyway (since 5.14). So it is probably not worth to fix it and instead + # we just eventually require kernel 5.14 for cgroupsv2 + # (before 5.19 memory measurements are missing as well). keep_child = self._delegated_to.path if self._delegated_to else None for child_cgroup in recursive_child_cgroups(self.path): kill_all_tasks_in_cgroup(child_cgroup) diff --git a/doc/INSTALL.md b/doc/INSTALL.md index fe1f1788d..401b13551 100644 --- a/doc/INSTALL.md +++ b/doc/INSTALL.md @@ -152,8 +152,10 @@ For other distributions, please read the following detailed requirements. Except on Ubuntu, the full feature set of BenchExec is only usable on **Linux 5.11 or newer**, so we suggest at least this kernel version. +And if your system is using cgroups v2 (cf. below), +the full feature set requires **Linux 5.19 or newer**. -On older kernels, you need to avoid using the overlay filesystem (cf. below), +On kernels than 5.11, you need to avoid using the overlay filesystem (cf. below), all other features are supported. However, we strongly recommend to use at least **Linux 4.14 or newer** because it reduces the overhead of BenchExec's memory measurements and limits. @@ -213,6 +215,9 @@ echo 'GRUB_CMDLINE_LINUX_DEFAULT="${GRUB_CMDLINE_LINUX_DEFAULT} systemd.unified_ sudo update-grub ``` +Furthermore, with cgroups v2 Linux 5.19 or newer is required +in order to have memory measurements. + ### Setting up Cgroups v2 on Machines with systemd This applies for example for Ubuntu 21.10 and newer, From 48564d78327bfe93cff74093281c83a5688e6c9e Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Fri, 30 Aug 2024 11:09:14 +0200 Subject: [PATCH 29/33] Declare minimum Python version as dependency in package metadata We should have done that a long time ago, but were not aware that this is possible. --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index be3291295..0c68b11dd 100644 --- a/setup.cfg +++ b/setup.cfg @@ -61,6 +61,7 @@ license_files = packages = benchexec, benchexec.tablegenerator, benchexec.tools install_requires = PyYAML >= 3.12 +python_requires = >= 3.8 zip_safe = True [options.extras_require] From 44c0a99a95d932d19c74207cd2d2a933478b4975 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Fri, 30 Aug 2024 12:48:43 +0200 Subject: [PATCH 30/33] Cleanup .gitignore For subcomponents we use a separate .gitignore file instead of adding paths to the main file, this is easier to maintain. --- .gitignore | 7 ++----- contrib/vcloud/lib/.gitignore | 9 +++++++++ 2 files changed, 11 insertions(+), 5 deletions(-) create mode 100644 contrib/vcloud/lib/.gitignore diff --git a/.gitignore b/.gitignore index eacea18cd..789154b01 100644 --- a/.gitignore +++ b/.gitignore @@ -12,8 +12,5 @@ /BenchExec.egg-info /.coverage* /coverage -/.pytype -.idea -node_modules/ -/contrib/vcloud/lib/ivy*.jar -/contrib/vcloud/lib/vcloud-jars +/coverage.xml +/.idea diff --git a/contrib/vcloud/lib/.gitignore b/contrib/vcloud/lib/.gitignore new file mode 100644 index 000000000..66bcbb67f --- /dev/null +++ b/contrib/vcloud/lib/.gitignore @@ -0,0 +1,9 @@ +# This file is part of BenchExec, a framework for reliable benchmarking: +# https://github.com/sosy-lab/benchexec +# +# SPDX-FileCopyrightText: 2007-2024 Dirk Beyer +# +# SPDX-License-Identifier: Apache-2.0 + +/ivy*.jar +/vcloud-jars From 5a9df4f98c7480549cb4a46898431c7f57de20f8 Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Fri, 30 Aug 2024 12:57:19 +0200 Subject: [PATCH 31/33] Add .rgignore ignoring some files that we never want to search in This is used by rg (ripgrep). --- benchexec/tablegenerator/react-table/.rgignore | 9 +++++++++ benchexec/tablegenerator/test_integration/.rgignore | 9 +++++++++ 2 files changed, 18 insertions(+) create mode 100644 benchexec/tablegenerator/react-table/.rgignore create mode 100644 benchexec/tablegenerator/test_integration/.rgignore diff --git a/benchexec/tablegenerator/react-table/.rgignore b/benchexec/tablegenerator/react-table/.rgignore new file mode 100644 index 000000000..2761807e9 --- /dev/null +++ b/benchexec/tablegenerator/react-table/.rgignore @@ -0,0 +1,9 @@ +# This file is part of BenchExec, a framework for reliable benchmarking: +# https://github.com/sosy-lab/benchexec +# +# SPDX-FileCopyrightText: 2024 Dirk Beyer +# +# SPDX-License-Identifier: Apache-2.0 + +/build +/src/tests/__snapshots__/ diff --git a/benchexec/tablegenerator/test_integration/.rgignore b/benchexec/tablegenerator/test_integration/.rgignore new file mode 100644 index 000000000..27489f8c5 --- /dev/null +++ b/benchexec/tablegenerator/test_integration/.rgignore @@ -0,0 +1,9 @@ +# This file is part of BenchExec, a framework for reliable benchmarking: +# https://github.com/sosy-lab/benchexec +# +# SPDX-FileCopyrightText: 2024 Dirk Beyer +# +# SPDX-License-Identifier: Apache-2.0 + +/expected +/results From e72cede78abd602748eb6265d7aaa845c2f1618a Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Fri, 30 Aug 2024 13:20:16 +0200 Subject: [PATCH 32/33] Fix unclosed file descriptor when reading PSI from cgroupsv2 --- benchexec/cgroupsv2.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/benchexec/cgroupsv2.py b/benchexec/cgroupsv2.py index 70107b8cb..b2b40e168 100644 --- a/benchexec/cgroupsv2.py +++ b/benchexec/cgroupsv2.py @@ -593,12 +593,13 @@ def read_max_mem_usage(self): return None def _read_pressure_stall_information(self, subsystem): - for line in open(self.path / (subsystem + ".pressure")): - if line.startswith("some "): - for item in line.split(" ")[1:]: - k, v = item.split("=") - if k == "total": - return Decimal(v) / 1_000_000 + with open(self.path / (subsystem + ".pressure")) as pressure_file: + for line in pressure_file: + if line.startswith("some "): + for item in line.split(" ")[1:]: + k, v = item.split("=") + if k == "total": + return Decimal(v) / 1_000_000 return None def read_mem_pressure(self): From b3debbe7df4795b308ca5fe5a8eca90d2a5dc69f Mon Sep 17 00:00:00 2001 From: Philipp Wendler Date: Fri, 30 Aug 2024 13:47:57 +0200 Subject: [PATCH 33/33] Enable warnings as errors when executing our test suite --- pyproject.toml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index acd0770cb..e1ff62eaa 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -72,5 +72,11 @@ ignore = [ ] [tool.pytest.ini_options] +filterwarnings = [ + "error", + # TODO cf. https://github.com/sosy-lab/benchexec/issues/1073 + # To make warning visible change "ignore" to "default". + "ignore:subprocess .* is still running:ResourceWarning", +] python_files = ["test_*.py", "test_integration/__init__.py", "test.py"] norecursedirs = ["contrib/p4/docker_files", "build", "benchexec/tablegenerator/react-table"]