Bug 1470223 - Prefer FUNC entries to PUBLIC entries when fixing symbols r=ted
authorGabriele Svelto <gsvelto@mozilla.com>
Fri, 21 Sep 2018 12:50:32 +0000
changeset 493482 552cf01fb131f58080232c47482d5b2ef5d979c3
parent 493481 3478b4d04eb9c86a88d8b6349f4015100e44ec6a
child 493483 9a90cc08d56d1039444aad89ed05c7adac3a097c
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersted
bugs1470223
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1470223 - Prefer FUNC entries to PUBLIC entries when fixing symbols r=ted This patch changes the way we search symbols when fixing up a stack. Previously we would find the closest PUBLIC or FUNC entry lower than a given address. Because of how symbol files were processed we preferred PUBLIC entries to FUNC ones. Now we look first for the function that contains the address (obtained from the FUNC entries) then if none is available we look for the closest, lower PUBLIC entry. Differential Revision: https://phabricator.services.mozilla.com/D5033
tools/moz.build
tools/rb/fix_stack_using_bpsyms.py
tools/rb/python.ini
tools/rb/test_fix_stack_using_bpsyms.py
--- a/tools/moz.build
+++ b/tools/moz.build
@@ -60,10 +60,11 @@ with Files('tryselect/docs/**'):
     SCHEDULES.exclusive = ['docs']
 
 CRAMTEST_MANIFESTS += [
     'tryselect/test/cram.ini',
 ]
 
 PYTHON_UNITTEST_MANIFESTS += [
     'lint/test/python.ini',
+    'rb/python.ini',
     'tryselect/test/python.ini',
 ]
--- a/tools/rb/fix_stack_using_bpsyms.py
+++ b/tools/rb/fix_stack_using_bpsyms.py
@@ -32,66 +32,93 @@ def prettyFileName(name):
             # return "http://%s/annotate/%s/%s#l" % (repo, rev, path)
             return path + ":"
     return name + ":"
 
 
 class SymbolFile:
     def __init__(self, fn):
         addrs = []  # list of addresses, which will be sorted once we're done initializing
-        funcs = {}  # hash: address --> (function name + possible file/line)
+        funcs = {}  # hash: address --> (function name, function size)
+        pubaddrs = []  # list of addresses for public symbols, sorted
+        pubs = {}  # hash: address --> public symbol name
         # hash: filenum (string) --> prettified filename ready to have a line number appended
         files = {}
         with open(fn) as f:
             for line in f:
                 line = line.rstrip()
                 # https://chromium.googlesource.com/breakpad/breakpad/+/master/docs/symbol_files.md
                 if line.startswith("FUNC "):
                     # FUNC [<multiple>] <address> <size> <stack_param_size> <name>
                     line = line.replace("FUNC m ", "FUNC ")  # Ignore the multiple marker
                     bits = line.split(None, 4)
                     if len(bits) < 5:
                         bits.append('unnamed_function')
                     (junk, rva, size, ss, name) = bits
                     rva = int(rva, 16)
-                    funcs[rva] = name
+                    funcs[rva] = {'name': name, 'size': int(size, 16)}
                     addrs.append(rva)
                     lastFuncName = name
                 elif line.startswith("PUBLIC "):
                     # PUBLIC [<multiple>] <address> <stack_param_size> <name>
                     line = line.replace("PUBLIC m ", "PUBLIC ")  # Ignore the multiple marker
                     (junk, rva, ss, name) = line.split(None, 3)
                     rva = int(rva, 16)
-                    funcs[rva] = name
-                    addrs.append(rva)
+                    pubs[rva] = name
+                    pubaddrs.append(rva)
                 elif line.startswith("FILE "):
                     # FILE <number> <name>
                     (junk, filenum, name) = line.split(None, 2)
                     files[filenum] = prettyFileName(name)
                 elif line[0] in "0123456789abcdef":
                     # This is one of the "line records" corresponding to the last FUNC record
                     # <address> <size> <line> <filenum>
                     (rva, size, line, filenum) = line.split(None)
                     rva = int(rva, 16)
                     file = files[filenum]
                     name = lastFuncName + " [" + file + line + "]"
-                    funcs[rva] = name
+                    funcs[rva] = {'name': name, 'size': int(size, 16)}
                     addrs.append(rva)
                 # skip everything else
-        # print "Loaded %d functions from symbol file %s" % (len(funcs), os.path.basename(fn))
         self.addrs = sorted(addrs)
         self.funcs = funcs
+        self.pubaddrs = sorted(pubaddrs)
+        self.pubs = pubs
+
+    def __find_matching_func(self, address):
+        # Look for a FUNC entry that contains the given address
+        i = bisect.bisect(self.addrs, address)
+        if i > 0:
+            func_addr = self.addrs[i - 1]
+            func = self.funcs[func_addr]
+            if address >= func_addr and address < func_addr + func['size']:
+                return func['name']
+
+        return None
+
+    def __find_closest_public_symbol(self, address):
+        # Find the closest PUBLIC entry that's lower than the given address
+        i = bisect.bisect(self.pubaddrs, address)
+        if i > 0:
+            return self.pubs[self.pubaddrs[i - 1]]
+        else:
+            return None
 
     def addrToSymbol(self, address):
-        i = bisect.bisect(self.addrs, address) - 1
-        if i > 0:
-            # offset = address - self.addrs[i]
-            return self.funcs[self.addrs[i]]
-        else:
-            return ""
+        entry = self.__find_matching_func(address)
+
+        if entry is not None:
+            return entry
+
+        entry = self.__find_closest_public_symbol(address)
+
+        if entry is not None:
+            return entry
+
+        return ""
 
 
 def findIdForPath(path):
     """Finds the breakpad id for the object file at the given path."""
     # We should always be packaged with a "fileid" executable.
     fileid_exe = os.path.join(here, 'fileid')
     if not os.path.isfile(fileid_exe):
         fileid_exe = fileid_exe + '.exe'
@@ -169,9 +196,9 @@ def fixSymbols(line, symbolsDir):
         return before + symbol + after + "\n"
     else:
         return line
 
 
 if __name__ == "__main__":
     symbolsDir = sys.argv[1]
     for line in iter(sys.stdin.readline, ''):
-        print fixSymbols(line, symbolsDir),
+        sys.stdout.write(fixSymbols(line, symbolsDir))
new file mode 100644
--- /dev/null
+++ b/tools/rb/python.ini
@@ -0,0 +1,3 @@
+[DEFAULT]
+
+[test_fix_stack_using_bpsyms.py]
new file mode 100755
--- /dev/null
+++ b/tools/rb/test_fix_stack_using_bpsyms.py
@@ -0,0 +1,65 @@
+#!/usr/bin/env python
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+import mozunit
+import platform
+import sys
+
+from mock import MagicMock, patch
+from fix_stack_using_bpsyms import fixSymbols, SymbolFile
+
+
+def test_fixSymbols_non_stack_line():
+    line = 'foobar\n'
+    res = fixSymbols(line, '.')
+    assert res == line
+
+
+def test_fix_Symbols_missing_symbol():
+    line = '#01: foobar[{:s}foo.{:s} +0xfc258]\n'
+    result = '#01: foo.{:s} + 0xfc258\n'
+    if platform.system() == "Windows":
+        line = line.format('C:\\application\\firefox\\', 'dll')
+        result = result.format('dll')
+    else:
+        line = line.format('/home/firerox/', 'so', '')
+        result = result.format('so')
+
+    output = fixSymbols(line, '.')
+    assert output == result
+
+
+symfile_data = '''FILE 0 source.c
+FUNC fc256 2 0 func
+FUNC fc258 4 0 func_line
+fc258 2 1 0
+fc25a 2 2 0
+FUNC m fc25c 2 0 func_multiple
+PUBLIC fc256 0 func_public
+PUBLIC fc260 0 other_public'''
+
+open_name = \
+    ('builtins.%s' if sys.version_info >= (3,) else '__builtin__.%s') % 'open'
+
+
+@patch(open_name, spec=open)
+def test_parse_SymbolFile(mock_open):
+    m = MagicMock()
+    m.__enter__.return_value.__iter__.return_value = (symfile_data.split('\n'))
+    m.__exit__.return_value = False
+    mock_open.side_effect = [m]
+    symfile = SymbolFile('foo.sym')
+    assert symfile is not None
+    assert symfile.addrToSymbol(int('fc255', 16)) == ''
+    assert symfile.addrToSymbol(int('fc256', 16)) == 'func'
+    assert symfile.addrToSymbol(int('fc258', 16)) == 'func_line [source.c:1]'
+    assert symfile.addrToSymbol(int('fc25a', 16)) == 'func_line [source.c:2]'
+    assert symfile.addrToSymbol(int('fc25b', 16)) == 'func_line [source.c:2]'
+    assert symfile.addrToSymbol(int('fc260', 16)) == 'other_public'
+    assert symfile.addrToSymbol(int('fc261', 16)) == 'other_public'
+
+
+if __name__ == '__main__':
+    mozunit.main()