[Predator] Add clusterfuzz callstack filters.

appengine/findit/crash/callstack_filters.py

Index: appengine/findit/crash/callstack_filters.py
diff --git a/appengine/findit/crash/callstack_filters.py b/appengine/findit/crash/callstack_filters.py
index 0ba96a5dc363f93410d03932a3a4976affe2da11..324e28d3732287dafc2d15d85dd3673f777aaaeb 100644
--- a/appengine/findit/crash/callstack_filters.py
+++ b/appengine/findit/crash/callstack_filters.py
@@ -4,12 +4,29 @@
 
 import re
 
+from crash.stacktrace import CallStack
+from crash.type_enums import LanguageType
+
 _INLINE_FUNCTION_FILE_PATH_MARKERS = [
     'third_party/llvm-build/Release+Asserts/include/c++/v1/',
     'linux/debian_wheezy_amd64-sysroot/usr/include/c++/4.6/bits/',
     'eglibc-3GlaMS/eglibc-2.19/sysdeps/unix/',
 ]
 
+# When checking the null pointer deference, any deference of a address within

[wrengr, 2016/12/19 23:51:05]

"deference" -> "dereference"

"a address" -> "an address"

[Sharu Jiang, 2016/12/22 01:59:43]

Done.

+# the threshold is considered as null pointer deference. Be consistent with

[wrengr, 2016/12/19 23:51:05]

ditto

[Sharu Jiang, 2016/12/22 01:59:43]

Done.

+# the threshold used in Clusterfuzz.
+NULL_POINTER_DEREFERENCE_THRESHOLD = 4096
+V8_JIT_CODE_MARKER = 'v8::internal::Invoke'
+V8_API_H_FILE_PATH = 'src/api.h'
+V8_API_CC_FILE_PATH = 'src/api.cc'

[stgao, 2016/12/20 19:07:36]

Could we make a list instead of two separate files?

[Sharu Jiang, 2016/12/22 01:59:43]

Actually making it seperate would make the implementation shorter, or we will
need to write for loop and it takes more lines...

+V8_DEP_PATH_MARKER = 'src/v8'
+BLINK_BINDINGS_GENERATED_PATH_REGEX = re.compile(
+    r'out/[^/]+/gen/blink/bindings/.*')
+JAVA_JRE_SDK_REGEX = re.compile(
+    r'(java\..*|javax\..*|org\.xml\..*|org\.w3c\..*|'
+    r'org\.omg\..*|org\.ietf\.jgss\..*)')
+
 
 class CallStackFilter(object):
   """Filters frames of a callstack buffer."""
@@ -62,3 +79,130 @@ class KeepTopNFrames(CallStackFilter):
 
     stack_buffer.frames = stack_buffer.frames[:self.top_n_frames]
     return stack_buffer
+
+
+class FilterJavaJreSdkFrames(CallStackFilter):

[wrengr, 2016/12/19 23:51:05]

It's inconsistent to lowercase JRE and SDK here when we keep acronyms like JIT
uppercased. 

[Sharu Jiang, 2016/12/22 01:59:43]

This is because if it is ``FilterJavaJRESDKFrames``, it will be hard to separate
``JRESDK``, or if I added And in it, ``FilterJavaJREAndSDKFrames`` is a bit
strange...

Any good naming idea?   

[wrengr, 2016/12/22 23:31:51]

yeah, JRESDK is hard to read too (though slightly less hard, imo). Don't have
any good suggestions, alas

[Sharu Jiang, 2016/12/23 00:33:20]

Ok... let's leave it as it is then.

+  """Filter out package names from Java JRE/SDK.

[stgao, 2016/12/20 19:07:36]

nit: "Filter" -> "Filters"

Same for other occurrences below.

[Sharu Jiang, 2016/12/22 01:59:43]

Done.

+
+  For example: java.*, javax.*, org.xml.*, org.w3c.*, org.omg.*,
+  org.ietf.jgss.*. These frames are misleading to Predator.
+  """
+  def __call__(self, stack_buffer):
+    if stack_buffer.language_type != LanguageType.JAVA:
+      return stack_buffer
+
+    stack_buffer.frames = filter(
+        lambda frame: not JAVA_JRE_SDK_REGEX.match(frame.function),
+        stack_buffer.frames)
+    return stack_buffer
+
+
+class KeepV8FramesIfV8GeneratedJITCrash(CallStackFilter):
+  """Keeps v8 frames if conditions met.
+
+  If the top-most frames don't have symbols, but the top frame that does is
+  ``v8::internal::Invoke``, the bug is likely a crash in
+  V8's generated JIT code.
+  """
+  def __call__(self, stack_buffer):
+    if (stack_buffer and V8_JIT_CODE_MARKER in stack_buffer.frames[0].function
+        and stack_buffer.metadata.get('top_frame_has_no_symbols')):
+      stack_buffer.frames = filter(lambda f: V8_DEP_PATH_MARKER in f.dep_path,
+                                   stack_buffer.frames)
+    return stack_buffer
+
+
+class FilterV8FramesForV8APIBindingCode(CallStackFilter):
+  """Filter all v8 frames if conditions met.

[wrengr, 2016/12/19 23:51:05]

should clarify whether it's all conditions or any condition.

[Sharu Jiang, 2016/12/22 01:59:43]

Done.

+
+  Conditions:
+  (1) src/v8/src/api.h or src/v8/src/api.cc appears as
+  the top file in the stack trace.
+  (2) the second file is not in src/v8/src
+  (e.g. src/out/Release/gen/blink/bindings) or the crash is caused by
+  dereference of null pointer, then V8 should not be responsible for
+  the crash (likely a bindings issue).

[stgao, 2016/12/20 19:07:36]

nit: Could we fill in the whole line before moving to the next line?

[Sharu Jiang, 2016/12/22 01:59:43]

Done.

+
+  Args:
+      crash_address: Address where crash happens.

[wrengr, 2016/12/19 23:51:05]

type?

[Sharu Jiang, 2016/12/22 01:59:43]

Done.

+
+  Returns:

[stgao, 2016/12/20 19:07:36]

What is the Returns for? I'm confused here.

[Sharu Jiang, 2016/12/22 01:59:43]

Oops, this should be removed.

+      A string of hint message.
+  """
+  def __init__(self, crash_address=None):
+    # Record the crash address of the to-be-parsed stack_buffer.
+    self.crash_address = crash_address
+
+  def __call__(self, stack_buffer):
+    if len(stack_buffer.frames) < 2:
+      return stack_buffer
+
+    first_frame_is_api_file = (
+        V8_DEP_PATH_MARKER in stack_buffer.frames[0].dep_path and
+        (V8_API_H_FILE_PATH == stack_buffer.frames[0].file_path or
+         V8_API_CC_FILE_PATH == stack_buffer.frames[0].file_path))
+
+    second_frame_not_from_v8_src = (
+        V8_DEP_PATH_MARKER not in stack_buffer.frames[1].dep_path or
+        not stack_buffer.frames[1].file_path.startswith('src'))
+
+    null_pointer_dereference = False
+    if self.crash_address:
+      try:
+        null_pointer_dereference = int(
+            self.crash_address,
+            base=16) < NULL_POINTER_DEREFERENCE_THRESHOLD
+      except ValueError:  # pragma: no cover
+        # some testcases like memcpy-param-overlap have crash addresses like
+        # '[0x621000017d00,0x621000018cea) and [0x621000017d16, 0x621000018d00)'
+        pass
+
+    if (first_frame_is_api_file and (second_frame_not_from_v8_src or
+                                     null_pointer_dereference)):
+      stack_buffer.frames = filter(
+          lambda f: V8_DEP_PATH_MARKER not in f.dep_path,
+          stack_buffer.frames)
+      # After deleting all v8 frames, if the top n frames are generated code,
+      # need to filter them out.
+      top_n_generated_code_frames = 0
+      for frame in stack_buffer.frames:
+        if not BLINK_BINDINGS_GENERATED_PATH_REGEX.match(frame.file_path):
+          break
+        top_n_generated_code_frames += 1
+      stack_buffer.frames = stack_buffer.frames[top_n_generated_code_frames:]
+
+    return stack_buffer
+
+
+class FilterFramesAfterBlinkGeneratedCode(CallStackFilter):
+  """Filter all the frames after blink generated code."""
+  def __call__(self, stack_buffer):
+    for index, frame in enumerate(stack_buffer):
+      if BLINK_BINDINGS_GENERATED_PATH_REGEX.match(frame.file_path):
+        stack_buffer.frames = stack_buffer.frames[:index]
+        break

[stgao, 2016/12/20 19:07:36]

What if there are two frames with generated code?

[Sharu Jiang, 2016/12/22 01:59:43]

This is to filter generated frames and frames after them, so we should break
after we found the first generated code frame.

+
+    return stack_buffer
+
+
+class FilterV8FramesIfV8NotInTopFrames(CallStackFilter):
+  """Filter all v8 frames if there is no v8 frames in top_n_frames."""
+  def __init__(self, top_n_frames=4):
+    self.top_n_frames = top_n_frames
+
+  def __call__(self, stack_buffer):
+    need_filter_v8 = False
+    for index, frame in enumerate(stack_buffer):
+      if index >= self.top_n_frames:
+        need_filter_v8 = True
+        break
+
+      if V8_DEP_PATH_MARKER in frame.dep_path:
+        break
+
+    if not need_filter_v8:
+      return stack_buffer
+
+    stack_buffer.frames = filter(
+        lambda f: not V8_DEP_PATH_MARKER in f.dep_path, stack_buffer.frames)

[wrengr, 2016/12/19 23:51:05]

The syntax "x not in xs" is more standard than "not x in xs"

[Sharu Jiang, 2016/12/22 01:59:43]

Done.

+    return stack_buffer