Fix gdb line number lookup on the mac.

1) need to quote filenames
2) need to know what address each library was loaded at
3) need to manually add symlink to chromium framework .dSYM file

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

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

4 files changed, 69 insertions, 14 deletions

diff --git a/tools/valgrind/build-valgrind-for-chromium.sh b/tools/valgrind/build-valgrind-for-chromium.sh
index 06b6ef6..4d8c513 100755
--- a/tools/valgrind/build-valgrind-for-chromium.sh
+++ b/tools/valgrind/build-valgrind-for-chromium.sh
@@ -64,6 +64,10 @@ then
   # "Want --show-possible option so I can ignore the bazillion possible leaks..."
   patch -p0 < "${THISDIR}/possible.patch"
 
+  # Add feature bug https://bugs.kde.org/show_bug.cgi?id=205000
+  # "Need library load address in log files"
+  patch -p0 < "${THISDIR}/xml-loadadr.patch"
+
   if [ "${INSTALL_TSAN}" = "yes" ]
   then
     # Add ThreadSanitier to the installation.
diff --git a/tools/valgrind/memcheck_analyze.py b/tools/valgrind/memcheck_analyze.py
index 15fe68c..8e7e563 100755
--- a/tools/valgrind/memcheck_analyze.py
+++ b/tools/valgrind/memcheck_analyze.py
@@ -31,10 +31,10 @@ def _GdbOutputToFileLine(output_line):
   else:
     return None
 
-def ResolveAddressesWithinABinary(binary_name, address_list):
+def ResolveAddressesWithinABinary(binary_name, load_address, address_list):
   ''' For each address, return a pair (file, line num) '''
   commands = tempfile.NamedTemporaryFile()
-  commands.write('file %s\n' % binary_name)
+  commands.write('add-symbol-file "%s" %s\n' % (binary_name, load_address))
   for addr in address_list:
     commands.write('info line *%s\n' % addr)
   commands.write('quit\n')
@@ -59,11 +59,18 @@ def ResolveAddressesWithinABinary(binary_name, address_list):
 class _AddressTable(object):
   ''' Object to do batched line number lookup. '''
   def __init__(self):
+    self._load_addresses = {}
     self._binaries = {}
     self._all_resolved = False
 
+  def AddBinaryAt(self, binary, load_address):
+    ''' Register a new shared library or executable. '''
+    self._load_addresses[binary] = load_address
+
   def Add(self, binary, address):
     ''' Register a lookup request. '''
+    if binary == '':
+      logging.warn('adding address %s in empty binary?' % address)
     if binary in self._binaries:
       self._binaries[binary].append(address)
     else:
@@ -74,8 +81,10 @@ class _AddressTable(object):
     ''' Carry out all lookup requests. '''
     self._translation = {}
     for binary in self._binaries.keys():
-      addr = ResolveAddressesWithinABinary(binary, self._binaries[binary])
-      self._translation[binary] = addr
+      if binary != '':
+        load_address = self._load_addresses[binary]
+        addr = ResolveAddressesWithinABinary(binary, load_address, self._binaries[binary])
+        self._translation[binary] = addr
     self._all_resolved = True
 
   def GetFileLine(self, binary, addr):
@@ -195,13 +204,20 @@ class ValgrindError:
     # Each frame looks like this:
     #  <frame>
     #    <ip>0x83751BC</ip>
-    #    <obj>/usr/local/google/bigdata/dkegel/chrome-build/src/out/Release/base_unittests</obj>
+    #    <obj>/data/dkegel/chrome-build/src/out/Release/base_unittests</obj>
     #    <fn>_ZN7testing8internal12TestInfoImpl7RunTestEPNS_8TestInfoE</fn>
     #    <dir>/data/dkegel/chrome-build/src/testing/gtest/src</dir>
     #    <file>gtest-internal-inl.h</file>
     #    <line>655</line>
     #  </frame>
-    # although the dir, file, and line elements are missing if there is no debug info.
+    # although the dir, file, and line elements are missing if there is
+    # no debug info.
+    #
+    # With our patch for https://bugs.kde.org/show_bug.cgi?id=205000 in,
+    # the file also includes records of the form
+    # <load_obj><obj>/usr/lib/libgcc_s.1.dylib</obj><ip>0x27000</ip></load_obj>
+    # giving the filename and load address of each binary that was mapped
+    # into the process.
 
     self._kind = getTextOf(error_node, "kind")
     self._backtraces = []
@@ -339,13 +355,7 @@ class MemcheckAnalyze:
         if origsize > newsize+1:
           logging.warn(str(origsize - newsize) + " bytes of junk were after </valgrindoutput> in %s!" % file)
         try:
-          raw_errors = parse(file).getElementsByTagName("error")
-          for raw_error in raw_errors:
-            # Ignore "possible" leaks for now by default.
-            if (show_all_leaks or
-                getTextOf(raw_error, "kind") != "Leak_PossiblyLost"):
-              error = ValgrindError(source_dir, raw_error)
-              self._errors.add(error)
+          parsed_file = parse(file);
         except ExpatError, e:
           self._parse_failed = True
           logging.warn("could not parse %s: %s" % (file, e))
@@ -364,6 +374,22 @@ class MemcheckAnalyze:
               logging.warn("> %s" % context_data)
           context_file.close()
           continue
+        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)
+
+        raw_errors = parsed_file.getElementsByTagName("error")
+        for raw_error in raw_errors:
+          # Ignore "possible" leaks for now by default.
+          if (show_all_leaks or
+              getTextOf(raw_error, "kind") != "Leak_PossiblyLost"):
+            error = ValgrindError(source_dir, raw_error)
+            self._errors.add(error)
+
     if len(badfiles) > 0:
       logging.warn("valgrind didn't finish writing %d files?!" % len(badfiles))
       for file in badfiles:
diff --git a/tools/valgrind/valgrind_test.py b/tools/valgrind/valgrind_test.py
index 5160ffe..14d3076 100755
--- a/tools/valgrind/valgrind_test.py
+++ b/tools/valgrind/valgrind_test.py
@@ -181,6 +181,18 @@ class ValgrindTool(object):
         shutil.copyfile(dsym_file, test_command)
         shutil.copymode(saved_test_command, test_command)
 
+        # Make sure the Chromium Framework symbols are available.
+        # See message from mmentovai on aug 5
+        build_dir = os.path.dirname(test_command)
+        symlink_source = '../../../Chromium Framework.framework.dSYM'
+        symlink_target = build_dir + '/Chromium.app/Contents/Frameworks/Chromium Framework.framework.dSYM'
+        try:
+          os.remove(symlink_target)
+        except IOError, (errno, strerror):
+          pass
+        logging.info('Trying ln -s "%s" "%s"' % (symlink_source, symlink_target))
+        os.symlink(symlink_source, symlink_target)
+
     if needs_dsymutil:
       if self._options.generate_dsym:
         # Remove the .dSYM bundle if it exists.
@@ -364,7 +376,7 @@ class Memcheck(ValgrindTool):
     filenames = glob.glob(self.TMP_DIR + "/memcheck.*")
 
     use_gdb = (sys.platform == 'darwin')
-    analyzer = memcheck_analyze.MemcheckAnalyze(self._source_dir, filenames, self._options.show_all_leaks, 
+    analyzer = memcheck_analyze.MemcheckAnalyze(self._source_dir, filenames, self._options.show_all_leaks,
                                                 use_gdb=use_gdb)
     return analyzer.Report()
 
diff --git a/tools/valgrind/xml-loadadr.patch b/tools/valgrind/xml-loadadr.patch
new file mode 100644
index 0000000..a398543
--- /dev/null
+++ b/tools/valgrind/xml-loadadr.patch
@@ -0,0 +1,13 @@
+Index: coregrind/m_debuginfo/readmacho.c
+===================================================================
+--- coregrind/m_debuginfo/readmacho.c	(revision 10771)
++++ coregrind/m_debuginfo/readmacho.c	(working copy)
+@@ -680,6 +682,8 @@
+    if (VG_(clo_verbosity) > 1)
+       VG_(message)(Vg_DebugMsg,
+                    "%s (%#lx)\n", di->filename, di->rx_map_avma );
++   if (VG_(clo_xml))
++      VG_(printf_xml)("<load_obj><obj>%s</obj><ip>%#lx</ip></load_obj>\n",  di->filename, di->rx_map_avma);
+ 
+    /* This should be ensured by our caller. */
+    vg_assert(di->have_rx_map);