Only update symbols on current thread when breaking or stepping in GDB.

mojo/devtools/common/android_gdb/session.py

 # 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.
 
 """
 Manages a debugging session with GDB.
 
 This module is meant to be imported from inside GDB. Once loaded, the
 |DebugSession| attaches GDB to a running Mojo Shell process on an Android
 device using a remote gdbserver.
@@ skipping 172 matching lines @@
         # This library file is not known locally. Download it from the device or
         # the cloud and put it in cache so, if it got symbols, we can see them.
         local_file = os.path.join(self._remote_file_cache, signature)
         if not os.path.exists(local_file):
           tmp_output = self._download_file(signature, remote_file)
           shutil.move(tmp_output, local_file)
         self._associate_symbols(mapping, local_file)
       return True
     return False
 
-  def _update_symbols(self):
+  def _find_mappings_current_thread(self, mapped_files):

[ppi, 2015/07/23 23:08:31]

this method is called "find", but doesn't return anything? please add a comment
explaining what it does.

Maybe this should be called _map_symbols_in_current_thread?

[etiennej, 2015/07/23 23:21:03]

Changed the method name.

+      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
+
+  def update_symbols(self, current_thread_only):
     """Updates the mapping between symbols as seen from GDB and local library
-    files."""
+    files.
+
+    If current_thread_only is True, only update symbols for the current thread.
+    """
     logging.info("Updating symbols")
     mapped_files = _get_mapped_files()
-    _gdb_execute("info threads")
-    nb_threads = len(_gdb_execute("info threads").split("\n")) - 2
     # Map all symbols from native libraries packages with the APK.
     for file_mappings in mapped_files:
       filename = file_mappings[0].filename
       if ((filename.startswith('/data/data/') or
            filename.startswith('/data/app')) and
           not filename.endswith('.apk') and
           not filename.endswith('.dex')):
-        logging.info('Pre-mapping: %s' % file_mappings[0].filename)
-        self._try_to_map(file_mappings)
-    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:
-        traceback.print_exc()
+          logging.info('Pre-mapping: %s' % file_mappings[0].filename)

[ppi, 2015/07/23 23:08:31]

Why this shift to the right? Not sure which one is right, but I think the old
alingment was easier to read.

[etiennej, 2015/07/23 23:21:03]

Done.

+          self._try_to_map(file_mappings)
+
+    if current_thread_only:
+      self._find_mappings_current_thread(mapped_files)
+    else:
+      logging.info('Updating all threads\' symbols')
+      current_thread = gdb.selected_thread()
+      nb_threads = len(_gdb_execute("info threads").split("\n")) - 2
+      for i in xrange(nb_threads):
+        try:
+          _gdb_execute("thread %d" % (i + 1))
+          self._find_mappings_current_thread(mapped_files)
+        except gdb.error:
+          traceback.print_exc()
+      current_thread.switch()
 
   def _get_device_application_pid(self, application):
     """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
 
@@ skipping 20 matching lines @@
                              remote_file_reader_pid])
     self._remote_file_reader_process = subprocess.Popen(
         [self._adb, 'shell', config.REMOTE_FILE_READER_DEVICE_PATH],
         stdout=subprocess.PIPE, preexec_fn = os.setpgrp)
     port = int(self._remote_file_reader_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()
+    self.update_symbols(False)

[ppi, 2015/07/23 23:08:31]

nit: (current_thread_only=True), similarly below

[ppi, 2015/07/23 23:09:32]

I meant (current_thread_only=False).

[etiennej, 2015/07/23 23:21:03]

Done.

     def on_stop(_):
-      self._update_symbols()
+      self.update_symbols(True)
     gdb.events.stop.connect(on_stop)
     gdb.events.exited.connect(self.stop)
+
+    # Register the update-symbols command.
+    UpdateSymbols(self)
+
+class UpdateSymbols(gdb.Command):
+  """Command to update symbols loaded into GDB.
+
+  GDB usage: update-symbols [all|current]
+  """
+  _UPDATE_COMMAND = "update-symbols"
+
+  def __init__(self, session):
+    super(UpdateSymbols, self).__init__(self._UPDATE_COMMAND, gdb.COMMAND_STACK)
+    self._session = session
+
+  def invoke(self, arg, _unused_from_tty):
+    if arg == 'current':
+      self._session.update_symbols(True)

[ppi, 2015/07/23 23:08:31]

nit: (current_thread_only=True), similarly below

[etiennej, 2015/07/23 23:21:03]

Done.

+    else:
+      self._session.update_symbols(False)
+
+  def complete(self, text, _unused_word):
+    if text == self._UPDATE_COMMAND:
+      return ('all', 'current')
+    elif text in self._UPDATE_COMMAND + ' all':
+      return ['all']
+    elif text in self._UPDATE_COMMAND + ' current':
+      return ['current']
+    else:
+      return []