Bug 1573090 - Part 2: Look up ELF symbol tables directly when pre-symbolicating Gecko profiles on Linux. r=gerald

Whereas previously MozDescribeCodeAddress would have handled demangling,
we need to explicitly do that from our new GetFunction method.  The string we
generate is now more useful for the profiler to merge -- having dropped the
address in the previous patch, and the file & line number and library in this
patch.

While we're at it, try to demangle Rust symbols too.

Ideally we'd add Rust symbol handling to DemangleSymbol in
StackWalk.cpp, but that lives in mozglue, which currently cannot have
any Rust crate dependencies.

Differential Revision: https://phabricator.services.mozilla.com/D43142

--HG--
extra : moz-landing-system : lando

Cargo.lock

@@ -2320,6 +2320,7 @@ dependencies = [
  "goblin 0.0.17 (registry+https://github.com/rust-lang/crates.io-index)",
  "memmap 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "object 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "rustc-demangle 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "thin-vec 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 

mozglue/misc/StackWalk.cpp

@@ -672,6 +672,8 @@ MFBT_API bool MozDescribeCodeAddress(void* aPC,
 #    include <cxxabi.h>
 #  endif  // MOZ_DEMANGLE_SYMBOLS
 
+namespace mozilla {
+
 void DemangleSymbol(const char* aSymbol, char* aBuffer, int aBufLen) {
   aBuffer[0] = '\0';
 
@@ -687,6 +689,8 @@ void DemangleSymbol(const char* aSymbol, char* aBuffer, int aBufLen) {
 #  endif  // MOZ_DEMANGLE_SYMBOLS
 }
 
+}  // namespace mozilla
+
 // {x86, ppc} x {Linux, Mac} stackwalking code.
 #  if ((defined(__i386) || defined(PPC) || defined(__ppc__)) && \
        (MOZ_STACKWALK_SUPPORTS_MACOSX || MOZ_STACKWALK_SUPPORTS_LINUX))

mozglue/misc/StackWalk.h

@@ -168,6 +168,10 @@ MFBT_API void FramePointerStackWalk(MozWalkStackCallback aCallback,
                                     void* aClosure, void** aBp,
                                     void* aStackEnd);
 
+#ifdef XP_LINUX
+MFBT_API void DemangleSymbol(const char* aSymbol, char* aBuffer, int aBufLen);
+#endif
+
 }  // namespace mozilla
 
 #endif

toolkit/moz.configure

@@ -78,10 +78,16 @@ set_define('MOZ_GECKO_PROFILER', gecko_profiler_define)
 # (for symbol table dumping).
 @depends(gecko_profiler, target)
 def gecko_profiler_parse_elf(value, target):
-    # Currently we only want to build this code on Android, in order to dump
-    # symbols from Android system libraries on the device. For other platforms
-    # there exist alternatives that don't require bloating up our binary size.
-    if value and target.os == 'Android':
+    # Currently we only want to build this code on Linux (including Android).
+    # For Android, this is in order to dump symbols from Android system, where
+    # on other platforms there exist alternatives that don't require bloating
+    # up our binary size. For Linux more generally, we use this in profile
+    # pre-symbolication support, since MozDescribeCodeAddress doesn't do
+    # anything useful on that platform. (Ideally, we would update
+    # MozDescribeCodeAddress to call into some Rust crates that parse ELF and
+    # DWARF data, but build system issues currently prevent Rust from being
+    # used in mozglue.)
+    if value and target.kernel == 'Linux':
         return True
 
 set_config('MOZ_GECKO_PROFILER_PARSE_ELF', gecko_profiler_parse_elf)

tools/profiler/core/ProfileBufferEntry.cpp

@@ -958,23 +958,19 @@ void ProfileBuffer::StreamSamplesToJSON(SpliceableJSONWriter& aWriter,
         void* pc = e.Get().GetPtr();
         e.Next();
 
-        static const uint32_t BUF_SIZE = 256;
-        char buf[BUF_SIZE];
+        nsCString buf;
 
-        if (aUniqueStacks.mCodeAddressService) {
-          // Add description after space. Note: Using a frame number of 0,
-          // as using `numFrames` wouldn't help here, and would prevent
-          // combining same function calls that happen at different depths.
-          // TODO: Remove unsightly "#00: " if too annoying. :-)
-          aUniqueStacks.mCodeAddressService->GetLocation(0, pc, buf, BUF_SIZE);
-        } else {
+        if (!aUniqueStacks.mCodeAddressService ||
+            !aUniqueStacks.mCodeAddressService->GetFunction(pc, buf) ||
+            buf.IsEmpty()) {
           // Bug 753041: We need a double cast here to tell GCC that we don't
           // want to sign extend 32-bit addresses starting with 0xFXXXXXX.
           unsigned long long pcULL = (unsigned long long)(uintptr_t)pc;
-          SprintfLiteral(buf, "%#llx", pcULL);
+          buf.AppendPrintf("0x%llx", pcULL);
         }
 
-        stack = aUniqueStacks.AppendFrame(stack, UniqueStacks::FrameKey(buf));
+        stack =
+            aUniqueStacks.AppendFrame(stack, UniqueStacks::FrameKey(buf.get()));
 
       } else if (e.Get().IsLabel()) {
         numFrames++;

tools/profiler/core/ProfilerCodeAddressService.cpp

@@ -0,0 +1,75 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* 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/. */
+
+#include "ProfilerCodeAddressService.h"
+
+#include "platform.h"
+#include "mozilla/StackWalk.h"
+
+using namespace mozilla;
+
+#ifdef XP_LINUX
+static char* SearchSymbolTable(SymbolTable& aTable, uint32_t aOffset) {
+  size_t index;
+  bool exact =
+      BinarySearch(aTable.mAddrs, 0, aTable.mAddrs.Length(), aOffset, &index);
+
+  if (index == 0 && !exact) {
+    // Our offset is before the first symbol in the table; no result.
+    return nullptr;
+  }
+
+  // Extract the (mangled) symbol name out of the string table.
+  auto strings = reinterpret_cast<char*>(aTable.mBuffer.Elements());
+  nsCString symbol;
+  symbol.Append(strings + aTable.mIndex[index - 1],
+                aTable.mIndex[index] - aTable.mIndex[index - 1]);
+
+  // First try demangling as a Rust identifier.
+  char demangled[1024];
+  if (!profiler_demangle_rust(symbol.get(), demangled,
+                              ArrayLength(demangled))) {
+    // Then as a C++ identifier.
+    DemangleSymbol(symbol.get(), demangled, ArrayLength(demangled));
+  }
+  demangled[ArrayLength(demangled) - 1] = '\0';
+
+  // Use the mangled name if we didn't successfully demangle.
+  return strdup(demangled[0] != '\0' ? demangled : symbol.get());
+}
+#endif
+
+bool ProfilerCodeAddressService::GetFunction(const void* aPc,
+                                             nsACString& aResult) {
+  Entry& entry = GetEntry(aPc);
+
+#ifdef XP_LINUX
+  // On Linux, most symbols will not be found by the MozDescribeCodeAddress call
+  // that GetEntry does.  So we read the symbol table directly from the ELF
+  // image.
+
+  // SymbolTable currently assumes library offsets will not be larger than
+  // 4 GiB.
+  if (entry.mLOffset <= 0xFFFFFFFF && !entry.mFunction) {
+    auto p = mSymbolTables.lookupForAdd(entry.mLibrary);
+    if (!p) {
+      if (!mSymbolTables.add(p, entry.mLibrary, SymbolTable())) {
+        MOZ_CRASH("ProfilerCodeAddressService OOM");
+      }
+      profiler_get_symbol_table(entry.mLibrary, nullptr, &p->value());
+    }
+    entry.mFunction =
+        SearchSymbolTable(p->value(), static_cast<uint32_t>(entry.mLOffset));
+  }
+#endif
+
+  if (!entry.mFunction || entry.mFunction[0] == '\0') {
+    return false;
+  }
+
+  aResult = nsDependentCString(entry.mFunction);
+  return true;
+}

tools/profiler/core/platform.cpp

@@ -3119,8 +3119,12 @@ UniquePtr<char[]> profiler_get_profile(double aSinceTime,
                                        bool aIsShuttingDown) {
   LOG("profiler_get_profile");
 
+  UniquePtr<ProfilerCodeAddressService> service =
+      profiler_code_address_service_for_presymbolication();
+
   SpliceableChunkedJSONWriter b;
-  if (!WriteProfileToJSONWriter(b, aSinceTime, aIsShuttingDown, nullptr)) {
+  if (!WriteProfileToJSONWriter(b, aSinceTime, aIsShuttingDown,
+                                service.get())) {
     return nullptr;
   }
   return b.WriteFunc()->CopyData();

tools/profiler/core/platform.h

@@ -41,6 +41,10 @@
 #include <functional>
 #include <stdint.h>
 
+namespace mozilla {
+struct SymbolTable;
+}
+
 extern mozilla::LazyLogModule gProfilerLog;
 
 // These are for MOZ_LOG="prof:3" or higher. It's the default logging level for
@@ -109,4 +113,14 @@ mozilla::Vector<nsCString> profiler_move_exit_profiles();
 mozilla::UniquePtr<ProfilerCodeAddressService>
 profiler_code_address_service_for_presymbolication();
 
+extern "C" {
+// This function is defined in the profiler rust module at
+// tools/profiler/rust-helper. mozilla::SymbolTable and CompactSymbolTable
+// have identical memory layout.
+bool profiler_get_symbol_table(const char* debug_path, const char* breakpad_id,
+                               mozilla::SymbolTable* symbol_table);
+
+bool profiler_demangle_rust(const char* mangled, char* buffer, size_t len);
+}
+
 #endif /* ndef TOOLS_PLATFORM_H_ */

tools/profiler/gecko/nsProfiler.cpp

@@ -41,14 +41,6 @@ using dom::AutoJSAPI;
 using dom::Promise;
 using std::string;
 
-extern "C" {
-// This function is defined in the profiler rust module at
-// tools/profiler/rust-helper. nsProfiler::SymbolTable and CompactSymbolTable
-// have identical memory layout.
-bool profiler_get_symbol_table(const char* debug_path, const char* breakpad_id,
-                               nsProfiler::SymbolTable* symbol_table);
-}
-
 NS_IMPL_ISUPPORTS(nsProfiler, nsIProfiler, nsIObserver)
 
 nsProfiler::nsProfiler()

tools/profiler/gecko/nsProfiler.h

@@ -16,6 +16,7 @@
 #include "mozilla/Vector.h"
 #include "nsServiceManagerUtils.h"
 #include "ProfileJSONWriter.h"
+#include "ProfilerCodeAddressService.h"
 
 class nsProfiler final : public nsIProfiler, public nsIObserver {
  public:
@@ -35,24 +36,12 @@ class nsProfiler final : public nsIProfiler, public nsIObserver {
 
   void GatheredOOPProfile(const nsACString& aProfile);
 
-  // This SymbolTable struct, and the CompactSymbolTable struct in the
-  // profiler rust module, have the exact same memory layout.
-  // nsTArray and ThinVec are FFI-compatible, because the thin-vec crate is
-  // being compiled with the "gecko-ffi" feature enabled.
-  struct SymbolTable {
-    SymbolTable() = default;
-    SymbolTable(SymbolTable&& aOther) = default;
-
-    nsTArray<uint32_t> mAddrs;
-    nsTArray<uint32_t> mIndex;
-    nsTArray<uint8_t> mBuffer;
-  };
-
  private:
   ~nsProfiler();
 
   typedef mozilla::MozPromise<nsCString, nsresult, false> GatheringPromise;
-  typedef mozilla::MozPromise<SymbolTable, nsresult, true> SymbolTablePromise;
+  typedef mozilla::MozPromise<mozilla::SymbolTable, nsresult, true>
+      SymbolTablePromise;
 
   RefPtr<GatheringPromise> StartGathering(double aSinceTime);
   void FinishGathering();

tools/profiler/moz.build

@@ -28,6 +28,7 @@ if CONFIG['MOZ_GECKO_PROFILER']:
         'core/ProfiledThreadData.cpp',
         'core/ProfileJSONWriter.cpp',
         'core/ProfilerBacktrace.cpp',
+        'core/ProfilerCodeAddressService.cpp',
         'core/ProfilerMarkerPayload.cpp',
         'core/RegisteredThread.cpp',
         'gecko/ChildProfilerController.cpp',

tools/profiler/public/ProfilerCodeAddressService.h

@@ -9,7 +9,43 @@
 
 #include "CodeAddressService.h"
 
-// XXX Will be extended with functionality in the next patch.
-class ProfilerCodeAddressService : public mozilla::CodeAddressService<> {};
+namespace mozilla {
+
+// This SymbolTable struct, and the CompactSymbolTable struct in the
+// profiler rust module, have the exact same memory layout.
+// nsTArray and ThinVec are FFI-compatible, because the thin-vec crate is
+// being compiled with the "gecko-ffi" feature enabled.
+struct SymbolTable {
+  SymbolTable() = default;
+  SymbolTable(SymbolTable&& aOther) = default;
+
+  nsTArray<uint32_t> mAddrs;
+  nsTArray<uint32_t> mIndex;
+  nsTArray<uint8_t> mBuffer;
+};
+
+}  // namespace mozilla
+
+/**
+ * Cache and look up function symbol names.
+ *
+ * We don't template this on AllocPolicy since we need to use nsTArray in
+ * SymbolTable above, which doesn't work with AllocPolicy.  (We can't switch
+ * to Vector, as we would lose FFI compatibility with ThinVec.)
+ */
+class ProfilerCodeAddressService : public mozilla::CodeAddressService<> {
+ public:
+  // Like GetLocation, but only returns the symbol name.
+  bool GetFunction(const void* aPc, nsACString& aResult);
+
+ private:
+#ifdef XP_LINUX
+  // Map of library names (owned by mLibraryStrings) to SymbolTables filled
+  // in by profiler_get_symbol_table.
+  mozilla::HashMap<const char*, mozilla::SymbolTable,
+                   mozilla::DefaultHasher<const char*>, AllocPolicy>
+      mSymbolTables;
+#endif
+};
 
 #endif  // ProfilerCodeAddressService_h

tools/profiler/rust-helper/Cargo.toml

@@ -5,6 +5,7 @@ authors = ["Markus Stange <mstange@themasta.com>"]
 
 [dependencies]
 memmap = "0.6.2"
+rustc-demangle = "0.1"
 
 [dependencies.object]
 version = "0.10.0"

tools/profiler/rust-helper/src/elf.rs

@@ -23,10 +23,10 @@ where
     .collect()
 }
 
-pub fn get_compact_symbol_table(buffer: &[u8], breakpad_id: &str) -> Option<CompactSymbolTable> {
+pub fn get_compact_symbol_table(buffer: &[u8], breakpad_id: Option<&str>) -> Option<CompactSymbolTable> {
   let elf_file = ElfFile::parse(buffer).ok()?;
   let elf_id = get_elf_id(&elf_file, buffer)?;
-  if format!("{:X}0", elf_id.simple()) != breakpad_id {
+  if !breakpad_id.map_or(true, |id| id == format!("{:X}0", elf_id.simple())) {
     return None;
   }
   return Some(CompactSymbolTable::from_map(get_symbol_map(&elf_file)));

tools/profiler/rust-helper/src/lib.rs

@@ -3,6 +3,7 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 extern crate memmap;
+extern crate rustc_demangle;
 extern crate thin_vec;
 
 #[cfg(feature = "parse_elf")]
@@ -21,13 +22,16 @@ use memmap::MmapOptions;
 use std::fs::File;
 
 use std::ffi::CStr;
+use std::mem;
 use std::os::raw::c_char;
+use std::ptr;
 use compact_symbol_table::CompactSymbolTable;
+use rustc_demangle::try_demangle;
 
 #[cfg(feature = "parse_elf")]
 pub fn get_compact_symbol_table_from_file(
     debug_path: &str,
-    breakpad_id: &str,
+    breakpad_id: Option<&str>,
 ) -> Option<CompactSymbolTable> {
     let file = File::open(debug_path).ok()?;
     let buffer = unsafe { MmapOptions::new().map(&file).ok()? };
@@ -37,7 +41,7 @@ pub fn get_compact_symbol_table_from_file(
 #[cfg(not(feature = "parse_elf"))]
 pub fn get_compact_symbol_table_from_file(
     _debug_path: &str,
-    _breakpad_id: &str,
+    _breakpad_id: Option<&str>,
 ) -> Option<CompactSymbolTable> {
     None
 }
@@ -49,9 +53,16 @@ pub extern "C" fn profiler_get_symbol_table(
     symbol_table: &mut CompactSymbolTable,
 ) -> bool {
     let debug_path = unsafe { CStr::from_ptr(debug_path).to_string_lossy() };
-    let breakpad_id = unsafe { CStr::from_ptr(breakpad_id).to_string_lossy() };
+    let breakpad_id = if breakpad_id.is_null() {
+        None
+    } else {
+        match unsafe { CStr::from_ptr(breakpad_id).to_str() } {
+            Ok(s) => Some(s),
+            Err(_) => return false,
+        }
+    };
 
-    match get_compact_symbol_table_from_file(&debug_path, &breakpad_id) {
+    match get_compact_symbol_table_from_file(&debug_path, breakpad_id.map(|id| id.as_ref())) {
         Some(mut st) => {
             std::mem::swap(symbol_table, &mut st);
             true
@@ -59,3 +70,41 @@ pub extern "C" fn profiler_get_symbol_table(
         None => false,
     }
 }
+
+#[no_mangle]
+pub extern "C" fn profiler_demangle_rust(
+    mangled: *const c_char,
+    buffer: *mut c_char,
+    buffer_len: usize,
+) -> bool {
+    assert!(!mangled.is_null());
+    assert!(!buffer.is_null());
+
+    if buffer_len == 0 {
+        return false;
+    }
+
+    let buffer: *mut u8 = unsafe { mem::transmute(buffer) };
+    let mangled = match unsafe { CStr::from_ptr(mangled).to_str() } {
+        Ok(s) => s,
+        Err(_) => return false,
+    };
+
+    match try_demangle(mangled) {
+        Ok(demangled) => {
+            let mut demangled = format!("{:#}", demangled);
+            if !demangled.is_ascii() {
+                return false;
+            }
+            demangled.truncate(buffer_len - 1);
+
+            let bytes = demangled.as_bytes();
+            unsafe {
+                ptr::copy(bytes.as_ptr(), buffer, bytes.len());
+                ptr::write(buffer.offset(bytes.len() as isize), 0);
+            }
+            true
+        }
+        Err(_) => false,
+    }
+}

xpcom/base/CodeAddressService.h

@@ -56,6 +56,7 @@ template <class AllocPolicy_ = MallocAllocPolicy,
               detail::DefaultDescribeCodeAddressLock>
 class CodeAddressService
     : private detail::CodeAddressServiceAllocPolicy<AllocPolicy_> {
+ protected:
   // GetLocation() is the key function in this class.  It's basically a wrapper
   // around MozDescribeCodeAddress.
   //
@@ -144,31 +145,7 @@ class CodeAddressService
     return newString;
   }
 
-  // A direct-mapped cache.  When doing dmd::Analyze() just after starting
-  // desktop Firefox (which is similar to analyzing after a longer-running
-  // session, thanks to the limit on how many records we print), a cache with
-  // 2^24 entries (which approximates an infinite-entry cache) has a ~91% hit
-  // rate.  A cache with 2^12 entries has a ~83% hit rate, and takes up ~85 KiB
-  // (on 32-bit platforms) or ~150 KiB (on 64-bit platforms).
-  static const size_t kNumEntries = 1 << 12;
-  static const size_t kMask = kNumEntries - 1;
-  Entry mEntries[kNumEntries];
-
-  size_t mNumCacheHits;
-  size_t mNumCacheMisses;
-
- public:
-  CodeAddressService()
-      : mLibraryStrings(64), mEntries(), mNumCacheHits(0), mNumCacheMisses(0) {}
-
-  ~CodeAddressService() {
-    for (auto iter = mLibraryStrings.iter(); !iter.done(); iter.next()) {
-      AllocPolicy::free_(const_cast<char*>(iter.get()));
-    }
-  }
-
-  void GetLocation(uint32_t aFrameNumber, const void* aPc, char* aBuf,
-                   size_t aBufLen) {
+  Entry& GetEntry(const void* aPc) {
     MOZ_ASSERT(DescribeCodeAddressLock::IsLocked());
 
     uint32_t index = HashGeneric(aPc) & kMask;
@@ -200,6 +177,35 @@ class CodeAddressService
 
     MOZ_ASSERT(entry.mPc == aPc);
 
+    return entry;
+  }
+
+  // A direct-mapped cache.  When doing dmd::Analyze() just after starting
+  // desktop Firefox (which is similar to analyzing after a longer-running
+  // session, thanks to the limit on how many records we print), a cache with
+  // 2^24 entries (which approximates an infinite-entry cache) has a ~91% hit
+  // rate.  A cache with 2^12 entries has a ~83% hit rate, and takes up ~85 KiB
+  // (on 32-bit platforms) or ~150 KiB (on 64-bit platforms).
+  static const size_t kNumEntries = 1 << 12;
+  static const size_t kMask = kNumEntries - 1;
+  Entry mEntries[kNumEntries];
+
+  size_t mNumCacheHits;
+  size_t mNumCacheMisses;
+
+ public:
+  CodeAddressService()
+      : mLibraryStrings(64), mEntries(), mNumCacheHits(0), mNumCacheMisses(0) {}
+
+  ~CodeAddressService() {
+    for (auto iter = mLibraryStrings.iter(); !iter.done(); iter.next()) {
+      AllocPolicy::free_(const_cast<char*>(iter.get()));
+    }
+  }
+
+  void GetLocation(uint32_t aFrameNumber, const void* aPc, char* aBuf,
+                   size_t aBufLen) {
+    Entry& entry = GetEntry(aPc);
     MozFormatCodeAddress(aBuf, aBufLen, aFrameNumber, entry.mPc,
                          entry.mFunction, entry.mLibrary, entry.mLOffset,
                          entry.mFileName, entry.mLineNo);