Skip to content

Commit

Permalink
cc: Symbol resolution with multiple executable regions per module
Browse files Browse the repository at this point in the history
The symbol resolution code used to assume for most purposes that
there is a single executable region per module. When there were
several, there was no crash, but symbols were not resolved correctly.
The reason is that the symbol offsets are relative to the first
executable region's start address, but bcc would resolve them
relative to the region in which they appeared. For example, given
the following regions and spans for a module libfoo.so loaded into
some process:

  1000-2000 r-xp libfoo.so
  2000-3000 rw-p libfoo.so
  3000-4000 r-xp libfoo.so
  4000-5000 r--- libfoo.so

Now, suppose there is a symbol bar() loaded at address 3500. In
the binary on disk, bar() is at offset 2500 from the beginning of
the module (but not the beginning of the 3000-4000 region!). When
we look at the candidate regions, we find 3000-4000, and discover
that 3500 lies within it. Then we subtract 3500-3000 to find the
offset from the beginning of the region, get 500, and now look
for a symbol that contains the relative address 500. As a result,
we might find some random symbol in the region 1000-2000, and
report that address 3500 corresponds to that random symbol rather
than to bar().

This commit fixes the situation by keeping only a single `Module`
instance for each module, even if that module spans multiple
executable regions. We remember all executable region start and
end ranges so we can determine whether an address (like 3500 in
the above example) lies within the module. But for the purpose of
finding the actual symbol, we need only the offset from the start
of the _first_ executable region, and then need to look up a symbol
based on that.

This was discovered and fixed while tracing .NET Core processes on
Linux, where libcoreclr.so (the main CLR binary) has several
executable regions. Resolving symbols from any but the first region
would produce totally bogus results.
  • Loading branch information
goldshtn committed Mar 3, 2017
1 parent 9ca8ac2 commit f141d1b
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 9 deletions.
27 changes: 21 additions & 6 deletions src/cc/bcc_syms.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,11 @@ void ProcSyms::refresh() {
int ProcSyms::_add_module(const char *modname, uint64_t start, uint64_t end,
void *payload) {
ProcSyms *ps = static_cast<ProcSyms *>(payload);
ps->modules_.emplace_back(modname, start, end);
auto it = std::find_if(ps->modules_.begin(), ps->modules_.end(),
[=](const ProcSyms::Module &m) { return m.name_ == modname; });
if (it == ps->modules_.end())
it = ps->modules_.insert(ps->modules_.end(), modname);
it->ranges_.push_back(ProcSyms::Module::Range(start, end));
return 0;
}

Expand All @@ -116,7 +120,7 @@ bool ProcSyms::resolve_addr(uint64_t addr, struct bcc_symbol *sym) {

const char *original_module = nullptr;
for (Module &mod : modules_) {
if (addr >= mod.start_ && addr < mod.end_) {
if (mod.contains(addr)) {
bool res = mod.find_addr(addr, sym);
if (sym->name) {
sym->demangle_name = abi::__cxa_demangle(sym->name, nullptr, nullptr, nullptr);
Expand Down Expand Up @@ -155,8 +159,8 @@ bool ProcSyms::resolve_name(const char *module, const char *name,
return false;
}

ProcSyms::Module::Module(const char *name, uint64_t start, uint64_t end)
: name_(name), start_(start), end_(end) {
ProcSyms::Module::Module(const char *name)
: name_(name) {
is_so_ = bcc_elf_is_shared_obj(name) == 1;
}

Expand Down Expand Up @@ -184,20 +188,28 @@ void ProcSyms::Module::load_sym_table() {
std::sort(syms_.begin(), syms_.end());
}

bool ProcSyms::Module::contains(uint64_t addr) const {
for (const auto &range : ranges_) {
if (addr >= range.start && addr < range.end)
return true;
}
return false;
}

bool ProcSyms::Module::find_name(const char *symname, uint64_t *addr) {
load_sym_table();

for (Symbol &s : syms_) {
if (*(s.name) == symname) {
*addr = is_so() ? start_ + s.start : s.start;
*addr = is_so() ? start() + s.start : s.start;
return true;
}
}
return false;
}

bool ProcSyms::Module::find_addr(uint64_t addr, struct bcc_symbol *sym) {
uint64_t offset = is_so() ? (addr - start_) : addr;
uint64_t offset = is_so() ? (addr - start()) : addr;

load_sym_table();

Expand Down Expand Up @@ -230,6 +242,9 @@ bool ProcSyms::Module::find_addr(uint64_t addr, struct bcc_symbol *sym) {
sym->offset = (offset - it->start);
return true;
}
// But don't step beyond begin()!
if (it == syms_.begin())
break;
}

return false;
Expand Down
13 changes: 10 additions & 3 deletions src/cc/syms.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,22 @@ class ProcSyms : SymbolCache {
};

struct Module {
Module(const char *name, uint64_t start, uint64_t end);
struct Range {
uint64_t start;
uint64_t end;
Range(uint64_t s, uint64_t e) : start(s), end(e) {}
};

Module(const char *name);
std::string name_;
uint64_t start_;
uint64_t end_;
std::vector<Range> ranges_;
bool is_so_;
std::unordered_set<std::string> symnames_;
std::vector<Symbol> syms_;

void load_sym_table();
bool contains(uint64_t addr) const;
uint64_t start() const { return ranges_.begin()->start; }
bool find_addr(uint64_t addr, struct bcc_symbol *sym);
bool find_name(const char *symname, uint64_t *addr);
bool is_so() const { return is_so_; }
Expand Down

0 comments on commit f141d1b

Please sign in to comment.