Run sio2jail self-check with Sio2jailExecutor (#280)

* Revert "Reverse breaking sio2jail change (#279)"

This reverts commit 28abda2.

* bump version

* Run sio2jail self-check with Sio2jailExecutor

We currently don't pass enough cmdline arguments to sio2jail self-check,
which makes it fail erroneously on some system configurations.
This change prevents that from happening, by using Sio2jailExecutor to
run the self check.

Notably this change required adding the original cmdline to
ExecutionResult for logging errors later - this feels like a leaky
abstraction, but that class actually already carries the process exit
signal and stderr.

Also improved sio2jail related error messages for better user experience.

* s/test/self-check

Author: j4b6ski

src/sinol_make/__init__.py

@@ -12,7 +12,7 @@
 from sinol_make.task_type.interactive import InteractiveTaskType # noqa
 
 
-__version__ = "1.9.3"
+__version__ = "1.9.4"
 
 
 def configure_parsers():
@@ -38,7 +38,7 @@ def configure_parsers():
 
 
 def check_sio2jail():
-    if util.is_linux() and not sio2jail.check_sio2jail():
+    if sio2jail.sio2jail_supported() and not sio2jail.check_sio2jail():
         print(util.warning('Up to date `sio2jail` in `~/.local/bin/` not found, installing new version...'))
         try:
             if sio2jail.install_sio2jail():

src/sinol_make/commands/run/__init__.py

@@ -280,7 +280,7 @@ def configure_subparser(self, subparser):
                 After running the solutions, it compares the solutions\' scores with the ones saved in config.yml.'
         )
 
-        default_timetool = 'sio2jail' if util.is_linux() else 'time'
+        default_timetool = 'sio2jail' if sio2jail.sio2jail_supported() else 'time'
 
         parser.add_argument('-s', '--solutions', type=str, nargs='+',
                             help='solutions to be run, for example prog/abc{b,s}*.{cpp,py}')
@@ -764,7 +764,7 @@ def validate_arguments(self, args):
 
         def use_sio2jail():
             timetool_path = None
-            if not util.is_linux():
+            if not sio2jail.sio2jail_supported():
                 util.exit_with_error('As `sio2jail` works only on Linux-based operating systems,\n'
                                      'we do not recommend using operating systems such as macOS.\n'
                                      'Nevertheless, you can still run sinol-make by specifying\n'
@@ -789,12 +789,12 @@ def use_time():
 
         timetool_path, timetool_name = None, None
         preferred_timetool = self.contest.preferred_timetool()
-        if preferred_timetool == 'sio2jail' and util.is_linux():
+        if preferred_timetool == 'sio2jail' and sio2jail.sio2jail_supported():
             use_default_timetool = use_sio2jail
         elif preferred_timetool == 'time':
             use_default_timetool = use_time
         else:
-            use_default_timetool = use_sio2jail if util.is_linux() else use_time
+            use_default_timetool = use_sio2jail if sio2jail.sio2jail_supported() else use_time
 
         if args.time_tool is None and self.config.get('sinol_undocumented_time_tool', '') != '':
             if self.config.get('sinol_undocumented_time_tool', '') == 'sio2jail':

src/sinol_make/executors/__init__.py

@@ -45,10 +45,12 @@ def execute(self, command: List[str], time_limit, hard_time_limit, memory_limit,
         """
 
         command = self._wrap_command(command, result_file_path, time_limit, memory_limit)
-        tle, mle, return_code, proc_stderr = self._execute(command, time_limit, hard_time_limit, memory_limit,
+        cmdline = " ".join(command)
+        tle, mle, return_code, proc_stderr = self._execute(cmdline, time_limit, hard_time_limit, memory_limit,
                                                            result_file_path, executable, execution_dir, stdin, stdout,
                                                            stderr, fds_to_close, *args, **kwargs)
         result = self._parse_result(tle, mle, return_code, result_file_path)
+        result.Cmdline = cmdline
         if not result.Stderr:
             result.Stderr = proc_stderr
         if tle:

src/sinol_make/executors/detailed.py

@@ -17,15 +17,15 @@ class DetailedExecutor(BaseExecutor):
     def _wrap_command(self, command: List[str], result_file_path: str, time_limit: int, memory_limit: int) -> List[str]:
         return command
 
-    def _execute(self, command: List[str], time_limit: int, hard_time_limit: int, memory_limit: int,
+    def _execute(self, cmdline: str, time_limit: int, hard_time_limit: int, memory_limit: int,
                  result_file_path: str, executable: str, execution_dir: str, stdin: int, stdout: int,
                  stderr: Union[None, int], fds_to_close: Union[None, List[int]],
                  *args, **kwargs) -> Tuple[bool, bool, int, List[str]]:
         timeout = False
         mem_used = 0
         if stderr is None:
             stderr = subprocess.PIPE
-        process = subprocess.Popen(" ".join(command), shell=True, *args, stdin=stdin, stdout=stdout, stderr=stderr,
+        process = subprocess.Popen(cmdline, shell=True, *args, stdin=stdin, stdout=stdout, stderr=stderr,
                                    preexec_fn=os.setpgrp, cwd=execution_dir, **kwargs)
         if fds_to_close is not None:
             for fd in fds_to_close:

src/sinol_make/executors/sio2jail.py

@@ -2,6 +2,7 @@
 import signal
 import subprocess
 import sys
+import traceback
 from typing import List, Tuple, Union
 
 from sinol_make import util
@@ -22,18 +23,18 @@ def _wrap_command(self, command: List[str], result_file_path: str, time_limit: i
                 '--rtimelimit', f'{int(16 * time_limit + 1000)}ms', '--memory-limit', f'{int(memory_limit)}K',
                 '--output-limit', '51200K', '--output', 'oiaug', '--stderr', '--'] + command
 
-    def _execute(self, command: List[str], time_limit: int, hard_time_limit: int, memory_limit: int,
+    def _execute(self, cmdline: str, time_limit: int, hard_time_limit: int, memory_limit: int,
                  result_file_path: str, executable: str, execution_dir: str, stdin: int, stdout: int,
                  stderr: Union[None, int], fds_to_close: Union[None, List[int]],
                  *args, **kwargs) -> Tuple[bool, bool, int, List[str]]:
         env = os.environ.copy()
         env['UNDER_SIO2JAIL'] = "1"
         with open(result_file_path, "w") as result_file:
             try:
-                process = subprocess.Popen(' '.join(command), *args, shell=True, stdin=stdin, stdout=stdout, env=env,
+                process = subprocess.Popen(cmdline, *args, shell=True, stdin=stdin, stdout=stdout, env=env,
                                            stderr=result_file, preexec_fn=os.setpgrp, cwd=execution_dir, **kwargs)
             except TypeError as e:
-                print(util.error("Invalid command: " + str(command)))
+                print(util.error(f"Invalid command: `{cmdline}`"))
                 raise e
             if fds_to_close is not None:
                 for fd in fds_to_close:
@@ -55,12 +56,17 @@ def _parse_result(self, _, mle, return_code, result_file_path) -> ExecutionResul
         with open(result_file_path, "r") as result_file:
             lines = result_file.readlines()
 
-        result.stderr = lines[:-2]
-
-        status, code, time_ms, _, memory_kb, _ = lines[-2].strip().split()
-        message = lines[-1].strip()
-        result.Time = int(time_ms)
-        result.Memory = int(memory_kb)
+        try:
+            result.stderr = lines[:-2]
+            status, code, time_ms, _, memory_kb, _ = lines[-2].strip().split()
+            message = lines[-1].strip()
+            result.Time = int(time_ms)
+            result.Memory = int(memory_kb)
+        except:
+            output = "".join(lines)
+            util.exit_with_error("Could not parse sio2jail output:"
+                f"\n---\n{output}"
+                f"\n---\n{traceback.format_exc()}")
 
         # ignoring `status` is weird, but sio2 does it this way
         if message == 'ok':

src/sinol_make/executors/time.py

@@ -22,15 +22,15 @@ def _wrap_command(self, command: List[str], result_file_path: str, time_limit: i
 
         return [f'{time_name}', '-f', '"%U\\n%M\\n%x"', '-o', result_file_path] + command
 
-    def _execute(self, command: List[str], time_limit: int, hard_time_limit: int, memory_limit: int,
+    def _execute(self, cmdline: str, time_limit: int, hard_time_limit: int, memory_limit: int,
                  result_file_path: str, executable: str, execution_dir: str, stdin: int, stdout: int,
                  stderr: Union[None, int], fds_to_close: Union[None, List[int]],
                  *args, **kwargs) -> Tuple[bool, bool, int, List[str]]:
         timeout = False
         mem_limit_exceeded = False
         if stderr is None:
             stderr = subprocess.PIPE
-        process = subprocess.Popen(" ".join(command), shell=True, *args, stdin=stdin, stdout=stdout, stderr=stderr,
+        process = subprocess.Popen(cmdline, shell=True, *args, stdin=stdin, stdout=stdout, stderr=stderr,
                                    preexec_fn=os.setpgrp, cwd=execution_dir, **kwargs)
         if fds_to_close is not None:
             for fd in fds_to_close:

src/sinol_make/sio2jail/__init__.py

@@ -7,7 +7,8 @@
 import requests
 
 from sinol_make import util
-
+from sinol_make.executors.sio2jail import Sio2jailExecutor
+from sinol_make.structs.status_structs import Status
 
 def sio2jail_supported():
     return util.is_linux()
@@ -74,29 +75,74 @@ def install_sio2jail(directory=None):
 
 def check_perf_counters_enabled():
     """
-    Checks if `kernel.perf_event_paranoid` is set to -1.
-    :return:
+    Checks if sio2jail is able to use perf counters to count instructions.
     """
-    if not util.is_linux() or not check_sio2jail():
+    if not sio2jail_supported() or not check_sio2jail():
         return
 
-    sio2jail = get_default_sio2jail_path()
+    with open('/proc/sys/kernel/perf_event_paranoid') as f:
+        perf_event_paranoid = int(f.read())
+
+    executor = Sio2jailExecutor(get_default_sio2jail_path())
     test_file = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'perf_test.py')
     python_executable = sys.executable
-
-    # subprocess.Pipe is not used, because than the code would hang on process.communicate()
-    with tempfile.TemporaryFile() as tmpfile:
-        process = subprocess.Popen([sio2jail, '--mount-namespace', 'off', '--', python_executable, test_file],
-                                   stdout=tmpfile, stderr=subprocess.DEVNULL)
-        process.wait()
-        tmpfile.seek(0)
-        output = tmpfile.read().decode('utf-8')
-        process.terminate()
-
-    if output != "Test string\n":
-        util.exit_with_error("To use the recommended tool for measuring time called `sio2jail`, please:\n"
-                             "- execute `sudo sysctl kernel.perf_event_paranoid=-1` to make `sio2jail` work for\n"
-                             "  the current system session,\n"
-                             "- or add `kernel.perf_event_paranoid=-1` to `/etc/sysctl.conf`\n"
-                             "  and reboot to permanently make sio2jail work.\n"
-                             "For more details, see https://github.com/sio2project/sio2jail#running.\n")
+    command = [python_executable, test_file]
+    time_limit = 1000
+    memory_limit = 65536
+
+    with (
+        tempfile.NamedTemporaryFile() as sio2jail_result,
+        tempfile.TemporaryFile() as command_stdout,
+        tempfile.TemporaryFile() as command_stderr
+    ):
+        result = executor.execute(
+            command=command,
+            time_limit=time_limit,
+            hard_time_limit=None,
+            memory_limit=memory_limit,
+            result_file_path=sio2jail_result.name,
+            executable=None,
+            execution_dir=None,
+            stdout=command_stdout,
+            stderr=command_stderr)
+        command_stdout.seek(0)
+        output_str = command_stdout.read().decode('utf-8')
+        command_stderr.seek(0)
+        error_str = command_stderr.read().decode('utf-8')
+        sio2jail_result.seek(0)
+        result_raw = sio2jail_result.read().decode('utf-8')
+
+    expected_output = "Test Successful!\n"
+    if result.Status != Status.OK or output_str != expected_output or error_str:
+        max_perf_event_paranoid = 2
+        if perf_event_paranoid > max_perf_event_paranoid:
+            hint = (f"You have sysctl kernel.perf_event_paranoid = {perf_event_paranoid}"
+                "\nThis might restrict access to instruction counting."
+                "\nTry relaxing this setting by running:"
+                f"\n\tsudo sysctl kernel.perf_event_paranoid={max_perf_event_paranoid}"
+                "\nIf that fixes the problem, you can set this permanently by adding:"
+                f"\n\tkernel.perf_event_paranoid={max_perf_event_paranoid}"
+                "\nto /etc/sysctl.conf and rebooting."
+            )
+        else:
+            hint = ("Your kernel, drivers, or hardware might be unsupported."
+                "\nDiagnose this further by trying the following commands:"
+                "\n1. Check if the `perf` tool is able to read performance counters correctly:"
+                "\n\tperf stat -e instructions:u -- sleep 0"
+                "\nIf `perf` can't be found, it might be located in: /usr/lib/linux-tools/*/perf"
+                "\n2. Check if the Performance Monitoring Unit driver was successfully loaded:"
+                "\n\tdmesg | grep PMU"
+            )
+        opt_stdout_hint = f"\nCommand stdout (expected {repr(expected_output)}):\n---\n{output_str}" if output_str != expected_output else ""
+        opt_stderr_hint = f"\nCommand stderr (expected none):\n---\n{error_str}" if error_str else ""
+        opt_sio2jail_hint = f"\nsio2jail result:\n---\n{result_raw}" if result.Status != Status.OK else ""
+        util.exit_with_error("Failed sio2jail instruction counting self-check!"
+            f"\n\nTest command:\n---\n{result.Cmdline}\n"
+            f"{opt_stdout_hint}"
+            f"{opt_stderr_hint}"
+            f"{opt_sio2jail_hint}"
+            f"\n\n{hint}"
+            "\n\nYou can also disable instruction counting by adding the `--time-tool time` flag."
+            "\nThis will make measured solution run times significantly different from SIO2."
+            "\nFor more details, see https://github.com/sio2project/sio2jail#running."
+        )

src/sinol_make/sio2jail/perf_test.py

@@ -1 +1 @@
-print("Test string")
+print("Test Successful!")

src/sinol_make/structs/status_structs.py

@@ -97,9 +97,11 @@ class ExecutionResult:
     Comment: str
     # Stderr of the program (used for checkers/interactors)
     Stderr: List[str]
+    # Original command line that was run
+    Cmdline: str
 
     def __init__(self, status=None, Time=None, Memory=None, Points=0, Error=None, Fail=False, ExitSignal=0, Comment="",
-                 Stderr=None):
+                 Stderr=None, Cmdline=None):
         self.Status = status
         self.Time = Time
         self.Memory = Memory
@@ -109,6 +111,7 @@ def __init__(self, status=None, Time=None, Memory=None, Points=0, Error=None, Fa
         self.ExitSignal = ExitSignal
         self.Comment = Comment
         self.Stderr = Stderr if Stderr is not None else []
+        self.Cmdline = Cmdline
 
     @staticmethod
     def from_dict(dict):
@@ -122,6 +125,7 @@ def from_dict(dict):
             ExitSignal=dict.get("ExitSignal", 0),
             Comment=dict.get("Comment", ""),
             Stderr=dict.get("Stderr", []),
+            Cmdline=dict.get("Cmdline", ""),
         )
 
     def to_dict(self):
@@ -135,4 +139,5 @@ def to_dict(self):
             "ExitSignal": self.ExitSignal,
             "Comment": self.Comment,
             "Stderr": self.Stderr,
+            "Cmdline": self.Cmdline,
         }

src/sinol_make/util.py

@@ -312,9 +312,9 @@ def is_wsl():
 
 def is_linux():
     """
-    Function to check if the program is running on Linux and not WSL.
+    Function to check if the program is running on Linux (including WSL).
     """
-    return sys.platform == "linux" and not is_wsl()
+    return sys.platform == "linux"
 
 
 def is_macos():