Check for counter overflows when merging profraw files.

R=sajjadm,liaoyuke

Bug: 1020404
Change-Id: I28e27f40aebbc9743da1fa7cc42deb03f77cee13
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1894619Reviewed-by: Yuke Liao <liaoyuke@chromium.org>
Commit-Queue: Roberto Carrillo <robertocn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712266}

testing/merge_scripts/code_coverage/merge_lib.py

@@ -113,6 +113,7 @@ def _validate_and_convert_profraws(profraw_files, profdata_tool_path):
     A tulple:
       A list of converted .profdata files of *valid* profraw files.
       A list of *invalid* profraw files.
+      A list of profraw files that have counter overflows.
   """
   logging.info('Validating and converting .profraw files.')
 
@@ -125,11 +126,13 @@ def _validate_and_convert_profraws(profraw_files, profdata_tool_path):
   pool = multiprocessing.Pool(counts)
   output_profdata_files = multiprocessing.Manager().list()
   invalid_profraw_files = multiprocessing.Manager().list()
+  counter_overflows = multiprocessing.Manager().list()
 
   for profraw_file in profraw_files:
     pool.apply_async(_validate_and_convert_profraw,
                      (profraw_file, output_profdata_files,
-                      invalid_profraw_files, profdata_tool_path))
+                      invalid_profraw_files, counter_overflows,
+                      profdata_tool_path))
 
   pool.close()
   pool.join()
@@ -138,7 +141,8 @@ def _validate_and_convert_profraws(profraw_files, profdata_tool_path):
   for input_file in profraw_files:
     os.remove(input_file)
 
-  return list(output_profdata_files), list(invalid_profraw_files)
+  return list(output_profdata_files), list(invalid_profraw_files), list(
+      counter_overflows)
 
 
 def _validate_and_convert_profraw(profraw_file, output_profdata_files,
@@ -148,19 +152,40 @@ def _validate_and_convert_profraw(profraw_file, output_profdata_files,
       profdata_tool_path, 'merge', '-o', output_profdata_file, '-sparse=true',
       profraw_file
   ]
+  profile_valid = False
+  counter_overflow = False
+  validation_output = None
 
+  # 1. Determine if the profile is valid.
   try:
     # Redirecting stderr is required because when error happens, llvm-profdata
     # writes the error output to stderr and our error handling logic relies on
     # that output.
-    output = subprocess.check_output(subprocess_cmd, stderr=subprocess.STDOUT)
-    logging.info('Validating and converting %r to %r succeeded with output: %r',
-                 profraw_file, output_profdata_file, output)
-    output_profdata_files.append(output_profdata_file)
+    validation_output = subprocess.check_output(
+        subprocess_cmd, stderr=subprocess.STDOUT)
+    if 'Counter overflow' in validation_output:
+      counter_overflow = True
+    else:
+      profile_valid = True
   except subprocess.CalledProcessError as error:
-    logging.warning('Validating and converting %r to %r failed with output: %r',
-                    profraw_file, output_profdata_file, error.output)
+    validation_output = error.output
+
+  # 2. Add the profile to the appropriate list(s).
+  if profile_valid:
+    output_profdata_files.append(output_profdata_file)
+  else:
     invalid_profraw_files.append(profraw_file)
+    if counter_overflow:
+      counter_overflows.append(profraw_file)
+
+  # 3. Log appropriate message
+  if not profile_valid:
+    template = 'Bad profile: %r, output: %r'
+    if counter_overflow:
+      template = 'Counter overflow: %r, output: %r'
+    logging.warning(template, profraw_file, validation_output)
+
+    # 4. Delete profdata for invalid profiles if present.
     if os.path.exists(output_profdata_file):
       # The output file may be created before llvm-profdata determines the
       # input is invalid. Delete it so that it does not leak and affect other
@@ -201,12 +226,13 @@ def merge_profiles(input_dir, output_file, input_extension, profdata_tool_path):
     profdata_tool_path: The path to the llvm-profdata executable.
   Returns:
     The list of profiles that had to be excluded to get the merge to
-    succeed.
+    succeed and a list of profiles that had a counter overflow.
   """
   profile_input_file_paths = _get_profile_paths(input_dir, input_extension)
   invalid_profraw_files = []
+  counter_overflows = []
   if input_extension == '.profraw':
-    profile_input_file_paths, invalid_profraw_files = (
+    profile_input_file_paths, invalid_profraw_files, counter_overflows = (
         _validate_and_convert_profraws(profile_input_file_paths,
                                        profdata_tool_path))
     logging.info('List of converted .profdata files: %r',
@@ -215,6 +241,10 @@ def merge_profiles(input_dir, output_file, input_extension, profdata_tool_path):
         'List of invalid .profraw files that failed to validate and convert: %r'
     ), invalid_profraw_files)
 
+    if counter_overflows:
+      logging.warning('There were %d profiles with counter overflows',
+                      len(counter_overflows))
+
   # The list of input files could be empty in the following scenarios:
   # 1. The test target is pure Python scripts test which doesn't execute any
   #    C/C++ binaries, such as devtools_closure_compile.
@@ -223,7 +253,7 @@ def merge_profiles(input_dir, output_file, input_extension, profdata_tool_path):
   if not profile_input_file_paths:
     logging.info('There is no valid profraw/profdata files to merge, skip '
                  'invoking profdata tools.')
-    return invalid_profraw_files
+    return invalid_profraw_files, counter_overflows
 
   invalid_profdata_files = _call_profdata_tool(
       profile_input_file_paths=profile_input_file_paths,
@@ -234,7 +264,7 @@ def merge_profiles(input_dir, output_file, input_extension, profdata_tool_path):
   for input_file in profile_input_file_paths:
     os.remove(input_file)
 
-  return invalid_profraw_files + invalid_profdata_files
+  return invalid_profraw_files + invalid_profdata_files, counter_overflows
 
 # We want to retry shards that contain one or more profiles that cannot be
 # merged (typically due to corruption described in crbug.com/937521).

testing/merge_scripts/code_coverage/merge_results.py

@@ -76,10 +76,19 @@ def main():
   # test results merge script such as layout tests will treat them as json test
   # results files and result in errors.
   logging.info('Merging code coverage profraw data')
-  invalid_profiles = coverage_merger.merge_profiles(
+  invalid_profiles, counter_overflows = coverage_merger.merge_profiles(
       params.task_output_dir,
       os.path.join(params.profdata_dir, 'default.profdata'), '.profraw',
       params.llvm_profdata)
+
+  # At the moment counter overflows overlap with invalid profiles, but this is
+  # not guaranteed to remain the case indefinitely. To avoid future conflicts
+  # treat these separately.
+  if counter_overflows:
+    with open(os.path.join(params.profdata_dir, 'profiles_with_overflows.json'),
+              'w') as f:
+      json.dump(counter_overflows, f)
+
   if invalid_profiles:
     with open(os.path.join(params.profdata_dir, 'invalid_profiles.json'),
               'w') as f:

testing/merge_scripts/code_coverage/merge_results_test.py

@@ -47,7 +47,7 @@ class MergeProfilesTest(unittest.TestCase):
         'llvm-profdata', 'a.json', 'b.json', 'c.json'
     ]
     with mock.patch.object(merger, 'merge_profiles') as mock_merge:
-      mock_merge.return_value = None
+      mock_merge.return_value = None, None
       with mock.patch.object(sys, 'argv', args):
         merge_results.main()
         self.assertEqual(
@@ -92,6 +92,8 @@ class MergeProfilesTest(unittest.TestCase):
     ], [
         '/b/some/path/0/default-2.profraw',
         '/b/some/path/1/default-1.profraw',
+    ], [
+        '/b/some/path/1/default-1.profraw',
     ]
 
     with mock.patch.object(os, 'walk') as mock_walk: