Fix the stack script issue when symbolizing tombstones.

Include the *.so files in lib.unstripped and the *.so files that are in apks so that we have the right symbols when symbolizing tombstones.

BUG=743268

Review-Url: https://codereview.chromium.org/2974163002
Cr-Commit-Position: refs/heads/master@{#489882}

build/android/gyp/create_test_runner_script.py

@@ -82,6 +82,7 @@ def main(args):
   group.add_argument('--android-manifest-path')
   group.add_argument('--resource-zips')
   group.add_argument('--robolectric-runtime-deps-dir')
+  group.add_argument('--enable-relocation-packing')
   args, test_runner_args = parser.parse_known_args(
       build_utils.ExpandFileArgs(args))
 
@@ -94,6 +95,8 @@ def RelativizePathToScript(path):
   test_runner_path = RelativizePathToScript(test_runner_path)
 
   test_runner_path_args = []
+  if args.enable_relocation_packing and args.enable_relocation_packing == "1":
+    test_runner_args.append('--enable-relocation-packing')
   if args.additional_apk_list:
     args.additional_apks.extend(
         build_utils.ParseGnList(args.additional_apk_list))

build/android/gyp/write_build_config.py

@@ -305,6 +305,8 @@ def main(argv):
   parser.add_option('--secondary-abi-shared-libraries-runtime-deps',
                     help='Path to file containing runtime deps for secondary '
                          'abi shared libraries.')
+  parser.add_option('--enable-relocation-packing',
+                    help='Whether relocation packing is enabled.')
 
   # apk options
   parser.add_option('--apk-path', help='Path to the target\'s apk output.')
@@ -472,6 +474,7 @@ def main(argv):
       deps_info['incremental_apk_path'] = options.incremental_apk_path
       deps_info['incremental_install_script_path'] = (
           options.incremental_install_script_path)
+      deps_info['enable_relocation_packing'] = options.enable_relocation_packing
 
   if options.type in ('java_binary', 'java_library', 'android_apk', 'dist_jar'):
     # Classpath values filled in below (after applying tested_apk_config).

build/android/pylib/instrumentation/instrumentation_test_instance.py

@@ -17,11 +17,13 @@
 from pylib.constants import host_paths
 from pylib.instrumentation import test_result
 from pylib.instrumentation import instrumentation_parser
+from pylib.symbols import stack_symbolizer
 from pylib.utils import dexdump
 from pylib.utils import instrumentation_tracing
 from pylib.utils import proguard
 from pylib.utils import shared_preference_utils
 
+
 with host_paths.SysPath(host_paths.BUILD_COMMON_PATH):
   import unittest_util # pylint: disable=import-error
 
@@ -482,6 +484,7 @@ def __init__(self, args, data_deps_delegate, error_func):
     self._initializeTestCoverageAttributes(args)
 
     self._store_tombstones = False
+    self._symbolizer = None
     self._initializeTombstonesAttributes(args)
 
     self._gs_results_bucket = None
@@ -665,6 +668,9 @@ def _initializeTestCoverageAttributes(self, args):
 
   def _initializeTombstonesAttributes(self, args):
     self._store_tombstones = args.store_tombstones
+    self._symbolizer = stack_symbolizer.Symbolizer(
+        self.apk_under_test.path if self.apk_under_test else None,
+        args.enable_relocation_packing)
 
   def _initializeLogAttributes(self, args):
     self._gs_results_bucket = args.gs_results_bucket
@@ -765,6 +771,10 @@ def store_tombstones(self):
   def suite(self):
     return self._suite
 
+  @property
+  def symbolizer(self):
+    return self._symbolizer
+
   @property
   def test_apk(self):
     return self._test_apk
@@ -891,4 +901,4 @@ def GenerateTestResults(
 
   #override
   def TearDown(self):
-    pass
+    self.symbolizer.CleanUp()

build/android/pylib/local/device/local_device_instrumentation_test_run.py

@@ -548,7 +548,8 @@ def handle_render_test_data():
                 device,
                 resolve_all_tombstones=True,
                 include_stack_symbols=False,
-                wipe_tombstones=True)
+                wipe_tombstones=True,
+                tombstone_symbolizer=self._test_instance.symbolizer)
             stream_name = 'tombstones_%s_%s' % (
                 time.strftime('%Y%m%dT%H%M%S-UTC', time.gmtime()),
                 device.serial)

build/android/pylib/symbols/stack_symbolizer.py

@@ -0,0 +1,102 @@
+# Copyright 2017 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import logging
+import os
+import re
+import shutil
+import tempfile
+import zipfile
+
+from devil.utils import cmd_helper
+from pylib import constants
+
+_STACK_TOOL = os.path.join(os.path.dirname(__file__), '..', '..', '..', '..',
+                          'third_party', 'android_platform', 'development',
+                          'scripts', 'stack')
+ABI_REG = re.compile('ABI: \'(.+?)\'')
+
+
+def _DeviceAbiToArch(device_abi):
+    # The order of this list is significant to find the more specific match
+    # (e.g., arm64) before the less specific (e.g., arm).
+    arches = ['arm64', 'arm', 'x86_64', 'x86_64', 'x86', 'mips']
+    for arch in arches:
+      if arch in device_abi:
+        return arch
+    raise RuntimeError('Unknown device ABI: %s' % device_abi)
+
+
+class Symbolizer(object):
+  """A helper class to symbolize stack."""
+
+  def __init__(self, apk_under_test=None,
+               enable_relocation_packing=None):
+    self._apk_under_test = apk_under_test
+    self._enable_relocation_packing = enable_relocation_packing
+    self._libs_dir = None
+    self._apk_libs = []
+    self._has_unzipped = False
+
+
+  def __del__(self):
+    if self._libs_dir:
+      logging.warning('Please call stack_symbolizer\'s'
+                      ' CleanUp method before it goes out of scope.')
+    self.CleanUp()
+
+
+  def CleanUp(self):
+    """Clean up the temporary directory of apk libs."""
+    if self._libs_dir:
+      shutil.rmtree(self._libs_dir)
+      self._libs_dir = None
+
+
+  def UnzipAPKIfNecessary(self):
+    """Unzip apk if packed relocation is enabled."""
+    if (self._has_unzipped
+        or not self._enable_relocation_packing
+        or not self._apk_under_test):
+      return
+    self._libs_dir = tempfile.mkdtemp()
+    with zipfile.ZipFile(self._apk_under_test) as z:
+      for name in z.namelist():
+        if name.endswith('.so'):
+          self._apk_libs.append(z.extract(name, self._libs_dir))
+
+    self._has_unzipped = True
+
+
+  def ExtractAndResolveNativeStackTraces(self, data_to_symbolize,
+                                         device_abi, include_stack=True):
+    """Run the stack tool for given input.
+
+    Args:
+      data_to_symbolize: a list of strings to symbolize.
+      include_stack: boolean whether to include stack data in output.
+      device_abi: the default ABI of the device which generated the tombstone.
+
+    Yields:
+      A string for each line of resolved stack output.
+    """
+    self.UnzipAPKIfNecessary()
+    arch = _DeviceAbiToArch(device_abi)
+    if not arch:
+      logging.warning('No device_abi can be found.')
+      return
+
+    cmd = [_STACK_TOOL, '--arch', arch, '--output-directory',
+           constants.GetOutDirectory(), '--more-info']
+    if self._enable_relocation_packing and self._apk_libs:
+      for apk_lib in self._apk_libs:
+        cmd.extend(['--packed-lib', apk_lib])
+    with tempfile.NamedTemporaryFile() as f:
+      f.write('\n'.join(data_to_symbolize))
+      f.flush()
+      _, output = cmd_helper.GetCmdStatusAndOutput(cmd + [f.name])
+    for line in output.splitlines():
+      if not include_stack and 'Stack Data:' in line:
+        break
+      yield line

build/android/test_runner.py

@@ -399,6 +399,11 @@ def AddInstrumentationTestOptions(parser):
       '--render-results-directory',
       dest='render_results_dir',
       help='Directory to pull render test result images off of the device to.')
+  parser.add_argument(
+      '--enable-relocation-packing',
+      dest='enable_relocation_packing',
+      action='store_true',
+      help='Whether relocation packing is enabled.')
   def package_replacement(arg):
     split_arg = arg.split(',')
     if len(split_arg) != 2:

build/android/test_runner.pydeps

@@ -166,6 +166,8 @@ pylib/results/flakiness_dashboard/json_results_generator.py
 pylib/results/flakiness_dashboard/results_uploader.py
 pylib/results/json_results.py
 pylib/results/report_results.py
+pylib/symbols/__init__.py
+pylib/symbols/stack_symbolizer.py
 pylib/utils/__init__.py
 pylib/utils/decorators.py
 pylib/utils/device_dependencies.py

build/android/tombstones.py

@@ -12,19 +12,19 @@
 import argparse
 import datetime
 import logging
-import multiprocessing
 import os
-import re
-import subprocess
 import sys
 
+from multiprocessing.pool import ThreadPool
+
 import devil_chromium
 
 from devil.android import device_blacklist
 from devil.android import device_errors
 from devil.android import device_utils
 from devil.utils import run_tests_helper
 from pylib import constants
+from pylib.symbols import stack_symbolizer
 
 
 _TZ_UTC = {'TZ': 'UTC'}
@@ -96,78 +96,41 @@ def _EraseTombstone(device, tombstone_file):
       as_root=True, check_return=True)
 
 
-def _DeviceAbiToArch(device_abi):
-  # The order of this list is significant to find the more specific match (e.g.,
-  # arm64) before the less specific (e.g., arm).
-  arches = ['arm64', 'arm', 'x86_64', 'x86_64', 'x86', 'mips']
-  for arch in arches:
-    if arch in device_abi:
-      return arch
-  raise RuntimeError('Unknown device ABI: %s' % device_abi)
-
-
-def _ResolveSymbols(tombstone_data, include_stack, device_abi):
-  """Run the stack tool for given tombstone input.
-
-  Args:
-    tombstone_data: a list of strings of tombstone data.
-    include_stack: boolean whether to include stack data in output.
-    device_abi: the default ABI of the device which generated the tombstone.
-
-  Yields:
-    A string for each line of resolved stack output.
-  """
-  # Check if the tombstone data has an ABI listed, if so use this in preference
-  # to the device's default ABI.
-  for line in tombstone_data:
-    found_abi = re.search('ABI: \'(.+?)\'', line)
-    if found_abi:
-      device_abi = found_abi.group(1)
-  arch = _DeviceAbiToArch(device_abi)
-  if not arch:
-    return
-
-  stack_tool = os.path.join(os.path.dirname(__file__), '..', '..',
-                            'third_party', 'android_platform', 'development',
-                            'scripts', 'stack')
-  cmd = [stack_tool, '--arch', arch, '--output-directory',
-         constants.GetOutDirectory()]
-  proc = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
-  output = proc.communicate(input='\n'.join(tombstone_data))[0]
-  for line in output.split('\n'):
-    if not include_stack and 'Stack Data:' in line:
-      break
-    yield line
-
-
-def _ResolveTombstone(tombstone):
+def _ResolveTombstone(args):
+  tombstone = args[0]
+  tombstone_symbolizer = args[1]
   lines = []
   lines += [tombstone['file'] + ' created on ' + str(tombstone['time']) +
             ', about this long ago: ' +
             (str(tombstone['device_now'] - tombstone['time']) +
             ' Device: ' + tombstone['serial'])]
   logging.info('\n'.join(lines))
   logging.info('Resolving...')
-  lines += _ResolveSymbols(tombstone['data'], tombstone['stack'],
-                           tombstone['device_abi'])
+  lines += tombstone_symbolizer.ExtractAndResolveNativeStackTraces(
+      tombstone['data'],
+      tombstone['device_abi'],
+      tombstone['stack'])
   return lines
 
 
-def _ResolveTombstones(jobs, tombstones):
+def _ResolveTombstones(jobs, tombstones, tombstone_symbolizer):
   """Resolve a list of tombstones.
 
   Args:
-    jobs: the number of jobs to use with multiprocess.
+    jobs: the number of jobs to use with multithread.
     tombstones: a list of tombstones.
   """
   if not tombstones:
     logging.warning('No tombstones to resolve.')
     return []
+  tombstone_symbolizer.UnzipAPKIfNecessary()
   if len(tombstones) == 1:
-    data = [_ResolveTombstone(tombstones[0])]
+    data = [_ResolveTombstone([tombstones[0], tombstone_symbolizer])]
   else:
-    pool = multiprocessing.Pool(processes=jobs)
-    data = pool.map(_ResolveTombstone, tombstones)
+    pool = ThreadPool(jobs)
+    data = pool.map(
+        _ResolveTombstone,
+        [[tombstone, tombstone_symbolizer] for tombstone in tombstones])
   resolved_tombstones = []
   for tombstone in data:
     resolved_tombstones.extend(tombstone)
@@ -236,7 +199,9 @@ def ClearAllTombstones(device):
 
 
 def ResolveTombstones(device, resolve_all_tombstones, include_stack_symbols,
-                      wipe_tombstones, jobs=4):
+                      wipe_tombstones, jobs=4,
+                      apk_under_test=None, enable_relocation_packing=None,
+                      tombstone_symbolizer=None):
   """Resolve tombstones in the device.
 
   Args:
@@ -253,7 +218,11 @@ def ResolveTombstones(device, resolve_all_tombstones, include_stack_symbols,
                             _GetTombstonesForDevice(device,
                                                     resolve_all_tombstones,
                                                     include_stack_symbols,
-                                                    wipe_tombstones))
+                                                    wipe_tombstones),
+                            (tombstone_symbolizer
+                             or stack_symbolizer.Symbolizer(
+                                apk_under_test,
+                                enable_relocation_packing)))
 
 
 def main():

build/config/android/internal_rules.gni

@@ -316,7 +316,12 @@ template("write_build_config") {
         }
       }
     }
-
+    if (defined(invoker.enable_relocation_packing)) {
+      args += [
+        "--enable-relocation-packing",
+        invoker.enable_relocation_packing,
+      ]
+    }
     if (defined(invoker.java_sources_file)) {
       args += [
         "--java-sources-file",
@@ -556,6 +561,10 @@ template("test_runner_script") {
           _apk_under_test = "@FileArg($_rebased_apk_under_test_build_config:deps_info:incremental_apk_path)"
         }
         test_runner_args += [ "--apk-under-test=$_apk_under_test" ]
+        test_runner_args += [
+          "--enable-relocation-packing",
+          "@FileArg($_rebased_apk_under_test_build_config:deps_info:enable_relocation_packing)",
+        ]
       }
       if (emma_coverage) {
         # Set a default coverage output directory (can be overridden by user

build/config/android/rules.gni

@@ -1843,6 +1843,11 @@ if (enable_java_templates) {
       if (_proguard_enabled) {
         proguard_info = "$_proguard_output_jar_path.info"
       }
+      if (_enable_relocation_packing) {
+        enable_relocation_packing = "1"
+      } else {
+        enable_relocation_packing = "0"
+      }
 
       # Don't depend on the runtime_deps target in order to avoid having to
       # build the native libraries just to create the .build_config file.