SuperSize: Ignore map-elf section size comparisons for some sections

Supersize compares sections sizes between the map and elf files to
ensure they match. However, with partitioned libraries, some sections
are automatically generated when extracting the partitions from the
original combined library, and hence impossible to compare sizes.

Similarly, the map file contains duplicate sections for each partition.
Since SuperSize currently analyzes the main .elf, we need to ignore
sections (eg. .text) for the partitions. In the future, the script can
be improved to return section sizes for all partitions (ie. a set of
sets of section sizes).

Bug: 982334
Change-Id: I240d06bdaa405c9c1984ddf82e4244fc9df5dda2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1708201
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#679952}

tools/binary_size/libsupersize/archive.py

@@ -52,6 +52,13 @@ _OutputDirectoryContext = collections.namedtuple('_OutputDirectoryContext', [
     'thin_archives',
 ])
 
+# When ensuring matching section sizes between .elf and .map files, these
+# sections should be ignored. When lld creates a combined library with
+# partitions, some sections (like .text) exist in each partition, but the ones
+# below are common. At library splitting time, llvm-objcopy pulls what's needed
+# from these sections into the new libraries. Hence, the ELF sections will end
+# up smaller than the combined .map file sections.
+_SECTION_SIZE_BLACKLIST = ['.symtab', '.shstrtab', '.strtab']
 
 # Tunable "knobs" for CreateSectionSizesAndSymbols().
 class SectionSizeKnobs(object):
@@ -857,7 +864,7 @@ def _ParseElfInfo(map_path, elf_path, tool_prefix, track_string_literals,
 
   logging.info('Parsing Linker Map')
   with _OpenMaybeGz(map_path) as map_file:
-    section_sizes, raw_symbols, linker_map_extras = (
+    map_section_sizes, raw_symbols, linker_map_extras = (
         linker_map_parser.MapFileParser().Parse(linker_name, map_file))
 
     if outdir_context and outdir_context.thin_archives:
@@ -866,12 +873,17 @@ def _ParseElfInfo(map_path, elf_path, tool_prefix, track_string_literals,
   if elf_path:
     logging.debug('Validating section sizes')
     elf_section_sizes = _SectionSizesFromElf(elf_path, tool_prefix)
+    differing_elf_section_sizes = {}
+    differing_map_section_sizes = {}
     for k, v in elf_section_sizes.iteritems():
-      if v != section_sizes.get(k):
-        logging.error('ELF file and .map file do not agree on section sizes.')
-        logging.error('.map file: %r', section_sizes)
-        logging.error('readelf: %r', elf_section_sizes)
-        sys.exit(1)
+      if k not in _SECTION_SIZE_BLACKLIST and v != map_section_sizes.get(k):
+        differing_map_section_sizes[k] = map_section_sizes.get(k)
+        differing_elf_section_sizes[k] = v
+    if differing_map_section_sizes:
+      logging.error('ELF file and .map file do not agree on section sizes.')
+      logging.error('readelf: %r', differing_elf_section_sizes)
+      logging.error('.map file: %r', differing_map_section_sizes)
+      sys.exit(1)
 
   if elf_path and outdir_context:
     missed_object_paths = _DiscoverMissedObjectPaths(
@@ -937,7 +949,10 @@ def _ParseElfInfo(map_path, elf_path, tool_prefix, track_string_literals,
 
   linker_map_parser.DeduceObjectPathsFromThinMap(raw_symbols, linker_map_extras)
 
-  return section_sizes, raw_symbols, object_paths_by_name
+  # If we have an ELF file, use its sizes as the source of truth, since some
+  # sections can differ from the .map.
+  return (elf_section_sizes if elf_path else map_section_sizes, raw_symbols,
+          object_paths_by_name)
 
 
 def _ComputePakFileSymbols(
@@ -1506,7 +1521,7 @@ def _DetectLinkerName(map_path):
 
 
 def _ElfInfoFromApk(apk_path, apk_so_path, tool_prefix):
-  """Returns a tuple of (build_id, section_sizes)."""
+  """Returns a tuple of (build_id, section_sizes, elf_overhead_size)."""
   with zipfile.ZipFile(apk_path) as apk, \
        tempfile.NamedTemporaryFile() as f:
     f.write(apk.read(apk_so_path))

tools/binary_size/libsupersize/linker_map_parser.py

@@ -485,6 +485,7 @@ class MapFileParserLld(object):
 
     tokenizer = self.Tokenize(lines)
 
+    in_partitions = False
     in_jump_table = False
     jump_tables_count = 0
     jump_entries_count = 0
@@ -493,15 +494,32 @@ class MapFileParserLld(object):
       # Level 1 data match the "Out" column. They specify sections or
       # PROVIDE_HIDDEN lines.
       if level == 1:
-        if not tok.startswith('PROVIDE_HIDDEN'):
-          self._section_sizes[tok] = size
-        cur_section = tok
-        # E.g., Want to convert "(.text._name)" -> "_name" later.
-        mangled_start_idx = len(cur_section) + 2
-        cur_section_is_useful = (cur_section in models.BSS_SECTIONS
-                                 or cur_section in (models.SECTION_RODATA,
-                                                    models.SECTION_TEXT)
-                                 or cur_section.startswith(models.SECTION_DATA))
+        # Ignore sections that belong to feature library partitions. Seeing a
+        # library name is an indicator that we've entered a list of feature
+        # partitions. After these, a single .part.end section will follow to
+        # reserve memory at runtime. Seeing the .part.end section also marks the
+        # end of partition sections in the map file.
+        if tok.startswith('lib') and tok.endswith('.so'):
+          in_partitions = True
+        elif tok == '.part.end':
+          # Note that we want to retain .part.end section, so it's fine to
+          # restart processing on this section, rather than the next one.
+          in_partitions = False
+
+        if in_partitions:
+          # For now, completely ignore feature partitions.
+          cur_section = None
+          cur_section_is_useful = False
+        else:
+          if not tok.startswith('PROVIDE_HIDDEN'):
+            self._section_sizes[tok] = size
+          cur_section = tok
+          # E.g., Want to convert "(.text._name)" -> "_name" later.
+          mangled_start_idx = len(cur_section) + 2
+          cur_section_is_useful = (
+              cur_section in models.BSS_SECTIONS
+              or cur_section in (models.SECTION_RODATA, models.SECTION_TEXT)
+              or cur_section.startswith(models.SECTION_DATA))
 
       elif cur_section_is_useful:
         # Level 2 data match the "In" column. They specify object paths and

tools/binary_size/libsupersize/testdata/linker_map_parser/Parser.golden

@@ -20,6 +20,7 @@
 .note.android.ident      152
 .note.crashpad.info      28
 .note.gnu.build-id       36
+.part.end                4096
 .plt                     6096
 .rel.dyn                 184789
 .rel.plt                 3032
@@ -124,3 +125,4 @@
 .bss@2f56000(size_without_padding=4,padding=0,full_name=WebRtcSpl_CrossCorrelation,object_path=,source_path=,flags={},num_aliases=1,component=)
 .bss@2f56004(size_without_padding=4,padding=0,full_name=WebRtcSpl_DownsampleFast,object_path=,source_path=,flags={},num_aliases=1,component=)
 .bss@2f56008(size_without_padding=4,padding=0,full_name=WebRtcSpl_MaxAbsValueW16,object_path=,source_path=,flags={},num_aliases=1,component=)
+.part.end@0(size_without_padding=4096,padding=0,full_name=** ,object_path=,source_path=,flags={},num_aliases=1,component=)

tools/binary_size/libsupersize/testdata/linker_map_parser/Tokenize.golden

@@ -226,6 +226,40 @@
  2F56008        4 (3)        0 WebRtcSpl_MaxAbsValueW16
  2F56008        4 (3)        0 WebRtcSpl_MaxAbsValueW16
  2F56008        4 (3)        4 WebRtcSpl_MaxAbsValueW16
+ 34BE000       34 (1) -------- libvr.so
+ 34BE000       34 (2) -------- <internal>:(libvr.so)
+ 34BE034      140 (1) -------- .phdrs
+ 34BE034      140 (2) -------- <internal>:(.phdrs)
+ 34BE174       13 (1) -------- .interp
+ 34BE174       13 (2) -------- <internal>:(.interp)
+ 34BE188       1C (1) -------- .note.crashpad.info
+ 34BE188       1C (2) -------- obj/third_party/crashpad/crashpad/client/libclient.a(client/crashpad_info_note.o):(.note.crashpad.info)
+ 34BE1A4       98 (1) -------- .note.android.ident
+ 34BE1A4       98 (2) -------- ../../third_party/android_ndk/platforms/android-16/arch-arm/usr/lib/crtbegin_so.o:(.note.android.ident)
+ 34BE23C       24 (1) -------- .note.gnu.build-id
+ 34BE23C       24 (2) -------- <internal>:(.note.gnu.build-id)
+ 34BE260       20 (1) -------- .dynsym
+ 34BE260       20 (2) -------- <internal>:(.dynsym)
+ 34BE280        4 (1) -------- .gnu.version
+ 34BE280        4 (2) -------- <internal>:(.gnu.version)
+ 34BE284       60 (1) -------- .gnu.version_r
+ 34BE284       60 (2) -------- <internal>:(.gnu.version_r)
+ 34BE2E4       1C (1) -------- .gnu.hash
+ 34BE2E4       1C (2) -------- <internal>:(.gnu.hash)
+ 34BE300       6C (1) -------- .dynstr
+ 34BE300       6C (2) -------- <internal>:(.dynstr)
+ 34BE36C      257 (1) -------- .rel.dyn
+ 34BE36C      257 (2) -------- <internal>:(.rel.dyn)
+ 34BE5C4       10 (1) -------- .ARM.exidx
+ 34BE5C4       10 (2) -------- <internal>:(.ARM.exidx)
+ 34BE5D8     2428 (1) -------- .rodata
+ 34BE5D8       10 (2) -------- thinlto-cache/Thin-5e976b.tmp.o:(.rodata._ZN2vrL17kRepositionIconIdE)
+ 34BE5D8       10 (3)       10 vr::kRepositionIconId
+ 34C0A00    2C9EC (1) -------- .text
+ 34C0A00       18 (2) -------- thinlto-cache/Thin-ec84a4.tmp.o:(.text._ZN6SkFont7setSizeEf)
+ 34C0A00       18 (3)       18 SkFont::setSize(float)
+       0     1000 (1) -------- .part.end
+       0     1000 (2) -------- <internal>:(.part.end)
        0       23 (1) -------- .ARM.attributes
        0       23 (2) -------- obj/third_party/boringssl/boringssl_asm/chacha-armv4.o:(.ARM.attributes)
        0       A8 (1) -------- .comment

tools/binary_size/libsupersize/testdata/linker_map_parser/test_lld-lto_v1.map

@@ -351,6 +351,46 @@
  2f56008  2f56008        4     1                 WebRtcSpl_MaxAbsValueW16
  2f56008  2f56008        4     1                 WebRtcSpl_MaxAbsValueW16
 
+# Partitions should be ignored at this point. Otherwise, their .text, .rodata,
+# etc. sections will overwrite those of the main partition.
+ 34be000  34be000       34     1 libvr.so
+ 34be000  34be000       34     1         <internal>:(libvr.so)
+ 34be034  34be034      140     1 .phdrs
+ 34be034  34be034      140     1         <internal>:(.phdrs)
+ 34be174  34be174       13     1 .interp
+ 34be174  34be174       13     1         <internal>:(.interp)
+ 34be188  34be188       1c     4 .note.crashpad.info
+ 34be188  34be188       1c     4         obj/third_party/crashpad/crashpad/client/libclient.a(client/crashpad_info_note.o):(.note.crashpad.info)
+ 34be1a4  34be1a4       98     4 .note.android.ident
+ 34be1a4  34be1a4       98     4         ../../third_party/android_ndk/platforms/android-16/arch-arm/usr/lib/crtbegin_so.o:(.note.android.ident)
+ 34be23c  34be23c       24     4 .note.gnu.build-id
+ 34be23c  34be23c       24     4         <internal>:(.note.gnu.build-id)
+ 34be260  34be260       20     4 .dynsym
+ 34be260  34be260       20     4         <internal>:(.dynsym)
+ 34be280  34be280        4     2 .gnu.version
+ 34be280  34be280        4     2         <internal>:(.gnu.version)
+ 34be284  34be284       60     4 .gnu.version_r
+ 34be284  34be284       60     4         <internal>:(.gnu.version_r)
+ 34be2e4  34be2e4       1c     4 .gnu.hash
+ 34be2e4  34be2e4       1c     4         <internal>:(.gnu.hash)
+ 34be300  34be300       6c     1 .dynstr
+ 34be300  34be300       6c     1         <internal>:(.dynstr)
+ 34be36c  34be36c      257     4 .rel.dyn
+ 34be36c  34be36c      257     4         <internal>:(.rel.dyn)
+ 34be5c4  34be5c4       10     4 .ARM.exidx
+ 34be5c4  34be5c4       10     4         <internal>:(.ARM.exidx)
+ 34be5d8  34be5d8     2428     8 .rodata
+ 34be5d8  34be5d8       10     1         thinlto-cache/Thin-5e976b.tmp.o:(.rodata._ZN2vrL17kRepositionIconIdE)
+ 34be5d8  34be5d8       10     1                 vr::kRepositionIconId
+ 34c0a00  34c0a00    2c9ec    16 .text
+ 34c0a00  34c0a00       18     4         thinlto-cache/Thin-ec84a4.tmp.o:(.text._ZN6SkFont7setSizeEf)
+ 34c0a00  34c0a00        0     1                 $t.11
+ 34c0a01  34c0a01       18     1                 SkFont::setSize(float)
+ 34c0a14  34c0a14        0     1                 $d.12
+# .part.end is unique, even if multiple lib*.so, .phdrs, etc. exist.
+       0        0     1000     1 .part.end
+       0        0     1000     1         <internal>:(.part.end)
+
 # Various .debug sections can exist, but they're omitted for simplicity.
 
 # Size-only sections.