Makes android.py support gdb

mojo/tools/mopy/android.py

Index: mojo/tools/mopy/android.py
diff --git a/mojo/tools/mopy/android.py b/mojo/tools/mopy/android.py
index 2354c8e5f9138fa1f63283ce97786cf21d4ecf0b..f74c561e12adb1605e06f68fe3f6e1b890818d5e 100644
--- a/mojo/tools/mopy/android.py
+++ b/mojo/tools/mopy/android.py
@@ -13,8 +13,10 @@ import math
 import os
 import os.path
 import random
+import shutil
 import subprocess
 import sys
+import tempfile
 import threading
 import time
 import urlparse
@@ -179,6 +181,21 @@ def _ExitIfNeeded(process):
     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.
 
@@ -187,11 +204,13 @@ class AndroidShell(object):
     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
@@ -312,7 +331,7 @@ class AndroidShell(object):
       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.
@@ -321,9 +340,10 @@ class AndroidShell(object):
     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 = []
@@ -333,10 +353,59 @@ class AndroidShell(object):
       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.
 
@@ -354,6 +423,12 @@ class AndroidShell(object):
            '-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(
@@ -375,7 +450,10 @@ class AndroidShell(object):
       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):
     """
@@ -392,7 +470,7 @@ class AndroidShell(object):
     """
     subprocess.check_call(self._CreateADBCommand(['logcat', '-c']))
 
-  def ShowLogs(self):
+  def ShowLogs(self, stdout=sys.stdout):
     """
     Displays the log for the mojo shell.
 
@@ -402,6 +480,53 @@ class AndroidShell(object):
                                '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.