Skip to content

Commit

Permalink
Start to use cilk data race detectors:
Browse files Browse the repository at this point in the history
1. Fix real races where getting block source edges in slicing is not
locked
2. Suppress false positves. Cilk race detectors cannot deal with thread
local bariables
  • Loading branch information
mxz297 committed Jul 16, 2018
1 parent 671d641 commit b869b50
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 36 deletions.
66 changes: 52 additions & 14 deletions common/src/linuxKludges.C
Expand Up @@ -203,20 +203,58 @@ unsigned long long PDYN_mulMillion(unsigned long long in) {
#include <boost/thread/mutex.hpp>
#include <boost/thread/lock_guard.hpp>

#include <race-detector-annotations.h>

using namespace abi;

inline void set_thread_local_pointer(char* &var, char* val) {
race_detector_fake_lock_acquire(race_detector_fake_lock(var));
var = val;
race_detector_fake_lock_release(race_detector_fake_lock(var));
}

inline void set_thread_local_bool(bool &var, bool val) {
race_detector_fake_lock_acquire(race_detector_fake_lock(var));
var = val;
race_detector_fake_lock_release(race_detector_fake_lock(var));
}

inline char* get_thread_local_pointer(char* &var) {
char *ret;
race_detector_fake_lock_acquire(race_detector_fake_lock(var));
ret = var;
race_detector_fake_lock_release(race_detector_fake_lock(var));
return ret;
}

inline bool get_thread_local_bool(bool &var) {
bool ret;
race_detector_fake_lock_acquire(race_detector_fake_lock(var));
ret = var;
race_detector_fake_lock_release(race_detector_fake_lock(var));
return ret;
}


char * P_cplus_demangle( const char * symbol, bool nativeCompiler,
bool includeTypes )
{
static __thread char* last_symbol = NULL;
static __thread bool last_native = false;
static __thread bool last_typed = false;
static __thread char* last_demangled = NULL;

if(last_symbol && last_demangled && (nativeCompiler == last_native)
&& (includeTypes == last_typed) && (strcmp(symbol, last_symbol) == 0))
static __thread char* last_symbol;
set_thread_local_pointer(last_symbol, NULL);
static __thread bool last_native;
set_thread_local_bool(last_native, false);
static __thread bool last_typed;
set_thread_local_bool(last_typed, false);
static __thread char* last_demangled;
set_thread_local_pointer(last_demangled, NULL);

if(get_thread_local_pointer(last_symbol) &&
get_thread_local_pointer(last_demangled) &&
(nativeCompiler == get_thread_local_bool(last_native)) &&
(includeTypes == get_thread_local_bool(last_typed)) &&
(strcmp(symbol, get_thread_local_pointer(last_symbol)) == 0))
{
return strdup(last_demangled);
return strdup(get_thread_local_pointer(last_demangled));
}
int status;
char* demangled;
Expand Down Expand Up @@ -246,12 +284,12 @@ char * P_cplus_demangle( const char * symbol, bool nativeCompiler,
demangled = dedemangled;
}

free(last_symbol);
free(last_demangled);
last_native = nativeCompiler;
last_typed = includeTypes;
last_symbol = strdup(symbol);
last_demangled = strdup(demangled);
free(get_thread_local_pointer(last_symbol));
free(get_thread_local_pointer(last_demangled));
set_thread_local_bool(last_native, nativeCompiler);
set_thread_local_bool(last_typed, includeTypes);
set_thread_local_pointer(last_symbol, strdup(symbol));
set_thread_local_pointer(last_demangled, strdup(demangled));

return demangled;
} /* end P_cplus_demangle() */
Expand Down
3 changes: 2 additions & 1 deletion dataflowAPI/src/slicing.C
Expand Up @@ -806,7 +806,8 @@ Slicer::getPredecessors(
// We force finalizing if necessary
cand.loc.func->num_blocks();
SingleContextOrInterproc epred(cand.loc.func, true, true);
const Block::edgelist & sources = cand.loc.block->sources();
Block::edgelist sources;
cand.loc.block->copy_sources(sources);
std::for_each(boost::make_filter_iterator(epred, sources.begin(), sources.end()),
boost::make_filter_iterator(epred, sources.end(), sources.end()),
boost::bind(&Slicer::handlePredecessorEdge,
Expand Down
4 changes: 4 additions & 0 deletions parseAPI/h/CFG.h
Expand Up @@ -325,6 +325,10 @@ class PARSER_EXPORT Block :
/* Edge access */
const edgelist & sources() const { return _srclist; }
const edgelist & targets() const { return _trglist; }
void copy_sources(edgelist & src) {
boost::lock_guard<Block> g(*this);
src = _srclist;
}

bool consistent(Address addr, Address & prev_insn);

Expand Down
19 changes: 11 additions & 8 deletions parseAPI/src/ParseData.h
Expand Up @@ -245,17 +245,20 @@ class region_data {

}
Block* record_block(Block* b) {
Block* ret = NULL;
race_detector_fake_lock_acquire(race_detector_fake_lock(blocksByAddr));
{
tbb::concurrent_hash_map<Address, Block*>::accessor a;
bool inserted = blocksByAddr.insert(a, std::make_pair(b->start(), b));
if(!inserted) {
{
tbb::concurrent_hash_map<Address, Block*>::accessor a;
bool inserted = blocksByAddr.insert(a, std::make_pair(b->start(), b));
// Inserting failed when another thread has inserted a block with the same starting address
return a->second;
} else {
return b;
}
if(!inserted) {
ret = a->second;
} else {
ret = b;
}
}
race_detector_fake_lock_release(race_detector_fake_lock(blocksByAddr));
return ret;
}
void insertBlockByRange(Block* b) {
blocksByRange.insert(b);
Expand Down
13 changes: 13 additions & 0 deletions parseAPI/src/Parser.C
Expand Up @@ -933,11 +933,24 @@ Parser::finalize_funcs(vector<Function *> &funcs)
{
vector<Function*> thread_local_funcs;
std::copy(funcs.begin(), funcs.end(), std::back_inserter(thread_local_funcs));
#if USE_OPENMP
#pragma omp parallel for schedule(auto)
for(int i = 0; i < thread_local_funcs.size(); ++i) {
Function *f = thread_local_funcs[i];
finalize(f);
}
#elif USE_CILK
cilk_for(int i = 0; i < thread_local_funcs.size(); ++i) {
Function *f = thread_local_funcs[i];
finalize(f);
}
#else
for(int i = 0; i < thread_local_funcs.size(); ++i) {
Function *f = thread_local_funcs[i];
finalize(f);
}
#endif

}

void
Expand Down
28 changes: 15 additions & 13 deletions parseAPI/src/SymtabCodeSource.C
Expand Up @@ -56,6 +56,19 @@ using namespace Dyninst::ParseAPI;

typedef tbb::concurrent_hash_map<Address, bool> SeenMap;

static const vector<std::string> skipped_symbols = {
"_non_rtti_object::`vftable'",
"bad_cast::`vftable'",
"exception::`vftable'",
"bad_typeid::`vftable'" ,
"sys_errlist",
"std::_non_rtti_object::`vftable'",
"std::__non_rtti_object::`vftable'",
"std::bad_cast::`vftable'",
"std::exception::`vftable'",
"std::bad_typeid::`vftable'" };


/** SymtabCodeRegion **/

SymtabCodeRegion::~SymtabCodeRegion()
Expand Down Expand Up @@ -505,26 +518,15 @@ SymtabCodeSource::init_hints(RegionMap &rmap, hint_filt * filt)
string fname_s = f->getFirstSymbol()->getPrettyName();
const char *fname = fname_s.c_str();
if(filt && (*filt)(f)) {
parsing_printf(" == filtered hint %s [%lx] ==\n",
FILE__,__LINE__,f->getOffset(), fname);
parsing_printf("[%s:%d} == filtered hint %s [%lx] ==\n",
FILE__,__LINE__,fname, f->getOffset());
continue;
}

// Cleaned-up version of a rather ugly series of strcmp's. Is this the
// right place to do this? Should these symbols not be filtered by the
// loop above?
/*Achin added code starts 12/15/2014*/
static dyn_tls const vector<std::string> skipped_symbols = {
"_non_rtti_object::`vftable'",
"bad_cast::`vftable'",
"exception::`vftable'",
"bad_typeid::`vftable'" ,
"sys_errlist",
"std::_non_rtti_object::`vftable'",
"std::__non_rtti_object::`vftable'",
"std::bad_cast::`vftable'",
"std::exception::`vftable'",
"std::bad_typeid::`vftable'" };
if (std::find(skipped_symbols.begin(), skipped_symbols.end(),
fsyms[i]->getFirstSymbol()->getPrettyName()) != skipped_symbols.end()) {
continue;
Expand Down

0 comments on commit b869b50

Please sign in to comment.