Make --gdb work for android

sky/tools/skydb

Index: sky/tools/skydb
diff --git a/sky/tools/skydb b/sky/tools/skydb
index 86c7243f9a491f112f992ebd4e785f12693f792a..b1c0e954faba7d4a00abea5e79d7247483781af7 100755
--- a/sky/tools/skydb
+++ b/sky/tools/skydb
@@ -17,6 +17,7 @@ import subprocess
 import sys
 import time
 import urlparse
+import re
 
 SRC_ROOT = skypy.paths.Paths('ignored').src_root
 sys.path.insert(0, os.path.join(SRC_ROOT, 'build', 'android'))
@@ -40,6 +41,7 @@ DEFAULT_URL = "https://raw.githubusercontent.com/domokit/mojo/master/sky/example
 ANDROID_PACKAGE = "org.chromium.mojo.shell"
 ANDROID_ACTIVITY = "%s/.MojoShellActivity" % ANDROID_PACKAGE
 
+
 # FIXME: Move this into mopy.config
 def gn_args_from_build_dir(build_dir):
     gn_cmd = [
@@ -97,7 +99,7 @@ class SkyDebugger(object):
             '--esa', 'parameters', ','.join(escaped_args),
         ]
 
-    def _build_mojo_shell_command(self, args):
+    def _build_mojo_shell_command(self, args, is_android):
         content_handlers = ['%s,%s' % (mime_type, 'mojo:sky_viewer')
             for mime_type in SUPPORTED_MIME_TYPES]
 
@@ -111,6 +113,14 @@ class SkyDebugger(object):
             'mojo:window_manager',
         ]
 
+        # Desktop-only work-around for mojo crashing under chromoting.
+        if not is_android and args.use_osmesa:
+            shell_args.append(
+                '--args-for=mojo:native_viewport_service --use-osmesa')
+
+        if is_android and args.gdb:
+            shell_args.append('--wait_for_debugger')
+
         if 'remote_sky_server_port' in self.pids:
             shell_command = self._wrap_for_android(shell_args)
         else:
@@ -139,6 +149,22 @@ class SkyDebugger(object):
         root_relative_build_dir = os.path.relpath(abs_build_dir, SRC_ROOT)
         return skypy.paths.Paths(root_relative_build_dir)
 
+    def _find_remote_pid_for_package(self, package):
+        ps_output = subprocess.check_output(['adb', 'shell', 'ps'])
+        for line in ps_output.split('\n'):
+            fields = line.split()
+            if fields and 'org' in fields[-1]:
+                print fields[-1]

[qsr, 2015/01/16 00:04:19]

Is that some debugging leftover?

[eseidel, 2015/01/16 00:14:06]

Yes!  Removed.

+            if fields and fields[-1] == package:
+                return fields[1]
+        return None
+
+    def _find_install_location_for_package(self, package):
+        pm_command = ['adb', 'shell', 'pm', 'path', package]
+        pm_output = subprocess.check_output(pm_command)
+        # e.g. package:/data/app/org.chromium.mojo.shell-1/base.apk
+        return pm_output.split(':')[-1]
+
     def start_command(self, args):
         # FIXME: Lame that we use self for a command-specific variable.
         self.paths = self._create_paths_for_build_dir(args.build_dir)
@@ -182,24 +208,50 @@ class SkyDebugger(object):
             ])
             self.pids['remote_sky_command_port'] = args.command_port
 
-        shell_command = self._build_mojo_shell_command(args)
-
-        if not is_android:
-            # Desktop-only work-around for mojo crashing under chromoting.
-            if args.use_osmesa:
-                shell_args.append(
-                    '--args-for=mojo:native_viewport_service --use-osmesa')
+        shell_command = self._build_mojo_shell_command(args, is_android)
 
-            # On android we can't launch inside gdb, but rather have to attach.
-            if args.gdb:
-                shell_command = ['gdbserver', ':%s' % GDB_PORT] + shell_command
+        # On android we can't launch inside gdb, but rather have to attach.
+        if not is_android and args.gdb:
+            shell_command = ['gdbserver', ':%s' % GDB_PORT] + shell_command

[qsr, 2015/01/16 00:04:19]

%d? Here and everywhere you format an integer

[eseidel, 2015/01/16 00:14:06]

OK. What's the benefit?

[qsr, 2015/01/16 00:46:40]

It will crash if this is not a number. Not much, but can catch some stupid
things.

 
         print ' '.join(map(pipes.quote, shell_command))
-        self.pids['mojo_shell_pid'] = subprocess.Popen(shell_command).pid
+        # This pid is meaningless on android (it's the adb shell pid)
+        start_command_pid = subprocess.Popen(shell_command).pid
+
+        if is_android:
+            # TODO(eseidel): am start -W does not seem to work?
+            pid_tries = 0
+            while True:
+                pid = self._find_remote_pid_for_package(ANDROID_PACKAGE)
+                if pid or pid_tries > 3:
+                    break
+                logging.warn('No pid for %s yet, waiting' % ANDROID_PACKAGE)
+                time.sleep(5)
+                pid_tries += 1
+
+            self.pids['mojo_shell_pid'] = pid
+            if not self.pids['mojo_shell_pid']:

[qsr, 2015/01/16 00:04:18]

Any reason not to write this:
if not pid:
  ...

 and do that before updating the dict?

[eseidel, 2015/01/16 00:14:06]

Done.

+                logging.error('Failed to find mojo_shell pid on device!')
+                return
+        else:
+            self.pids['mojo_shell_pid'] = start_command_pid
 
         if args.gdb and is_android:
-            gdbserver_cmd = ['gdbserver', '--attach', ':%s' % GDB_PORT]
-            self.pids['remote_gdbserver_pid'] = subprocess.Popen(shell_command).pid
+            # We push our own copy of gdbserver with the package since
+            # the default gdbserver is a different version from our gdb.
+            package_path = \
+                self._find_install_location_for_package(ANDROID_PACKAGE)
+            gdb_server_path = os.path.join(
+                os.path.dirname(package_path), 'lib/arm/gdbserver')
+            gdbserver_cmd = [
+                'adb', 'shell',
+                gdb_server_path, '--attach',
+                ':%s' % GDB_PORT,
+                str(self.pids['mojo_shell_pid'])
+            ]
+            print ' '.join(map(pipes.quote, gdbserver_cmd))
+            self.pids['adb_shell_gdbserver_pid'] = \
+                subprocess.Popen(gdbserver_cmd).pid

[qsr, 2015/01/16 00:04:19]

I am missing something here. Why are you transforming gdbserver_cmd into a
string and using the string version of Popen instead of using the array version?

[eseidel, 2015/01/16 00:14:06]

I'm only transforming it into a string for the print command (which I could
remove).  I'm passing the array to popen.

[qsr, 2015/01/16 00:46:40]

 That's what I was missing. Sorry.

 
             port_string = 'tcp:%s' % GDB_PORT
             subprocess.check_call([
@@ -230,9 +282,9 @@ class SkyDebugger(object):
     def stop_command(self, args):
         # TODO(eseidel): mojo_shell crashes when attempting graceful shutdown.
         # self._send_command_to_sky('/quit')
-        self._kill_if_exists('mojo_shell_pid', 'mojo_shell')
 
         self._kill_if_exists('sky_server_pid', 'sky_server')
+
         # We could be much more surgical here:
         if 'remote_sky_server_port' in self.pids:
             device = android_commands.AndroidCommands(
@@ -246,11 +298,18 @@ class SkyDebugger(object):
 
             subprocess.call([
                 'adb', 'shell', 'am', 'force-stop', ANDROID_PACKAGE])
+        else:
+            # Only try to kill mojo_shell if it's running locally.
+            self._kill_if_exists('mojo_shell_pid', 'mojo_shell')
 
         if 'remote_gdbserver_port' in self.pids:
+            self._kill_if_exists('adb_shell_gdbserver_pid', 'adb shell gdbserver')
+
             port_string = 'tcp:%s' % self.pids['remote_gdbserver_port']
             subprocess.call(['adb', 'forward', '--remove', port_string])
 
+        self._kill_if_exists('mojo_cache_linker_pid', 'mojo cache linker')
+
     def load_command(self, args):
         if not urlparse.urlparse(args.url_or_path).scheme:
             # The load happens on the remote device, use the remote port.
@@ -323,9 +382,61 @@ class SkyDebugger(object):
 
     def gdb_attach_command(self, args):
         self.paths = self._create_paths_for_build_dir(self.pids['build_dir'])
+
+        self._kill_if_exists('mojo_cache_linker_pid', 'mojo cache linker')
+
+        links_path = '/tmp/mojo_cache_links'
+        if not os.path.exists(links_path):
+            os.makedirs(links_path)

[qsr, 2015/01/16 00:04:18]

You do not seem to ever be cleaning this directory. Should you delete it on
startup?

[eseidel, 2015/01/16 00:14:06]

Unclear.  I could?  Originally I had the new script manage this directory, but
that seemed error-prone.  I certainly could have this script delete it and
create it fresh every time...

[qsr, 2015/01/16 00:46:40]

I fear this will quickly be full on unused files. Clearing it each time should
help.

+        shell_link_path = os.path.join(links_path, 'libmojo_shell.so')
+        if os.path.lexists(shell_link_path):
+            os.unlink(shell_link_path)
+        os.symlink(self.paths.mojo_shell_path, shell_link_path)
+
+        logcat_cmd = ['adb', 'logcat']
+        logcat = subprocess.Popen(logcat_cmd, stdout=subprocess.PIPE)
+
+        mojo_cache_linker_path = os.path.join(
+            self.paths.sky_tools_directory, 'mojo_cache_linker.py')
+        cache_linker_cmd = [
+            mojo_cache_linker_path,
+            links_path,
+            self.pids['build_dir'],
+            'http://localhost:%s' % self.pids['remote_sky_server_port']
+        ]
+        self.pids['mojo_cache_linker_pid'] = \
+            subprocess.Popen(cache_linker_cmd, stdin=logcat.stdout).pid
+
+        # TODO(eseidel): Need to sync down system libraries into a directory.
+        # For example, this is what adb_gdb uses:
+        # set print pretty 1
+        # python
+        # import sys
+        # sys.path.insert(0, '/src/mojo/src/tools/gdb/')
+        # try:
+        #   import gdb_chrome
+        # finally:
+        #   sys.path.pop(0)
+        # end
+        # file /tmp/eseidel-adb-gdb-tmp-19640/app_process
+        # directory /src/mojo/src
+        # set solib-absolute-prefix /tmp/eseidel-adb-gdb-libs
+        # set solib-search-path /tmp/eseidel-adb-gdb-libs/system:/tmp/eseidel-adb-gdb-libs/system/vendor:/tmp/eseidel-adb-gdb-libs/system/vendor/lib:/tmp/eseidel-adb-gdb-libs/system/vendor/lib/egl:/tmp/eseidel-adb-gdb-libs/system/bin:/tmp/eseidel-adb-gdb-libs/system/lib:/tmp/eseidel-adb-gdb-libs/system/lib/hw::/tmp/eseidel-adb-gdb-libs:/src/mojo/src/foo/Debug/lib
+        # echo Attaching and reading symbols, this may take a while..
+        # target remote :5039
+
+        symbol_search_paths = [
+            links_path,
+            self.pids['build_dir'],
+        ]
+        gdb_path = os.path.join(SRC_ROOT, 'third_party/android_tools/ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-gdb')

[qsr, 2015/01/16 00:04:19]

This should be 80 chars long... You also might want to separate all the
directory:
os.path.join(SRC_ROOT, a, b, c, d)...

[eseidel, 2015/01/16 00:14:06]

Eek!  I need to fix this lookup before landing. Thanks.

         gdb_command = [
-            '/usr/bin/gdb', self.paths.mojo_shell_path,
-            '--eval-command', 'target remote localhost:%s' % GDB_PORT
+            gdb_path,
+            '--eval-command', 'file %s' % self.paths.mojo_shell_path,
+            '--eval-command', 'directory %s' % self.paths.src_root,
+            '--eval-command', 'target remote localhost:%s' % GDB_PORT,
+            '--eval-command', 'set solib-search-path %s' %
+                ':'.join(symbol_search_paths),
         ]
         print " ".join(gdb_command)
         # We don't want python listenting for signals or anything, so exec