Makes android.py support gdb

mojo/tools/mopy/android.py

 # Copyright 2014 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 atexit
 import datetime
 import email.utils
 import hashlib
 import itertools
 import json
 import logging
 import math
 import os
 import os.path
 import random
+import shutil
 import subprocess
 import sys
+import tempfile
 import threading
 import time
 import urlparse
 
 import SimpleHTTPServer
 import SocketServer
 
 
 # Tags used by the mojo shell application logs.
 LOGCAT_TAGS = [
@@ skipping 144 matching lines @@
 
 
 def _ExitIfNeeded(process):
   """
   Exits |process| if it is still alive.
   """
   if process.poll() is None:
     process.kill()
 
 
+class Tmpdir(object):

[msw, 2015/05/22 02:41:43]

nit: Maybe just inline this in _WaitForProcessIdAndStartGdb:
  temp_dir = tempfile.mkdtemp()
  atexit.register(shutil.rmtree, temp_dir, True)

[sky, 2015/05/22 17:27:24]

Done.

+  """
+  Creates a temp directory that is deleted when either Destroy() is called, or
+  normal exit.
+  """
+  def __init__(self):
+    self.dir = tempfile.mkdtemp()
+    atexit.register(self.Destroy)
+
+  def Destroy(self):
+    if self.dir:
+      shutil.rmtree(self.dir, True)
+      self.dir = None
+
+
 class AndroidShell(object):
   """ Allows to set up and run a given mojo shell binary on an Android device.
 
   Args:
     shell_apk_path: path to the shell Android binary
     local_dir: directory where locally build Mojo apps will be served, optional
     adb_path: path to adb, optional if adb is in PATH
     target_device: device to run on, if multiple devices are connected
+    src_root: root of the source tree
   """
   def __init__(
       self, shell_apk_path, local_dir=None, adb_path="adb", target_device=None,
-      target_package=MOJO_SHELL_PACKAGE_NAME):
+      target_package=MOJO_SHELL_PACKAGE_NAME, src_root=None):
     self.shell_apk_path = shell_apk_path
+    self.src_root = src_root
     self.adb_path = adb_path
     self.local_dir = local_dir
     self.target_device = target_device
     self.target_package = target_package
 
   def _CreateADBCommand(self, args):
     adb_command = [self.adb_path]
     if self.target_device:
       adb_command.extend(['-s', self.target_device])
     adb_command.extend(args)
@@ skipping 100 matching lines @@
       return []
 
     original_values = list(itertools.chain(
         *map(lambda x: x[len(MAPPING_PREFIX):].split(','), map_parameters)))
     sorted(original_values)
     result = []
     for value in original_values:
       result.append(self._StartHttpServerForOriginMapping(value, 0))
     return [MAPPING_PREFIX + ','.join(result)]
 
-  def PrepareShellRun(self, origin=None):
+  def PrepareShellRun(self, origin=None, install=True):

[msw, 2015/05/22 02:41:43]

nit: The class member |shell_apk_path| isn't used elsewhere afaict. Maybe remove
that and change |install| to an optional |apk_to_install=None|.

[sky, 2015/05/22 17:27:24]

That would make the call sites more complex. I left this as is.

     """ Prepares for StartShell: runs adb as root and installs the apk.  If the
     origin specified is 'localhost', a local http server will be set up to serve
     files from the build directory along with port forwarding.
 
     Returns arguments that should be appended to shell argument list."""
     if 'cannot run as root' in subprocess.check_output(
         self._CreateADBCommand(['root'])):
       raise Exception("Unable to run adb as root.")
-    subprocess.check_call(
-        self._CreateADBCommand(['install', '-r', self.shell_apk_path, '-i',
-                                self.target_package]))
+    if install:
+      subprocess.check_call(
+          self._CreateADBCommand(['install', '-r', self.shell_apk_path, '-i',
+                                  self.target_package]))
     atexit.register(self.StopShell)
 
     extra_shell_args = []
     if origin is 'localhost':
       origin = self._StartHttpServerForDirectory(self.local_dir, 0)
     if origin:
       extra_shell_args.append("--origin=" + origin)
     return extra_shell_args
 
+  def PrepareGdb(self):
+    subprocess.check_call(self._CreateADBCommand(['forward', 'tcp:5039',

[msw, 2015/05/22 02:41:44]

nit: Could this be inlined in (or called from) StartShell?

[sky, 2015/05/22 17:27:24]

Done.

+                                                  'tcp:5039']))
+
+  def _GetProcessId(self, process):
+    """Returns the process id of the process on the remote device."""
+    while True:
+      line = process.stdout.readline()
+      pid_command = 'launcher waiting for GDB. pid: '
+      index = line.find(pid_command)
+      if index != -1:
+        return line[index + len(pid_command):].strip()
+    return 0
+
+  def _GetLocalGdbPath(self):

[msw, 2015/05/22 02:41:43]

nit: inline in caller?

[sky, 2015/05/22 17:27:24]

IMO the separate function makes this more readable.

+    """Returns the path to the android gdb."""
+    return os.path.join(self.src_root, "third_party", "android_tools", "ndk",
+                        "toolchains", "arm-linux-androideabi-4.9", "prebuilt",
+                        "linux-x86_64", "bin", "arm-linux-androideabi-gdb")
+
+  def _WaitForProcessIdAndStartGdb(self, process):
+    """Waits until we see the process id from the remote device, starts up
+    gdbserver on the remote device, and gdb on the local device."""
+    # Wait until we see "PID"
+    pid = self._GetProcessId(process)

[msw, 2015/05/22 02:41:44]

nit: maybe ensure this doesn't return 0?

[sky, 2015/05/22 17:27:24]

Done.

+    gdbserver_process = subprocess.Popen(self._CreateADBCommand(['shell',
+                                                                 'gdbserver',
+                                                                 '--attach',
+                                                                 ':5039',
+                                                                 pid]))
+    atexit.register(_ExitIfNeeded, gdbserver_process)

[msw, 2015/05/22 02:41:43]

Interesting, will this do the right thing if the process is being run on the
device?

[sky, 2015/05/22 17:27:24]

Earlier code makes sure the app isn't already running.

+
+    tmpdir = Tmpdir()
+    gdbinit_path = os.path.join(tmpdir.dir, 'gdbinit')
+    _CreateGdbInit(tmpdir.dir, gdbinit_path)
+
+    _CreateSOLinks(tmpdir.dir, self.local_dir)
+
+    # Wait a second for gdb to start up on the device.

[msw, 2015/05/22 02:41:43]

Hmm, I wonder how reliable this is, and what the failure looks like...

[sky, 2015/05/22 17:27:24]

No doubt it's flakey. I added a TODO to try a couple of times and then give up.

+    time.sleep(1)
+
+    local_gdb_process = subprocess.Popen([self._GetLocalGdbPath(),
+                                          "-x",
+                                          gdbinit_path],
+                                         cwd=tmpdir.dir)
+    atexit.register(_ExitIfNeeded, local_gdb_process)
+    local_gdb_process.wait()
+
   def StartShell(self,
                  arguments,
                  stdout=None,
-                 on_application_stop=None):
+                 on_application_stop=None,
+                 gdb=False):
     """
     Starts the mojo shell, passing it the given arguments.
 
     The |arguments| list must contain the "--origin=" arg from PrepareShellRun.
     If |stdout| is not None, it should be a valid argument for subprocess.Popen.
     """
     STDOUT_PIPE = "/data/data/%s/stdout.fifo" % self.target_package
 
     cmd = self._CreateADBCommand([
            'shell',
            'am',
            'start',
            '-S',
            '-a', 'android.intent.action.VIEW',
            '-n', '%s/%s.MojoShellActivity' % (self.target_package,
                                               MOJO_SHELL_PACKAGE_NAME)])
 
+    logcat_process = None
+
+    if gdb:
+      arguments += ['--wait-for-debugger']
+      logcat_process = self.ShowLogs(stdout=subprocess.PIPE)
+
     parameters = []
     if stdout or on_application_stop:
       subprocess.check_call(self._CreateADBCommand(
           ['shell', 'rm', '-f', STDOUT_PIPE]))
       parameters.append('--fifo-path=%s' % STDOUT_PIPE)
       max_attempts = 5
       if '--wait-for-debugger' in arguments:
         max_attempts = 200
       self._ReadFifo(STDOUT_PIPE, stdout, on_application_stop,
                      max_attempts=max_attempts)
 
     # Extract map-origin arguments.
     map_parameters, other_parameters = _Split(arguments, _IsMapOrigin)
     parameters += other_parameters
     parameters += self._StartHttpServerForOriginMappings(map_parameters)
 
     if parameters:
       encodedParameters = json.dumps(parameters)
       cmd += ['--es', 'encodedParameters', encodedParameters]
 
     with open(os.devnull, 'w') as devnull:
-      subprocess.Popen(cmd, stdout=devnull).wait()
+      cmd_process = subprocess.Popen(cmd, stdout=devnull)
+      if logcat_process:
+        self._WaitForProcessIdAndStartGdb(logcat_process)
+      cmd_process.wait()
 
   def StopShell(self):
     """
     Stops the mojo shell.
     """
     subprocess.check_call(self._CreateADBCommand(['shell',
                                                   'am',
                                                   'force-stop',
                                                   self.target_package]))
 
   def CleanLogs(self):
     """
     Cleans the logs on the device.
     """
     subprocess.check_call(self._CreateADBCommand(['logcat', '-c']))
 
-  def ShowLogs(self):
+  def ShowLogs(self, stdout=sys.stdout):
     """
     Displays the log for the mojo shell.
 
     Returns the process responsible for reading the logs.
     """
     logcat = subprocess.Popen(self._CreateADBCommand([
                                'logcat',
                                '-s',
                                ' '.join(LOGCAT_TAGS)]),
-                              stdout=sys.stdout)
+                              stdout=stdout)
     atexit.register(_ExitIfNeeded, logcat)
     return logcat
+

[msw, 2015/05/22 02:41:43]

nit: the rest of this file uses one blank line between functions, ditto below.

[sky, 2015/05/22 17:27:24]

That is true for class functions, but not top level functions (see line 163).

+
+def _CreateGdbInit(tmp_dir, gdb_init_path):
+  """
+  Creates the gdbinit file.
+  Args:
+    tmp_dir: the directory where the gdbinit and other files lives.
+    gdb_path: path to gdbinit

[msw, 2015/05/22 02:41:43]

nit: gdb_init_path

[sky, 2015/05/22 17:27:24]

Done.

+  """
+  gdbinit = ('target remote localhost:5039\n'

[msw, 2015/05/22 02:41:43]

How odd, I've never seen this before...

+             'def reload-symbols\n'
+             '  set solib-search-path %s\n'
+             'end\n'
+             'def info-symbols\n'
+             '  info sharedlibrary\n'
+             'end\n'
+             'echo \\n\\n'
+             'You are now in gdb and need to type continue (or c) to continue '
+             'execution.\\n'
+             'gdb is in the directory %s\\n'
+             'The following functions have been defined:\\n'
+             'reload-symbols: forces reloading symbols. If after a crash you\\n'
+             'still do not see symbols you likely need to create a link in\\n'
+             'the directory you are in.\\n'
+             'info-symbols: shows status of current shared libraries.\\n\\n' %
+             (tmp_dir, tmp_dir))
+  with open(gdb_init_path, 'w') as f:
+    f.write(gdbinit)
+
+
+def _CreateSOLinks(dest_dir, build_dir):
+  """
+  Creates links from files (such as mojo files) to the real .so so that gdb can
+  find them.
+  """
+  # The files to create links for. The key is the name as seen on the device,
+  # and the target an array of path elements as to where the .so lives (relative
+  # to the output directory).
+  files_to_link = {
+    'html_viewer.mojo': ['libhtml_viewer', 'html_viewer_library.so']

[msw, 2015/05/22 02:41:44]

nit: trailing comma

[sky, 2015/05/22 17:27:24]

Done.

+  }
+  for android_name, so_path in files_to_link.iteritems():
+    src = os.path.join(build_dir, *so_path)
+    if not os.path.isfile(src):
+      print 'Expected file not found', src
+      sys.exit(-1)
+    os.symlink(src, os.path.join(dest_dir, android_name))

[msw, 2015/05/22 02:41:43]

Do we need to clean these up at all?

[sky, 2015/05/22 17:27:24]

This is put in the directory that is removed by shutil.rmtree.