Update to valgrind-3.5.0. From timur, http://codereview.chromium.org/190008

Also use it if present.  Especially use its new xml suppression record
if present, which is more correct than the one we synthesize.
Also fix a warning about global use before declare in memcheck_analyze.py.

Review URL: http://codereview.chromium.org/191006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@25353 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 89 insertions, 80 deletions

diff --git a/tools/valgrind/build-valgrind-for-chromium.sh b/tools/valgrind/build-valgrind-for-chromium.sh
index 32f600e..3437222 100755
--- a/tools/valgrind/build-valgrind-for-chromium.sh
+++ b/tools/valgrind/build-valgrind-for-chromium.sh
@@ -12,11 +12,11 @@
 # fetch valgrind source via svn.  Most users won't need those options.
 
 # Checkout by date doesn't work unless you specify the friggin' timezone
-VALGRIND_SVN_REV=10771
+VALGRIND_SVN_REV=10880
 # And svn isn't smart enough to figure out what rev of the linked tree to get
-VEX_SVN_REV=1913
+VEX_SVN_REV=1914
 # and TSAN may be out of sync, so you have to check that out by rev anyway
-TSAN_SVN_REV=1111
+TSAN_SVN_REV=1129
 
 THISDIR=$(dirname "${0}")
 THISDIR=$(cd "${THISDIR}" && /bin/pwd)
@@ -75,8 +75,8 @@ then
     # ThreadSanitizer is an experimental dynamic data race detector.
     # See http://code.google.com/p/data-race-test/wiki/ThreadSanitizer
     svn checkout -r "${TSAN_SVN_REV}" http://data-race-test.googlecode.com/svn/trunk/tsan tsan
-    mkdir tsan/{docs,tests}
-    touch tsan/{docs,tests}/Makefile.am
+    mkdir tsan/tests
+    touch tsan/tests/Makefile.am
     patch -p 0 < tsan/valgrind.patch
   fi
 
diff --git a/tools/valgrind/chrome_tests.sh b/tools/valgrind/chrome_tests.sh
index 666ff0c..cea7777 100755
--- a/tools/valgrind/chrome_tests.sh
+++ b/tools/valgrind/chrome_tests.sh
@@ -12,7 +12,7 @@ if test x"$CHROME_VALGRIND_BIN" = x
 then
   # Figure out which valgrind is installed.  Use most recent one.
   # See build-valgrind-for-chromium.sh and its history for these constants.
-  for SVNREV in '10771' '{2009-07-15}'
+  for SVNREV in '10880' '10771' '{2009-07-15}'
   do
     SHORTSVNREV=`echo $SVNREV | tr -d '{\-}'`
     CHROME_VALGRIND_BIN=/usr/local/valgrind-$SHORTSVNREV/bin
diff --git a/tools/valgrind/fork.patch b/tools/valgrind/fork.patch
index ba3e268..1461941 100644
--- a/tools/valgrind/fork.patch
+++ b/tools/valgrind/fork.patch
@@ -1,6 +1,6 @@
 Index: coregrind/m_syswrap/syswrap-generic.c
 ===================================================================
---- coregrind/m_syswrap/syswrap-generic.c	(revision 10771)
+--- coregrind/m_syswrap/syswrap-generic.c	(revision 10880)
 +++ coregrind/m_syswrap/syswrap-generic.c	(working copy)
 @@ -2563,6 +2563,11 @@
  
@@ -21,7 +21,7 @@ Index: coregrind/m_syswrap/syswrap-generic.c
 -
 Index: coregrind/pub_core_libcfile.h
 ===================================================================
---- coregrind/pub_core_libcfile.h	(revision 10771)
+--- coregrind/pub_core_libcfile.h	(revision 10880)
 +++ coregrind/pub_core_libcfile.h	(working copy)
 @@ -41,6 +41,7 @@
  
@@ -33,7 +33,7 @@ Index: coregrind/pub_core_libcfile.h
  /* Convert an fd into a filename */
 Index: coregrind/m_main.c
 ===================================================================
---- coregrind/m_main.c	(revision 10771)
+--- coregrind/m_main.c	(revision 10880)
 +++ coregrind/m_main.c	(working copy)
 @@ -313,6 +313,48 @@
     }
@@ -217,21 +217,7 @@ Index: coregrind/m_main.c
     } else {
        // If they said --xml-fd=-1, don't print anything.  Plausible for use in
        // regression testing suites that use client requests to count errors.
-@@ -950,10 +925,9 @@
-                   i++;
-                }
- 
--               VG_(message_no_f_c)(Vg_UserMsg,
--                                   "<logfilequalifier> <var>%t</var> "
--                                   "<value>%t</value> </logfilequalifier>\n",
--                                   qualname,qual);
-+               VG_(printf_xml_no_f_c)( "<logfilequalifier> <var>%t</var> "
-+                                       "<value>%t</value> </logfilequalifier>\n",
-+                                       qualname,qual);
- 	       format[i] = '}';
- 	       i++;
- 	    }
-@@ -988,9 +962,12 @@
+@@ -989,9 +964,12 @@
     If logging to file or a socket, write details of parent PID and
     command line args, to help people trying to interpret the
     results of a run which encompasses multiple processes. */
@@ -247,7 +233,7 @@ Index: coregrind/m_main.c
  {
     Int    i;
     HChar* xpre  = VG_(clo_xml) ? "  <line>" : "";
-@@ -998,9 +975,15 @@
+@@ -999,9 +977,15 @@
     UInt (*umsg_or_xml)( const HChar*, ... )
        = VG_(clo_xml) ? VG_(printf_xml) : VG_(umsg);
  
@@ -263,7 +249,7 @@ Index: coregrind/m_main.c
  
     if (VG_(clo_xml)) {
        VG_(printf_xml)("<?xml version=\"1.0\"?>\n");
-@@ -1012,7 +995,7 @@
+@@ -1013,7 +997,7 @@
        VG_(printf_xml)("\n");
     }
  
@@ -272,7 +258,7 @@ Index: coregrind/m_main.c
  
        if (VG_(clo_xml))
           VG_(printf_xml)("<preamble>\n");
-@@ -1065,6 +1048,7 @@
+@@ -1067,6 +1051,7 @@
     }
     else
     if (VG_(clo_xml)) {
@@ -280,16 +266,7 @@ Index: coregrind/m_main.c
        VG_(printf_xml)("\n");
        VG_(printf_xml)("<pid>%d</pid>\n", VG_(getpid)());
        VG_(printf_xml)("<ppid>%d</ppid>\n", VG_(getppid)());
-@@ -1112,7 +1096,7 @@
-    }
- 
-    // Empty line after the preamble
--   if (VG_(clo_verbosity) > 0)
-+   if (VG_(clo_verbosity) > 1)
-       VG_(umsg)("\n");
-    if (VG_(clo_xml))
-       VG_(printf_xml)("\n");
-@@ -1372,7 +1356,6 @@
+@@ -1374,7 +1359,6 @@
     Int     need_help          = 0; // 0 = no, 1 = --help, 2 = --help-debug
     ThreadId tid_main          = VG_INVALID_THREADID;
     Bool    logging_to_fd      = False;
@@ -297,7 +274,7 @@ Index: coregrind/m_main.c
     Int     loglevel, i;
     struct vki_rlimit zero = { 0, 0 };
     XArray* addr2dihandle = NULL;
-@@ -1870,8 +1853,7 @@
+@@ -1872,8 +1856,7 @@
     VG_(debugLog)(1, "main",
                      "(main_) Process Valgrind's command line options, "
                      "setup logging\n");
@@ -307,7 +284,7 @@ Index: coregrind/m_main.c
  
     //--------------------------------------------------------------
     // Zeroise the millisecond counter by doing a first read of it.
-@@ -1884,10 +1866,10 @@
+@@ -1886,10 +1869,10 @@
     //   p: tl_pre_clo_init            [for 'VG_(details).name' and friends]
     //   p: main_process_cmd_line_options()
     //         [for VG_(clo_verbosity), VG_(clo_xml),
@@ -320,18 +297,9 @@ Index: coregrind/m_main.c
     VG_(debugLog)(1, "main", "...finished the preamble\n");
  
     //--------------------------------------------------------------
-@@ -2394,7 +2376,7 @@
-    // Finalisation: cleanup, messages, etc.  Order not so important, only
-    // affects what order the messages come.
-    //--------------------------------------------------------------
--   if (VG_(clo_verbosity) > 0)
-+   if (VG_(clo_verbosity) > 1)
-       VG_(message)(Vg_UserMsg, "\n");
-    if (VG_(clo_xml))
-       VG_(printf_xml)("\n");
 Index: coregrind/m_libcproc.c
 ===================================================================
---- coregrind/m_libcproc.c	(revision 10771)
+--- coregrind/m_libcproc.c	(revision 10880)
 +++ coregrind/m_libcproc.c	(working copy)
 @@ -33,9 +33,12 @@
  #include "pub_core_vkiscnums.h"
@@ -388,7 +356,7 @@ Index: coregrind/m_libcproc.c
           (*atforks[i].child)(tid);
 Index: coregrind/pub_core_options.h
 ===================================================================
---- coregrind/pub_core_options.h	(revision 10771)
+--- coregrind/pub_core_options.h	(revision 10880)
 +++ coregrind/pub_core_options.h	(working copy)
 @@ -74,6 +74,9 @@
  extern Char* VG_(clo_log_fname_expanded);
diff --git a/tools/valgrind/memcheck_analyze.py b/tools/valgrind/memcheck_analyze.py
index 9b9d404..ceade09 100755
--- a/tools/valgrind/memcheck_analyze.py
+++ b/tools/valgrind/memcheck_analyze.py
@@ -116,6 +116,19 @@ def getTextOf(top_node, name):
                      if node.nodeType == node.TEXT_NODE])
   return text
 
+def getCDATAOf(top_node, name):
+  ''' Returns all CDATA in all DOM nodes with a certain |name| that are children
+  of |top_node|.
+  '''
+
+  text = ""
+  for nodes_named in top_node.getElementsByTagName(name):
+    text += "".join([node.data for node in nodes_named.childNodes
+                     if node.nodeType == node.CDATA_SECTION_NODE])
+  if (text == ""):
+    return None
+  return text
+
 def removeCommonRoot(source_dir, directory):
   '''Returns a string with the string prefix |source_dir| removed from
   |directory|.'''
@@ -171,7 +184,8 @@ class ValgrindError:
     '''
 
     # Valgrind errors contain one <what><stack> pair, plus an optional
-    # <auxwhat><stack> pair, plus an optional <origin><what><stack></origin>.
+    # <auxwhat><stack> pair, plus an optional <origin><what><stack></origin>,
+    # plus (since 3.5.0) a <suppression></suppression> pair.
     # (Origin is nicely enclosed; too bad the other two aren't.)
     # The most common way to see all three in one report is
     # a syscall with a parameter that points to uninitialized memory, e.g.
@@ -200,6 +214,27 @@ class ValgrindError:
     #     </frame>
     #   </stack>
     #   </origin>
+    #   <suppression>
+    #     <sname>insert_a_suppression_name_here</sname>
+    #     <skind>Memcheck:Param</skind>
+    #     <skaux>write(buf)</skaux>
+    #     <sframe> <fun>__write_nocancel</fun> </sframe>
+    #     ...
+    #     <sframe> <fun>main</fun> </sframe>
+    #     <rawtext>
+    # <![CDATA[
+    # {
+    #    <insert_a_suppression_name_here>
+    #    Memcheck:Param
+    #    write(buf)
+    #    fun:__write_nocancel
+    #    ...
+    #    fun:main
+    # }
+    # ]]>
+    #     </rawtext>
+    #   </suppression>
+    # </error>
     #
     # Each frame looks like this:
     #  <frame>
@@ -221,6 +256,7 @@ class ValgrindError:
 
     self._kind = getTextOf(error_node, "kind")
     self._backtraces = []
+    self._suppression = None
 
     # Iterate through the nodes, parsing <what|auxwhat><stack> pairs.
     description = None
@@ -241,6 +277,8 @@ class ValgrindError:
         description = None
         stack = None
         frames = None
+      elif node.localName == "suppression":
+        self._suppression = getCDATAOf(node, "rawtext");
 
   def __str__(self):
     ''' Pretty print the type and backtrace(s) of this specific error,
@@ -267,19 +305,27 @@ class ValgrindError:
         global TheAddressTable
         if TheAddressTable != None and frame[SRC_FILE_DIR] == "":
            # Try using gdb
-           foo = TheAddressTable.GetFileLine(frame[OBJECT_FILE], frame[INSTRUCTION_POINTER])
+           foo = TheAddressTable.GetFileLine(frame[OBJECT_FILE],
+                                             frame[INSTRUCTION_POINTER])
            if foo[0] != None:
              output += (" (" + foo[0] + ":" + foo[1] + ")")
         elif frame[SRC_FILE_DIR] != "":
-          output += (" (" + frame[SRC_FILE_DIR] + "/" + frame[SRC_FILE_NAME] + ":" +
-                     frame[SRC_LINE] + ")")
+          output += (" (" + frame[SRC_FILE_DIR] + "/" + frame[SRC_FILE_NAME] +
+                     ":" + frame[SRC_LINE] + ")")
         else:
           output += " (" + frame[OBJECT_FILE] + ")"
         output += "\n"
 
-      output += "Suppression:\n"
-      for frame in backtrace[1]:
-        output += "  fun:" + (frame[FUNCTION_NAME] or "*") + "\n"
+      # TODO(dank): stop synthesizing suppressions once everyone has
+      # valgrind-3.5 and we can rely on xml
+      if (self._suppression == None):
+        output += "Ssuppression:\n"
+        for frame in backtrace[1]:
+          output += "  fun:" + (frame[FUNCTION_NAME] or "*") + "\n"
+
+    if (self._suppression != None):
+      output += "Suppression:"
+      output += self._suppression
 
     return output
 
@@ -327,8 +373,8 @@ class MemcheckAnalyze:
       show_all_leaks: whether to show even less important leaks
     '''
 
+    global TheAddressTable
     if use_gdb:
-      global TheAddressTable
       TheAddressTable = _AddressTable()
     self._errors = set()
     badfiles = set()
@@ -377,7 +423,6 @@ class MemcheckAnalyze:
         if TheAddressTable != None:
           load_objs = parsed_file.getElementsByTagName("load_obj")
           for load_obj in load_objs:
-            global TheAddressTable
             obj = getTextOf(load_obj, "obj")
             ip = getTextOf(load_obj, "ip")
             TheAddressTable.AddBinaryAt(obj, ip)
@@ -404,6 +449,7 @@ class MemcheckAnalyze:
     if self._errors:
       logging.error("FAIL! There were %s errors: " % len(self._errors))
 
+      global TheAddressTable
       if TheAddressTable != None:
         TheAddressTable.ResolveAll()
 
diff --git a/tools/valgrind/possible.patch b/tools/valgrind/possible.patch
index 1091844..b5e4841 100644
--- a/tools/valgrind/possible.patch
+++ b/tools/valgrind/possible.patch
@@ -1,6 +1,6 @@
 Index: memcheck/mc_main.c
 ===================================================================
---- memcheck/mc_main.c	(revision 10771)
+--- memcheck/mc_main.c	(revision 10880)
 +++ memcheck/mc_main.c	(working copy)
 @@ -4658,6 +4658,7 @@
  LeakCheckMode MC_(clo_leak_check)             = LC_Summary;
@@ -28,9 +28,9 @@ Index: memcheck/mc_main.c
  "    --partial-loads-ok=no|yes        too hard to explain here; see manual [no]\n"
 Index: memcheck/mc_include.h
 ===================================================================
---- memcheck/mc_include.h	(revision 10771)
+--- memcheck/mc_include.h	(revision 10880)
 +++ memcheck/mc_include.h	(working copy)
-@@ -394,6 +394,9 @@
+@@ -395,6 +395,9 @@
  /* In leak check, show reachable-but-not-freed blocks?  default: NO */
  extern Bool MC_(clo_show_reachable);
  
@@ -42,19 +42,15 @@ Index: memcheck/mc_include.h
  extern Bool MC_(clo_workaround_gcc296_bugs);
 Index: memcheck/mc_leakcheck.c
 ===================================================================
---- memcheck/mc_leakcheck.c	(revision 10771)
+--- memcheck/mc_leakcheck.c	(revision 10880)
 +++ memcheck/mc_leakcheck.c	(working copy)
-@@ -843,7 +843,7 @@
-       //
-       lr = lr_array[i];
-       count_as_error = Unreached == lr->key.state || 
--                       Possible  == lr->key.state;
-+                       (MC_(clo_show_possible) && Possible == lr->key.state);
+@@ -845,7 +845,8 @@
        print_record = is_full_check &&
-                      ( MC_(clo_show_reachable) || count_as_error );
-       is_suppressed = 
-@@ -1113,4 +1113,3 @@
- /*--------------------------------------------------------------------*/
- /*--- end                                                          ---*/
- /*--------------------------------------------------------------------*/
--
+                      ( MC_(clo_show_reachable) ||
+                        Unreached == lr->key.state || 
+-                       Possible  == lr->key.state );
++                       ( MC_(clo_show_possible) && 
++                         Possible  == lr->key.state ) );
+       // We don't count a leaks as errors with --leak-check=summary.
+       // Otherwise you can get high error counts with few or no error
+       // messages, which can be confusing.  Also, you could argue that
diff --git a/tools/valgrind/valgrind.sh b/tools/valgrind/valgrind.sh
index 33ae8a2..5db5bfd 100755
--- a/tools/valgrind/valgrind.sh
+++ b/tools/valgrind/valgrind.sh
@@ -72,7 +72,7 @@ if test x"$CHROME_VALGRIND_BIN" = x
 then
   # Figure out which valgrind is installed.  Use most recent one.
   # See build-valgrind-for-chromium.sh and its history for these constants.
-  for SVNREV in '10771' '{2009-07-15}'
+  for SVNREV in '10880' '10771' '{2009-07-15}'
   do
     SHORTSVNREV=`echo $SVNREV | tr -d '{\-}'`
     CHROME_VALGRIND_BIN=/usr/local/valgrind-$SHORTSVNREV/bin
diff --git a/tools/valgrind/valgrind_test.py b/tools/valgrind/valgrind_test.py
index d0e2f65..d7b809f 100755
--- a/tools/valgrind/valgrind_test.py
+++ b/tools/valgrind/valgrind_test.py
@@ -210,8 +210,7 @@ class ValgrindTool(object):
     proc = ["valgrind",
             "--tool=%s" % tool_name,
             "--smc-check=all",
-            "--num-callers=%i" % self._num_callers,
-            "--demangle=no"]
+            "--num-callers=%i" % self._num_callers]
 
     if self._options.trace_children:
       proc += ["--trace-children=yes"];
@@ -347,7 +346,7 @@ class Memcheck(ValgrindTool):
                       help="Show whence uninitialized bytes came. 30% slower.")
 
   def ToolSpecificFlags(self):
-    ret = ["--leak-check=full"]
+    ret = ["--leak-check=full", "--gen-suppressions=all", "--demangle=no"]
 
     if self._options.show_all_leaks:
       ret += ["--show-reachable=yes"];