Improve memcheck_analyze

tools/valgrind/memcheck_analyze.py

 #!/usr/bin/python
 # Copyright (c) 2011 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.
 
 # memcheck_analyze.py
 
 ''' Given a valgrind XML file, parses errors and uniques them.'''
 
 import gdb_helper
 
 import hashlib
 import logging
 import optparse
 import os
 import re
 import subprocess
 import sys
 import time
 from xml.dom.minidom import parse
 from xml.parsers.expat import ExpatError
 
 import common
 
 # Global symbol table (yuck)
 TheAddressTable = None
 
-# These are functions (using C++ mangled names) that we look for in stack
-# traces. We don't show stack frames while pretty printing when they are below
-# any of the following:
-_TOP_OF_STACK_POINTS = [
-  # Don't show our testing framework.
-  "testing::Test::Run()",

[Timur Iskhodzhanov, 2011/08/16 13:48:02]

this wasn't matching anything

+# These are regexps that define functions (using C++ mangled names)
+# we don't want to see in stack traces while pretty printing
+# or generating suppressions.
+# Just stop printing the stack/suppression frames when the current one
+# matches any of these.
+_BORING_CALLERS = [
+  # TODO(timurrrr): add more boring callers when needed
+
+  # Don't show our testing framework:
   "_ZN7testing4Test3RunEv",
+  "_ZN7testing8internal35HandleExceptionsInMethodIfSupported",
+  "_ZN7testing8internal38HandleSehExceptionsInMethodIfSupported",
+
+  # Depends on scheduling:
+  "_ZN11MessageLoop3RunEv",
+  "_ZN14RunnableMethod.*",

[Lei Zhang, 2011/08/16 18:49:04]

Indeed, stuff below this is quite boring and can probably be safely omitted.

+
   # Also don't show the internals of libc/pthread.
   "start_thread"
 ]
 
 def getTextOf(top_node, name):
   ''' Returns all text in all DOM nodes with a certain |name| that are children
   of |top_node|.
   '''
 
   text = ""
@@ skipping 43 matching lines @@
   for frame in node.getElementsByTagName("frame"):
     frame_dict = {
       INSTRUCTION_POINTER : getTextOf(frame, INSTRUCTION_POINTER),
       OBJECT_FILE         : getTextOf(frame, OBJECT_FILE),
       FUNCTION_NAME       : getTextOf(frame, FUNCTION_NAME),
       SRC_FILE_DIR        : shortenFilePath(
           source_dir, getTextOf(frame, SRC_FILE_DIR)),
       SRC_FILE_NAME       : getTextOf(frame, SRC_FILE_NAME),
       SRC_LINE            : getTextOf(frame, SRC_LINE)
     }
+
+    # Ignore this frame and all the following if it's a "boring" function.
+    enough_frames = False
+    for regexp in _BORING_CALLERS:
+      if re.match(regexp, frame_dict[FUNCTION_NAME]):
+        enough_frames = True
+        break
+    if enough_frames:
+      break
+
     frames += [frame_dict]
-    if frame_dict[FUNCTION_NAME] in _TOP_OF_STACK_POINTS:
-      break
+
     global TheAddressTable
     if TheAddressTable != None and frame_dict[SRC_LINE] == "":
       # Try using gdb
       TheAddressTable.Add(frame_dict[OBJECT_FILE],
                           frame_dict[INSTRUCTION_POINTER])
   return frames
 
 class ValgrindError:
   ''' Takes a <DOM Element: error> node and reads all the data from it. A
   ValgrindError is immutable and is hashed on its pretty printed output.
@@ skipping 147 matching lines @@
     output += "Suppression (error hash=#%016X#):\n" % self.ErrorHash()
     output += ("  For more info on using suppressions see "
                "http://dev.chromium.org/developers/how-tos/using-valgrind#TOC-Suppressing-Errors")
 
     # Widen suppression slightly to make portable between mac and linux
     supp = self._suppression;
     supp = supp.replace("fun:_Znwj", "fun:_Znw*")
     supp = supp.replace("fun:_Znwm", "fun:_Znw*")
     supp = supp.replace("fun:_Znaj", "fun:_Zna*")
     supp = supp.replace("fun:_Znam", "fun:_Zna*")
+
     # Split into lines so we can enforce length limits
     supplines = supp.split("\n")
+    supp = None  # to avoid re-use
 
     # Truncate at line 26 (VG_MAX_SUPP_CALLERS plus 2 for name and type)
     # or at the first 'boring' caller.
     # (https://bugs.kde.org/show_bug.cgi?id=199468 proposes raising
     # VG_MAX_SUPP_CALLERS, but we're probably fine with it as is.)
-    # TODO(dkegel): add more boring callers
-    newlen = 26;
-    for boring_caller in ["   fun:_ZN11MessageLoop3RunEv",
-                          "   fun:_ZN7testing4Test3RunEv"]:
-      try:
-        newlen = min(newlen, supplines.index(boring_caller))
-      except ValueError:
-        pass
+    newlen = min(26, len(supplines));
+    for boring_caller in _BORING_CALLERS:

[Lei Zhang, 2011/08/16 18:49:04]

I think this double for-loop can exit a bit earlier if we write it as:

enough_frames = False
for frameno in range(len(supplines)):
  for boring_caller in _BORING_CALLERS:
    if re.search(boring_caller, supplines[frameno]):
      newlen = frameno
      enough_frames = True
      break
  if enough_frames:
    break

[Timur Iskhodzhanov, 2011/08/17 09:09:19]

Absolutely!
I've started from this one (not the gatherFrames part) and for some reason my
mind had stuck on this sub-optimal solution.
In fact, "for frameno in range(newlen)" should be even better

On 2011/08/16 18:49:04, Lei Zhang wrote:

+      for frame in range(len(supplines)):
+        if re.search(boring_caller, supplines[frame]):
+          newlen = min(newlen, frame)
+          break
     if (len(supplines) > newlen):
       supplines = supplines[0:newlen]
       supplines.append("}")
 
+    for frame in range(len(supplines)):
+      # Replace the always-changing anonymous namespace prefix with "*".
+      m = re.match("( +fun:)_ZN.*_GLOBAL__N_.*\.cc_" +
+                   "[0-9a-fA-F]{8}_[0-9a-fA-F]{8}(.*)",
+                   supplines[frame])
+      if m:
+        supplines[frame] = "*".join(m.groups())
+
     output += "\n".join(supplines) + "\n"
 
     return output
 
   def UniqueString(self):
     ''' String to use for object identity. Don't print this, use str(obj)
     instead.'''
     rep = self._kind + " "
     for backtrace in self._backtraces:
       for frame in backtrace[1]:
@@ skipping 284 matching lines @@
     parser.error("no filename specified")
   filenames = args
 
   analyzer = MemcheckAnalyzer(options.source_dir, use_gdb=True)
   retcode = analyzer.Report(filenames)
 
   sys.exit(retcode)
 
 if __name__ == "__main__":
   _main()