Supersize: Fix archive producing different results for the same input (part 2)

Problem is the same, but didn't get the sort key exactly right.

This also adds a test for idempotency (but our test data is too trivial to catch
this bug).

Bug: 779895
Change-Id: I3021ec2cf222c27b51cfaba1b90bd3b7ed1f912f
Reviewed-on: https://chromium-review.googlesource.com/748809
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513232}

tools/binary_size/libsupersize/archive.py

@@ -388,7 +388,10 @@ def _CreateMergeStringsReplacements(merge_string_syms,
   logging.debug('Created %d string literal symbols', sum(len(x) for x in ret))
   logging.debug('Sorting string literals')
   for symbols in ret:
-    symbols.sort(key=lambda x: (x.address, -x.size))
+    # In order to achieve a total ordering in the presense of aliases, need to
+    # include both |address| and |object_path|.
+    # In order to achieve consistent deduping, need to include |size|.
+    symbols.sort(key=lambda x: (x.address, -x.size, x.object_path))
 
   logging.debug('Deduping string literals')
   num_removed = 0

tools/binary_size/libsupersize/integration_test.py

@@ -113,19 +113,25 @@ class IntegrationTest(unittest.TestCase):
               output_directory)
     return copy.deepcopy(IntegrationTest.cached_size_info[i])
 
+  def _DoArchive(self, archive_path, use_output_directory=True, use_elf=True,
+                 debug_measures=False):
+    args = [archive_path, '--map-file', _TEST_MAP_PATH]
+    if use_output_directory:
+      # Let autodetection find output_directory when --elf-file is used.
+      if not use_elf:
+        args += ['--output-directory', _TEST_OUTPUT_DIR]
+    else:
+      args += ['--no-source-paths']
+    if use_elf:
+      args += ['--elf-file', _TEST_ELF_PATH]
+    _RunApp('archive', args, debug_measures=debug_measures)
+
   def _DoArchiveTest(self, use_output_directory=True, use_elf=True,
                      debug_measures=False):
     with tempfile.NamedTemporaryFile(suffix='.size') as temp_file:
-      args = [temp_file.name, '--map-file', _TEST_MAP_PATH]
-      if use_output_directory:
-        # Let autodetection find output_directory when --elf-file is used.
-        if not use_elf:
-          args += ['--output-directory', _TEST_OUTPUT_DIR]
-      else:
-        args += ['--no-source-paths']
-      if use_elf:
-        args += ['--elf-file', _TEST_ELF_PATH]
-      _RunApp('archive', args, debug_measures=debug_measures)
+      self._DoArchive(
+          temp_file.name, use_output_directory=use_output_directory,
+          use_elf=use_elf, debug_measures=debug_measures)
       size_info = archive.LoadAndPostProcessSizeInfo(temp_file.name)
     # Check that saving & loading is the same as directly parsing the .map.
     expected_size_info = self._CloneSizeInfo(
@@ -200,6 +206,16 @@ class IntegrationTest(unittest.TestCase):
       file_format.SaveSizeInfo(self._CloneSizeInfo(), temp_file.name)
       return _RunApp('diff', [temp_file.name, temp_file.name])
 
+  # Runs archive 3 times, and asserts the contents are the same each time.
+  def test_Idempotent(self):
+    prev_contents = None
+    for _ in xrange(3):
+      with tempfile.NamedTemporaryFile(suffix='.size') as temp_file:
+        self._DoArchive(temp_file.name)
+        contents = temp_file.read()
+        self.assertTrue(prev_contents is None or contents == prev_contents)
+        prev_contents = contents
+
   @_CompareWithGolden()
   def test_Diff_Basic(self):
     size_info1 = self._CloneSizeInfo(use_elf=False)

tools/binary_size/libsupersize/testdata/mock_toolchain/mock_readelf.py

@@ -150,10 +150,7 @@ There are 68 section headers, starting at offset 0x5650:
 Section Headers:
   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
   [ 0]                   NULL            00000000 000000 000000 00      0   0  0
-
-Section Headers:
-  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
-  [ 0]                   NULL            00000000 000000 000000 00      0   0  0
+  [ 1] .rodata.str1.1    PROGBITS        00000000 000015 000005 01 AMS  0   0  1
 Key to Flags:
   W (write), A (alloc), X (execute), M (merge), S (strings)
   I (info), L (link order), G (group), T (TLS), E (exclude), x (unknown)
@@ -162,10 +159,6 @@ Key to Flags:
 File: obj/third_party/ffmpeg/libffmpeg_internal.a(fft_fixed.o)
 There are 68 section headers, starting at offset 0x5650:
 
-Section Headers:
-  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
-  [ 0]                   NULL            00000000 000000 000000 00      0   0  0
-
 Section Headers:
   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
   [ 0]                   NULL            00000000 000000 000000 00      0   0  0
@@ -180,10 +173,6 @@ File: ../../third_party/gvr-android-sdk/libgvr_shim_static_arm.a(\
 libcontroller_api_impl.a_controller_api_impl.o)
 There are 68 section headers, starting at offset 0x5650:
 
-Section Headers:
-  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
-  [ 0]                   NULL            00000000 000000 000000 00      0   0  0
-
 Section Headers:
   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
   [ 0]                   NULL            00000000 000000 000000 00      0   0  0
@@ -196,10 +185,6 @@ File: ../../third_party/gvr-android-sdk/libgvr_shim_static_arm.a(\
 libport_android_jni.a_jni_utils.o)
 There are 68 section headers, starting at offset 0x5650:
 
-Section Headers:
-  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
-  [ 0]                   NULL            00000000 000000 000000 00      0   0  0
-
 Section Headers:
   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
   [ 0]                   NULL            00000000 000000 000000 00      0   0  0