From b869b502770e9a8872d121b9ae77b451698e7941 Mon Sep 17 00:00:00 2001 From: Xiaozhu Meng Date: Mon, 16 Jul 2018 16:52:17 -0500 Subject: [PATCH] Start to use cilk data race detectors: 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 --- common/src/linuxKludges.C | 66 ++++++++++++++++++++++++++------- dataflowAPI/src/slicing.C | 3 +- parseAPI/h/CFG.h | 4 ++ parseAPI/src/ParseData.h | 19 ++++++---- parseAPI/src/Parser.C | 13 +++++++ parseAPI/src/SymtabCodeSource.C | 28 +++++++------- 6 files changed, 97 insertions(+), 36 deletions(-) diff --git a/common/src/linuxKludges.C b/common/src/linuxKludges.C index 11421c5e05..6284f516f1 100644 --- a/common/src/linuxKludges.C +++ b/common/src/linuxKludges.C @@ -203,20 +203,58 @@ unsigned long long PDYN_mulMillion(unsigned long long in) { #include #include +#include + 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; @@ -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() */ diff --git a/dataflowAPI/src/slicing.C b/dataflowAPI/src/slicing.C index aef0026997..d2704b2dec 100644 --- a/dataflowAPI/src/slicing.C +++ b/dataflowAPI/src/slicing.C @@ -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, diff --git a/parseAPI/h/CFG.h b/parseAPI/h/CFG.h index 4f734d3390..4110a1fa70 100644 --- a/parseAPI/h/CFG.h +++ b/parseAPI/h/CFG.h @@ -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 g(*this); + src = _srclist; + } bool consistent(Address addr, Address & prev_insn); diff --git a/parseAPI/src/ParseData.h b/parseAPI/src/ParseData.h index 9b1baadd1c..3380c14c58 100644 --- a/parseAPI/src/ParseData.h +++ b/parseAPI/src/ParseData.h @@ -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::accessor a; - bool inserted = blocksByAddr.insert(a, std::make_pair(b->start(), b)); - if(!inserted) { + { + tbb::concurrent_hash_map::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); diff --git a/parseAPI/src/Parser.C b/parseAPI/src/Parser.C index 7716019c0a..d272e7cbe2 100644 --- a/parseAPI/src/Parser.C +++ b/parseAPI/src/Parser.C @@ -933,11 +933,24 @@ Parser::finalize_funcs(vector &funcs) { vector 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 diff --git a/parseAPI/src/SymtabCodeSource.C b/parseAPI/src/SymtabCodeSource.C index 3e5a966063..a9cd9b3eca 100644 --- a/parseAPI/src/SymtabCodeSource.C +++ b/parseAPI/src/SymtabCodeSource.C @@ -56,6 +56,19 @@ using namespace Dyninst::ParseAPI; typedef tbb::concurrent_hash_map SeenMap; +static const vector 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() @@ -505,8 +518,8 @@ 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; } @@ -514,17 +527,6 @@ SymtabCodeSource::init_hints(RegionMap &rmap, hint_filt * filt) // 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 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;