Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hide implementations of complex data structures in SymtabAPI::Symtab #1531

Merged
merged 13 commits into from
Sep 22, 2023
Merged

Hide implementations of complex data structures in SymtabAPI::Symtab #1531

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
98e5061
Moved indexed_symbols into a private implementation
hainest Aug 29, 2023
eda971c
Move indexed_modules into private implementation
hainest Aug 29, 2023
49cc373
Move funcRangesAreParsed into private implementation
hainest Aug 29, 2023
c338aa6
Move types_parsed into private implementation
hainest Aug 29, 2023
4569d45
Move funcsByOffset into private implementation
hainest Aug 29, 2023
6385274
Move varsByOffset into private implementation
hainest Aug 29, 2023
77f8ee0
Remove 'concurrent.h' from 'Symtab.h'
hainest Aug 29, 2023
24654ae
clang-format
hainest Sep 20, 2023
0c1a0c6
Move mod_lookup_ into private implementation.
hainest Sep 20, 2023
14e0f0c
Move func_lookup into private implementation
hainest Sep 20, 2023
4c8a55c
run clang-format
hainest Sep 21, 2023
752d370
Make pImpl const
hainest Sep 21, 2023
ff68724
Merge branch 'master' into thaines/symtab_impl_hiding
hainest Sep 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion symtabAPI/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ set(_private_headers
src/Object-elf.h
src/Object.h
src/Object-nt.h
src/Type-mem.h)
src/Type-mem.h
src/indexed_symbols.hpp
src/symtab_impl.hpp)

set(_sources
src/AddrLookup.C
Expand Down
2 changes: 0 additions & 2 deletions symtabAPI/h/Function.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
#include "Annotatable.h"
#include "Aggregate.h"
#include "Variable.h"
#include "IBSTree.h"
#include "concurrent.h"
#include "VariableLocation.h"

Expand Down Expand Up @@ -77,7 +76,6 @@ class SYMTAB_EXPORT FuncRange {
typedef Dyninst::Offset type;
};

typedef IBSTree<FuncRange> FuncRangeLookup;
typedef std::vector<FuncRange> FuncRangeCollection;
typedef std::vector<FunctionBase *> InlineCollection;
typedef std::vector<FuncRange> FuncRangeCollection;
Expand Down
104 changes: 5 additions & 99 deletions symtabAPI/h/Symtab.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,39 +38,19 @@
#include <string>
#include <vector>
#include <set>
#include <mutex>
#include <memory>

#include "Symbol.h"
#include "Module.h"
#include "Region.h"
#include "Function.h"
#include "Annotatable.h"
#include "ProcReader.h"
#include "IBSTree.h"
#include "Type.h"

#include "dyninstversion.h"

#include "concurrent.h"

#include "boost/shared_ptr.hpp"
#include "boost/multi_index_container.hpp"
#include <boost/multi_index/member.hpp>
#include <boost/multi_index/mem_fun.hpp>
#include <boost/multi_index/ordered_index.hpp>
#include <boost/multi_index/hashed_index.hpp>
#include <boost/multi_index/identity.hpp>
#include <boost/multi_index/random_access_index.hpp>
using boost::multi_index_container;
using boost::multi_index::indexed_by;
using boost::multi_index::ordered_unique;
using boost::multi_index::ordered_non_unique;
using boost::multi_index::hashed_non_unique;

using boost::multi_index::identity;
using boost::multi_index::tag;
using boost::multi_index::const_mem_fun;
using boost::multi_index::member;

class MappedFile;

Expand All @@ -92,8 +72,8 @@ class Object;
class localVar;
class relocationEntry;
class Type;
struct symtab_impl;

typedef IBSTree< ModRange > ModRangeLookup;
typedef Dyninst::ProcessReader MemRegReader;

class SYMTAB_EXPORT Symtab : public LookupInterface,
Expand All @@ -111,6 +91,9 @@ class SYMTAB_EXPORT Symtab : public LookupInterface,
friend class relocationEntry;
friend class Object;

// Hide implementation details that are complex or add large dependencies
std::unique_ptr<symtab_impl> impl;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const std::unique_ptr...? The impl never changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Symtab isn't moveable, that is a good idea.


public:


Expand Down Expand Up @@ -538,81 +521,13 @@ class SYMTAB_EXPORT Symtab : public LookupInterface,

//symbols
unsigned no_of_symbols{};

struct indexed_symbols {
typedef dyn_c_hash_map<Symbol*, Offset> master_t;
typedef std::vector<Symbol*> symvec_t;
typedef dyn_c_hash_map<Offset, symvec_t> by_offset_t;
typedef dyn_c_hash_map<std::string, symvec_t> by_name_t;

master_t master;
by_offset_t by_offset;
by_name_t by_mangled;
by_name_t by_pretty;
by_name_t by_typed;

// Only inserts if not present. Returns whether it inserted.
bool insert(Symbol* s);

// Clears the table. Do not use in parallel.
void clear();

// Erases symbols from the table. Do not use in parallel.
void erase(Symbol* s);

// Iterator for the symbols. Do not use in parallel.
class iterator {
master_t::iterator m;
public:
using iterator_category = std::forward_iterator_tag;
using value_type = Symbol*;
using difference_type = std::ptrdiff_t;
using pointer = value_type*;
using reference = value_type&;

iterator(master_t::iterator i) : m(i) {}
bool operator==(const iterator& x) const { return m == x.m; }
bool operator!=(const iterator& x) const { return !operator==(x); }
Symbol* const& operator*() const { return m->first; }
Symbol* const* operator->() const { return &operator*(); }
iterator& operator++() { ++m; return *this; }
iterator operator++(int) {
iterator old(m);
operator++();
return old;
}
};

iterator begin() { return iterator(master.begin()); }
iterator end() { return iterator(master.end()); }
};

indexed_symbols everyDefinedSymbol{};
indexed_symbols undefDynSyms{};

// We also need per-Aggregate indices
bool sorted_everyFunction{false};
std::vector<Function *> everyFunction{};
// Since Functions are unique by address we require this structure to
// efficiently track them.
dyn_c_hash_map <Offset, Function *> funcsByOffset{};

// Similar for Variables
std::vector<Variable *> everyVariable{};
using VarsByOffsetMap = dyn_c_hash_map<Offset, std::vector<Variable *> >;
VarsByOffsetMap varsByOffset{};

dyn_mutex im_lock{};
boost::multi_index_container<Module*,
boost::multi_index::indexed_by<
boost::multi_index::random_access<>,
boost::multi_index::ordered_unique<boost::multi_index::identity<Module*> >,
boost::multi_index::ordered_non_unique<
boost::multi_index::const_mem_fun<Module, const std::string&, &Module::fileName> >
>
>
indexed_modules{};


std::vector<relocationEntry > relocation_table_{};
std::vector<ExceptionBlock *> excpBlocks{};
Expand All @@ -626,8 +541,6 @@ class SYMTAB_EXPORT Symtab : public LookupInterface,
bool getExplicitSymtabRefs(std::set<Symtab *> &refs);
std::set<Symtab *> explicitSymtabRefs_{};

std::once_flag types_parsed;

//Relocation sections
bool hasRel_{false};
bool hasRela_{false};
Expand All @@ -639,17 +552,10 @@ class SYMTAB_EXPORT Symtab : public LookupInterface,
bool isStaticBinary_{false};
bool isDefensiveBinary_{false};

FuncRangeLookup func_lookup{};
std::once_flag funcRangesAreParsed;

ModRangeLookup mod_lookup_{};


//Don't use obj_private, use getObject() instead.
public:
Object *getObject();
const Object *getObject() const;
ModRangeLookup* mod_lookup();
void dumpModRanges();
void dumpFuncRanges();

Expand Down
40 changes: 20 additions & 20 deletions symtabAPI/src/Symtab-edit.C
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
#include "Collections.h"
#include "Function.h"
#include "Variable.h"

#include "symtab_impl.hpp"
#include "symtabAPI/src/Object.h"

#include "boost/tuple/tuple.hpp"
Expand Down Expand Up @@ -109,7 +109,7 @@ bool Symtab::deleteFunction(Function *func) {
}
}
*/
funcsByOffset.erase(func->getOffset());
impl->funcsByOffset.erase(func->getOffset());

// Now handle the Aggregate stuff
return deleteAggregate(func);
Expand All @@ -121,13 +121,13 @@ bool Symtab::deleteVariable(Variable *var) {

// remove variable from varsByOffset
{
VarsByOffsetMap::accessor a;
bool found = !varsByOffset.find(a, var->getOffset());
decltype(impl->varsByOffset)::accessor a;
bool found = !impl->varsByOffset.find(a, var->getOffset());
if (found) {
VarsByOffsetMap::mapped_type &vars = a->second;
decltype(impl->varsByOffset)::mapped_type &vars = a->second;
vars.erase(std::remove(vars.begin(), vars.end(), var), vars.end());
if (vars.empty()) {
varsByOffset.erase(a);
impl->varsByOffset.erase(a);
}
}
}
Expand All @@ -148,8 +148,8 @@ bool Symtab::deleteAggregate(Aggregate *agg) {
}

bool Symtab::deleteSymbolFromIndices(Symbol *sym) {
everyDefinedSymbol.erase(sym);
undefDynSyms.erase(sym);
impl->everyDefinedSymbol.erase(sym);
impl->undefDynSyms.erase(sym);
return true;
}

Expand All @@ -172,17 +172,17 @@ bool Symtab::changeSymbolOffset(Symbol *sym, Offset newOffset) {
// the aggregate, and make a new aggregate.
{
indexed_symbols::master_t::accessor a;
if (!everyDefinedSymbol.master.find(a, sym)) {
if (!impl->everyDefinedSymbol.master.find(a, sym)) {
assert(!"everyDefinedSymbol.master.find(a, sym)");
}

indexed_symbols::by_offset_t::accessor oa;
if (!everyDefinedSymbol.by_offset.find(oa, sym->offset_)) {
if (!impl->everyDefinedSymbol.by_offset.find(oa, sym->offset_)) {
assert(!"everyDefinedSymbol.by_offset.find(oa, sym->offset_)");
}
auto &syms = oa->second;
syms.erase(std::remove(syms.begin(), syms.end(), sym), syms.end());
everyDefinedSymbol.by_offset.insert(oa, newOffset);
impl->everyDefinedSymbol.by_offset.insert(oa, newOffset);
oa->second.push_back(sym);

a->second = newOffset;
Expand All @@ -198,25 +198,25 @@ bool Symtab::changeAggregateOffset(Aggregate *agg, Offset oldOffset, Offset newO
Variable *var = dynamic_cast<Variable *>(agg);

if (func) {
funcsByOffset.erase(oldOffset);
if (!funcsByOffset.insert({newOffset, func})) {
impl->funcsByOffset.erase(oldOffset);
if (!impl->funcsByOffset.insert({newOffset, func})) {
// Already someone there... odd, so don't do anything.
}
}
if (var) {
VarsByOffsetMap::accessor a;
bool found = !varsByOffset.find(a, oldOffset);
VarsByOffsetMap::mapped_type &vars = a->second;
decltype(impl->varsByOffset)::accessor a;
bool found = !impl->varsByOffset.find(a, oldOffset);
decltype(impl->varsByOffset)::mapped_type &vars = a->second;
if (found) {
vars.erase(std::remove(vars.begin(), vars.end(), var), vars.end());
if (vars.empty()) {
varsByOffset.erase(a);
impl->varsByOffset.erase(a);
}
} else {
assert(0);
}

found = !varsByOffset.insert(a, newOffset);
found = !impl->varsByOffset.insert(a, newOffset);
if (found) {
found = false;
for (auto v: vars) {
Expand Down Expand Up @@ -308,7 +308,7 @@ Function *Symtab::createFunction(std::string name,
}

// Check to see if we contain this module...
if(indexed_modules.get<1>().find(mod) == indexed_modules.get<1>().end()) return NULL;
if(impl->indexed_modules.get<1>().find(mod) == impl->indexed_modules.get<1>().end()) return NULL;
//
// bool found = false;
// for (unsigned i = 0; i < indexed_modules.size(); i++) {
Expand Down Expand Up @@ -356,7 +356,7 @@ Variable *Symtab::createVariable(std::string name,
mod = getDefaultModule();
}
// Check to see if we contain this module...
if(indexed_modules.get<1>().find(mod) == indexed_modules.get<1>().end()) return NULL;
if(impl->indexed_modules.get<1>().find(mod) == impl->indexed_modules.get<1>().end()) return NULL;
//
// bool found = false;
// for (unsigned i = 0; i < indexed_modules.size(); i++) {
Expand Down