maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

Revert "Thread Safety Analysis: add support for before/after annotations on mutexes." 

This reverts r227997, as well as r228009. It does not pass check-clang
for me locally on Linux.

llvm-svn: 228020
This commit is contained in:
Reid Kleckner 2015-02-03 19:51:16 +00:00
parent cf7248bcaf
commit 6c5e36ae3b
8 changed files with 22 additions and 482 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
clang/include/clang/Analysis/Analyses/ThreadSafety.h
View File

@ -26,8 +26,6 @@
namespace clang {
namespace threadSafety {
class BeforeSet;
/// This enum distinguishes between different kinds of operations that may
/// need to be protected by locks. We use this enum in error handling.
enum ProtectedOperationKind {
@ -185,14 +183,6 @@ public:
virtual void handleFunExcludesLock(StringRef Kind, Name FunName,
Name LockName, SourceLocation Loc) {}
/// Warn that L1 cannot be acquired before L2.
virtual void handleLockAcquiredBefore(StringRef Kind, Name L1Name,
Name L2Name, SourceLocation Loc) {}
/// Warn that there is a cycle in acquired_before/after dependencies.
virtual void handleBeforeAfterCycle(Name L1Name, SourceLocation Loc) {}
/// Called by the analysis when starting analysis of a function.
/// Used to issue suggestions for changes to annotations.
virtual void enterFunction(const FunctionDecl *FD) {}
@ -213,8 +203,7 @@ private:
/// at the end of each block, and issue warnings for thread safety violations.
/// Each block in the CFG is traversed exactly once.
void runThreadSafetyAnalysis(AnalysisDeclContext &AC,
ThreadSafetyHandler &Handler,
BeforeSet **Bset);
ThreadSafetyHandler &Handler);
/// \brief Helper function that returns a LockKind required for the given level
/// of access.

8
clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
View File

@ -286,14 +286,6 @@ public:
sx::partiallyMatches(CapExpr, other.CapExpr);
}
const ValueDecl* valueDecl() const {
if (Negated)
return nullptr;
if (auto *P = dyn_cast<til::Project>(CapExpr))
return P->clangDecl();
return nullptr;
}
std::string toString() const {
if (Negated)
return "!" + sx::toString(CapExpr);

7
clang/include/clang/Basic/DiagnosticSemaKinds.td
View File

@ -2394,13 +2394,6 @@ def warn_fun_excludes_mutex : Warning<
def warn_cannot_resolve_lock : Warning<
"cannot resolve lock expression">,
InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
def warn_acquired_before : Warning<
"%0 '%1' must be acquired before '%2'">,
InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
def warn_acquired_before_after_cycle : Warning<
"Cycle in acquired_before/after dependencies, starting with '%0'">,
InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
// Thread safety warnings negative capabilities
def warn_acquire_requires_negative_cap : Warning<

6
clang/include/clang/Sema/Sema.h
View File

@ -199,11 +199,6 @@ namespace sema {
class TemplateDeductionInfo;
}
namespace threadSafety {
class BeforeSet;
void threadSafetyCleanup(BeforeSet* Cache);
}
// FIXME: No way to easily map from TemplateTypeParmTypes to
// TemplateTypeParmDecls, so we have this horrible PointerUnion.
typedef std::pair<llvm::PointerUnion<const TemplateTypeParmType*, NamedDecl*>,
@ -6713,7 +6708,6 @@ public:
/// \brief Worker object for performing CFG-based warnings.
sema::AnalysisBasedWarnings AnalysisWarnings;
threadSafety::BeforeSet *ThreadSafetyDeclCache;
/// \brief An entity for which implicit template instantiation is required.
///

233
clang/lib/Analysis/ThreadSafety.cpp
View File

@ -101,22 +101,17 @@ private:
LockKind LKind; ///< exclusive or shared
SourceLocation AcquireLoc; ///< where it was acquired.
bool Asserted; ///< true if the lock was asserted
bool Declared; ///< true if the lock was declared
public:
FactEntry(const CapabilityExpr &CE, LockKind LK, SourceLocation Loc,
bool Asrt, bool Declrd = false)
: CapabilityExpr(CE), LKind(LK), AcquireLoc(Loc), Asserted(Asrt),
Declared(Declrd) {}
bool Asrt)
: CapabilityExpr(CE), LKind(LK), AcquireLoc(Loc), Asserted(Asrt) {}
virtual ~FactEntry() {}
LockKind kind() const { return LKind; }
LockKind kind() const { return LKind; }
SourceLocation loc() const { return AcquireLoc; }
bool asserted() const { return Asserted; }
bool declared() const { return Declared; }
void setDeclared(bool D) { Declared = D; }
virtual void
handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan,
@ -236,58 +231,14 @@ public:
FactEntry *findPartialMatch(FactManager &FM,
const CapabilityExpr &CapE) const {
auto I = std::find_if(begin(), end(), [&](FactID ID) -> bool {
auto I = std::find_if(begin(), end(), [&](FactID ID) {
return FM[ID].partiallyMatches(CapE);
});
return I != end() ? &FM[*I] : nullptr;
}
bool containsMutexDecl(FactManager &FM, const ValueDecl* Vd) const {
auto I = std::find_if(begin(), end(), [&](FactID ID) -> bool {
return FM[ID].valueDecl() == Vd;
});
return I != end();
}
};
class ThreadSafetyAnalyzer;
class BeforeSet {
private:
typedef SmallVector<const ValueDecl*, 4> BeforeVect;
struct BeforeInfo {
BeforeInfo() : Vect(nullptr), Visited(false) { }
BeforeInfo(BeforeInfo &&O) : Vect(std::move(O.Vect)), Visited(O.Visited) {}
std::unique_ptr<BeforeVect> Vect;
int Visited;
};
typedef llvm::DenseMap<const ValueDecl*, BeforeInfo> BeforeMap;
typedef llvm::DenseMap<const ValueDecl*, bool> CycleMap;
public:
BeforeSet() { }
BeforeInfo* insertAttrExprs(const ValueDecl* Vd,
ThreadSafetyAnalyzer& Analyzer);
void checkBeforeAfter(const ValueDecl* Vd,
const FactSet& FSet,
ThreadSafetyAnalyzer& Analyzer,
SourceLocation Loc, StringRef CapKind);
private:
BeforeMap BMap;
CycleMap CycMap;
};
typedef llvm::ImmutableMap<const NamedDecl*, unsigned> LocalVarContext;
class LocalVariableMap;
@ -902,7 +853,6 @@ public:
/// \brief Class which implements the core thread safety analysis routines.
class ThreadSafetyAnalyzer {
friend class BuildLockset;
friend class BeforeSet;
llvm::BumpPtrAllocator Bpa;
threadSafety::til::MemRegionRef Arena;
@ -914,11 +864,9 @@ class ThreadSafetyAnalyzer {
FactManager FactMan;
std::vector<CFGBlockInfo> BlockInfo;
BeforeSet* GlobalBeforeSet;
public:
ThreadSafetyAnalyzer(ThreadSafetyHandler &H, BeforeSet* Bset)
: Arena(&Bpa), SxBuilder(Arena), Handler(H), GlobalBeforeSet(Bset) {}
ThreadSafetyAnalyzer(ThreadSafetyHandler &H)
: Arena(&Bpa), SxBuilder(Arena), Handler(H) {}
bool inCurrentScope(const CapabilityExpr &CapE);
@ -959,136 +907,6 @@ public:
void runAnalysis(AnalysisDeclContext &AC);
};
/// Process acquired_before and acquired_after attributes on Vd.
BeforeSet::BeforeInfo* BeforeSet::insertAttrExprs(const ValueDecl* Vd,
ThreadSafetyAnalyzer& Analyzer) {
// Create a new entry for Vd.
auto& Entry = BMap.FindAndConstruct(Vd);
BeforeInfo* Info = &Entry.second;
BeforeVect* Bv = nullptr;
const AttrVec &ArgAttrs = Vd->getAttrs();
for (Attr* At : ArgAttrs) {
switch (At->getKind()) {
case attr::AcquiredBefore: {
auto *A = cast<AcquiredBeforeAttr>(At);
// Create a new BeforeVect for Vd if necessary.
if (!Bv) {
Bv = new BeforeVect;
Info->Vect.reset(Bv);
}
// Read exprs from the attribute, and add them to BeforeVect.
for (const auto *Arg : A->args()) {
CapabilityExpr Cp =
Analyzer.SxBuilder.translateAttrExpr(Arg, nullptr);
if (const ValueDecl *Cpvd = Cp.valueDecl()) {
Bv->push_back(Cpvd);
auto It = BMap.find(Cpvd);
if (It == BMap.end())
insertAttrExprs(Cpvd, Analyzer);
}
}
break;
}
case attr::AcquiredAfter: {
auto *A = cast<AcquiredAfterAttr>(At);
// Read exprs from the attribute, and add them to BeforeVect.
for (const auto *Arg : A->args()) {
CapabilityExpr Cp =
Analyzer.SxBuilder.translateAttrExpr(Arg, nullptr);
if (const ValueDecl *ArgVd = Cp.valueDecl()) {
// Get entry for mutex listed in attribute
BeforeInfo* ArgInfo;
auto It = BMap.find(ArgVd);
if (It == BMap.end())
ArgInfo = insertAttrExprs(ArgVd, Analyzer);
else
ArgInfo = &It->second;
// Create a new BeforeVect if necessary.
BeforeVect* ArgBv = ArgInfo->Vect.get();
if (!ArgBv) {
ArgBv = new BeforeVect;
ArgInfo->Vect.reset(ArgBv);
}
ArgBv->push_back(Vd);
}
}
break;
}
default:
break;
}
}
return Info;
}
/// Return true if any mutexes in FSet are in the acquired_before set of Vd.
void BeforeSet::checkBeforeAfter(const ValueDecl* StartVd,
const FactSet& FSet,
ThreadSafetyAnalyzer& Analyzer,
SourceLocation Loc, StringRef CapKind) {
SmallVector<BeforeInfo*, 8> InfoVect;
// Do a depth-first traversal of Vd.
// Return true if there are cycles.
std::function<bool (const ValueDecl*)> traverse = [&](const ValueDecl* Vd) {
if (!Vd)
return false;
BeforeSet::BeforeInfo* Info;
auto It = BMap.find(Vd);
if (It == BMap.end())
Info = insertAttrExprs(Vd, Analyzer);
else
Info = &It->second;
if (Info->Visited == 1)
return true;
if (Info->Visited == 2)
return false;
BeforeVect* Bv = Info->Vect.get();
if (!Bv)
return false;
InfoVect.push_back(Info);
Info->Visited = 1;
for (auto *Vdb : *Bv) {
// Exclude mutexes in our immediate before set.
if (FSet.containsMutexDecl(Analyzer.FactMan, Vdb)) {
StringRef L1 = StartVd->getName();
StringRef L2 = Vdb->getName();
Analyzer.Handler.handleLockAcquiredBefore(CapKind, L1, L2, Loc);
}
// Transitively search other before sets, and warn on cycles.
if (traverse(Vdb)) {
if (CycMap.find(Vd) == CycMap.end()) {
CycMap.insert(std::make_pair(Vd, true));
StringRef L1 = Vd->getName();
Analyzer.Handler.handleBeforeAfterCycle(L1, Vd->getLocation());
}
}
}
Info->Visited = 2;
return false;
};
traverse(StartVd);
for (auto* Info : InfoVect)
Info->Visited = 0;
}
/// \brief Gets the value decl pointer from DeclRefExprs or MemberExprs.
static const ValueDecl *getValueDecl(const Expr *Exp) {
if (const auto *CE = dyn_cast<ImplicitCastExpr>(Exp))
@ -1202,13 +1020,7 @@ void ThreadSafetyAnalyzer::addLock(FactSet &FSet,
}
}
// Check before/after constraints
if (Handler.issueBetaWarnings() &&
!Entry->asserted() && !Entry->declared()) {
GlobalBeforeSet->checkBeforeAfter(Entry->valueDecl(), FSet, *this,
Entry->loc(), DiagKind);
}
// FIXME: deal with acquired before/after annotations.
// FIXME: Don't always warn when we have support for reentrant locks.
if (FSet.findLock(FactMan, *Entry)) {
if (!Entry->asserted())
@ -2167,16 +1979,14 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
}
// FIXME -- Loc can be wrong here.
for (const auto &Mu : ExclusiveLocksToAdd) {
auto Entry = llvm::make_unique<LockableFactEntry>(Mu, LK_Exclusive, Loc);
Entry->setDeclared(true);
addLock(InitialLockset, std::move(Entry), CapDiagKind, true);
}
for (const auto &Mu : SharedLocksToAdd) {
auto Entry = llvm::make_unique<LockableFactEntry>(Mu, LK_Shared, Loc);
Entry->setDeclared(true);
addLock(InitialLockset, std::move(Entry), CapDiagKind, true);
}
for (const auto &Mu : ExclusiveLocksToAdd)
addLock(InitialLockset,
llvm::make_unique<LockableFactEntry>(Mu, LK_Exclusive, Loc),
CapDiagKind, true);
for (const auto &Mu : SharedLocksToAdd)
addLock(InitialLockset,
llvm::make_unique<LockableFactEntry>(Mu, LK_Shared, Loc),
CapDiagKind, true);
}
for (const auto *CurrBlock : *SortedGraph) {
@ -2370,20 +2180,11 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
/// at the end of each block, and issue warnings for thread safety violations.
/// Each block in the CFG is traversed exactly once.
void runThreadSafetyAnalysis(AnalysisDeclContext &AC,
ThreadSafetyHandler &Handler,
BeforeSet **BSet) {
if (!*BSet)
*BSet = new BeforeSet;
ThreadSafetyAnalyzer Analyzer(Handler, *BSet);
ThreadSafetyHandler &Handler) {
ThreadSafetyAnalyzer Analyzer(Handler);
Analyzer.runAnalysis(AC);
}
void threadSafetyCleanup(BeforeSet* Cache) {
delete Cache;
}
/// \brief Helper function that returns a LockKind required for the given level
/// of access.
LockKind getLockKindFromAccessKind(AccessKind AK) {

21
clang/lib/Sema/AnalysisBasedWarnings.cpp
View File

@ -1679,22 +1679,6 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler {
Warnings.push_back(DelayedDiag(Warning, getNotes()));
}
virtual void handleLockAcquiredBefore(StringRef Kind, Name L1Name,
Name L2Name, SourceLocation Loc)
override {
PartialDiagnosticAt Warning(Loc,
S.PDiag(diag::warn_acquired_before) << Kind << L1Name << L2Name);
Warnings.push_back(DelayedDiag(Warning, getNotes()));
}
virtual void handleBeforeAfterCycle(Name L1Name, SourceLocation Loc)
override {
PartialDiagnosticAt Warning(Loc,
S.PDiag(diag::warn_acquired_before_after_cycle) << L1Name);
Warnings.push_back(DelayedDiag(Warning, getNotes()));
}
void enterFunction(const FunctionDecl* FD) override {
CurrentFunction = FD;
}
@ -1720,7 +1704,7 @@ class ConsumedWarningsHandler : public ConsumedWarningsHandlerBase {
DiagList Warnings;
public:
ConsumedWarningsHandler(Sema &S) : S(S) {}
void emitDiagnostics() override {
@ -1997,8 +1981,7 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
if (!Diags.isIgnored(diag::warn_thread_safety_verbose, D->getLocStart()))
Reporter.setVerbose(true);
threadSafety::runThreadSafetyAnalysis(AC, Reporter,
&S.ThreadSafetyDeclCache);
threadSafety::runThreadSafetyAnalysis(AC, Reporter);
Reporter.emitDiagnostics();
}

4
clang/lib/Sema/Sema.cpp
View File

@ -102,7 +102,7 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
AccessCheckingSFINAE(false), InNonInstantiationSFINAEContext(false),
NonInstantiationEntries(0), ArgumentPackSubstitutionIndex(-1),
CurrentInstantiationScope(nullptr), DisableTypoCorrection(false),
TyposCorrected(0), AnalysisWarnings(*this), ThreadSafetyDeclCache(nullptr),
TyposCorrected(0), AnalysisWarnings(*this),
VarDataSharingAttributesStack(nullptr), CurScope(nullptr),
Ident_super(nullptr), Ident___float128(nullptr)
{
@ -243,8 +243,6 @@ Sema::~Sema() {
if (isMultiplexExternalSource)
delete ExternalSource;
threadSafety::threadSafetyCleanup(ThreadSafetyDeclCache);
// Destroys data sharing attributes stack for OpenMP
DestroyDataSharingAttributesStack();

212
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
View File

@ -4835,7 +4835,7 @@ public:
read2(10, *foosp); // expected-warning {{reading the value pointed to by 'foosp' requires holding mutex 'mu'}}
destroy(mymove(*foosp)); // expected-warning {{reading the value pointed to by 'foosp' requires holding mutex 'mu'}}
// TODO -- these require better smart pointer handling.
// TODO -- these requires better smart pointer handling.
copy(*foosp.get());
write1(*foosp.get());
write2(10, *foosp.get());
@ -4848,213 +4848,3 @@ public:
} // end namespace PassByRefTest
namespace AcquiredBeforeAfterText {
class Foo {
Mutex mu1 ACQUIRED_BEFORE(mu2, mu3);
Mutex mu2;
Mutex mu3;
void test1() {
mu1.Lock();
mu2.Lock();
mu3.Lock();
mu3.Unlock();
mu2.Unlock();
mu1.Unlock();
}
void test2() {
mu2.Lock();
mu1.Lock(); // expected-warning {{mutex 'mu1' must be acquired before 'mu2'}}
mu1.Unlock();
mu2.Unlock();
}
void test3() {
mu3.Lock();
mu1.Lock(); // expected-warning {{mutex 'mu1' must be acquired before 'mu3'}}
mu1.Unlock();
mu3.Unlock();
}
void test4() EXCLUSIVE_LOCKS_REQUIRED(mu1) {
mu2.Lock();
mu2.Unlock();
}
void test5() EXCLUSIVE_LOCKS_REQUIRED(mu2) {
mu1.Lock(); // expected-warning {{mutex 'mu1' must be acquired before 'mu2'}}
mu1.Unlock();
}
void test6() EXCLUSIVE_LOCKS_REQUIRED(mu2) {
mu1.AssertHeld();
}
void test7() EXCLUSIVE_LOCKS_REQUIRED(mu1, mu2, mu3) { }
void test8() EXCLUSIVE_LOCKS_REQUIRED(mu3, mu2, mu1) { }
};
class Foo2 {
Mutex mu1;
Mutex mu2 ACQUIRED_AFTER(mu1);
Mutex mu3 ACQUIRED_AFTER(mu1);
void test1() {
mu1.Lock();
mu2.Lock();
mu3.Lock();
mu3.Unlock();
mu2.Unlock();
mu1.Unlock();
}
void test2() {
mu2.Lock();
mu1.Lock(); // expected-warning {{mutex 'mu1' must be acquired before 'mu2'}}
mu1.Unlock();
mu2.Unlock();
}
void test3() {
mu3.Lock();
mu1.Lock(); // expected-warning {{mutex 'mu1' must be acquired before 'mu3'}}
mu1.Unlock();
mu3.Unlock();
}
};
class Foo3 {
Mutex mu1 ACQUIRED_BEFORE(mu2);
Mutex mu2;
Mutex mu3 ACQUIRED_AFTER(mu2) ACQUIRED_BEFORE(mu4);
Mutex mu4;
void test1() {
mu1.Lock();
mu2.Lock();
mu3.Lock();
mu4.Lock();
mu4.Unlock();
mu3.Unlock();
mu2.Unlock();
mu1.Unlock();
}
void test2() {
mu4.Lock();
mu2.Lock(); // expected-warning {{mutex 'mu2' must be acquired before 'mu4'}}
mu2.Unlock();
mu4.Unlock();
}
void test3() {
mu4.Lock();
mu1.Lock(); // expected-warning {{mutex 'mu1' must be acquired before 'mu4'}}
mu1.Unlock();
mu4.Unlock();
}
void test4() {
mu3.Lock();
mu1.Lock(); // expected-warning {{mutex 'mu1' must be acquired before 'mu3'}}
mu1.Unlock();
mu3.Unlock();
}
};
// Test transitive DAG traversal with AFTER
class Foo4 {
Mutex mu1;
Mutex mu2 ACQUIRED_AFTER(mu1);
Mutex mu3 ACQUIRED_AFTER(mu1);
Mutex mu4 ACQUIRED_AFTER(mu2, mu3);
Mutex mu5 ACQUIRED_AFTER(mu4);
Mutex mu6 ACQUIRED_AFTER(mu4);
Mutex mu7 ACQUIRED_AFTER(mu5, mu6);
Mutex mu8 ACQUIRED_AFTER(mu7);
void test() {
mu8.Lock();
mu1.Lock(); // expected-warning {{mutex 'mu1' must be acquired before 'mu8'}}
mu1.Unlock();
mu8.Unlock();
}
};
// Test transitive DAG traversal with BEFORE
class Foo5 {
Mutex mu1 ACQUIRED_BEFORE(mu2, mu3);
Mutex mu2 ACQUIRED_BEFORE(mu4);
Mutex mu3 ACQUIRED_BEFORE(mu4);
Mutex mu4 ACQUIRED_BEFORE(mu5, mu6);
Mutex mu5 ACQUIRED_BEFORE(mu7);
Mutex mu6 ACQUIRED_BEFORE(mu7);
Mutex mu7 ACQUIRED_BEFORE(mu8);
Mutex mu8;
void test() {
mu8.Lock();
mu1.Lock(); // expected-warning {{mutex 'mu1' must be acquired before 'mu8'}}
mu1.Unlock();
mu8.Unlock();
}
};
class Foo6 {
Mutex mu1 ACQUIRED_AFTER(mu3); // expected-warning {{Cycle in acquired_before/after dependencies, starting with 'mu1'}}
Mutex mu2 ACQUIRED_AFTER(mu1); // expected-warning {{Cycle in acquired_before/after dependencies, starting with 'mu2'}}
Mutex mu3 ACQUIRED_AFTER(mu2); // expected-warning {{Cycle in acquired_before/after dependencies, starting with 'mu3'}}
Mutex mu_b ACQUIRED_BEFORE(mu_b); // expected-warning {{Cycle in acquired_before/after dependencies, starting with 'mu_b'}}
Mutex mu_a ACQUIRED_AFTER(mu_a); // expected-warning {{Cycle in acquired_before/after dependencies, starting with 'mu_a'}}
void test0() {
mu_a.Lock();
mu_b.Lock();
mu_b.Unlock();
mu_a.Unlock();
}
void test1a() {
mu1.Lock();
mu1.Unlock();
}
void test1b() {
mu1.Lock();
mu_a.Lock();
mu_b.Lock();
mu_b.Unlock();
mu_a.Unlock();
mu1.Unlock();
}
void test() {
mu2.Lock();
mu2.Unlock();
}
void test3() {
mu3.Lock();
mu3.Unlock();
}
};
} // end namespace AcquiredBeforeAfterTest