GDB support for Android in devtools' debugger.

mojo/devtools/common/android_gdb/session.py

Index: mojo/devtools/common/android_gdb/session.py
diff --git a/mojo/devtools/common/android_gdb/session.py b/mojo/devtools/common/android_gdb/session.py
new file mode 100644
index 0000000000000000000000000000000000000000..60d104d8f8d918af40f6be996493397b04da52ca
--- /dev/null
+++ b/mojo/devtools/common/android_gdb/session.py
@@ -0,0 +1,260 @@
+# Copyright 2015 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.
+

[ppi, 2015/07/09 13:12:40]

please add a top-level pydoc explaining what this module does (ie. that we use
it to script gdb, and what is the goal of that).

(we could list/explain here the key gdb commands we're issuing, as a high-level
guide to all this black magic - wdyt?)

[etiennej, 2015/07/15 07:13:21]

I gave a high-level description, and put the details into |DebugSession|
documentation.

+import logging
+import gdb
+import glob
+import itertools
+import os
+import os.path
+import shutil
+import subprocess
+import sys
+import tempfile
+import time
+
+FILE = __file__

[ppi, 2015/07/09 13:12:39]

Why? __file__ is a known thing, should we stick with it?

[etiennej, 2015/07/15 07:13:20]

There was an issue with __file__ not being defined after the import phase. As we
are providing all the paths to DebugSession (instead of having it guess), we no
longer need it.

+
+sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(FILE))))
+import android_gdb.config as config
+from android_gdb.remote_file_connection import RemoteFileConnection
+
+
+def _GetDirAbove(dirname):

[ppi, 2015/07/09 13:12:39]

Let's make this return None if the dir is not found, instead of getting into an
infinite loop.

[ppi, 2015/07/09 13:12:39]

_get_dir_above

[ppi, 2015/07/09 13:12:40]

Can you infer all paths in `debugger` and always pass them to the DebugSession,
so that we don't have this get_dir_above both here and there?

[etiennej, 2015/07/15 07:13:20]

Done.

+  """Returns the directory "above" this file containing |dirname|."""
+  path = os.path.abspath(FILE)
+  while True:
+    path, _ = os.path.split(path)
+    assert path
+    if dirname in os.listdir(path):
+      return path
+
+
+def gdb_execute(command):

[ppi, 2015/07/09 13:12:40]

_gdb_execute?

[etiennej, 2015/07/15 07:13:20]

Done.

+  """Executes a GDB command."""
+  return gdb.execute(command, to_string=True)
+
+
+class Mapping(object):

[ppi, 2015/07/09 13:12:39]

Please add a pydoc - what does a "Mapping" represent?

[etiennej, 2015/07/15 07:13:20]

Done.

+  def __init__(self, line):
+    self.start = int(line[0], 16)
+    self.end = int(line[1], 16)
+    self.size = int(line[2], 16)
+    self.offset = int(line[3], 16)
+    self.filename = line[4]
+
+
+def get_mapped_files():

[ppi, 2015/07/09 13:12:39]

Please describe the output format / value.

[ppi, 2015/07/09 13:12:40]

_get_mapped_files?

[etiennej, 2015/07/15 07:13:20]

Done.

[etiennej, 2015/07/15 07:13:20]

Done.

+  mappings = [Mapping(x) for x in
+              [x.split() for x in
+               gdb_execute("info proc map").split('\n')]
+              if len(x) == 5 and x[4][0] == '/']

[ppi, 2015/07/09 13:12:39]

Please explain what's happening here - example return value from `info proc map`
would be helpful.

[etiennej, 2015/07/15 07:13:20]

Done.

+  res = {}
+  for m in mappings:
+    libname = m.filename[m.filename.rfind('/') + 1:]
+    res[libname] = res.get(libname, []) + [m]
+  return res.values()
+
+
+class DebugSession(object):
+  def __init__(self,
+               build_directory=None,
+               package_name='org.chromium.mojo.shell',

[ppi, 2015/07/09 13:12:39]

Let's push this default up to `debugger`? It's likely that this won't be the
only tool for which the shell package name will matter.

[etiennej, 2015/07/15 07:13:20]

Done.

+               adb='adb',
+               pyelftools_dir=None):
+    if build_directory == None:
+      self.build_directory = os.path.join(
+          _GetDirAbove('out'), 'out', 'android_Debug')
+    else:
+      self.build_directory = build_directory
+    if not os.path.exists(self.build_directory):
+      logging.fatal("Please pass a valid build directory")
+      sys.exit(1)
+    self.package_name = package_name
+    self.adb = adb
+    self.remote_file_cache = os.path.join(os.getenv('HOME'), '.mojosymbols')
+
+
+    if pyelftools_dir == None:
+      # No directory for pyelftools is specified. Try first to load it from the
+      # system installation. If that fails, then try to guess.
+      try:
+        import elftools.elf.elffile as elffile
+      except ImportError:
+        pyelftools_dir = os.path.join(_GetDirAbove('third_party'),
+                                     'third_party', 'pyelftools')
+        sys.path.append(pyelftools_dir)
+        try:
+          import elftools.elf.elffile as elffile
+        except ImportError:
+          logging.fatal("Unable to find elftools module; please install it "
+                        "(for exmple, using 'pip install elftools')")
+          sys.exit(1)
+    else:
+      sys.path.append(pyelftools_dir)
+      import elftools.elf.elffile as elffile
+
+    self._elffile = elffile

[ppi, 2015/07/09 13:12:40]

_elffile_module?

[etiennej, 2015/07/15 07:13:21]

Done.

+
+    self.libraries = self._find_libraries(build_directory)

[ppi, 2015/07/09 13:12:39]

unify the naming convention, should all of these be _libraries, _rfc, etc.?

[etiennej, 2015/07/15 07:13:21]

Done.

+    self.rfc = RemoteFileConnection('localhost', 10000)
+    if not os.path.exists(self.remote_file_cache):
+      os.makedirs(self.remote_file_cache)
+    self.done_mapping = set()
+    self._downloaded_files = []
+
+  def __del__(self):
+    gdbserver_pid = self._get_pid('gdbserver')
+    if gdbserver_pid is not None:
+      subprocess.check_call([self.adb, 'shell', 'kill', gdbserver_pid])
+    remote_file_reader_pid = self._get_pid('remote_file_reader')
+    if remote_file_reader_pid is not None:
+      subprocess.check_call([self.adb, 'shell', 'kill', remote_file_reader_pid])
+
+  def _find_libraries(self, lib_dir):

[ppi, 2015/07/09 13:12:40]

Please add a pydoc explaining the return value.

[etiennej, 2015/07/15 07:13:21]

Done.

+    res = {}
+    for fn in glob.glob('%s/*.so' % lib_dir):
+      with open(fn, 'r') as f:
+        s = self._get_signature(f)
+        if s is not None:
+          res[s] = fn
+    return res
+
+  def _get_signature(self, file_object):

[ppi, 2015/07/09 13:12:39]

file_object -> file?

[etiennej, 2015/07/15 07:13:20]

I would prefer not redefining python internals.

+    """Compute a unique signature of a library file."""

[ppi, 2015/07/09 13:12:40]

Computes

[etiennej, 2015/07/15 07:13:21]

Done.

+    elf = self._elffile.ELFFile(file_object)
+    text_section = elf.get_section_by_name('.text')
+    file_object.seek(text_section['sh_offset'])
+    data = file_object.read(min(4096, text_section['sh_size']))

[ppi, 2015/07/09 13:12:39]

Are we hashing only the first 4096 bytes of the text section? Why?

Can we just hash the entire file without using elffile?

[qsr, 2015/07/09 13:54:18]

You cannot hash the entire file, because on the host you have the file wiht
debug information and on the device you have the stripped file.

We only use 4096 bytes because this is enough and otherwise we would need to
pass multiple Mo of data from the device to the host, which is slow.

This scheme is what breakpad does to identity libraries.

[ppi, 2015/07/09 14:16:11]

Makes sense. Etienne, could you please import these explanation as a comment?

[etiennej, 2015/07/15 07:13:20]

Done.

+    def combine((i, c)):
+      return i ^ ord(c)
+    result = 16 * [0]
+    for i in xrange(0, len(data), len(result)):
+      result = map(combine,
+                   itertools.izip_longest(result,
+                                          data[i:i + len(result)],
+                                          fillvalue='\0'))
+    return ''.join(["%02x" % x for x in result])
+
+  def _associate_symbols(self, mapping, local_file):
+    with open(local_file, "r") as f:
+      elf = self._elffile.ELFFile(f)
+      s = elf.get_section_by_name(".text")
+      text_address = mapping[0].start + s['sh_offset']
+      gdb_execute("add-symbol-file %s 0x%x" % (local_file, text_address))
+
+  def _download_file(self, remote):
+    """Downloads a remote file through GDB connection.
+
+    Returns:
+      The filename of the downloaded file
+    """
+    temp_file = tempfile.NamedTemporaryFile()
+    gdb_execute("remote get %s %s" % (remote, temp_file.name))
+    # This allows the deletion of temporary files on disk when the debugging
+    # session terminates.
+    self._downloaded_files.append(temp_file)
+    return temp_file.name
+
+  def _download_and_associate_symbol(self, mapping):
+    self._associate_symbols(mapping, self._download_file(mapping[0].filename))
+
+  def _find_mapping_for_address(self, mappings, address):
+    for m in mappings:
+      for s in m:

[ppi, 2015/07/09 13:12:39]

what is 's'?

[etiennej, 2015/07/15 07:13:21]

Done.

+        if address >= s.start and address <= s.end:
+          return m
+    return None
+
+  def _try_to_map(self, mapping):
+    remote_file = mapping[0].filename
+    if remote_file in self.done_mapping:
+      return False
+    self.done_mapping.add(remote_file)
+    self.rfc.open(remote_file)
+    s = self._get_signature(self.rfc)
+    if s is not None:
+      if s in self.libraries:
+        self._associate_symbols(mapping, self.libraries[s])
+      else:

[ppi, 2015/07/09 13:12:40]

Can you comment on when we hit this case and what we're doing there?

[etiennej, 2015/07/15 07:13:20]

Done.

+        local_file = os.path.join(self.remote_file_cache, s)
+        if not os.path.exists(local_file):
+          tmp_output = self._download_file(remote_file)
+          shutil.move(tmp_output, local_file)
+        self._associate_symbols(mapping, local_file)
+      return True
+    return False
+
+  def _update_symbols(self):
+    """Updates the mapping between symbols as seen from GDB and local library
+    files."""
+    mapped_files = get_mapped_files()
+    gdb_execute("info threads")
+    nb_threads = len(gdb_execute("info threads").split("\n")) - 2
+    # Map all symbols in /data/

[ppi, 2015/07/09 13:12:39]

Which are the symbols in "/data"? Are these the native libraries packaged with
the apk?

[qsr, 2015/07/09 13:54:18]

Yes.

[ppi, 2015/07/09 14:16:10]

Thanks! I guess I meant that this could be said in the comment:).

[etiennej, 2015/07/15 07:13:20]

Done.

+    for m in mapped_files:
+      filename = m[0].filename
+      if ((filename.startswith('/data/data/') or
+           filename.startswith('/data/app')) and
+          not filename.endswith('.apk') and
+          not filename.endswith('.dex')):
+        print 'Pre-mapping: %s' % m[0].filename
+        self._try_to_map(m)
+    for i in xrange(nb_threads):
+      try:
+        gdb_execute("thread %d" % (i + 1))
+        frame = gdb.newest_frame()
+        while frame and frame.is_valid():
+          if frame.name() is None:
+            m = self._find_mapping_for_address(mapped_files, frame.pc())
+            if m is not None and self._try_to_map(m):
+              # Force gdb to recompute its frames.
+              gdb_execute("info threads")
+              frame = gdb.newest_frame()
+              assert frame.is_valid()
+          if (frame.older() is not None and
+              frame.older().is_valid() and
+              frame.older().pc() != frame.pc()):
+            frame = frame.older()
+          else:
+            frame = None
+      except gdb.error as _:
+        import traceback
+        traceback.print_exc()
+
+  def _get_pid(self, application):

[ppi, 2015/07/09 13:12:39]

_get_device_app_pid?

[etiennej, 2015/07/15 07:13:21]

Done.

+    """Gets the PID of an application running on a device."""
+    output = subprocess.check_output([self.adb, 'shell', 'ps'])
+    for line in output.split('\n'):
+      elements = line.split()
+      if len(elements) > 0 and elements[-1] == application:
+        return elements[1]
+    return None
+
+  def start_debug(self):

[ppi, 2015/07/09 13:12:39]

The class is already called DebugSession, maybe just "start" ?

[etiennej, 2015/07/15 07:13:21]

Done.

+    """Starts a debugging session."""
+    gdbserver_pid = self._get_pid('gdbserver')
+    if gdbserver_pid is not None:
+      subprocess.check_call([self.adb, 'shell', 'kill', gdbserver_pid])
+    shell_pid = self._get_pid(self.package_name)
+    if shell_pid is None:
+      raise Exception('Unable to find a running mojo shell.')
+    subprocess.check_call([self.adb, 'forward', 'tcp:9999', 'tcp:9999'])
+    subprocess.Popen(
+        [self.adb, 'shell', 'gdbserver', '--attach', ':9999', shell_pid])
+    time.sleep(0.1)

[ppi, 2015/07/09 13:12:40]

Why? Please explain the sleep in a comment.

[etiennej, 2015/07/15 07:13:20]

Done.

+
+    read_process = subprocess.Popen(

[ppi, 2015/07/09 13:12:40]

reader_process?

[etiennej, 2015/07/15 07:13:20]

Done.

+        [self.adb, 'shell', config.REMOTE_FILE_READER_DEVICE_PATH],
+        stdout=subprocess.PIPE)
+    port = int(read_process.stdout.readline())
+    subprocess.check_call([self.adb, 'forward', 'tcp:10000', 'tcp:%d' % port])
+    self.rfc.connect()
+
+    gdb_execute('target remote localhost:9999')
+
+    self._update_symbols()
+    def on_stop(_):
+      self._update_symbols()
+    gdb.events.stop.connect(on_stop)