Orderfile: Only use symbols in orderfile generation

Removes gold-style section orderfile, as that is now not possible to
generate with ThinLTO in lld.

Bug: 869997
Change-Id: I9d6970cf9c251cefdc69fbef6c87118fde844374
Reviewed-on: https://chromium-review.googlesource.com/1162221
Commit-Queue: Matthew Cary <mattcary@chromium.org>
Reviewed-by: Egor Pasko <pasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582282}

tools/cygprofile/check_orderfile.py

@@ -16,58 +16,6 @@ import patch_orderfile
 import symbol_extractor
 
 
-_MAX_WARNINGS_TO_PRINT = 200
-
-
-def _IsSameMethod(name1, name2):
-  """Returns true if name1 or name2 are split method forms of the other."""
-  return patch_orderfile.RemoveSuffixes(name1) == \
-         patch_orderfile.RemoveSuffixes(name2)
-
-
-def _CountMisorderedSymbols(symbols, symbol_infos):
-  """Count the number of misordered symbols, and log them.
-
-  Args:
-    symbols: ordered sequence of symbols from the orderfile
-    symbol_infos: ordered list of SymbolInfo from the binary
-
-  Returns:
-    (misordered_pairs_count, matched_symbols_count, unmatched_symbols_count)
-  """
-  name_to_symbol_info = symbol_extractor.CreateNameToSymbolInfo(symbol_infos)
-  matched_symbol_infos = []
-  missing_count = 0
-  misordered_count = 0
-
-  # Find the SymbolInfo matching the orderfile symbols in the binary.
-  for symbol in symbols:
-    if symbol in name_to_symbol_info:
-      matched_symbol_infos.append(name_to_symbol_info[symbol])
-    else:
-      missing_count += 1
-      if missing_count < _MAX_WARNINGS_TO_PRINT:
-        logging.warning('Symbol "%s" is in the orderfile, not in the binary' %
-                        symbol)
-  logging.info('%d matched symbols, %d un-matched (Only the first %d unmatched'
-               ' symbols are shown)' % (
-                   len(matched_symbol_infos), missing_count,
-                   _MAX_WARNINGS_TO_PRINT))
-
-  # In the order of the orderfile, find all the symbols that are at an offset
-  # smaller than their immediate predecessor, and record the pair.
-  previous_symbol_info = symbol_extractor.SymbolInfo(
-      name='', offset=-1, size=0, section='')
-  for symbol_info in matched_symbol_infos:
-    if symbol_info.offset < previous_symbol_info.offset and not (
-        _IsSameMethod(symbol_info.name, previous_symbol_info.name)):
-      logging.warning('Misordered pair: %s - %s' % (
-          str(previous_symbol_info), str(symbol_info)))
-      misordered_count += 1
-    previous_symbol_info = symbol_info
-  return (misordered_count, len(matched_symbol_infos), missing_count)
-
-
 def _VerifySymbolOrder(orderfile_symbols, symbol_infos):
   """Verify symbol ordering.
 
@@ -98,6 +46,7 @@ def _VerifySymbolOrder(orderfile_symbols, symbol_infos):
   logging.warning('Missing symbols in verification: %d', missing_count)
   return True
 
+
 def main():
   parser = optparse.OptionParser(usage=
       'usage: %prog [options] <binary> <orderfile>')
@@ -115,25 +64,12 @@ def main():
   (binary_filename, orderfile_filename) = argv[1:]
 
   symbol_extractor.SetArchitecture(options.arch)
-  obj_dir = cygprofile_utils.GetObjDir(binary_filename)
-  symbol_to_sections_map = cyglog_to_orderfile.ObjectFileProcessor(
-      obj_dir).GetSymbolToSectionsMap()
-  section_to_symbols_map = cygprofile_utils.InvertMapping(
-      symbol_to_sections_map)
-  symbols = patch_orderfile.GetSymbolsFromOrderfile(orderfile_filename,
-                                                    section_to_symbols_map)
   symbol_infos = symbol_extractor.SymbolInfosFromBinary(binary_filename)
 
   if not _VerifySymbolOrder([sym.strip() for sym in file(orderfile_filename)],
                             symbol_infos):
     return 1
 
-  # Missing symbols is not an error since some of them can be eliminated through
-  # inlining.
-  (misordered_pairs_count, matched_symbols, _) = _CountMisorderedSymbols(
-      symbols, symbol_infos)
-  return (misordered_pairs_count > options.threshold) or (matched_symbols == 0)
-
 
 if __name__ == '__main__':
   logging.basicConfig(level=logging.INFO)

tools/cygprofile/check_orderfile_unittest.py

@@ -15,32 +15,6 @@ class TestCheckOrderFile(unittest.TestCase):
                    symbol_extractor.SymbolInfo('notProfiled', 0x4, 0, ''),
                    symbol_extractor.SymbolInfo('third', 0x3, 0, ''),]
 
-  def testMatchesSymbols(self):
-    symbols = ['first', 'second', 'third']
-    (misordered_pairs_count, matched_count, missing_count) = (
-        check_orderfile._CountMisorderedSymbols(symbols, self._SYMBOL_INFOS))
-    self.assertEquals(
-        (misordered_pairs_count, matched_count, missing_count), (0, 3, 0))
-
-  def testMissingMatches(self):
-    symbols = ['second', 'third', 'other', 'first']
-    (_, matched_count, unmatched_count) = (
-        check_orderfile._CountMisorderedSymbols(symbols, self._SYMBOL_INFOS))
-    self.assertEquals(matched_count, 3)
-    self.assertEquals(unmatched_count, 1)
-
-  def testNoUnorderedSymbols(self):
-    symbols = ['first', 'other', 'second', 'third', 'noMatchEither']
-    (misordered_pairs_count, _, _) = (
-        check_orderfile._CountMisorderedSymbols(symbols, self._SYMBOL_INFOS))
-    self.assertEquals(misordered_pairs_count, 0)
-
-  def testUnorderedSymbols(self):
-    symbols = ['first', 'other', 'third', 'second', 'noMatchEither']
-    (misordered_pairs_count, _, _) = (
-        check_orderfile._CountMisorderedSymbols(symbols, self._SYMBOL_INFOS))
-    self.assertEquals(misordered_pairs_count, 1)
-
   def testVerifySymbolOrder(self):
     self.assertTrue(check_orderfile._VerifySymbolOrder(
         ['.second', 'first', 'eighth', 'third'],

tools/cygprofile/cyglog_to_orderfile.py

@@ -256,18 +256,13 @@ def main():
     args.arch = cygprofile_utils.DetectArchitecture()
   symbol_extractor.SetArchitecture(args.target_arch)
 
-  obj_dir = cygprofile_utils.GetObjDir(args.native_library)
-
   offsets = _ReadReachedOffsets(args.reached_offsets)
   assert offsets
   _WarnAboutDuplicates(offsets)
 
-  generator = OffsetOrderfileGenerator(
-      process_profiles.SymbolOffsetProcessor(args.native_library),
-      ObjectFileProcessor(obj_dir))
-
-  ordered_sections = generator.GetOrderedSections(offsets)
-  if ordered_sections is None:
+  processor = process_profiles.SymbolOffsetProcessor(args.native_library)
+  ordered_symbols = processor.GetOrderedSymbols(offsets)
+  if ordered_symbols is None:
     return 1
 
   success = False
@@ -276,7 +271,7 @@ def main():
   try:
     (fd, temp_filename) = tempfile.mkstemp(dir=os.path.dirname(args.output))
     output_file = os.fdopen(fd, 'w')
-    output_file.write('\n'.join(ordered_sections))
+    output_file.write('\n'.join(ordered_symbols))
     output_file.close()
     os.rename(temp_filename, args.output)
     temp_filename = None

tools/cygprofile/orderfile_generator_backend.py

@@ -236,12 +236,11 @@ class ClankCompiler(object):
     self._max_load = max_load
     self._use_goma = use_goma
     self._goma_dir = goma_dir
-    lib_chrome_so_dir = 'lib.unstripped'
     self._system_health_profiling = system_health_profiling
 
     self.obj_dir = os.path.join(self._out_dir, 'Release', 'obj')
     self.lib_chrome_so = os.path.join(
-        self._out_dir, 'Release', lib_chrome_so_dir, 'libchrome.so')
+        self._out_dir, 'Release', 'lib.unstripped', 'libchrome.so')
     self.chrome_apk = os.path.join(
         self._out_dir, 'Release', 'apks', 'Chrome.apk')
 
@@ -401,9 +400,6 @@ class OrderfileGenerator(object):
   generates an updated orderfile.
   """
   _CLANK_REPO = os.path.join(constants.DIR_SOURCE_ROOT, 'clank')
-  _CYGLOG_TO_ORDERFILE_SCRIPT = os.path.join(
-      constants.DIR_SOURCE_ROOT, 'tools', 'cygprofile',
-      'cyglog_to_orderfile.py')
   _CHECK_ORDERFILE_SCRIPT = os.path.join(
       constants.DIR_SOURCE_ROOT, 'tools', 'cygprofile', 'check_orderfile.py')
   _BUILD_ROOT = os.path.abspath(os.path.dirname(os.path.dirname(
@@ -411,8 +407,6 @@ class OrderfileGenerator(object):
 
   _UNPATCHED_ORDERFILE_FILENAME = os.path.join(
       _CLANK_REPO, 'orderfiles', 'unpatched_orderfile.%s')
-  _MERGED_CYGLOG_FILENAME = os.path.join(
-      constants.GetOutDirectory(), 'merged_cyglog')
 
   _PATH_TO_ORDERFILE = os.path.join(_CLANK_REPO, 'orderfiles',
                                     'orderfile.%s.out')
@@ -506,6 +500,7 @@ class OrderfileGenerator(object):
       except Exception:
         for f in files:
           self._SaveForDebugging(f)
+        self._SaveForDebugging(self._compiler.lib_chrome_so)
         raise
       finally:
         self._profiler.Cleanup()
@@ -541,14 +536,11 @@ class OrderfileGenerator(object):
     offsets_list = (profile_offsets.startup +
                     profile_offsets.common +
                     profile_offsets.interaction)
-    generator = cyglog_to_orderfile.OffsetOrderfileGenerator(
-        processor, cyglog_to_orderfile.ObjectFileProcessor(
-            self._compiler.obj_dir))
-    ordered_sections = generator.GetOrderedSections(offsets_list)
-    if not ordered_sections:
-      raise Exception('Failed to get ordered sections')
+    ordered_symbols = processor.GetOrderedSymbols(offsets_list)
+    if not ordered_symbols:
+      raise Exception('Failed to get ordered symbols')
     with open(self._GetUnpatchedOrderfileFilename(), 'w') as orderfile:
-      orderfile.write('\n'.join(ordered_sections))
+      orderfile.write('\n'.join(ordered_symbols))
 
   def _CollectLegacyProfile(self):
     try:
@@ -563,8 +555,14 @@ class OrderfileGenerator(object):
       if not offsets:
         raise Exception('No profiler offsets found in {}'.format(
             '\n'.join(files)))
-      with open(self._MERGED_CYGLOG_FILENAME, 'w') as f:
-        f.write('\n'.join(map(str, offsets)))
+      processor = process_profiles.SymbolOffsetProcessor(
+          self._compiler.lib_chrome_so)
+      ordered_symbols = processor.GetOrderedSymbols(offsets)
+      if not ordered_symbols:
+        raise Exception('No symbol names from  offsets found in {}'.format(
+            '\n'.join(files)))
+      with open(self._GetUnpatchedOrderfileFilename(), 'w') as orderfile:
+        orderfile.write('\n'.join(ordered_symbols))
     except Exception:
       for f in files:
         self._SaveForDebugging(f)
@@ -572,19 +570,6 @@ class OrderfileGenerator(object):
     finally:
       self._profiler.Cleanup()
 
-    try:
-      command_args = [
-          '--target-arch=' + self._options.arch,
-          '--native-library=' + self._compiler.lib_chrome_so,
-          '--output=' + self._GetUnpatchedOrderfileFilename()]
-      command_args.append('--reached-offsets=' + self._MERGED_CYGLOG_FILENAME)
-      self._step_recorder.RunCommand(
-          [self._CYGLOG_TO_ORDERFILE_SCRIPT] + command_args)
-    except CommandError:
-      self._SaveForDebugging(self._MERGED_CYGLOG_FILENAME)
-      self._SaveForDebuggingWithOverwrite(self._compiler.lib_chrome_so)
-      raise
-
   def _MaybeSaveProfile(self, files):
     if self._options.profile_save_dir:
       logging.info('Saving profiles to %s', self._options.profile_save_dir)
@@ -592,13 +577,6 @@ class OrderfileGenerator(object):
         shutil.copy(f, self._options.profile_save_dir)
         logging.info('Saved profile %s', f)
 
-  def _DeleteTempFiles(self):
-    """Deletes intermediate step output files."""
-    print 'Delete %s' % (
-        self._MERGED_CYGLOG_FILENAME)
-    if os.path.isfile(self._MERGED_CYGLOG_FILENAME):
-      os.unlink(self._MERGED_CYGLOG_FILENAME)
-
   def _PatchOrderfile(self):
     """Patches the orderfile using clean version of libchrome.so."""
     self._step_recorder.BeginStep('Patch Orderfile')
@@ -721,7 +699,6 @@ class OrderfileGenerator(object):
         self._MaybeArchiveOrderfile(self._GetUnpatchedOrderfileFilename())
         profile_uploaded = True
       finally:
-        self._DeleteTempFiles()
         _StashOutputDirectory(self._instrumented_out_dir)
     elif self._options.manual_symbol_offsets:
       assert self._options.manual_libname

tools/cygprofile/patch_orderfile_unittest.py

@@ -20,80 +20,6 @@ class TestPatchOrderFile(unittest.TestCase):
     self.assertEquals(
         'this.is.a', patch_orderfile.RemoveSuffixes(with_part))
 
-  def testSymbolsWithSameOffset(self):
-    symbol_name = "dummySymbol"
-    symbol_name2 = "other"
-    name_to_symbol_infos = {symbol_name: [
-        symbol_extractor.SymbolInfo(symbol_name, 0x42, 0x12,
-                                    section='.text')]}
-    offset_to_symbol_infos = {
-        0x42: [symbol_extractor.SymbolInfo(symbol_name, 0x42, 0x12,
-                                           section='.text'),
-               symbol_extractor.SymbolInfo(symbol_name2, 0x42, 0x12,
-                                           section='.text')]}
-    symbol_names = patch_orderfile._SymbolsWithSameOffset(
-        symbol_name, name_to_symbol_infos, offset_to_symbol_infos)
-    self.assertEquals(len(symbol_names), 2)
-    self.assertEquals(symbol_names[0], symbol_name)
-    self.assertEquals(symbol_names[1], symbol_name2)
-    self.assertEquals([], patch_orderfile._SymbolsWithSameOffset(
-        "symbolThatShouldntMatch",
-        name_to_symbol_infos, offset_to_symbol_infos))
-
-  def testSectionNameToSymbols(self):
-    mapping = {'.text.foo': ['foo'],
-               '.text.hot.bar': ['bar', 'bar1']}
-    self.assertEquals(list(patch_orderfile._SectionNameToSymbols(
-                      '.text.foo', mapping)),
-                      ['foo'])
-    self.assertEquals(list(patch_orderfile._SectionNameToSymbols(
-                      '.text.hot.bar', mapping)),
-                      ['bar', 'bar1'])
-    self.assertEquals(list(patch_orderfile._SectionNameToSymbols(
-                      '.text.hot.bar', mapping)),
-                      ['bar', 'bar1'])
-    self.assertEquals(list(patch_orderfile._SectionNameToSymbols(
-                      '.text.hot.foobar', mapping)),
-                      ['foobar'])
-    self.assertEquals(list(patch_orderfile._SectionNameToSymbols(
-                      '.text.unlikely.*', mapping)),
-                      [])
-
-  def testSectionMatchingRules(self):
-    symbol_name1 = 'symbol1'
-    symbol_name2 = 'symbol2'
-    symbol_name3 = 'symbol3'
-    section_name1 = '.text.' + symbol_name1
-    section_name3 = '.text.foo'
-    suffixed = set([section_name3])
-    name_to_symbol_infos = {symbol_name1: [
-        symbol_extractor.SymbolInfo(symbol_name1, 0x42, 0x12,
-                                    section='.text')]}
-    offset_to_symbol_infos = {
-        0x42: [symbol_extractor.SymbolInfo(symbol_name1, 0x42, 0x12,
-                                           section='.text'),
-               symbol_extractor.SymbolInfo(symbol_name2, 0x42, 0x12,
-                                           section='.text')]}
-    section_to_symbols_map = {section_name1: [symbol_name1],
-                              section_name3: [symbol_name1, symbol_name3]}
-    symbol_to_sections_map = {symbol_name1:
-                                  [section_name1, section_name3],
-                              symbol_name3: [section_name3]}
-    expected = [
-        section_name1,
-        section_name3,
-        section_name3 + '.*',
-        '.text.hot.' + symbol_name1,
-        '.text.unlikely.' + symbol_name1,
-        symbol_name1,
-        '.text.hot.symbol2',
-        '.text.unlikely.symbol2',
-        '.text.symbol2',
-        'symbol2']
-    self.assertEqual(expected, list(patch_orderfile._SectionMatchingRules(
-        section_name1, name_to_symbol_infos, offset_to_symbol_infos,
-        section_to_symbols_map, symbol_to_sections_map, suffixed)))
-
   def testUniqueGenerator(self):
     @patch_orderfile._UniqueGenerator
     def TestIterator():
@@ -104,21 +30,6 @@ class TestPatchOrderFile(unittest.TestCase):
 
     self.assertEqual(list(TestIterator()), [1,2,3])
 
-  def testCombineSectionListsByPrimaryName(self):
-    self.assertEqual(patch_orderfile._CombineSectionListsByPrimaryName(
-        {'foo': ['.text.foo', '.text.bar.constprop.1'],
-         'foo.part.1': ['.text.baz'],
-         'foobar': ['.text.foobar']}),
-        {'foo': ['.text.foo', '.text.bar', '.text.baz'],
-         'foobar': ['.text.foobar']})
-
-  def testSectionsWithSuffixes(self):
-     self.assertEqual(patch_orderfile._SectionsWithSuffixes(
-        {'foo': ['.text.foo', '.text.bar.constprop.1'],
-         'foo.part.1': ['.text.baz'],
-         'foobar': ['.text.foobar']}),
-        set(['.text.bar']))
-
 
 if __name__ == "__main__":
   unittest.main()

tools/cygprofile/process_profiles.py

@@ -126,6 +126,30 @@ class SymbolOffsetProcessor(object):
           self.SymbolInfos())
     return self._offset_to_symbols
 
+  def GetOrderedSymbols(self, offsets):
+    """Maps a list of offsets to symbol names, retaining ordering.
+
+    The symbol name is the primary symbol. This also deals with thumb
+    instruction (which have odd offsets).
+
+    Args::
+      offsets (int iterable) a set of offsets.
+
+    Returns
+      [str] list of symbol names.
+    """
+    symbols = []
+    not_found = 0
+    for o in offsets:
+      if o in self.OffsetToPrimaryMap():
+        symbols.append(self.OffsetToPrimaryMap()[o].name)
+      elif o % 2 and (o - 1) in self.OffsetToPrimaryMap():
+        symbols.append(self.OffsetToPrimaryMap()[o - 1].name)
+      else:
+        not_found += 1
+    logging.warning('%d offsets do not have matching symbol', not_found)
+    return symbols
+
   def OffsetsPrimarySize(self, offsets):
     """Computes the total primary size of a set of offsets.
 

tools/cygprofile/process_profiles_unittest.py

@@ -58,6 +58,11 @@ class ProcessProfilesTestCase(unittest.TestCase):
     self.assertDictEqual({8: symbol_infos[0],
                           40: symbol_infos[2]}, processor.OffsetToPrimaryMap())
 
+  def testGetOrderedSymbols(self):
+    processor = TestSymbolOffsetProcessor(self.symbol_infos)
+    self.assertListEqual(['1', '3', '0'],
+                         processor.GetOrderedSymbols([8, 41, 6, 0]))
+
   def testOffsetToSymbolsMap(self):
     symbol_infos = [SimpleTestSymbol('1', 8, 16),
                     SimpleTestSymbol('AnAlias', 8, 16),