[Findit] Plain objects to represent and parse stack trace.

tools/findit/stacktrace.py

+# Copyright (c) 2014 The Chromium Authors. All rights reserved.

[stgao, 2014/08/01 17:22:21]

It might be better to re-order the classes in this file, as I pointed out in my
code review.

[jeun, 2014/08/06 18:22:59]

Reorder the classes as in reorder the order of imports?

+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import os
+import re
+import crash_utils

[stgao, 2014/08/01 17:22:21]

One blank line between groups of imports.
For grouping, please refer to
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Imports_format...

[jeun, 2014/08/06 18:23:00]

Done.

+
+
+class Stacktrace(object):
+  """Represents Stacktrace object.
+
+  Contains a list of callstacks, because one stacktrace might have more than
+  one callstacks.
+  """
+
+  def __init__(self, stacktrace, chrome_build):
+    self.stack_list = []
+    self.ParseStacktrace(stacktrace, chrome_build)
+
+  def ParseStacktrace(self, stacktrace, chrome_build):

[stgao, 2014/08/01 17:22:21]

I'm OK if you don't want to create a parser class for the parsing logic.

If CF folks point it out, then we'd better do that as in the comment of my first
code review.

[jeun, 2014/08/06 18:23:00]

Acknowledged.

+    """Parses stacktrace and normalizes it.
+
+    If there are multiple callstacks within the stacktrace,
+    it will parse each of them separately, and store them in the stack_list
+    variable.
+
+    Args:
+      stacktrace: a string containing stacktrace
+      chrome_build: a string containing the job type of the crash

[stgao, 2014/08/01 17:22:21]

Could we reword "job type" to "build type"?  Same for other occurrences.

[jeun, 2014/08/06 18:22:59]

Done.

+    """
+    # If the passed in string is empty, the object does not represent anything.
+    if not stacktrace:
+      self.stack_list = None
+      return
+
+    # Reset the stack list, and assume we start from main thread.
+    self.stack_list = []
+    stack_priority = 0
+    reached_new_callstack = False
+    # Note that we do not need exact stack frame index, we only need relative
+    # position of a frame within a callstack. The reason for not extracting
+    # index from a line is that some stack frames do not have index.
+    stack_frame_index = 0
+    current_stack = CallStack(stack_priority)
+
+    for line in stacktrace:
+      # Check if current line is the start of new callstack
+      is_new_callstack = self.CheckIfNewStackFrame(line, chrome_build)

[stgao, 2014/08/01 17:22:21]

"CheckIfNewStackFrame" is not that clear. Based on the name, we are looking for
a new stack frame. However, we are checking for a new stack.

[jeun, 2014/08/06 18:22:59]

Done.

+      if is_new_callstack:
+
+        # If this callstack is from main thread, update the boolean.

[stgao, 2014/08/01 17:22:21]

Comment needs updating.

Please make sure comments are up-to-date after you update code during code
review.

[jeun, 2014/08/06 18:22:59]

I think this comment is correct. If it is new call stack and we have not reached
the new callstack, it means that this callstack is from main thread.

+        if not reached_new_callstack:
+          reached_new_callstack = True
+
+        # If this is from freed or prev-alloc, add the callstack we have
+        # to the list of callstacks, and increment the stack priority.
+        else:
+          stack_frame_index = 0
+          self.stack_list.append(current_stack)
+          stack_priority += 1
+          current_stack = CallStack(stack_priority)
+
+      # Parse function name, file path and line number from the line.
+      parsed_stack_frame_line = self.__ExtractFromStackFrame(
+          line, chrome_build)
+
+      # If the line is malformed, ignore this line. Else, get the info.
+      if not parsed_stack_frame_line:
+        continue
+
+      (function, file_path, crashed_line_number) = parsed_stack_frame_line
+
+      # Normalize the file path so that it can be compared to repository path.
+      file_name = os.path.basename(file_path)
+      (component, file_path) = crash_utils.NormalizePathLinux(file_path)
+
+      # Currently supports only blink and chromium.
+      if component == 'blink' or component == 'chromium':
+        current_stack.Add(
+            StackFrame(stack_frame_index, component, file_name,
+                       function, file_path, crashed_line_number))
+
+      stack_frame_index += 1
+
+    self.stack_list.append(current_stack)

[stgao, 2014/08/01 17:22:21]

bug: if there is no frame found, an empty stack (initialized in line 45) will be
added.

[jeun, 2014/08/06 18:23:00]

Done.

+
+  def CheckIfNewStackFrame(self, line, chrome_build):

[stgao, 2014/08/01 17:22:21]

Will this function be used outside of this class?

[jeun, 2014/08/06 18:23:00]

Renamed with two underscores.

+    """Check if this line is the start of the new stack frame.

[stgao, 2014/08/01 17:22:21]

why new stack frame?

[jeun, 2014/08/06 18:23:00]

changed to callstack

+
+    Since each builds have different syntax of stacktrace, the logic for

[stgao, 2014/08/01 17:22:21]

syntax -> format?

[jeun, 2014/08/06 18:23:00]

Done.

+    checking the line for all builds is handled in here.
+
+    Args:
+      line: line to check for
+      chrome_build: the name of the build
+
+    Returns:
+      True if the line is the start of new callstack, False otherwise
+    """
+    # Currently not supported
+    if 'android' in chrome_build:
+      return False
+
+    if 'syzyasan' in chrome_build:
+      # In syzyasan build, new stack starts with 'crash stack:',
+      # 'freed stack:', etc
+      callstack_start_pattern = re.compile(r'^.* stack:$')
+      return callstack_start_pattern.match(line)
+
+    # Stack frame pattern for asan, lsan, tsan, etc
+    stack_frame_index_pattern = re.compile(r'#(\d+)')
+
+    # Match the line to see if it is a valid stack frame.
+    stack_frame_index_match = stack_frame_index_pattern.match(line)
+
+    # If this line does not represent the crashing information, continue.

[stgao, 2014/08/01 17:22:20]

continue?

[jeun, 2014/08/06 18:23:00]

changed to return false.

+    if not stack_frame_index_match:
+      return False
+
+    # Get index from the frame. If the index is 0, it means that new callstack
+    # starts in this line.
+    stack_frame_index = int(stack_frame_index_match.group(1))
+    return stack_frame_index == 0
+
+  def __ExtractFromStackFrame(self, line, chrome_build):
+    """Extracts information from a line in stacktrace.
+
+    Args:
+      line: a stacktrace string to extract data from
+      chrome_build: a string containing the job type
+               of this crash (e.g. linux_asan_chrome_mp)
+
+    Returns:
+      A triple containing the name of the function, the path of the file and
+      the line of crash

[stgao, 2014/08/01 17:22:20]

line content or line number?

[jeun, 2014/08/06 18:23:00]

changed to crashed line number.

+    """
+    line_parts = line.split()
+    try:
+      # tsan has different stack frame style from other builds.
+      if chrome_build.startswith('linux_tsan'):
+        file_path_and_line = line_parts[-2]
+        function = ' '.join(line_parts[1:-2])
+
+      else:
+        file_path_and_line = line_parts[-1]
+        function = ' '.join(line_parts[3:-1])
+
+      # Get file path and line info from the line.
+      file_path_and_line = file_path_and_line.split(':')
+      file_path = file_path_and_line[0]
+      crashed_line_number = int(file_path_and_line[1])
+      return (function, file_path, crashed_line_number)
+
+    # Return None if the line is malformed.
+    except IndexError:
+      return None
+    except ValueError:
+      return None
+
+  def __getitem__(self, index):
+    return self.stack_list[index]
+
+  def GetStackFromMainThread(self):

[stgao, 2014/08/01 17:22:21]

Is it true that the first callstack is from the main thread for all build types?

[jeun, 2014/08/06 18:23:00]

I believe this is a valid assumption, from the stacktraces I have seen.

+    return self.stack_list[0]
+
+
+class CallStack(object):
+  """Represents a call stack within a stacktrace.
+
+  It is a list of StackFrame object, and the stack represented by
+  this object is from main thread, freed thread or previously-allocated thread.
+  """
+
+  def __init__(self, stack_priority):
+    self.frame_list = []
+    self.priority = stack_priority
+
+  def Add(self, stacktrace_line):
+    self.frame_list.append(stacktrace_line)
+
+  def GetFirstN(self, n):
+    return self.frame_list[:n]
+
+
+class StackFrame(object):
+  """Represents a line in stacktrace.

[stgao, 2014/08/01 17:22:21]

"a frame in callstack"?

[jeun, 2014/08/06 18:22:59]

Done.

+
+  Attributes:
+    index: index in the stacktrace
+    component: a component this line represents, such as blink, chrome, etc.
+    file_name: name of the file that crashed
+    function: function that caused the crash
+    file_path: path of the crashed file
+    crashed_line_number: line of the file that caused the crash
+  """
+
+  def __init__(self, stack_frame_index, component, file_name,
+               function, file_path, crashed_line_number):
+    self.index = stack_frame_index
+    self.component = component
+    self.file_name = file_name
+    self.function = function
+    self.file_path = file_path
+    self.crashed_line_number = crashed_line_number