Bug 1483913 - prettify gdb unwinder comments; r=nbp
authorTom Tromey <tom@tromey.com>
Mon, 14 Jan 2019 17:09:23 +0000
changeset 510929 d8a71e4402fc20fba8095a49fb96db7592cdb3e8
parent 510928 36b1b39a97754dc150db4d96749b59efb510ee6e
child 510930 bd071dbcecad945f22ef53c976e312d3a2ee0c21
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp
bugs1483913, 1464869
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1483913 - prettify gdb unwinder comments; r=nbp Bug 1464869 resulted in blank lines being inserted between comments and the classes they describe in the gdb unwinder code. This patch changes most of these comments into doc strings instead. MozReview-Commit-ID: 6XQwheUNRxI Differential Revision: https://phabricator.services.mozilla.com/D7174
js/src/gdb/mozilla/unwind.py
--- a/js/src/gdb/mozilla/unwind.py
+++ b/js/src/gdb/mozilla/unwind.py
@@ -115,25 +115,24 @@ class UnwinderTypeCache(object):
             # Strip off "js::jit::".
             name = field.name[9:]
             enumval = long(field.enumval)
             self.d[name] = enumval
             self.frame_enum_names[enumval] = name
             class_type = gdb.lookup_type('js::jit::' + SizeOfFramePrefix[name])
             self.frame_class_types[enumval] = class_type.pointer()
 
-# gdb doesn't have a direct way to tell us if a given address is
-# claimed by some shared library or the executable.  See
-# https://sourceware.org/bugzilla/show_bug.cgi?id=19288
-# In the interest of not requiring a patched gdb, instead we read
-# /proc/.../maps.  This only works locally, but maybe could work
-# remotely using "remote get".  FIXME.
-
 
 def parse_proc_maps():
+    # gdb doesn't have a direct way to tell us if a given address is
+    # claimed by some shared library or the executable.  See
+    # https://sourceware.org/bugzilla/show_bug.cgi?id=19288
+    # In the interest of not requiring a patched gdb, instead we read
+    # /proc/.../maps.  This only works locally, but maybe could work
+    # remotely using "remote get".  FIXME.
     mapfile = '/proc/' + str(gdb.selected_inferior().pid) + '/maps'
     # Note we only examine executable mappings here.
     matcher = re.compile("^([a-fA-F0-9]+)-([a-fA-F0-9]+)\s+..x.\s+\S+\s+\S+\s+\S*(.*)$")
     mappings = []
     with open(mapfile, "r") as inp:
         for line in inp:
             match = matcher.match(line)
             if not match:
@@ -143,36 +142,36 @@ def parse_proc_maps():
             end = match.group(2)
             name = match.group(3).strip()
             if name is '' or (name.startswith('[') and name is not '[vdso]'):
                 # Skip entries not corresponding to a file.
                 continue
             mappings.append((long(start, 16), long(end, 16)))
     return mappings
 
-# A symbol/value pair as expected from gdb frame decorators.
-
 
 class FrameSymbol(object):
+    "A symbol/value pair as expected from gdb frame decorators."
+
     def __init__(self, sym, val):
         self.sym = sym
         self.val = val
 
     def symbol(self):
         return self.sym
 
     def value(self):
         return self.val
 
-# This represents a single JIT frame for the purposes of display.
-# That is, the frame filter creates instances of this when it sees a
-# JIT frame in the stack.
-
 
 class JitFrameDecorator(FrameDecorator):
+    """This represents a single JIT frame for the purposes of display.
+    That is, the frame filter creates instances of this when it sees a
+    JIT frame in the stack."""
+
     def __init__(self, base, info, cache):
         super(JitFrameDecorator, self).__init__(base)
         self.info = info
         self.cache = cache
 
     def _decode_jitframe(self, this_frame):
         calleetoken = long(this_frame['calleeToken_'])
         tag = calleetoken & 3
@@ -253,20 +252,20 @@ class JitFrameDecorator(FrameDecorator):
             # anything better to do.
             if i == 0:
                 name = 'this'
             else:
                 name = 'arg%d' % i
             result.append(FrameSymbol(name, args_ptr[i]))
         return result
 
-# A frame filter for SpiderMonkey.
-
 
 class SpiderMonkeyFrameFilter(object):
+    "A frame filter for SpiderMonkey."
+
     # |state_holder| is either None, or an instance of
     # SpiderMonkeyUnwinder.  If the latter, then this class will
     # reference the |unwinder_state| attribute to find the current
     # unwinder state.
     def __init__(self, cache, state_holder):
         self.name = "SpiderMonkey"
         self.enabled = True
         self.priority = 100
@@ -280,41 +279,41 @@ class SpiderMonkeyFrameFilter(object):
         info = self.state_holder.unwinder_state.get_frame(base)
         if info is None:
             return frame
         return JitFrameDecorator(frame, info, self.cache)
 
     def filter(self, frame_iter):
         return imap(self.maybe_wrap_frame, frame_iter)
 
-# A frame id class, as specified by the gdb unwinder API.
-
 
 class SpiderMonkeyFrameId(object):
+    "A frame id class, as specified by the gdb unwinder API."
+
     def __init__(self, sp, pc):
         self.sp = sp
         self.pc = pc
 
-# This holds all the state needed during a given unwind.  Each time a
-# new unwind is done, a new instance of this class is created.  It
-# keeps track of all the state needed to unwind JIT frames.  Note that
-# this class is not directly instantiated.
-#
-# This is a base class, and must be specialized for each target
-# architecture, both because we need to use arch-specific register
-# names, and because entry frame unwinding is arch-specific.
-# See https://sourceware.org/bugzilla/show_bug.cgi?id=19286 for info
-# about the register name issue.
-#
-# Each subclass must define SP_REGISTER, PC_REGISTER, and
-# SENTINEL_REGISTER (see x64UnwinderState for info); and implement
-# unwind_entry_frame_registers.
-
 
 class UnwinderState(object):
+    """This holds all the state needed during a given unwind.  Each time a
+    new unwind is done, a new instance of this class is created.  It
+    keeps track of all the state needed to unwind JIT frames.  Note that
+    this class is not directly instantiated.
+
+    This is a base class, and must be specialized for each target
+    architecture, both because we need to use arch-specific register
+    names, and because entry frame unwinding is arch-specific.
+    See https://sourceware.org/bugzilla/show_bug.cgi?id=19286 for info
+    about the register name issue.
+
+    Each subclass must define SP_REGISTER, PC_REGISTER, and
+    SENTINEL_REGISTER (see x64UnwinderState for info); and implement
+    unwind_entry_frame_registers."""
+
     def __init__(self, typecache):
         self.next_sp = None
         self.next_type = None
         self.activation = None
         # An unwinder instance is specific to a thread.  Record the
         # selected thread for later verification.
         self.thread = gdb.selected_thread()
         self.frame_map = {}
@@ -500,20 +499,20 @@ class UnwinderState(object):
             if self.next_type == self.typecache.CppToJSJit:
                 return self.unwind_entry_frame(pc, pending_frame)
             return self.unwind_ordinary(pc, pending_frame)
         # Maybe we've found an exit frame.  FIXME I currently don't
         # know how to identify these precisely, so we'll just hope for
         # the time being.
         return self.unwind_exit_frame(pc, pending_frame)
 
-# The UnwinderState subclass for x86-64.
-
 
 class x64UnwinderState(UnwinderState):
+    "The UnwinderState subclass for x86-64."
+
     SP_REGISTER = 'rsp'
     PC_REGISTER = 'rip'
 
     # A register unique to this architecture, that is also likely to
     # have been saved in any frame.  The best thing to use here is
     # some arch-specific name for PC or SP.
     SENTINEL_REGISTER = 'rip'
 
@@ -529,22 +528,22 @@ class x64UnwinderState(UnwinderState):
         sp = sp + 1
         for reg in self.PUSHED_REGS:
             data = sp.dereference()
             sp = sp + 1
             unwind_info.add_saved_register(reg, data)
             if reg is "rbp":
                 unwind_info.add_saved_register(self.SP_REGISTER, sp)
 
-# The unwinder object.  This provides the "user interface" to the JIT
-# unwinder, and also handles constructing or destroying UnwinderState
-# objects as needed.
-
 
 class SpiderMonkeyUnwinder(Unwinder):
+    """The unwinder object.  This provides the "user interface" to the JIT
+    unwinder, and also handles constructing or destroying UnwinderState
+    objects as needed."""
+
     # A list of all the possible unwinders.  See |self.make_unwinder|.
     UNWINDERS = [x64UnwinderState]
 
     def __init__(self, typecache):
         super(SpiderMonkeyUnwinder, self).__init__("SpiderMonkey")
         self.typecache = typecache
         self.unwinder_state = None
 
@@ -596,21 +595,21 @@ class SpiderMonkeyUnwinder(Unwinder):
             self.unwinder_state = self.make_unwinder(pending_frame)
         if not self.unwinder_state:
             return None
         return self.unwinder_state.unwind(pending_frame)
 
     def invalidate_unwinder_state(self, *args, **kwargs):
         self.unwinder_state = None
 
-# Register the unwinder and frame filter with |objfile|.  If |objfile|
-# is None, register them globally.
-
 
 def register_unwinder(objfile):
+    """Register the unwinder and frame filter with |objfile|.  If |objfile|
+    is None, register them globally."""
+
     type_cache = UnwinderTypeCache()
     unwinder = None
     # This currently only works on Linux, due to parse_proc_maps.
     if _have_unwinder and platform.system() == "Linux":
         unwinder = SpiderMonkeyUnwinder(type_cache)
         gdb.unwinder.register_unwinder(objfile, unwinder, replace=True)
     # We unconditionally register the frame filter, because at some
     # point we'll add interpreter frame filtering.