Android: Refactor native stack decoding for bundles

This change does the following:

- Allow disambiguation of which APK(s) should be used for decoding by
  adding an APK directory override argument to stack.py. The alternate
  directory is also handy if using bundles, where the APK files need to
  be generated from an .aab file.

- Augment Clank binary wrapper scripts with a "stack" command,
  effectively acting as a stack.py wrapper that points stack decoding at
  only the relevant APK files (as opposed to a typical stack.py
  invocation, forced to look at out/../apks/, which may contain many
  APKs).

- For lines in the trace that already have symbols, but cannot be
  decoded by the script (ie. lines from on-device Android runtime
  libraries), don't print <UNKNOWN> - fall back to the symbol given in
  logcat instead.

Bug: 1015159
Change-Id: Ib65b82f3a6d85e79f61c8381b3aa1828f7214a22
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1903926Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714638}

build/android/apk_operations.py

@@ -18,9 +18,12 @@ import posixpath
 import random
 import re
 import shlex
+import shutil
+import subprocess
 import sys
 import tempfile
 import textwrap
+import zipfile
 
 import adb_command_line
 import devil_chromium
@@ -34,8 +37,11 @@ from devil.android.sdk import intent
 from devil.android.sdk import version_codes
 from devil.utils import run_tests_helper
 
-with devil_env.SysPath(os.path.join(os.path.dirname(__file__), '..', '..',
-                                    'third_party', 'colorama', 'src')):
+_DIR_SOURCE_ROOT = os.path.normpath(
+    os.path.join(os.path.dirname(__file__), '..', '..'))
+
+with devil_env.SysPath(
+    os.path.join(_DIR_SOURCE_ROOT, 'third_party', 'colorama', 'src')):
   import colorama
 
 from incremental_install import installer
@@ -44,8 +50,8 @@ from pylib.symbols import deobfuscator
 from pylib.utils import simpleperf
 from pylib.utils import app_bundle_utils
 
-with devil_env.SysPath(os.path.join(os.path.dirname(__file__), '..', '..',
-                                    'build', 'android', 'gyp')):
+with devil_env.SysPath(
+    os.path.join(_DIR_SOURCE_ROOT, 'build', 'android', 'gyp')):
   import bundletool
 
 # Matches messages only on pre-L (Dalvik) that are spammy and unimportant.
@@ -1480,6 +1486,55 @@ class _ManifestCommand(_Command):
     ])
 
 
+class _StackCommand(_Command):
+  name = 'stack'
+  description = 'Decodes an Android stack.'
+  need_device_args = False
+
+  def _RegisterExtraArgs(self, group):
+    group.add_argument(
+        'file',
+        nargs='?',
+        help='File to decode. If not specified, stdin is processed.')
+
+  def Run(self):
+    try:
+      # In many cases, stack decoding requires APKs to map trace lines to native
+      # libraries. Create a temporary directory, and either unpack a bundle's
+      # APKS into it, or simply symlink the standalone APK into it. This
+      # provides an unambiguous set of APK files for the stack decoding process
+      # to inspect.
+      apks_directory = tempfile.mkdtemp()
+
+      if self.is_bundle:
+        output_path = self.bundle_generation_info.bundle_apks_path
+        _GenerateBundleApks(self.bundle_generation_info, output_path)
+
+        with zipfile.ZipFile(output_path, 'r') as archive:
+          files_to_extract = [
+              f for f in archive.namelist() if f.endswith('-master.apk')
+          ]
+          archive.extractall(apks_directory, files_to_extract)
+      else:
+        output = os.path.join(apks_directory,
+                              os.path.basename(self.args.apk_path))
+        os.symlink(self.args.apk_path, output)
+
+      stack_script = os.path.join(
+          constants.host_paths.ANDROID_PLATFORM_DEVELOPMENT_SCRIPTS_PATH,
+          'stack.py')
+      stack_command = [
+          stack_script, '--output-directory', self.args.output_directory,
+          '--apks-directory', apks_directory
+      ]
+      if self.args.file:
+        stack_command.append(self.args.file)
+      subprocess.call(stack_command)
+
+    finally:
+      shutil.rmtree(apks_directory)
+
+
 # Shared commands for regular APKs and app bundles.
 _COMMANDS = [
     _DevicesCommand,
@@ -1500,6 +1555,7 @@ _COMMANDS = [
     _CompileDexCommand,
     _ProfileCommand,
     _RunCommand,
+    _StackCommand,
 ]
 
 # Commands specific to app bundles.

third_party/android_platform/development/scripts/stack.py

@@ -141,20 +141,12 @@ def UnzipSymbols(symbolfile, symdir=None):
 
 def main(argv):
   try:
-    options, arguments = getopt.getopt(argv, "",
-                                       ["packed-relocation-adjustments",
-                                        "no-packed-relocation-adjustments",
-                                        "more-info",
-                                        "less-info",
-                                        "chrome-symbols-dir=",
-                                        "output-directory=",
-                                        "symbols-dir=",
-                                        "symbols-zip=",
-                                        "packed-lib=",
-                                        "arch=",
-                                        "fallback-monochrome",
-                                        "verbose",
-                                        "help"])
+    options, arguments = getopt.getopt(argv, "", [
+        "packed-relocation-adjustments", "no-packed-relocation-adjustments",
+        "more-info", "less-info", "chrome-symbols-dir=", "output-directory=",
+        "apks-directory=", "symbols-dir=", "symbols-zip=", "packed-lib=",
+        "arch=", "fallback-monochrome", "verbose", "help"
+    ])
   except getopt.GetoptError, unused_error:
     PrintUsage()
 
@@ -163,6 +155,7 @@ def main(argv):
   fallback_monochrome = False
   arch_defined = False
   packed_libs = []
+  apks_directory = None
   for option, value in options:
     if option == "--help":
       PrintUsage()
@@ -178,6 +171,8 @@ def main(argv):
                                                value)
     elif option == "--output-directory":
       constants.SetOutputDirectory(os.path.abspath(value))
+    elif option == "--apks-directory":
+      apks_directory = os.path.abspath(value)
     elif option == "--packed-lib":
       packed_libs.append(os.path.abspath(os.path.expanduser(value)))
     elif option == "--more-info":
@@ -216,7 +211,7 @@ def main(argv):
     with llvm_symbolizer.LLVMSymbolizer() as symbolizer:
       stack_core.StreamingConvertTrace(sys.stdin, {}, more_info,
                                        fallback_monochrome, arch_defined,
-                                       symbolizer)
+                                       symbolizer, apks_directory)
   else:
     print "Searching for native crashes in: " + os.path.realpath(arguments[0])
     f = open(arguments[0], "r")
@@ -240,8 +235,9 @@ def main(argv):
     with llvm_symbolizer.LLVMSymbolizer() as symbolizer:
       print ("Searching for Chrome symbols from within: "
              + ':'.join((os.path.normpath(d) for d in chrome_search_path)))
-      stack_core.ConvertTrace(lines, load_vaddrs, more_info, fallback_monochrome,
-                              arch_defined, symbolizer)
+      stack_core.ConvertTrace(lines, load_vaddrs, more_info,
+                              fallback_monochrome, arch_defined, symbolizer,
+                              apks_directory)
 
   if rootdir:
     # be a good citizen and clean up...os.rmdir and os.removedirs() don't work

third_party/android_platform/development/scripts/stack_core.py

@@ -154,7 +154,9 @@ def PrintDivider():
   print
   print '-----------------------------------------------------\n'
 
-def StreamingConvertTrace(input, load_vaddrs, more_info, fallback_monochrome, arch_defined, llvm_symbolizer):
+
+def StreamingConvertTrace(input, load_vaddrs, more_info, fallback_monochrome,
+                          arch_defined, llvm_symbolizer, apks_directory):
   """Symbolize stacks on the fly as they are read from an input stream."""
   InitWidthRelatedLineMatchers()
 
@@ -178,7 +180,8 @@ def StreamingConvertTrace(input, load_vaddrs, more_info, fallback_monochrome, ar
 
   for line in iter(sys.stdin.readline, b''):
     print line,
-    maybe_line, maybe_so_dir = PreProcessLog(load_vaddrs)([line])
+    maybe_line, maybe_so_dir = PreProcessLog(load_vaddrs,
+                                             apks_directory)([line])
     useful_lines.extend(maybe_line)
     so_dirs.extend(maybe_so_dir)
     if in_stack:
@@ -194,7 +197,9 @@ def StreamingConvertTrace(input, load_vaddrs, more_info, fallback_monochrome, ar
   if in_stack:
     ConvertStreamingChunk()
 
-def ConvertTrace(lines, load_vaddrs, more_info, fallback_monochrome, arch_defined, llvm_symbolizer):
+
+def ConvertTrace(lines, load_vaddrs, more_info, fallback_monochrome,
+                 arch_defined, llvm_symbolizer, apks_directory):
   """Convert strings containing native crash to a stack."""
   InitWidthRelatedLineMatchers()
 
@@ -205,7 +210,7 @@ def ConvertTrace(lines, load_vaddrs, more_info, fallback_monochrome, arch_define
 
   chunks = [lines[i: i+_CHUNK_SIZE] for i in xrange(0, len(lines), _CHUNK_SIZE)]
   pool = multiprocessing.Pool(processes=_DEFAULT_JOBS)
-  results = pool.map(PreProcessLog(load_vaddrs), chunks)
+  results = pool.map(PreProcessLog(load_vaddrs, apks_directory), chunks)
   useful_log = []
   so_dirs = []
   for result in results:
@@ -232,12 +237,13 @@ def ConvertTrace(lines, load_vaddrs, more_info, fallback_monochrome, arch_define
 
 class PreProcessLog:
   """Closure wrapper, for multiprocessing.Pool.map."""
-  def __init__(self, load_vaddrs):
+  def __init__(self, load_vaddrs, apks_directory):
     """Bind load_vaddrs to the PreProcessLog closure.
     Args:
       load_vaddrs: LOAD segment min_vaddrs keyed on mapped executable
     """
-    self._load_vaddrs = load_vaddrs;
+    self._load_vaddrs = load_vaddrs
+    self._apks_directory = apks_directory
     # This is mapping from apk's offset to shared libraries.
     self._shared_libraries_mapping = dict()
     # The list of directires in which instead of default output dir,
@@ -258,7 +264,8 @@ class PreProcessLog:
       soname = self._shared_libraries_mapping[key]
     else:
       soname, host_so = _FindSharedLibraryFromAPKs(constants.GetOutDirectory(),
-                                                  int(offset, 16))
+                                                   self._apks_directory,
+                                                   int(offset, 16))
       if soname:
         self._shared_libraries_mapping[key] = soname
         so_dir = os.path.dirname(host_so)
@@ -318,9 +325,11 @@ class PreProcessLog:
           soname = self._DetectSharedLibrary(lib, symbol_present)
           if soname:
             line = line.replace('/' + os.path.basename(lib), '/' + soname)
-          else:
+          elif not apks_directory:
             # If the trace line suggests a direct load from APK, replace the
-            # APK name with _FALLBACK_SO.
+            # APK name with _FALLBACK_SO, unless an APKs directory was
+            # explicitly specified (in which case, the correct .so should always
+            # be identified, and using a fallback could be misleading).
             line = line.replace('/' + _BASE_APK, '/' + _FALLBACK_SO)
             logging.debug("Can't detect shared library in APK, fallback to" +
                           " library " + _FALLBACK_SO)
@@ -432,6 +441,10 @@ def ResolveCrashSymbol(lines, more_info, llvm_symbolizer):
           if nest_count > 0:
             nest_count = nest_count - 1
             trace_lines.append(('v------>', source_symbol, source_location))
+          elif '<UNKNOWN>' in source_symbol and symbol_name:
+            # If the symbolizer couldn't find a symbol name, but the trace had
+            # one, use what the trace had.
+            trace_lines.append((code_addr, symbol_name, source_location))
           else:
             trace_lines.append((code_addr,
                                 source_symbol,
@@ -513,11 +526,12 @@ def _GetSharedLibraryInHost(soname, dirs):
     return host_so_file
 
 
-def _FindSharedLibraryFromAPKs(out_dir, offset):
+def _FindSharedLibraryFromAPKs(output_directory, apks_directory, offset):
   """Find the shared library at the specifc offset of an APK file.
 
-    WARNING: This function will look at *all* the apks under $out_dir/apks/
-    looking for native libraries they may contain at |offset|.
+    WARNING: This function will look at *all* the apks under
+    $output_directory/apks/ looking for native libraries they may contain at
+    |offset|, unless an APKs directory is explicitly specified.
 
     This is error-prone, since a typical full Chrome build has more than a
     hundred APKs these days, meaning that several APKs might actually match
@@ -531,24 +545,34 @@ def _FindSharedLibraryFromAPKs(out_dir, offset):
     If there are more than one library at offset from the pool of all APKs,
     the function prints an error message and fails.
 
-    TODO(digit): Either find a way to pass a list of valid APKs here, or
-    rewrite this script entirely to avoid so many other problematic things
-    in it.
-
   Args:
-    out_dir: Chromium output directory.
+    output_directory: Chromium output directory.
+    apks_directory: A optional directory containing (only) the APK in question,
+        or in the case of a bundle, all split APKs. This overrides the default
+        apks directory derived from the output directory, and allows for
+        disambiguation.
     offset: APK file offset, as extracted from the stack trace.
   Returns:
     A (library_name, host_library_path) tuple on success, or (None, None)
     in case of failure.
   """
-  apk_dir = os.path.join(out_dir, "apks")
-  if not os.path.isdir(apk_dir):
-    return (None, None)
 
-  apks = [os.path.join(apk_dir, f) for f in os.listdir(apk_dir)
-              if (os.path.isfile(os.path.join(apk_dir, f)) and
-                   os.path.splitext(f)[1] == '.apk')]
+  if apks_directory:
+    if not os.path.isdir(apks_directory):
+      raise Exception('Explicit APKs directory does not exist: %s',
+                      repr(apks_directory))
+  else:
+    apks_directory = os.path.join(out_dir, 'apks')
+    if not os.path.isdir(apks_directory):
+      return (None, None)
+
+  apks = []
+  # Walk subdirectories here, in case the directory contains an unzipped bundle
+  # .apks file, with splits in it.
+  for d, _, files in os.walk(apks_directory):
+    apks.extend(
+        os.path.join(d, f) for f in files if os.path.splitext(f)[1] == '.apk')
+
   shared_libraries = []
   for apk in apks:
     soname, sosize = GetUncompressedSharedLibraryFromAPK(apk, offset)
@@ -557,13 +581,13 @@ def _FindSharedLibraryFromAPKs(out_dir, offset):
     # The relocation section of libraries in APK is packed, we can't
     # detect library by its size, and have to rely on the order of lib
     # directories; for a specific ARCH, the libraries are in either
-    # out_dir or out_dir/android_ARCH, and android_ARCH directory could
-    # exists or not, so having android_ARCH directory be in first, we
-    # will find the correct lib.
+    # output_directory or output_directory/android_ARCH, and android_ARCH
+    # directory could exists or not, so having android_ARCH directory be in
+    # first, we will find the correct lib.
     dirs = [
-           os.path.join(out_dir, "android_%s" % symbol.ARCH),
-           out_dir,
-           ]
+        os.path.join(output_directory, "android_%s" % symbol.ARCH),
+        output_directory,
+    ]
     host_so_file = _GetSharedLibraryInHost(soname, dirs)
     if host_so_file:
       shared_libraries += [(soname, host_so_file)]