[CIFuzz] Fix handling of sanitizer artifacts (#5182)

Fixes #5175
1. Put sanitizer in artifact name.
2. Fix parsing of non-ASAN stacks.

8 files changed, 109 insertions, 23 deletions

diff --git a/docs/getting-started/continuous_integration.md b/docs/getting-started/continuous_integration.md
index 14ed9380..5ef0cb9d 100644
--- a/docs/getting-started/continuous_integration.md
+++ b/docs/getting-started/continuous_integration.md
@@ -214,9 +214,9 @@ The results of CIFuzz can be found in two different places.
     1. When a crash is found by CIFuzz the Upload Artifact event is triggered.
     1. This will cause a pop up in the right hand corner, allowing
     you to download a zip file called `artifacts`.
-    1. `artifacts` contains two files:
-        * `test_case` - a test case that can be used to reproduce the crash.
-        * `bug_summary` - the stack trace and summary of the crash.
+    1. `artifacts` contains two files for each crash:
+        * A test case that can be used to reproduce the crash.
+        * The sanitizer stack trace of the crash.
 
 ![Finding uploaded artifacts](../images/artifacts.png)
 
diff --git a/infra/cifuzz/run_fuzzers.py b/infra/cifuzz/run_fuzzers.py
index 2a2a89e5..de2914f1 100644
--- a/infra/cifuzz/run_fuzzers.py
+++ b/infra/cifuzz/run_fuzzers.py
@@ -100,9 +100,12 @@ class BaseFuzzTargetRunner:
     raise NotImplementedError('Child class must implement method')
 
   def get_fuzz_target_artifact(self, target, artifact_name):
-    """Returns the path of a fuzzing |artifact| named |artifact_name| for
-    |target|."""
-    artifact_name = target.target_name + '-' + artifact_name
+    """Returns the path of a fuzzing artifact named |artifact_name| for
+    |fuzz_target|."""
+    artifact_name = '{target_name}-{sanitizer}-{artifact_name}'.format(
+        target_name=target.target_name,
+        sanitizer=self.config.sanitizer,
+        artifact_name=artifact_name)
     return os.path.join(self.artifacts_dir, artifact_name)
 
   def create_fuzz_target_obj(self, target_path, run_seconds):
diff --git a/infra/cifuzz/run_fuzzers_test.py b/infra/cifuzz/run_fuzzers_test.py
index 847ddf39..1f925592 100644
--- a/infra/cifuzz/run_fuzzers_test.py
+++ b/infra/cifuzz/run_fuzzers_test.py
@@ -227,7 +227,8 @@ class BaseFuzzTargetRunnerTest(unittest.TestCase):
     target.target_name = target_name
     fuzz_target_artifact = runner.get_fuzz_target_artifact(
         target, artifact_name)
-    expected_fuzz_target_artifact = 'artifacts-dir/target_name-artifact-name'
+    expected_fuzz_target_artifact = (
+        'artifacts-dir/target_name-address-artifact-name')
     self.assertEqual(fuzz_target_artifact, expected_fuzz_target_artifact)
 
 
@@ -263,7 +264,7 @@ class CiFuzzTargetRunnerTest(fake_filesystem_unittest.TestCase):
     magic_mock.target_name = 'target1'
     mocked_create_fuzz_target_obj.return_value = magic_mock
     self.assertTrue(runner.run_fuzz_targets())
-    self.assertIn('target1-testcase', os.listdir(runner.artifacts_dir))
+    self.assertIn('target1-address-testcase', os.listdir(runner.artifacts_dir))
     self.assertEqual(mocked_run_fuzz_target.call_count, 1)
 
 
@@ -312,7 +313,7 @@ class BatchFuzzTargetRunnerTest(fake_filesystem_unittest.TestCase):
     magic_mock.target_name = 'target1'
     mocked_create_fuzz_target_obj.return_value = magic_mock
     self.assertTrue(runner.run_fuzz_targets())
-    self.assertIn('target1-testcase', os.listdir(runner.artifacts_dir))
+    self.assertIn('target1-address-testcase', os.listdir(runner.artifacts_dir))
     self.assertEqual(mocked_run_fuzz_target.call_count, 2)
 
 
diff --git a/infra/cifuzz/stack_parser.py b/infra/cifuzz/stack_parser.py
index 0077caae..69c44bc2 100644
--- a/infra/cifuzz/stack_parser.py
+++ b/infra/cifuzz/stack_parser.py
@@ -13,6 +13,8 @@
 # limitations under the License.
 """Module for parsing stacks from fuzz targets."""
 
+import logging
+
 # From clusterfuzz: src/python/crash_analysis/crash_analyzer.py
 # Used to get the beginning of the stacktrace.
 STACKTRACE_TOOL_MARKERS = [
@@ -51,25 +53,33 @@ def parse_fuzzer_output(fuzzer_output, parsed_output_file_path):
     parsed_output_file_path: The location to store the parsed output.
   """
   # Get index of key file points.
+  begin_stack = None
   for marker in STACKTRACE_TOOL_MARKERS:
     marker_index = fuzzer_output.find(marker)
-    if marker_index:
+    if marker_index != -1:
       begin_stack = marker_index
       break
 
-  end_stack = -1
+  if begin_stack is None:
+    logging.error(
+        b'Could not find a begin stack marker (%s) in fuzzer output:\n%s',
+        STACKTRACE_TOOL_MARKERS, fuzzer_output)
+    return
+
+  end_stack = None
   for marker in STACKTRACE_END_MARKERS:
     marker_index = fuzzer_output.find(marker)
-    if marker_index:
+    if marker_index != -1:
       end_stack = marker_index + len(marker)
       break
 
-  if begin_stack is None or end_stack is None:
+  if end_stack is None:
+    logging.error(
+        b'Could not find an end stack marker (%s) in fuzzer output:\n%s',
+        STACKTRACE_END_MARKERS, fuzzer_output)
     return
 
   summary_str = fuzzer_output[begin_stack:end_stack]
-  if not summary_str:
-    return
 
   # Write sections of fuzzer output to specific files.
   with open(parsed_output_file_path, 'ab') as summary_handle:
diff --git a/infra/cifuzz/stack_parser_test.py b/infra/cifuzz/stack_parser_test.py
index 9b05710f..a7b57f5e 100644
--- a/infra/cifuzz/stack_parser_test.py
+++ b/infra/cifuzz/stack_parser_test.py
@@ -14,7 +14,9 @@
 """Tests for stack_parser."""
 import os
 import unittest
+from unittest import mock
 
+import parameterized
 from pyfakefs import fake_filesystem_unittest
 
 import stack_parser
@@ -33,33 +35,42 @@ class ParseOutputTest(fake_filesystem_unittest.TestCase):
 
   def setUp(self):
     self.setUpPyfakefs()
+    self.maxDiff = None  # pylint: disable=invalid-name
 
-  def test_parse_valid_output(self):
+  @parameterized.parameterized.expand([('example_crash_fuzzer_output.txt',
+                                        'example_crash_fuzzer_bug_summary.txt'),
+                                       ('msan_crash_fuzzer_output.txt',
+                                        'msan_crash_fuzzer_bug_summary.txt')])
+  def test_parse_valid_output(self, fuzzer_output_file, bug_summary_file):
     """Checks that the parse fuzzer output can correctly parse output."""
     # Read the fuzzer output from disk.
-    fuzzer_output_path = os.path.join(TEST_FILES_PATH,
-                                      'example_crash_fuzzer_output.txt')
+    fuzzer_output_path = os.path.join(TEST_FILES_PATH, fuzzer_output_file)
     self.fs.add_real_file(fuzzer_output_path)
     with open(fuzzer_output_path, 'rb') as fuzzer_output_handle:
       fuzzer_output = fuzzer_output_handle.read()
     bug_summary_path = '/bug-summary.txt'
-    stack_parser.parse_fuzzer_output(fuzzer_output, bug_summary_path)
+    with mock.patch('logging.info') as mocked_info:
+      stack_parser.parse_fuzzer_output(fuzzer_output, bug_summary_path)
+      mocked_info.assert_not_called()
+
     with open(bug_summary_path) as bug_summary_handle:
       bug_summary = bug_summary_handle.read()
 
     # Compare the bug to the expected one.
-    expected_bug_summary_path = os.path.join(TEST_FILES_PATH,
-                                             'bug_summary_example.txt')
+    expected_bug_summary_path = os.path.join(TEST_FILES_PATH, bug_summary_file)
     self.fs.add_real_file(expected_bug_summary_path)
     with open(expected_bug_summary_path) as expected_bug_summary_handle:
       expected_bug_summary = expected_bug_summary_handle.read()
+
     self.assertEqual(expected_bug_summary, bug_summary)
 
   def test_parse_invalid_output(self):
     """Checks that no files are created when an invalid input was given."""
     artifact_path = '/bug-summary.txt'
-    stack_parser.parse_fuzzer_output(b'not a valid output_string',
-                                     artifact_path)
+    with mock.patch('logging.error') as mocked_error:
+      stack_parser.parse_fuzzer_output(b'not a valid output_string',
+                                       artifact_path)
+      assert mocked_error.call_count
     self.assertFalse(os.path.exists(artifact_path))
 
 
diff --git a/infra/cifuzz/test_files/bug_summary_example.txt b/infra/cifuzz/test_files/example_crash_fuzzer_bug_summary.txt
index 8caebad0..8caebad0 100644
--- a/infra/cifuzz/test_files/bug_summary_example.txt
+++ b/infra/cifuzz/test_files/example_crash_fuzzer_bug_summary.txt
diff --git a/infra/cifuzz/test_files/msan_crash_fuzzer_bug_summary.txt b/infra/cifuzz/test_files/msan_crash_fuzzer_bug_summary.txt
new file mode 100644
index 00000000..b55e9c6b
--- /dev/null
+++ b/infra/cifuzz/test_files/msan_crash_fuzzer_bug_summary.txt
@@ -0,0 +1,22 @@
+MemorySanitizer: use-of-uninitialized-value
+#0 0x52675f in LLVMFuzzerTestOneInput /src/cifuzz-example/do_stuff_fuzzer.cpp:13:7
+#1 0x45a431 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15
+#2 0x45ba46 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:792:3
+#3 0x45bed9 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:845:3
+#4 0x44a4bc in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:906:6
+#5 0x474432 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
+#6 0x7eff5562683f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f)
+#7 0x41eab8 in _start (out/do_stuff_fuzzer+0x41eab8)
+
+DEDUP_TOKEN: LLVMFuzzerTestOneInput--fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long)--fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&)
+Uninitialized value was created by a heap allocation
+#0 0x4d57ad in malloc /src/llvm-project/compiler-rt/lib/msan/msan_interceptors.cpp:901:3
+#1 0x437c07 in operator new(unsigned long) (out/do_stuff_fuzzer+0x437c07)
+#2 0x45ba46 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:792:3
+#3 0x45bed9 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:845:3
+#4 0x44a4bc in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:906:6
+#5 0x474432 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
+#6 0x7eff5562683f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f)
+DEDUP_TOKEN: malloc--operator new(unsigned long)--fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&)
+
+SUMMARY:
\ No newline at end of file
diff --git a/infra/cifuzz/test_files/msan_crash_fuzzer_output.txt b/infra/cifuzz/test_files/msan_crash_fuzzer_output.txt
new file mode 100644
index 00000000..c803bfb1
--- /dev/null
+++ b/infra/cifuzz/test_files/msan_crash_fuzzer_output.txt
@@ -0,0 +1,39 @@
+Dictionary: 3 entries
+INFO: Running with entropic power schedule (0xFF, 100).
+INFO: Seed: 1337
+INFO: Loaded 1 modules   (184 inline 8-bit counters): 184 [0x829300, 0x8293b8), 
+INFO: Loaded 1 PC tables (184 PCs): 184 [0x5dc910,0x5dd490), 
+INFO:        5 files found in /tmp/do_stuff_fuzzer_corpus
+INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
+==13==WARNING: MemorySanitizer: use-of-uninitialized-value
+#0 0x52675f in LLVMFuzzerTestOneInput /src/cifuzz-example/do_stuff_fuzzer.cpp:13:7
+#1 0x45a431 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15
+#2 0x45ba46 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:792:3
+#3 0x45bed9 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:845:3
+#4 0x44a4bc in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:906:6
+#5 0x474432 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
+#6 0x7eff5562683f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f)
+#7 0x41eab8 in _start (out/do_stuff_fuzzer+0x41eab8)
+
+DEDUP_TOKEN: LLVMFuzzerTestOneInput--fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long)--fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&)
+Uninitialized value was created by a heap allocation
+#0 0x4d57ad in malloc /src/llvm-project/compiler-rt/lib/msan/msan_interceptors.cpp:901:3
+#1 0x437c07 in operator new(unsigned long) (out/do_stuff_fuzzer+0x437c07)
+#2 0x45ba46 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:792:3
+#3 0x45bed9 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:845:3
+#4 0x44a4bc in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:906:6
+#5 0x474432 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
+#6 0x7eff5562683f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f)
+DEDUP_TOKEN: malloc--operator new(unsigned long)--fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&)
+
+SUMMARY: MemorySanitizer: use-of-uninitialized-value /src/cifuzz-example/do_stuff_fuzzer.cpp:13:7 in LLVMFuzzerTestOneInput
+Unique heap origins: 65
+Stack depot allocated bytes: 4424
+Unique origin histories: 29
+History depot allocated bytes: 696
+Exiting
+MS: 0 ; base unit: 0000000000000000000000000000000000000000
+
+
+artifact_prefix='./'; Test unit written to ./crash-da39a3ee5e6b4b0d3255bfef95601890afd80709
+Base64: