[CIFuzz] Timeout orphan process fix (#3462)

This fix adds the -max_total_time argument to the cifuzz run_fuzzers command. The bug that was occurring was the docker process was being killed but the fuzzing was continuing wasting valuable compute resources. So a project with a large number of fuzzers would have all of them running by the end of CIFUzz leading to no actual fuzzing getting done because of resource scarcity. This patch should fix that.

5 files changed, 37 insertions, 14 deletions

diff --git a/infra/cifuzz/cifuzz_test.py b/infra/cifuzz/cifuzz_test.py
index 8988052a..44b2517d 100644
--- a/infra/cifuzz/cifuzz_test.py
+++ b/infra/cifuzz/cifuzz_test.py
@@ -36,8 +36,18 @@ EXAMPLE_PROJECT = 'example'
 TEST_FILES_PATH = os.path.join(os.path.dirname(os.path.abspath(__file__)),
                                'test_files')
 
-# An example fuzzer that triggers an error.
-EXAMPLE_FUZZER = 'do_stuff_fuzzer'
+# An example fuzzer that triggers an crash.
+# Binary is a copy of the example project's do_stuff_fuzzer and can be
+# generated by running "python3 infra/helper.py build_fuzzers example".
+EXAMPLE_CRASH_FUZZER = 'example_crash_fuzzer'
+
+# An example fuzzer that does not trigger a crash.
+# Binary is a modified version of example project's do_stuff_fuzzer. It is
+# created by removing the bug in my_api.cpp.
+EXAMPLE_NOCRASH_FUZZER = 'example_nocrash_fuzzer'
+
+# A fuzzer to be built in build_fuzzers integration tests.
+EXAMPLE_BUILD_FUZZER = 'do_stuff_fuzzer'
 
 
 class BuildFuzzersIntegrationTest(unittest.TestCase):
@@ -54,7 +64,8 @@ class BuildFuzzersIntegrationTest(unittest.TestCase):
               'oss-fuzz',
               tmp_dir,
               commit_sha='0b95fe1039ed7c38fea1f97078316bfc1030c523'))
-      self.assertTrue(os.path.exists(os.path.join(out_path, EXAMPLE_FUZZER)))
+      self.assertTrue(
+          os.path.exists(os.path.join(out_path, EXAMPLE_BUILD_FUZZER)))
 
   def test_valid_pull_request(self):
     """Test building fuzzers with valid pull request."""
@@ -66,7 +77,8 @@ class BuildFuzzersIntegrationTest(unittest.TestCase):
                                'oss-fuzz',
                                tmp_dir,
                                pr_ref='refs/pull/1757/merge'))
-      self.assertTrue(os.path.exists(os.path.join(out_path, EXAMPLE_FUZZER)))
+      self.assertTrue(
+          os.path.exists(os.path.join(out_path, EXAMPLE_BUILD_FUZZER)))
 
   def test_invalid_pull_request(self):
     """Test building fuzzers with invalid pull request."""
@@ -127,7 +139,8 @@ class RunFuzzersIntegrationTest(unittest.TestCase):
     out_dir = os.path.join(TEST_FILES_PATH, 'out')
     for out_file in os.listdir(out_dir):
       out_path = os.path.join(out_dir, out_file)
-      if out_file == EXAMPLE_FUZZER:
+      #pylint: disable=consider-using-in
+      if out_file == EXAMPLE_CRASH_FUZZER or out_file == EXAMPLE_NOCRASH_FUZZER:
         continue
       if os.path.isdir(out_path):
         shutil.rmtree(out_path)
@@ -142,7 +155,7 @@ class RunFuzzersIntegrationTest(unittest.TestCase):
     with unittest.mock.patch.object(fuzz_target.FuzzTarget,
                                     'is_reproducible',
                                     side_effect=[True, False]):
-      run_success, bug_found = cifuzz.run_fuzzers(100, TEST_FILES_PATH,
+      run_success, bug_found = cifuzz.run_fuzzers(10, TEST_FILES_PATH,
                                                   EXAMPLE_PROJECT)
       build_dir = os.path.join(TEST_FILES_PATH, 'out', 'oss_fuzz_latest')
       self.assertTrue(os.path.exists(build_dir))
@@ -155,7 +168,7 @@ class RunFuzzersIntegrationTest(unittest.TestCase):
     with unittest.mock.patch.object(fuzz_target.FuzzTarget,
                                     'is_reproducible',
                                     side_effect=[True, True]):
-      run_success, bug_found = cifuzz.run_fuzzers(100, TEST_FILES_PATH,
+      run_success, bug_found = cifuzz.run_fuzzers(10, TEST_FILES_PATH,
                                                   EXAMPLE_PROJECT)
       build_dir = os.path.join(TEST_FILES_PATH, 'out', 'oss_fuzz_latest')
       self.assertTrue(os.path.exists(build_dir))
@@ -168,7 +181,7 @@ class RunFuzzersIntegrationTest(unittest.TestCase):
     with tempfile.TemporaryDirectory() as tmp_dir:
       out_path = os.path.join(tmp_dir, 'out')
       os.mkdir(out_path)
-      run_success, bug_found = cifuzz.run_fuzzers(100, tmp_dir, EXAMPLE_PROJECT)
+      run_success, bug_found = cifuzz.run_fuzzers(10, tmp_dir, EXAMPLE_PROJECT)
     self.assertFalse(run_success)
     self.assertFalse(bug_found)
 
@@ -195,7 +208,7 @@ class ParseOutputUnitTest(unittest.TestCase):
   def test_parse_valid_output(self):
     """Checks that the parse fuzzer output can correctly parse output."""
     test_output_path = os.path.join(TEST_FILES_PATH,
-                                    'example_fuzzer_output.txt')
+                                    'example_crash_fuzzer_output.txt')
     test_summary_path = os.path.join(TEST_FILES_PATH, 'bug_summary_example.txt')
     with tempfile.TemporaryDirectory() as tmp_dir:
       with open(test_output_path, 'r') as test_fuzz_output:
diff --git a/infra/cifuzz/fuzz_target.py b/infra/cifuzz/fuzz_target.py
index 22c7b5f3..1aede43e 100644
--- a/infra/cifuzz/fuzz_target.py
+++ b/infra/cifuzz/fuzz_target.py
@@ -57,6 +57,9 @@ SANITIZER = 'address'
 # The number of reproduce attempts for a crash.
 REPRODUCE_ATTEMPTS = 10
 
+# Seconds on top of duration till a timeout error is raised.
+BUFFER_TIME = 10
+
 
 class FuzzTarget:
   """A class to manage a single fuzz target.
@@ -82,7 +85,7 @@ class FuzzTarget:
       project_name: The name of the relevant OSS-Fuzz project.
     """
     self.target_name = os.path.basename(target_path)
-    self.duration = duration
+    self.duration = int(duration)
     self.target_path = target_path
     self.out_dir = out_dir
     self.project_name = project_name
@@ -109,7 +112,8 @@ class FuzzTarget:
         'bash', '-c'
     ]
     run_fuzzer_command = 'run_fuzzer {fuzz_target} {options}'.format(
-        fuzz_target=self.target_name, options=LIBFUZZER_OPTIONS)
+        fuzz_target=self.target_name,
+        options=LIBFUZZER_OPTIONS + ' -max_total_time=' + str(self.duration))
 
     # If corpus can be downloaded use it for fuzzing.
     latest_corpus_path = self.download_latest_corpus()
@@ -121,13 +125,19 @@ class FuzzTarget:
     process = subprocess.Popen(command,
                                stdout=subprocess.PIPE,
                                stderr=subprocess.PIPE)
-
     try:
-      _, err = process.communicate(timeout=self.duration)
+      _, err = process.communicate(timeout=self.duration + BUFFER_TIME)
     except subprocess.TimeoutExpired:
-      logging.info('Fuzzer %s, finished with timeout.', self.target_name)
+      logging.error('Fuzzer %s timed out, ending fuzzing.', self.target_name)
+      return None, None
+
+    # Libfuzzer timeout has been reached.
+    if not process.returncode:
+      logging.info('Fuzzer %s finished with no crashes discovered.',
+                   self.target_name)
       return None, None
 
+    # Crash has been discovered.
     logging.info('Fuzzer %s, ended before timeout.', self.target_name)
     err_str = err.decode('ascii')
     test_case = self.get_test_case(err_str)
diff --git a/infra/cifuzz/test_files/example_fuzzer_output.txt b/infra/cifuzz/test_files/example_crash_fuzzer_output.txt
index d316f5f4..d316f5f4 100644
--- a/infra/cifuzz/test_files/example_fuzzer_output.txt
+++ b/infra/cifuzz/test_files/example_crash_fuzzer_output.txt
diff --git a/infra/cifuzz/test_files/out/do_stuff_fuzzer b/infra/cifuzz/test_files/out/example_crash_fuzzer
index 704800dd..704800dd 100755
--- a/infra/cifuzz/test_files/out/do_stuff_fuzzer
+++ b/infra/cifuzz/test_files/out/example_crash_fuzzer
diff --git a/infra/cifuzz/test_files/out/example_nocrash_fuzzer b/infra/cifuzz/test_files/out/example_nocrash_fuzzer
new file mode 100755
index 00000000..e4ff8604
--- /dev/null
+++ b/infra/cifuzz/test_files/out/example_nocrash_fuzzer