From a1e18da1b1207b2439f332ea276c3b751ae88575 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Fri, 23 Aug 2019 05:45:16 +0000 Subject: [PATCH] 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 | 1 + mozglue/misc/StackWalk.cpp | 4 + mozglue/misc/StackWalk.h | 4 + toolkit/moz.configure | 14 +++- tools/profiler/core/ProfileBufferEntry.cpp | 18 ++--- .../core/ProfilerCodeAddressService.cpp | 75 +++++++++++++++++++ tools/profiler/core/platform.cpp | 6 +- tools/profiler/core/platform.h | 14 ++++ tools/profiler/gecko/nsProfiler.cpp | 8 -- tools/profiler/gecko/nsProfiler.h | 17 +---- tools/profiler/moz.build | 1 + .../public/ProfilerCodeAddressService.h | 40 +++++++++- tools/profiler/rust-helper/Cargo.toml | 1 + tools/profiler/rust-helper/src/elf.rs | 4 +- tools/profiler/rust-helper/src/lib.rs | 57 +++++++++++++- xpcom/base/CodeAddressService.h | 56 +++++++------- 16 files changed, 249 insertions(+), 71 deletions(-) create mode 100644 tools/profiler/core/ProfilerCodeAddressService.cpp diff --git a/Cargo.lock b/Cargo.lock index 0b98ce79ffe8..71abfd7a9057 100644 --- a/Cargo.lock +++ b/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)", ] diff --git a/mozglue/misc/StackWalk.cpp b/mozglue/misc/StackWalk.cpp index e503c58e86b7..821a99c04b23 100644 --- a/mozglue/misc/StackWalk.cpp +++ b/mozglue/misc/StackWalk.cpp @@ -672,6 +672,8 @@ MFBT_API bool MozDescribeCodeAddress(void* aPC, # include # 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)) diff --git a/mozglue/misc/StackWalk.h b/mozglue/misc/StackWalk.h index e722af64945e..8144abbad7d7 100644 --- a/mozglue/misc/StackWalk.h +++ b/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 diff --git a/toolkit/moz.configure b/toolkit/moz.configure index 571044abba23..02cb63ac0324 100644 --- a/toolkit/moz.configure +++ b/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) diff --git a/tools/profiler/core/ProfileBufferEntry.cpp b/tools/profiler/core/ProfileBufferEntry.cpp index 7bd223f2b2b1..b2f118937aef 100644 --- a/tools/profiler/core/ProfileBufferEntry.cpp +++ b/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++; diff --git a/tools/profiler/core/ProfilerCodeAddressService.cpp b/tools/profiler/core/ProfilerCodeAddressService.cpp new file mode 100644 index 000000000000..286440b2a198 --- /dev/null +++ b/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(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(entry.mLOffset)); + } +#endif + + if (!entry.mFunction || entry.mFunction[0] == '\0') { + return false; + } + + aResult = nsDependentCString(entry.mFunction); + return true; +} diff --git a/tools/profiler/core/platform.cpp b/tools/profiler/core/platform.cpp index 76085008cddb..f86b443fe3ad 100644 --- a/tools/profiler/core/platform.cpp +++ b/tools/profiler/core/platform.cpp @@ -3119,8 +3119,12 @@ UniquePtr profiler_get_profile(double aSinceTime, bool aIsShuttingDown) { LOG("profiler_get_profile"); + UniquePtr 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(); diff --git a/tools/profiler/core/platform.h b/tools/profiler/core/platform.h index 1386ba13a8b8..58f7295bb7ce 100644 --- a/tools/profiler/core/platform.h +++ b/tools/profiler/core/platform.h @@ -41,6 +41,10 @@ #include #include +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 profiler_move_exit_profiles(); mozilla::UniquePtr 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_ */ diff --git a/tools/profiler/gecko/nsProfiler.cpp b/tools/profiler/gecko/nsProfiler.cpp index e83192a3ba77..6fb583888e4f 100644 --- a/tools/profiler/gecko/nsProfiler.cpp +++ b/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() diff --git a/tools/profiler/gecko/nsProfiler.h b/tools/profiler/gecko/nsProfiler.h index 089ed3786b03..23702a9a9e74 100644 --- a/tools/profiler/gecko/nsProfiler.h +++ b/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 mAddrs; - nsTArray mIndex; - nsTArray mBuffer; - }; - private: ~nsProfiler(); typedef mozilla::MozPromise GatheringPromise; - typedef mozilla::MozPromise SymbolTablePromise; + typedef mozilla::MozPromise + SymbolTablePromise; RefPtr StartGathering(double aSinceTime); void FinishGathering(); diff --git a/tools/profiler/moz.build b/tools/profiler/moz.build index 1f5e79dac4a4..0a91b282a489 100644 --- a/tools/profiler/moz.build +++ b/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', diff --git a/tools/profiler/public/ProfilerCodeAddressService.h b/tools/profiler/public/ProfilerCodeAddressService.h index 0c3a87158c59..e393b38b06d7 100644 --- a/tools/profiler/public/ProfilerCodeAddressService.h +++ b/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 mAddrs; + nsTArray mIndex; + nsTArray 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, AllocPolicy> + mSymbolTables; +#endif +}; #endif // ProfilerCodeAddressService_h diff --git a/tools/profiler/rust-helper/Cargo.toml b/tools/profiler/rust-helper/Cargo.toml index b71bfb6d4764..9b5becb053b6 100644 --- a/tools/profiler/rust-helper/Cargo.toml +++ b/tools/profiler/rust-helper/Cargo.toml @@ -5,6 +5,7 @@ authors = ["Markus Stange "] [dependencies] memmap = "0.6.2" +rustc-demangle = "0.1" [dependencies.object] version = "0.10.0" diff --git a/tools/profiler/rust-helper/src/elf.rs b/tools/profiler/rust-helper/src/elf.rs index d0d7f6e10370..43924d1a32f7 100644 --- a/tools/profiler/rust-helper/src/elf.rs +++ b/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 { +pub fn get_compact_symbol_table(buffer: &[u8], breakpad_id: Option<&str>) -> Option { 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))); diff --git a/tools/profiler/rust-helper/src/lib.rs b/tools/profiler/rust-helper/src/lib.rs index b9bc7b022e8a..fc34cde5f2c1 100644 --- a/tools/profiler/rust-helper/src/lib.rs +++ b/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 { 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 { 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, + } +} diff --git a/xpcom/base/CodeAddressService.h b/xpcom/base/CodeAddressService.h index 2a2d34eb6043..028c61aa3900 100644 --- a/xpcom/base/CodeAddressService.h +++ b/xpcom/base/CodeAddressService.h @@ -56,6 +56,7 @@ template class CodeAddressService : private detail::CodeAddressServiceAllocPolicy { + 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(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(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);