From a7a3147fd4b51db9c3fa77126f135e411ba1fe05 Mon Sep 17 00:00:00 2001 From: Eric Seidel Date: Fri, 23 Jan 2015 15:18:21 -0800 Subject: [PATCH] Fix skydb --gdb to always have symbols This replaces my previous --gdb work: https://codereview.chromium.org/848013004/ And obsoletes my build-id based attempt: https://codereview.chromium.org/788903011 Context: mojo_shell downloads arbitrary binaries from urls copying them to temp files before calling dlopen. Because the names it used were random this broke gdb, pprof, etc. tools which wanted to make address -> symbol translations based on the library load path. The major thing this change does is move away from the previous method of watching the logs of mojo_shell for 'Caching %url as %file...' messages or the /tmp/mojo_shell.%pid.map file to having mojo_shell use a priori knowable names for all of its library loads. Thus we can similarly build a directory of correctly named symboled binaries corresponding to the expected load names of libraries. This change does this in 3 pieces: 1. Introduces the concept of 'app ids' (which are currently just the md5 of the distributed app binary) and teaches dynamic_application_loader to rename all apps to APP_ID.mojo before loading them. This has the nice side-effect of always loading an app with a dlopen/library name which is both unique to the application as well as predictable. 2. Re-writes the mojo_cache_linker script to no longer watch stdin/adb logcat for 'caching...' messages and instead build a links directory based on pre-determined app_ids (md5s) linking back to the symboled binaries. 3. Remove a bunch of the former mojo_cache_linker calling code which is no longer needed now that the library_names of loaded application .so's are predictable before launching mojo_shell I'm happy to make app_ids fancier, originally I was going to use ELF's build-id's directly: https://codereview.chromium.org/788903011 but unfortunately gdbserver does not know how to do a build-id lookup on the serverside: https://groups.google.com/a/google.com/forum/#!topic/gdb-discuss/Fd0R-gFaqXk R=aa@chromium.org, abarth@chromium.org Review URL: https://codereview.chromium.org/866383004 --- engine/src/flutter/tools/mojo_cache_linker.py | 66 +++++++++--------- engine/src/flutter/tools/skydb | 67 +++++++++---------- 2 files changed, 65 insertions(+), 68 deletions(-) diff --git a/engine/src/flutter/tools/mojo_cache_linker.py b/engine/src/flutter/tools/mojo_cache_linker.py index 4592eba39a..4636c9b943 100755 --- a/engine/src/flutter/tools/mojo_cache_linker.py +++ b/engine/src/flutter/tools/mojo_cache_linker.py @@ -6,59 +6,59 @@ import argparse import logging import os -import re import sys +import subprocess -# TODO(eseidel): This should be shared with tools/android_stack_parser/stack -# TODO(eseidel): This could be replaced by using build-ids on Android -# TODO(eseidel): mojo_shell should write out a cache mapping file. +# TODO(eseidel): Share logic with tools/android_stack_parser/stack def main(): logging.basicConfig(level=logging.INFO) parser = argparse.ArgumentParser( - description='Watches mojo_shell logcat output and builds a directory ' - 'of symlinks to symboled binaries for seen cache names.') + description='Builds a directory of app_id symlinks to symbols' + ' to match expected dlopen names from mojo_shell\'s NetworkLoader.') parser.add_argument('links_dir', type=str) - parser.add_argument('symbols_dir', type=str) - parser.add_argument('base_url', type=str) + parser.add_argument('build_dir', type=str) args = parser.parse_args() - regex = re.compile('Caching mojo app (?P\S+) at (?P\S+)') - if not os.path.isdir(args.links_dir): logging.fatal('links_dir: %s is not a directory' % args.links_dir) sys.exit(1) - for line in sys.stdin: - result = regex.search(line) - if not result: + for name in os.listdir(args.build_dir): + path = os.path.join(args.build_dir, name) + if not os.path.isfile(path): continue - url = result.group('url') - if not url.startswith(args.base_url): - logging.debug('%s does not match base %s' % (url, args.base_url)) - continue - full_name = os.path.basename(url) - name, ext = os.path.splitext(full_name) - if ext != '.mojo': - logging.debug('%s is not a .mojo library' % url) + # md5sum is slow, so only bother for suffixes we care about: + basename, ext = os.path.splitext(name) + if ext not in ('', '.mojo', '.so'): continue - symboled_name = 'lib%s_library.so' % name - cache_link_path = os.path.join(args.links_dir, - os.path.basename(result.group('path'))) - symboled_path = os.path.realpath( - os.path.join(args.symbols_dir, symboled_name)) - if not os.path.isfile(symboled_path): - logging.warn('symboled path %s does not exist' % symboled_path) + # Ignore ninja's dot-files. + if basename.startswith('.'): continue - print "%s -> %s" % (cache_link_path, symboled_path) + # Example output: + # f82a3551478a9a0e010adccd675053b9 png_viewer.mojo + md5 = subprocess.check_output(['md5sum', path]).strip().split()[0] + link_path = os.path.join(args.links_dir, '%s.mojo' % md5) - if os.path.lexists(cache_link_path): - logging.debug('link already exists %s, replacing' % symboled_path) - os.unlink(cache_link_path) + lib_path = os.path.realpath(os.path.join(args.build_dir, name)) - os.symlink(symboled_path, cache_link_path) + # On android foo.mojo is stripped, but libfoo_library.so is not. + if ext == '.mojo': + symboled_name = 'lib%s_library.so' % basename + symboled_path = os.path.realpath( + os.path.join(args.build_dir, symboled_name)) + if os.path.exists(symboled_path): + lib_path = symboled_path + + print "%s -> %s" % (link_path, lib_path) + + if os.path.lexists(link_path): + logging.debug('link already exists %s, replacing' % lib_path) + os.unlink(link_path) + + os.symlink(lib_path, link_path) if __name__ == '__main__': main() diff --git a/engine/src/flutter/tools/skydb b/engine/src/flutter/tools/skydb index eb13caab5e..69e4b932d0 100755 --- a/engine/src/flutter/tools/skydb +++ b/engine/src/flutter/tools/skydb @@ -40,6 +40,7 @@ DEFAULT_URL = "https://raw.githubusercontent.com/domokit/mojo/master/sky/example ANDROID_PACKAGE = "org.chromium.mojo.shell" ANDROID_ACTIVITY = "%s/.MojoShellActivity" % ANDROID_PACKAGE +ANDROID_APK_NAME = 'MojoShell.apk' CACHE_LINKS_PATH = '/tmp/mojo_cache_links' SYSTEM_LIBS_ROOT_PATH = '/tmp/device_libs' @@ -195,6 +196,11 @@ class SkyDebugger(object): # Pray to the build/android gods in their misspelled tongue. constants.SetOutputDirectort(self.paths.build_dir) + # We could make installing conditional on an argument. + apk_path = os.path.join(self.paths.build_dir, 'apks', + ANDROID_APK_NAME) + subprocess.check_call(['adb', 'install', '-r', apk_path]) + device = self._connect_to_device() self.pids['device_serial'] = device.GetDevice() @@ -311,8 +317,6 @@ class SkyDebugger(object): subprocess.call(['adb', 'forward', '--remove', port_string]) self.pids = {} # Clear out our pid file. - 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. @@ -425,29 +429,6 @@ class SkyDebugger(object): ] subprocess.call(['adb', 'logcat', '-d', '-s'] + TAGS) - def _start_mojo_cache_linker(self, links_path): - self._kill_if_exists('mojo_cache_linker_pid', 'mojo cache linker') - - if not os.path.exists(links_path): - os.makedirs(links_path) - 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'] - ] - return subprocess.Popen(cache_linker_cmd, stdin=logcat.stdout).pid - def _pull_system_libraries(self, system_libs_root): # Pull down the system libraries this pid has already mapped in. # TODO(eseidel): This does not handle dynamic loads. @@ -466,30 +447,45 @@ class SkyDebugger(object): '-type', 'd', ]).strip().split('\n') + def _add_android_library_links(self, links_path): + # TODO(eseidel): This might not match mojo_shell on the device? + # TODO(eseidel): Should we pass libmojo_shell.so as 'file' to gdb? + 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) def gdb_attach_command(self, args): self.paths = self._create_paths_for_build_dir(self.pids['build_dir']) - symbol_search_paths = [self.pids['build_dir']] + if not os.path.exists(CACHE_LINKS_PATH): + os.makedirs(CACHE_LINKS_PATH) + cache_linker_path = os.path.join( + self.paths.sky_tools_directory, 'mojo_cache_linker.py') + subprocess.check_call([ + cache_linker_path, CACHE_LINKS_PATH, self.paths.build_dir]) + + symbol_search_paths = [ + self.pids['build_dir'], + CACHE_LINKS_PATH, + ] gdb_path = '/usr/bin/gdb' - init_commands = [ - 'file %s' % self.paths.mojo_shell_path, + eval_commands = [ 'directory %s' % self.paths.src_root, + 'file %s' % self.paths.mojo_shell_path, 'target remote localhost:%s' % GDB_PORT, ] # A bunch of extra work is needed for android: if 'remote_sky_server_port' in self.pids: - pid = self._start_mojo_cache_linker(CACHE_LINKS_PATH) - self.pids['mojo_cache_linker_pid'] = pid + self._add_android_library_links(CACHE_LINKS_PATH) system_lib_dirs = self._pull_system_libraries(SYSTEM_LIBS_ROOT_PATH) - init_commands.append( + eval_commands.append( 'set solib-absolute-prefix %s' % SYSTEM_LIBS_ROOT_PATH) symbol_search_paths = system_lib_dirs + symbol_search_paths - symbol_search_paths.append(CACHE_LINKS_PATH) # TODO(eseidel): We need to look up the toolchain somehow? gdb_path = os.path.join(SRC_ROOT, 'third_party/android_tools/ndk/' @@ -497,12 +493,13 @@ class SkyDebugger(object): 'bin/arm-linux-androideabi-gdb') # Set solib-search-path after letting android modify symbol_search_paths - init_commands.append( + eval_commands.append( 'set solib-search-path %s' % ':'.join(symbol_search_paths)) - exec_command = [gdb_path] - for command in init_commands: + exec_command = ['/usr/bin/strace', '-o', 'trace.txt', gdb_path] + for command in eval_commands: exec_command += ['--eval-command', command] + print " ".join(exec_command) # Write out our pid file before we exec ourselves.