Stack trace support for libraries loaded directly from APKs

third_party/android_platform/development/scripts/symbol.py

Index: third_party/android_platform/development/scripts/symbol.py
diff --git a/third_party/android_platform/development/scripts/symbol.py b/third_party/android_platform/development/scripts/symbol.py
index d1fbaf7d55da88464960ac1f3970c19dc2ccd3c1..4cc415a247dfa08e20d2e60c8e3b5ab4692808fe 100755
--- a/third_party/android_platform/development/scripts/symbol.py
+++ b/third_party/android_platform/development/scripts/symbol.py
@@ -19,9 +19,12 @@
 The information can include symbol names, offsets, and source locations.
 """
 
+import glob
+import itertools
 import os
 import re
 import subprocess
+import zipfile
 
 CHROME_SRC = os.path.join(os.path.realpath(os.path.dirname(__file__)),
                           os.pardir, os.pardir, os.pardir, os.pardir)
@@ -130,6 +133,117 @@ def FindToolchain():
 
   raise Exception("Could not find tool chain")
 
+def GetAapt():
+  """Returns the path to aapt.
+
+  Args:
+    None
+
+  Returns:
+    the pathname of the 'aapt' executable.
+  """
+  sdk_home = os.path.join('third_party', 'android_tools', 'sdk')
+  sdk_home = os.environ.get('SDK_HOME', sdk_home)
+  aapt_exe = glob.glob(os.path.join(sdk_home, 'build-tools', '*', 'aapt'))
+  if not aapt_exe:
+    return None
+  return sorted(aapt_exe, key=os.path.getmtime, reverse=True)[0]
+
+def GetPackageName(aapt, apk_path):
+  """Returns the package name of the apk.
+
+  Args:
+    aapt: pathname for the 'aapt' executable.
+    apk_path: pathname of the APK file.
+
+  Returns:
+    the package name of the APK file.
+  """
+  if not aapt:
+    return None

[rmcilroy, 2014/07/21 16:05:31]

return apk_path here for allowing false positives and avoid the "if not aapt:"
on L223

[anton1, 2014/07/22 13:54:03]

apk_path is not the same as the package_name so that does not work.

[rmcilroy, 2014/07/22 14:22:31]

Ahh right.  How about instead of GetPackageName you have a function
"ApkMatchesPackageName(aapt, candidate_apk, package_name)", which just returns
true if "aapt" is None - I think this would simplify the code in
GetMatchingApks.

+  aapt_output = subprocess.check_output(
+      [aapt, 'dump', 'badging', apk_path]).split('\n')
+  package_name_re = re.compile(r'package: .*name=\'(\S*)\'')
+  for line in aapt_output:
+    m = package_name_re.match(line)

[rmcilroy, 2014/07/21 16:05:30]

nit - more descriptive variable names than "m" and "z" please (here and below).

[anton1, 2014/07/22 13:54:03]

Done.

+    if m:
+      return m.group(1)
+
+def GetCandidateApks():
+  """Returns a list of APKs which could contain the library.
+
+  Args:
+    None
+
+  Returns:
+    list of APK filename which could contain the library.
+  """
+  out_dir = os.environ.get('CHROMIUM_OUT_DIR', 'out')
+  buildtype = os.environ.get('BUILDTYPE')
+  if buildtype:
+    candidate_dirs = [os.path.join(out_dir, buildtype, 'apks')]
+  else:
+    candidate_dirs = [os.path.join(out_dir, 'Debug', 'apks'),
+                      os.path.join(out_dir, 'Release', 'apks'),
+                      ]
+  candidate_apks = list(itertools.chain.from_iterable(
+      map(lambda d: glob.glob(os.path.join(d, '*.apk')), candidate_dirs)))
+  candidate_apks = sorted(candidate_apks, key=os.path.getmtime, reverse=True)
+  return candidate_apks

[rmcilroy, 2014/07/21 16:05:30]

Could you put MapDeviceApkToLibrary after L274 and reuse the candidate_dirs? 
Also maybe invert the process, doing the filtering of sorted candidate_apks in
TranslateLibPath and passing this candidate set to MapDeviceApkToLibrary?  If
so, pull out both the candidate_libraries and candidate_apks filtering to a
helper function.

[anton1, 2014/07/22 13:54:03]

The candidate_dirs at L274 are for "libs" at 184 they are for "apks"
I don't quite follow what you are suggesting here, but I think you want to try
and factor something out, but there isn't much in common. It looks possible to
factor filter and sort, but seems contrived and is already succinct. Another
possible factor is lists of os.path.join.

The other possibility is to change the existing code to use glob, then I think
it factors, but it may change the behavior a bit and I would rather existing
behavior was preserved. 

[rmcilroy, 2014/07/22 14:22:31]

As discussed offline, you could factor this out to a helper function which takes
a list of subdirectories to search (e.g., ['apks'] here and ['lib',
'lib.target'] below, and a lambda to do the filtering.

+
+def GetCrazyLib(apk_filename):
+  """Returns the name of the first crazy library from this APK.
+
+  Args:
+    apk_filename: name of an APK file.
+
+  Returns:
+    Name of the first library which would be crazy loaded from this APK.
+  """
+  z = zipfile.ZipFile(apk_filename, 'r')
+  for filename in z.namelist():
+    m = re.match('lib/[^/]*/crazy.(lib.*[.]so)', filename)
+    if m:
+      return m.group(1)
+
+def GetMatchingApks(device_apk_name):
+  """Find any APKs which match the package indicated by the device_apk_name.
+
+  Args:
+     device_apk_name: name of the APK on the device.
+
+  Returns:
+     a list of APK filenames which could contain the desired library.

[rmcilroy, 2014/07/21 16:05:32]

/s/a/A

[anton1, 2014/07/22 13:54:03]

Done.

+  """
+  m = re.match('(.*)-[0-9]+[.]apk$', device_apk_name)
+  if not m:
+    return None
+  package_name = m.group(1)
+  aapt = GetAapt()
+  if not aapt:
+    # Allow false positives
+    return GetCandidateApks()
+  candidate_apks = GetCandidateApks()
+  return filter(
+      lambda candidate_apk :
+      package_name == GetPackageName(aapt, candidate_apk),

[rmcilroy, 2014/07/21 16:05:30]

indent (if you can)

[anton1, 2014/07/22 13:54:03]

Done.

+      candidate_apks)
+
+def MapDeviceApkToLibrary(device_apk_name):
+  """Provide a library name which corresponds with device_apk_name.
+
+  Args:
+    device_apk_name: name of the APK on the device.
+
+  Returns:
+    Name of the library which corresponds to that APK.
+  """
+  matching_apks = GetMatchingApks(device_apk_name)
+  for matching_apk in matching_apks:
+    crazy = GetCrazyLib(matching_apk)

[rmcilroy, 2014/07/21 16:05:32]

nit - crazy_lib

[anton1, 2014/07/22 13:54:03]

Done.

+    if crazy:
+      return crazy
+
 def TranslateLibPath(lib):
   # SymbolInformation(lib, addr) receives lib as the path from symbols
   # root to the symbols file. This needs to be translated to point to the
@@ -138,6 +252,12 @@ def TranslateLibPath(lib):
   # If the .so is not found somewhere in CHROME_SYMBOLS_DIR, leave it
   # untranslated in case it is an Android symbol in SYMBOLS_DIR.
   library_name = os.path.basename(lib)
+
+  if re.search('-[0-9]+[.]apk$', library_name):

[rmcilroy, 2014/07/21 16:05:32]

Add a comment stating what you are doing here

[anton1, 2014/07/22 13:54:03]

Done.

+    mapping = MapDeviceApkToLibrary(library_name)
+    if mapping:
+      library_name = mapping
+
   out_dir = os.environ.get('CHROMIUM_OUT_DIR', 'out')
   candidate_dirs = ['.',
                     os.path.join(out_dir, 'Debug', 'lib'),
@@ -145,7 +265,6 @@ def TranslateLibPath(lib):
                     os.path.join(out_dir, 'Release', 'lib'),
                     os.path.join(out_dir, 'Release', 'lib.target'),
                     ]
-
   candidate_libraries = map(
       lambda d: ('%s/%s/%s' % (CHROME_SYMBOLS_DIR, d, library_name)),
       candidate_dirs)