Update to current Valgrind and tsan.

New fork and possible patches from Timur.

Accept new xml output format.
Use --xml-file option if present.
Drop two patches already in trunk.
Remove obsolete --generate_suppressions code.
Accept but ignore --generate_dsym option on non-darwin platforms, so scripts don't need conditionals

Switch to using svn revision numbers rather than dates,
because svn uses the local timezone to interpret the dates,
and this caused patches to fail.  Feh.

Check to make sure a good copy of valgrind was found before trying to
use valgrind.

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

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

8 files changed, 386 insertions, 147 deletions

diff --git a/tools/valgrind/build-valgrind-for-chromium.sh b/tools/valgrind/build-valgrind-for-chromium.sh
index 823e0f7..1c94a68 100755
--- a/tools/valgrind/build-valgrind-for-chromium.sh
+++ b/tools/valgrind/build-valgrind-for-chromium.sh
@@ -1,9 +1,12 @@
 #!/bin/sh
 # Script to build valgrind for use with chromium
 
-SVNDATE='{2009-07-15}'
-SHORTSVNDATE=20090715
-TSAN_SVN_REV='1096'
+# Checkout by date doesn't work unless you specify the friggin' timezone
+VALGRIND_SVN_REV=10771
+# And svn isn't smart enough to figure out what rev of the linked tree to get
+VEX_SVN_REV=1913
+# and TSAN may be out of sync, so you have to check that out by rev anyway
+TSAN_SVN_REV=1111
 
 THISDIR=`dirname $0`
 THISDIR=`cd $THISDIR && /bin/pwd`
@@ -41,7 +44,7 @@ then
 fi
 
 # Desired parent directory for valgrind's bin, include, etc.
-PREFIX="${1:-/usr/local/valgrind-$SHORTSVNDATE}"
+PREFIX="${1:-/usr/local/valgrind-$VALGRIND_SVN_REV}"
 parent_of_prefix="`dirname $PREFIX`"
 if test ! -d "$parent_of_prefix"
 then
@@ -50,26 +53,18 @@ then
 fi
 
 # Check out latest version that following patches known to apply against
-rm -rf valgrind-$SHORTSVNDATE
-svn co -r $SVNDATE svn://svn.valgrind.org/valgrind/trunk valgrind-$SHORTSVNDATE
+rm -rf valgrind-$VALGRIND_SVN_REV
+svn co -r $VALGRIND_SVN_REV svn://svn.valgrind.org/valgrind/trunk valgrind-$VALGRIND_SVN_REV
 
-cd valgrind-$SHORTSVNDATE
+cd valgrind-$VALGRIND_SVN_REV
 
 # Make sure svn gets the right version of the external VEX repo, too
-svn update -r $SVNDATE VEX/
+svn update -r $VEX_SVN_REV VEX/
 
 # Work around bug https://bugs.kde.org/show_bug.cgi?id=162848
 # "fork() not handled properly"
 patch -p0 < "$THISDIR"/fork.patch
 
-# Work around bug https://bugs.kde.org/show_bug.cgi?id=186796
-# "long suppressions truncated"
-patch -p0 < "$THISDIR"/longlines.patch
-
-# Work around bug http://bugs.kde.org/186790
-# "Suppression counts do not include leak suppressions"
-patch -p0 < "$THISDIR"/leak.patch
-
 # Add feature bug https://bugs.kde.org/show_bug.cgi?id=201170
 # "Want --show-possible option so I can ignore the bazillion possible leaks..."
 patch -p0 < "$THISDIR"/possible.patch
@@ -83,7 +78,6 @@ then
   mkdir tsan/{docs,tests}
   touch tsan/{docs,tests}/Makefile.am
   patch -p 0 < tsan/valgrind.patch
-  patch -p 0 -d VEX < tsan/vex.patch
 fi
 
 sh autogen.sh
diff --git a/tools/valgrind/chrome_tests.sh b/tools/valgrind/chrome_tests.sh
index fbf45e8..5523e36 100755
--- a/tools/valgrind/chrome_tests.sh
+++ b/tools/valgrind/chrome_tests.sh
@@ -3,9 +3,25 @@
 # Select the valgrind built by build-valgrind-for-chromium.sh by default,
 # but allow users to override this default without editing scripts and
 # without specifying a commandline option
-# SHORTSVNDATE should track value of same var in build-valgrind-for-chromium.sh
-SHORTSVNDATE=20090715
-CHROME_VALGRIND_BIN="${CHROME_VALGRIND_BIN:-/usr/local/valgrind-$SHORTSVNDATE/bin}"
+
+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}'
+  do
+    SHORTSVNREV=`echo $SVNREV | tr -d '{\-}'`
+    CHROME_VALGRIND_BIN=/usr/local/valgrind-$SHORTSVNREV/bin
+    test -x $CHROME_VALGRIND_BIN/valgrind && break
+  done
+fi
+
+if ! test -x $CHROME_VALGRIND_BIN/valgrind
+then
+  echo "Could not find chromium's version of valgrind."
+  echo "Please run build-valgrind-for-chromium.sh or set CHROME_VALGRIND_BIN."
+  exit 1
+fi
 PATH="${CHROME_VALGRIND_BIN}:$PATH"
 
 export THISDIR=`dirname $0`
diff --git a/tools/valgrind/fork.patch b/tools/valgrind/fork.patch
index 7e9281c..ba3e268 100644
--- a/tools/valgrind/fork.patch
+++ b/tools/valgrind/fork.patch
@@ -1,52 +1,260 @@
 Index: coregrind/m_syswrap/syswrap-generic.c
 ===================================================================
---- coregrind/m_syswrap/syswrap-generic.c	(revision 10399)
+--- coregrind/m_syswrap/syswrap-generic.c	(revision 10771)
 +++ coregrind/m_syswrap/syswrap-generic.c	(working copy)
-@@ -2558,6 +2558,15 @@
+@@ -2563,6 +2563,11 @@
  
     } else {
        path = (Char*)ARG1;
 +      if (VG_(clo_xml)) {
-+        VG_(message)(Vg_UserMsg, "");
-+        VG_(message)(Vg_UserMsg, "<execv/>");
-+        VG_(message)(Vg_UserMsg, "");
-+        VG_(message)(Vg_UserMsg, "</valgrindoutput>");
-+        VG_(message)(Vg_UserMsg, "");
++        VG_(printf_xml)("\n<execv/>\n\n</valgrindoutput>\n\n");
 +      } else {
-+        VG_(message)(Vg_UserMsg, "execv called - the tool will now quit");
++        VG_(umsg)("execv called - the tool will now quit\n");
 +      }
     }
  
     // Set up the child's environment.
-@@ -4093,4 +4102,3 @@
+@@ -4103,4 +4108,3 @@
  /*--------------------------------------------------------------------*/
  /*--- end                                                          ---*/
  /*--------------------------------------------------------------------*/
 -
+Index: coregrind/pub_core_libcfile.h
+===================================================================
+--- coregrind/pub_core_libcfile.h	(revision 10771)
++++ coregrind/pub_core_libcfile.h	(working copy)
+@@ -41,6 +41,7 @@
+ 
+ /* Move an fd into the Valgrind-safe range */
+ extern Int VG_(safe_fd) ( Int oldfd );
++extern Int reopen_output_fd(Bool xml);
+ extern Int VG_(fcntl)   ( Int fd, Int cmd, Addr arg );
+ 
+ /* Convert an fd into a filename */
 Index: coregrind/m_main.c
 ===================================================================
---- coregrind/m_main.c	(revision 10399)
+--- coregrind/m_main.c	(revision 10771)
 +++ coregrind/m_main.c	(working copy)
-@@ -755,15 +755,26 @@
+@@ -313,6 +313,48 @@
+    }
+ }
+ 
++Int reopen_output_fd(Bool xml) {
++  // Returns FD
++  Char *filename = NULL;
++  Char *fsname_unexpanded = xml ? VG_(clo_xml_fname_unexpanded) :
++                                 VG_(clo_log_fname_unexpanded);
++  const Char *output_type = xml ? "xml" : "log";
++  Int ret = -1;
++  SysRes sres;
++
++  vg_assert(fsname_unexpanded != NULL);
++  vg_assert(VG_(strlen)(fsname_unexpanded) <= 900); /* paranoia */
++
++  // Nb: we overwrite an existing file of this name without asking
++  // any questions.
++  filename = VG_(expand_file_name)(xml ? "--xml-file" : "--log-file",
++                                   fsname_unexpanded);
++  sres = VG_(open)(filename,
++                   VKI_O_CREAT|VKI_O_WRONLY|VKI_O_TRUNC,
++                   VKI_S_IRUSR|VKI_S_IWUSR);
++  if (!sr_isError(sres)) {
++    ret = sr_Res(sres);
++    if (xml)
++      VG_(clo_xml_fname_expanded) = filename;
++    else
++      VG_(clo_log_fname_expanded) = filename;
++
++    /* strdup here is probably paranoid overkill, but ... */
++    // TODO: do we need to do anything with it?
++    /* *fsname_unexpanded = VG_(strdup)( "main.mpclo.2",
++                                         xml_fsname_unexpanded ); */
++  } else {
++    VG_(message)(Vg_UserMsg,
++                 "Can't create %s file '%s' (%s); giving up!\n",
++                 output_type, filename, VG_(strerror)(sr_Err(sres)));
++    VG_(err_bad_option)(
++        "--[xml|log]-file=<file> (didn't work out for some reason.)");
++    /*NOTREACHED*/
++  }
++
++  return ret;
++}
++
+ /* The main processing for command line options.  See comments above
+    on early_process_cmd_line_options.
+ 
+@@ -339,13 +381,11 @@
+ */
+ static
+ void main_process_cmd_line_options ( /*OUT*/Bool* logging_to_fd,
+-                                     /*OUT*/Char** xml_fname_unexpanded,
+                                      const HChar* toolname )
+ {
+    // VG_(clo_log_fd) is used by all the messaging.  It starts as 2 (stderr)
+    // and we cannot change it until we know what we are changing it to is
+    // ok.  So we have tmp_log_fd to hold the tmp fd prior to that point.
+-   SysRes sres;
+    Int    i, tmp_log_fd, tmp_xml_fd;
+    Int    toolname_len = VG_(strlen)(toolname);
+    Char*  tmp_str;         // Used in a couple of places.
+@@ -514,9 +554,13 @@
+ 
+       else if VG_STR_CLO(arg, "--log-file", log_fsname_unexpanded) {
+          log_to = VgLogTo_File;
++         VG_(clo_log_fname_unexpanded) =
++             VG_(strdup)("", log_fsname_unexpanded);
+       }
+       else if VG_STR_CLO(arg, "--xml-file", xml_fsname_unexpanded) {
+          xml_to = VgLogTo_File;
++         VG_(clo_xml_fname_unexpanded) =
++             VG_(strdup)("", xml_fsname_unexpanded);
+       }
+  
+       else if VG_STR_CLO(arg, "--log-socket", log_fsname_unexpanded) {
+@@ -716,29 +760,7 @@
+          break;
+ 
+       case VgLogTo_File: {
+-         Char* logfilename;
+-
+-         vg_assert(log_fsname_unexpanded != NULL);
+-         vg_assert(VG_(strlen)(log_fsname_unexpanded) <= 900); /* paranoia */
+-
+-         // Nb: we overwrite an existing file of this name without asking
+-         // any questions.
+-         logfilename = VG_(expand_file_name)("--log-file",
+-                                             log_fsname_unexpanded);
+-         sres = VG_(open)(logfilename, 
+-                          VKI_O_CREAT|VKI_O_WRONLY|VKI_O_TRUNC, 
+-                          VKI_S_IRUSR|VKI_S_IWUSR);
+-         if (!sr_isError(sres)) {
+-            tmp_log_fd = sr_Res(sres);
+-            VG_(clo_log_fname_expanded) = logfilename;
+-         } else {
+-            VG_(message)(Vg_UserMsg, 
+-                         "Can't create log file '%s' (%s); giving up!\n", 
+-                         logfilename, VG_(strerror)(sr_Err(sres)));
+-            VG_(err_bad_option)(
+-               "--log-file=<file> (didn't work out for some reason.)");
+-            /*NOTREACHED*/
+-         }
++         tmp_log_fd = reopen_output_fd(False);
+          break;
+       }
+ 
+@@ -784,32 +806,7 @@
+          break;
+ 
+       case VgLogTo_File: {
+-         Char* xmlfilename;
+-
+-         vg_assert(xml_fsname_unexpanded != NULL);
+-         vg_assert(VG_(strlen)(xml_fsname_unexpanded) <= 900); /* paranoia */
+-
+-         // Nb: we overwrite an existing file of this name without asking
+-         // any questions.
+-         xmlfilename = VG_(expand_file_name)("--xml-file",
+-                                             xml_fsname_unexpanded);
+-         sres = VG_(open)(xmlfilename, 
+-                          VKI_O_CREAT|VKI_O_WRONLY|VKI_O_TRUNC, 
+-                          VKI_S_IRUSR|VKI_S_IWUSR);
+-         if (!sr_isError(sres)) {
+-            tmp_xml_fd = sr_Res(sres);
+-            VG_(clo_xml_fname_expanded) = xmlfilename;
+-            /* strdup here is probably paranoid overkill, but ... */
+-            *xml_fname_unexpanded = VG_(strdup)( "main.mpclo.2",
+-                                                 xml_fsname_unexpanded );
+-         } else {
+-            VG_(message)(Vg_UserMsg, 
+-                         "Can't create XML file '%s' (%s); giving up!\n", 
+-                         xmlfilename, VG_(strerror)(sr_Err(sres)));
+-            VG_(err_bad_option)(
+-               "--xml-file=<file> (didn't work out for some reason.)");
+-            /*NOTREACHED*/
+-         }
++         tmp_xml_fd = reopen_output_fd(True);
+          break;
+       }
+ 
+@@ -864,18 +861,7 @@
+    // Finalise the output fds: the log fd ..
+ 
+    if (tmp_log_fd >= 0) {
+-      // Move log_fd into the safe range, so it doesn't conflict with
+-      // any app fds.
+-      tmp_log_fd = VG_(fcntl)(tmp_log_fd, VKI_F_DUPFD, VG_(fd_hard_limit));
+-      if (tmp_log_fd < 0) {
+-         VG_(message)(Vg_UserMsg, "valgrind: failed to move logfile fd "
+-                                  "into safe range, using stderr\n");
+-         VG_(log_output_sink).fd = 2;   // stderr
+-         VG_(log_output_sink).is_socket = False;
+-      } else {
+-         VG_(log_output_sink).fd = tmp_log_fd;
+-         VG_(fcntl)(VG_(log_output_sink).fd, VKI_F_SETFD, VKI_FD_CLOEXEC);
+-      }
++      VG_(log_output_sink).fd = tmp_log_fd = VG_(safe_fd)(tmp_log_fd);
+    } else {
+       // If they said --log-fd=-1, don't print anything.  Plausible for use in
+       // regression testing suites that use client requests to count errors.
+@@ -886,18 +872,7 @@
+    // Finalise the output fds: and the XML fd ..
+ 
+    if (tmp_xml_fd >= 0) {
+-      // Move xml_fd into the safe range, so it doesn't conflict with
+-      // any app fds.
+-      tmp_xml_fd = VG_(fcntl)(tmp_xml_fd, VKI_F_DUPFD, VG_(fd_hard_limit));
+-      if (tmp_xml_fd < 0) {
+-         VG_(message)(Vg_UserMsg, "valgrind: failed to move XML file fd "
+-                                  "into safe range, using stderr\n");
+-         VG_(xml_output_sink).fd = 2;   // stderr
+-         VG_(xml_output_sink).is_socket = False;
+-      } else {
+-         VG_(xml_output_sink).fd = tmp_xml_fd;
+-         VG_(fcntl)(VG_(xml_output_sink).fd, VKI_F_SETFD, VKI_FD_CLOEXEC);
+-      }
++      VG_(xml_output_sink).fd = tmp_xml_fd = VG_(safe_fd)(tmp_xml_fd);
+    } 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 @@
     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. */
--static void print_preamble(Bool logging_to_fd, const char* toolname)
-+
+-static void print_preamble ( Bool logging_to_fd, 
+-                             Char* xml_fname_unexpanded,
+-                             const HChar* toolname )
 +// TODO(timurrrr): we add a non-static declaration of this function since
 +// we need it in coregrind/m_libcproc.c
 +// Should we move it to some header file?
-+void print_preamble(Bool logging_to_fd, const char* toolname);
++void print_preamble ( Bool logging_to_fd, const HChar* toolname );
 +
-+void print_preamble(Bool logging_to_fd, const char* toolname)
++void print_preamble ( Bool logging_to_fd, const HChar* toolname )
  {
-    HChar* xpre  = VG_(clo_xml) ? "  <line>" : "";
-    HChar* xpost = VG_(clo_xml) ? "</line>" : "";
     Int    i;
-+   static const char* last_toolname = NULL;
+    HChar* xpre  = VG_(clo_xml) ? "  <line>" : "";
+@@ -998,9 +975,15 @@
+    UInt (*umsg_or_xml)( const HChar*, ... )
+       = VG_(clo_xml) ? VG_(printf_xml) : VG_(umsg);
  
++   static const char* last_toolname = NULL;
     vg_assert( VG_(args_for_client) );
     vg_assert( VG_(args_for_valgrind) );
++
 +   // This way you may pass toolname == NULL provided the first invocation
 +   // with toolname != NULL takes place in valgrind_main().
 +   toolname = (toolname == NULL ? last_toolname : toolname);
@@ -54,10 +262,76 @@ Index: coregrind/m_main.c
 +   last_toolname = toolname;
  
     if (VG_(clo_xml)) {
-       VG_(message)(Vg_UserMsg, "<?xml version=\"1.0\"?>");
+       VG_(printf_xml)("<?xml version=\"1.0\"?>\n");
+@@ -1012,7 +995,7 @@
+       VG_(printf_xml)("\n");
+    }
+ 
+-   if (VG_(clo_xml) || VG_(clo_verbosity > 0)) {
++   if (VG_(clo_xml) || VG_(clo_verbosity) > 0) {
+ 
+       if (VG_(clo_xml))
+          VG_(printf_xml)("<preamble>\n");
+@@ -1065,6 +1048,7 @@
+    }
+    else
+    if (VG_(clo_xml)) {
++      Char *xml_fname_unexpanded = VG_(clo_xml_fname_unexpanded);
+       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 @@
+    Int     need_help          = 0; // 0 = no, 1 = --help, 2 = --help-debug
+    ThreadId tid_main          = VG_INVALID_THREADID;
+    Bool    logging_to_fd      = False;
+-   Char* xml_fname_unexpanded = NULL;
+    Int     loglevel, i;
+    struct vki_rlimit zero = { 0, 0 };
+    XArray* addr2dihandle = NULL;
+@@ -1870,8 +1853,7 @@
+    VG_(debugLog)(1, "main",
+                     "(main_) Process Valgrind's command line options, "
+                     "setup logging\n");
+-   main_process_cmd_line_options ( &logging_to_fd, &xml_fname_unexpanded,
+-                                   toolname );
++   main_process_cmd_line_options ( &logging_to_fd, toolname );
+ 
+    //--------------------------------------------------------------
+    // Zeroise the millisecond counter by doing a first read of it.
+@@ -1884,10 +1866,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),
+-   //          logging_to_fd, xml_fname_unexpanded]
++   //          logging_to_fd]
+    //--------------------------------------------------------------
+    VG_(debugLog)(1, "main", "Print the preamble...\n");
+-   print_preamble(logging_to_fd, xml_fname_unexpanded, toolname);
++   print_preamble(logging_to_fd, toolname);
+    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 10399)
+--- coregrind/m_libcproc.c	(revision 10771)
 +++ coregrind/m_libcproc.c	(working copy)
 @@ -33,9 +33,12 @@
  #include "pub_core_vkiscnums.h"
@@ -72,52 +346,32 @@ Index: coregrind/m_libcproc.c
  #include "pub_core_seqmatch.h"
  #include "pub_core_mallocfree.h"
  #include "pub_core_syscall.h"
-@@ -703,10 +706,59 @@
+@@ -708,10 +711,39 @@
           (*atforks[i].parent)(tid);
  }
  
 +// Defined in m_main.c
 +void print_preamble(Bool logging_to_fd, const char* toolname);
 +
++Char* VG_(clo_log_fname_unexpanded) = NULL;
++Char* VG_(clo_xml_fname_unexpanded) = NULL;
++
 +// If --log-file=ABC%pXYZ is specified, we'd like to have separate log files
 +// for each forked child.
 +// If %p is present in the --log-file option, this function creates
 +// a new log file and redirects the child's output to it.
 +static void open_new_logfile_for_forked_child(void)
 +{
-+   SysRes sres;
-+   Int tmp_log_fd = -1;
-+   Char *logfilename, *clo_log_name;
++   Int tmp_fd = -1;
 +
-+   clo_log_name = VG_(clo_log_name);
-+   if (clo_log_name == NULL || !VG_(strstr)(clo_log_name, "%p")) {
-+      // Don't create new log streams unless --log-file=ABC%pXYZ is specified.
-+      return;
++   if (VG_(log_output_sink).is_socket == False && VG_(clo_log_fname_unexpanded) != NULL) {
++     tmp_fd = reopen_output_fd(False);
++     VG_(log_output_sink).fd = VG_(safe_fd)(tmp_fd);
 +   }
 +
-+   logfilename = VG_(expand_file_name)("--log-file", clo_log_name);
-+   sres = VG_(open) (logfilename,
-+                    VKI_O_CREAT | VKI_O_WRONLY | VKI_O_TRUNC,
-+                    VKI_S_IRUSR | VKI_S_IWUSR);
-+   if (!sr_isError(sres)) {
-+      tmp_log_fd = sr_Res(sres);
-+      // Move log_fd into the safe range,
-+      // so it doesn't conflict with any app fds.
-+      tmp_log_fd = VG_(fcntl) (tmp_log_fd, VKI_F_DUPFD, VG_(fd_hard_limit));
-+      if (tmp_log_fd >= 0) {
-+         VG_(clo_log_fd) = tmp_log_fd;
-+         VG_(fcntl) (VG_(clo_log_fd), VKI_F_SETFD, VKI_FD_CLOEXEC);
-+      } else {
-+         VG_(message) (Vg_UserMsg,
-+                       "valgrind: failed to move logfile fd into safe range, "
-+                       "using stderr");
-+         VG_(clo_log_fd) = 2;    // stderr
-+      }
-+   } else {
-+      VG_(message) (Vg_UserMsg,
-+                    "Can't create log file '%s' (%s); giving up!",
-+                    logfilename, VG_(strerror) (sr_Err(sres)));
-+      VG_(core_panic)("Error creating log file for child process");
++   if (VG_(xml_output_sink).is_socket == False && VG_(clo_xml_fname_unexpanded) != NULL) {
++     tmp_fd = reopen_output_fd(True);
++     VG_(xml_output_sink).fd = VG_(safe_fd)(tmp_fd);
 +   }
 +
 +   print_preamble(False, NULL);
@@ -132,3 +386,17 @@ Index: coregrind/m_libcproc.c
     for (i = 0; i < n_atfork; i++)
        if (atforks[i].child != NULL)
           (*atforks[i].child)(tid);
+Index: coregrind/pub_core_options.h
+===================================================================
+--- coregrind/pub_core_options.h	(revision 10771)
++++ coregrind/pub_core_options.h	(working copy)
+@@ -74,6 +74,9 @@
+ extern Char* VG_(clo_log_fname_expanded);
+ extern Char* VG_(clo_xml_fname_expanded);
+ 
++extern Char* VG_(clo_log_fname_unexpanded);
++extern Char* VG_(clo_xml_fname_unexpanded);
++
+ /* Add timestamps to log messages?  default: NO */
+ extern Bool  VG_(clo_time_stamp);
+ 
diff --git a/tools/valgrind/leak.patch b/tools/valgrind/leak.patch
index 5526f11..e69de29 100644
--- a/tools/valgrind/leak.patch
+++ b/tools/valgrind/leak.patch
@@ -1,18 +0,0 @@
-Index: coregrind/m_main.c
-===================================================================
---- coregrind/m_main.c	(revision 10461)
-+++ coregrind/m_main.c	(working copy)
-@@ -2175,11 +2186,11 @@
-    if (VG_(clo_track_fds))
-       VG_(show_open_fds)();
- 
-+   VG_TDICT_CALL(tool_fini, 0/*exitcode*/);
-+
-    if (VG_(needs).core_errors || VG_(needs).tool_errors)
-       VG_(show_all_errors)();
- 
--   VG_TDICT_CALL(tool_fini, 0/*exitcode*/);
--
-    if (VG_(clo_xml)) {
-       VG_(message)(Vg_UserMsg, "");
-       VG_(message)(Vg_UserMsg, "</valgrindoutput>");
diff --git a/tools/valgrind/longlines.patch b/tools/valgrind/longlines.patch
index a3c0619..e69de29 100644
--- a/tools/valgrind/longlines.patch
+++ b/tools/valgrind/longlines.patch
@@ -1,11 +0,0 @@
---- /data/dkegel/valgrind-10414-vex-1906/coregrind/m_errormgr.c	2009-07-08 15:49:30.000000000 -0700
-+++ coregrind/m_errormgr.c	2009-07-08 17:17:36.000000000 -0700
-@@ -942,7 +942,7 @@
- */
- static void load_one_suppressions_file ( Char* filename )
- {
--#  define N_BUF 200
-+#  define N_BUF 2000
-    SysRes sres;
-    Int    fd, i, j, lineno = 0;
-    Bool   eof;
diff --git a/tools/valgrind/memcheck_analyze.py b/tools/valgrind/memcheck_analyze.py
index c8c566a..b5e192e 100755
--- a/tools/valgrind/memcheck_analyze.py
+++ b/tools/valgrind/memcheck_analyze.py
@@ -127,6 +127,8 @@ class ValgrindError:
       if node.localName == "what" or node.localName == "auxwhat":
         description = "".join([n.data for n in node.childNodes
                               if n.nodeType == n.TEXT_NODE])
+      elif node.localName == "xwhat":
+        description = getTextOf(node, "text")
       elif node.localName == "stack":
         self._backtraces.append([description, gatherFrames(node, source_dir)])
         description = None
diff --git a/tools/valgrind/possible.patch b/tools/valgrind/possible.patch
index af4a1ef..1091844 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 10536)
+--- memcheck/mc_main.c	(revision 10771)
 +++ 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 10536)
+--- memcheck/mc_include.h	(revision 10771)
 +++ memcheck/mc_include.h	(working copy)
-@@ -390,6 +390,9 @@
+@@ -394,6 +394,9 @@
  /* In leak check, show reachable-but-not-freed blocks?  default: NO */
  extern Bool MC_(clo_show_reachable);
  
@@ -42,15 +42,19 @@ Index: memcheck/mc_include.h
  extern Bool MC_(clo_workaround_gcc296_bugs);
 Index: memcheck/mc_leakcheck.c
 ===================================================================
---- memcheck/mc_leakcheck.c	(revision 10536)
+--- memcheck/mc_leakcheck.c	(revision 10771)
 +++ memcheck/mc_leakcheck.c	(working copy)
-@@ -844,7 +844,8 @@
+@@ -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);
        print_record = is_full_check &&
-                      ( MC_(clo_show_reachable) || 
-                        Unreached == lr->key.state || 
--                       Possible  == lr->key.state );
-+                       ( MC_(clo_show_possible) && 
-+                         Possible  == lr->key.state ) );
+                      ( MC_(clo_show_reachable) || count_as_error );
        is_suppressed = 
-          MC_(record_leak_error) ( tid, i+1, n_lossrecords, lr, print_record );
- 
+@@ -1113,4 +1113,3 @@
+ /*--------------------------------------------------------------------*/
+ /*--- end                                                          ---*/
+ /*--------------------------------------------------------------------*/
+-
diff --git a/tools/valgrind/valgrind_test.py b/tools/valgrind/valgrind_test.py
index c837235..5990107 100755
--- a/tools/valgrind/valgrind_test.py
+++ b/tools/valgrind/valgrind_test.py
@@ -43,6 +43,10 @@ class ValgrindTool(object):
       shutil.rmtree(self.TMP_DIR)
     os.mkdir(self.TMP_DIR)
 
+  def UseXML(self):
+    # Override if tool prefers nonxml output
+    return True
+
   def ToolName(self):
     raise RuntimeError, "This method should be implemented " \
                         "in the tool-specific subclass"
@@ -71,9 +75,6 @@ class ValgrindTool(object):
     self._parser.add_option("", "--trace_children", action="store_true",
                             default=False,
                             help="also trace child processes")
-    self._parser.add_option("", "--gen_suppressions", action="store_true",
-                            dest="generate_suppressions", default=False,
-                            help="skip analysis and generate suppressions")
     self._parser.add_option("", "--num-callers",
                             dest="num_callers", default=30,
                             help="number of callers to show in stack traces")
@@ -84,10 +85,9 @@ class ValgrindTool(object):
     self._parser.description = __doc__
 
   def ExtendOptionParser(self, parser):
-    if sys.platform == 'darwin':
-      parser.add_option("", "--generate_dsym", action="store_true",
-                            default=False,
-                            help="Generate .dSYM file on Mac if needed. Slow!")
+    parser.add_option("", "--generate_dsym", action="store_true",
+                          default=False,
+                          help="Generate .dSYM file on Mac if needed. Slow!")
 
   def ParseArgv(self, args):
     self.CreateOptionParser()
@@ -118,7 +118,6 @@ class ValgrindTool(object):
 
     self._timeout = int(self._options.timeout)
     self._num_callers = int(self._options.num_callers)
-    self._generate_suppressions = self._options.generate_suppressions
     self._suppressions = self._options.suppressions
     self._source_dir = self._options.source_dir
     self._nocleanup_on_exit = self._options.nocleanup_on_exit
@@ -216,9 +215,6 @@ class ValgrindTool(object):
     proc += self.ToolSpecificFlags()
     proc += self._tool_flags
 
-    if self._generate_suppressions:
-      proc += ["--gen-suppressions=all"]
-
     suppression_count = 0
     for suppression_file in self._suppressions:
       if os.path.exists(suppression_file):
@@ -228,7 +224,15 @@ class ValgrindTool(object):
     if not suppression_count:
       logging.warning("WARNING: NOT USING SUPPRESSIONS!")
 
-    proc += ["--log-file=" + self.TMP_DIR + ("/%s." % tool_name) + "%p"]
+    logfilename = self.TMP_DIR + ("/%s." % tool_name) + "%p"
+    if self.UseXML():
+      if os.system("valgrind --help | grep -q xml-file") == 0:
+        proc += ["--xml=yes", "--xml-file=" + logfilename]
+      else:
+        # TODO(dank): remove once valgrind-3.5 is deployed everywhere
+        proc += ["--xml=yes", "--log-file=" + logfilename]
+    else:
+      proc += ["--log-file=" + logfilename]
 
     # The Valgrind command is constructed.
 
@@ -349,35 +353,12 @@ class Memcheck(ValgrindTool):
     if self._options.track_origins:
       ret += ["--track-origins=yes"];
 
-    """Either generate suppressions or load them.
-    TODO(dkegel): enhance valgrind to support generating
-    suppressions in xml mode.  See
-    http://bugs.kde.org/show_bug.cgi?id=191189
-    """
-    if self._generate_suppressions:
-      ret += ["--gen-suppressions=all"]
-    else:
-      ret += ["--xml=yes"]
-
     return ret
 
   def Analyze(self):
     # Glob all the files in the "valgrind.tmp" directory
     filenames = glob.glob(self.TMP_DIR + "/memcheck.*")
 
-    # TODO(dkegel): use new xml suppressions feature when it lands
-    if self._generate_suppressions:
-      # Just concatenate all the output files.  Lame...
-      for filename in filenames:
-        print "## %s" % filename
-        f = file(filename)
-        while True:
-          line = f.readline()
-          if len(line) == 0:
-            break
-          print line, # comma means don't add newline
-        f.close()
-      return 0
     analyzer = memcheck_analyze.MemcheckAnalyze(self._source_dir, filenames, self._options.show_all_leaks)
     return analyzer.Report()
 
@@ -390,6 +371,9 @@ class ThreadSanitizer(ValgrindTool):
   def ToolName(self):
     return "tsan"
 
+  def UseXML(self):
+    return False
+
   def ExtendOptionParser(self, parser):
     ValgrindTool.ExtendOptionParser(self, parser)
     parser.add_option("", "--suppressions", default=[],