Commit 88cc7c93 authored by Yedidya Feldblum's avatar Yedidya Feldblum Committed by Facebook Github Bot

Revise SignalSafeElfCache to allocate with mmap

Summary:
[Folly] Revise `SignalSafeElfCache` to use `mmap` for dynamic allocation and not to require static preallocation.

While, technically, `mmap` is not documented to be async-signal-safe, in practice it is so at least on Linux. Take advantage.

Prior to this change, the `SignalSafeElfCache` with all of its storage would have to be preallocated before setting the signal handler and must not be destroyed ever. Prior to this change, the preallocation would require at least `sizeof(Path) * capacity`, which defaults to ~2MB.

It is now possible to use stack-allocated SignalSafeElfCache in signal handlers.

This change adds a multi-thread-safe and async-signal-safe `mmap`-based allocator. Details as follows:
* All allocations are rounded up to the nearest power of two in size and alignment.
* Page or larger allocations are handled directly by `mmap` and free by `munmap` and are not tracked.
* Smaller allocations are handled by an `mmap`-backed refcounted arena. Arena sections are 16 pages in size; they are allocated on demand; and they are linked together to allow for `munmap` when the allocator refcounted arena is destroyed.

Reviewed By: davidtgoldblatt

Differential Revision: D18760282

fbshipit-source-id: 56a34abfe39a7108f790537afeda832fd39714d7
parent 09446426
......@@ -66,7 +66,6 @@ ElfFile::OpenResult ElfFile::openNoThrow(
FOLLY_SAFE_CHECK(fd_ == -1, "File already open");
// Always close fd and unmap in case of failure along the way to avoid
// check failure above if we leave fd != -1 and the object is recycled
// like it is inside SignalSafeElfCache
auto guard = makeGuard([&] { reset(); });
strncat(filepath_, name, kFilepathMaxLen - 1);
fd_ = ::open(name, options.writable() ? O_RDWR : O_RDONLY);
......
......@@ -17,6 +17,10 @@
#include <folly/experimental/symbolizer/ElfCache.h>
#include <link.h>
#include <signal.h>
#include <folly/ScopeGuard.h>
#include <folly/portability/SysMman.h>
/*
* This is declared in `link.h' on Linux platforms, but apparently not on the
......@@ -50,42 +54,44 @@ size_t countLoadedElfFiles() {
return count;
}
SignalSafeElfCache::SignalSafeElfCache(size_t capacity) {
map_.reserve(capacity);
slots_.reserve(capacity);
// Preallocate
for (size_t i = 0; i < capacity; ++i) {
slots_.push_back(std::make_shared<ElfFile>());
}
}
std::shared_ptr<ElfFile> SignalSafeElfCache::getFile(StringPiece p) {
if (p.size() > Path::kMaxSize) {
return nullptr;
struct cmp {
bool operator()(Entry const& a, StringPiece b) const noexcept {
return a.path < b;
}
bool operator()(StringPiece a, Entry const& b) const noexcept {
return a < b.path;
}
};
sigset_t newsigs;
sigfillset(&newsigs);
sigset_t oldsigs;
sigemptyset(&oldsigs);
sigprocmask(SIG_SETMASK, &newsigs, &oldsigs);
SCOPE_EXIT {
sigprocmask(SIG_SETMASK, &oldsigs, nullptr);
};
if (!state_) {
state_.emplace();
}
scratchpad_.assign(p);
auto pos = map_.find(scratchpad_);
if (pos != map_.end()) {
return slots_[pos->second];
auto pos = state_->map.find(p, cmp{});
if (pos == state_->map.end()) {
state_->list.emplace_front(p, state_->alloc);
pos = state_->map.insert(state_->list.front()).first;
}
size_t n = map_.size();
if (n >= slots_.size()) {
DCHECK_EQ(map_.size(), slots_.size());
return nullptr;
if (!pos->init) {
int r = pos->file->openAndFollow(pos->path.c_str());
pos->init = r == ElfFile::kSuccess;
}
auto& f = slots_[n];
int r = f->openAndFollow(scratchpad_.data());
if (r != ElfFile::kSuccess) {
if (!pos->init) {
return nullptr;
}
map_[scratchpad_] = n;
return f;
return pos->file;
}
ElfCache::ElfCache(size_t capacity) : capacity_(capacity) {}
......
......@@ -18,13 +18,14 @@
#include <climits> // for PATH_MAX
#include <cstring>
#include <forward_list>
#include <memory>
#include <mutex>
#include <string>
#include <unordered_map>
#include <vector>
#include <boost/container/flat_map.hpp>
#include <boost/intrusive/avl_set.hpp>
#include <boost/intrusive/list.hpp>
#include <boost/operators.hpp>
#include <glog/logging.h>
......@@ -32,6 +33,7 @@
#include <folly/Range.h>
#include <folly/experimental/symbolizer/Elf.h>
#include <folly/hash/Hash.h>
#include <folly/memory/ReentrantAllocator.h>
namespace folly {
namespace symbolizer {
......@@ -48,66 +50,43 @@ class ElfCacheBase {
};
/**
* Cache ELF files. Async-signal-safe: does memory allocation upfront.
*
* Will not grow; once the capacity is reached, lookups for files that
* aren't already in the cache will fail (return nullptr).
* Cache ELF files. Async-signal-safe: does memory allocation via mmap.
*
* Not MT-safe. May not be used concurrently from multiple threads.
*
* NOTE that async-signal-safety is preserved only as long as the
* SignalSafeElfCache object exists; after the SignalSafeElfCache object
* is destroyed, destroying returned shared_ptr<ElfFile> objects may
* cause ElfFile objects to be destroyed, and that's not async-signal-safe.
*/
class SignalSafeElfCache : public ElfCacheBase {
public:
explicit SignalSafeElfCache(size_t capacity);
std::shared_ptr<ElfFile> getFile(StringPiece path) override;
private:
// We can't use std::string (allocating memory is bad!) so we roll our
// own wrapper around a fixed-size, null-terminated string.
class Path : private boost::totally_ordered<Path> {
public:
Path() {
assign(folly::StringPiece());
}
explicit Path(StringPiece s) {
assign(s);
}
void assign(StringPiece s) {
DCHECK_LE(s.size(), kMaxSize);
if (!s.empty()) {
memcpy(data_, s.data(), s.size());
}
data_[s.size()] = '\0';
}
using Path = std::basic_string< //
char,
std::char_traits<char>,
reentrant_allocator<char>>;
bool operator<(const Path& other) const {
return strcmp(data_, other.data_) < 0;
}
struct Entry : boost::intrusive::avl_set_base_hook<> {
Path path;
std::shared_ptr<ElfFile> file;
bool init = false;
bool operator==(const Path& other) const {
return strcmp(data_, other.data_) == 0;
}
explicit Entry(StringPiece p, reentrant_allocator<char> alloc) noexcept
: path{p.data(), p.size(), alloc},
file{std::allocate_shared<ElfFile>(alloc)} {}
Entry(Entry const&) = delete;
Entry& operator=(Entry const& that) = delete;
const char* data() const {
return data_;
friend bool operator<(Entry const& a, Entry const& b) noexcept {
return a.path < b.path;
}
static constexpr size_t kMaxSize = PATH_MAX - 1;
private:
char data_[kMaxSize + 1];
};
Path scratchpad_; // Preallocated key for map_ lookups.
boost::container::flat_map<Path, int> map_;
std::vector<std::shared_ptr<ElfFile>> slots_;
struct State {
reentrant_allocator<void> alloc{
reentrant_allocator_options().block_size_lg(16).large_size_lg(12)};
std::forward_list<Entry, reentrant_allocator<Entry>> list{alloc};
// note: map entry dtors check that they have already been unlinked
boost::intrusive::avl_set<Entry> map; // must follow list
};
Optional<State> state_;
};
/**
......
......@@ -458,11 +458,8 @@ void StringSymbolizePrinter::doPrint(StringPiece sp) {
buf_.append(sp.data(), sp.size());
}
SafeStackTracePrinter::SafeStackTracePrinter(
size_t minSignalSafeElfCacheSize,
int fd)
SafeStackTracePrinter::SafeStackTracePrinter(int fd)
: fd_(fd),
elfCache_(std::max(countLoadedElfFiles(), minSignalSafeElfCacheSize)),
printer_(
fd,
SymbolizePrinter::COLOR_IF_TTY,
......@@ -481,6 +478,7 @@ void SafeStackTracePrinter::printSymbolizedStackTrace() {
// Do our best to populate location info, process is going to terminate,
// so performance isn't critical.
SignalSafeElfCache elfCache_;
Symbolizer symbolizer(&elfCache_, LocationInfoMode::FULL);
symbolizer.symbolize(*addresses_);
......
......@@ -342,11 +342,7 @@ class StringSymbolizePrinter : public SymbolizePrinter {
*/
class SafeStackTracePrinter {
public:
static constexpr size_t kDefaultMinSignalSafeElfCacheSize = 500;
explicit SafeStackTracePrinter(
size_t minSignalSafeElfCacheSize = kDefaultMinSignalSafeElfCacheSize,
int fd = STDERR_FILENO);
explicit SafeStackTracePrinter(int fd = STDERR_FILENO);
virtual ~SafeStackTracePrinter() {}
......@@ -374,7 +370,6 @@ class SafeStackTracePrinter {
static constexpr size_t kMaxStackTraceDepth = 100;
int fd_;
SignalSafeElfCache elfCache_;
FDSymbolizePrinter printer_;
std::unique_ptr<FrameArray<kMaxStackTraceDepth>> addresses_;
};
......
......@@ -159,7 +159,7 @@ void testStackTracePrinter(StackTracePrinter& printer, int fd) {
TEST(StackTraceTest, SafeStackTracePrinter) {
test::TemporaryFile file;
SafeStackTracePrinter printer{10, file.fd()};
SafeStackTracePrinter printer{file.fd()};
testStackTracePrinter<SafeStackTracePrinter>(printer, file.fd());
}
......
......@@ -109,7 +109,7 @@ TEST_F(ElfCacheTest, TinyElfCache) {
}
TEST_F(ElfCacheTest, SignalSafeElfCache) {
SignalSafeElfCache cache(100);
SignalSafeElfCache cache;
Symbolizer symbolizer(&cache);
for (size_t i = 0; i < 2; ++i) {
runElfCacheTest(symbolizer);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment