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

Thread safety analysis: Replace flags in FactEntry by SourceKind (NFC) 

The motivation here is to make it available in the base class whether a
fact is managed or not. That would have meant three flags on the base
class, so I had a look whether we really have 8 possible combinations.

It turns out we don't: asserted and declared are obviously mutually
exclusive. Managed facts are only created when we acquire a capability
through a scoped capability. Adopting an asserted or declared lock will
not (in fact can not, because Facts are immutable) make them managed.

We probably don't want to allow adopting an asserted lock (because then
the function should probably have a release attribute, and then the
assertion is pointless), but we might at some point decide to replace a
declared fact on adoption.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D100801
This commit is contained in:
Aaron Puchert 2021-05-03 13:57:44 +02:00
parent 876bf516e7
commit 530e074faa
1 changed files with 46 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -105,32 +105,37 @@ class FactSet;
///
/// FIXME: this analysis does not currently support re-entrant locking.
class FactEntry : public CapabilityExpr {
public:
/// Where a fact comes from.
enum SourceKind {
Acquired, ///< The fact has been directly acquired.
Asserted, ///< The fact has been asserted to be held.
Declared, ///< The fact is assumed to be held by callers.
Managed, ///< The fact has been acquired through a scoped capability.
};
private:
/// Exclusive or shared.
LockKind LKind;
LockKind LKind : 8;
// How it was acquired.
SourceKind Source : 8;
/// Where it was acquired.
SourceLocation AcquireLoc;
/// True if the lock was asserted.
bool Asserted;
/// True if the lock was declared.
bool 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) {}
SourceKind Src)
: CapabilityExpr(CE), LKind(LK), Source(Src), AcquireLoc(Loc) {}
virtual ~FactEntry() = default;
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; }
bool asserted() const { return Source == Asserted; }
bool declared() const { return Source == Declared; }
bool managed() const { return Source == Managed; }
virtual void
handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan,
@ -851,20 +856,16 @@ static void findBlockLocations(CFG *CFGraph,
namespace {
class LockableFactEntry : public FactEntry {
private:
/// managed by ScopedLockable object
bool Managed;
public:
LockableFactEntry(const CapabilityExpr &CE, LockKind LK, SourceLocation Loc,
bool Mng = false, bool Asrt = false)
: FactEntry(CE, LK, Loc, Asrt), Managed(Mng) {}
SourceKind Src = Acquired)
: FactEntry(CE, LK, Loc, Src) {}
void
handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan,
SourceLocation JoinLoc, LockErrorKind LEK,
ThreadSafetyHandler &Handler) const override {
if (!Managed && !asserted() && !negative() && !isUniversal()) {
if (!managed() && !asserted() && !negative() && !isUniversal()) {
Handler.handleMutexHeldEndOfScope("mutex", toString(), loc(), JoinLoc,
LEK);
}
@ -903,7 +904,7 @@ private:
public:
ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc)
: FactEntry(CE, LK_Exclusive, Loc, false) {}
: FactEntry(CE, LK_Exclusive, Loc, Acquired) {}
void addLock(const CapabilityExpr &M) {
UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
@ -983,7 +984,7 @@ private:
} else {
FSet.removeLock(FactMan, !Cp);
FSet.addLock(FactMan,
std::make_unique<LockableFactEntry>(Cp, kind, loc, true));
std::make_unique<LockableFactEntry>(Cp, kind, loc, Managed));
}
}
@ -1854,10 +1855,11 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
CapExprSet AssertLocks;
Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
for (const auto &AssertLock : AssertLocks)
Analyzer->addLock(FSet,
std::make_unique<LockableFactEntry>(
AssertLock, LK_Exclusive, Loc, false, true),
ClassifyDiagnostic(A));
Analyzer->addLock(
FSet,
std::make_unique<LockableFactEntry>(AssertLock, LK_Exclusive, Loc,
FactEntry::Asserted),
ClassifyDiagnostic(A));
break;
}
case attr::AssertSharedLock: {
@ -1866,10 +1868,11 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
CapExprSet AssertLocks;
Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
for (const auto &AssertLock : AssertLocks)
Analyzer->addLock(FSet,
std::make_unique<LockableFactEntry>(
AssertLock, LK_Shared, Loc, false, true),
ClassifyDiagnostic(A));
Analyzer->addLock(
FSet,
std::make_unique<LockableFactEntry>(AssertLock, LK_Shared, Loc,
FactEntry::Asserted),
ClassifyDiagnostic(A));
break;
}
@ -1882,7 +1885,7 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
std::make_unique<LockableFactEntry>(
AssertLock,
A->isShared() ? LK_Shared : LK_Exclusive, Loc,
false, true),
FactEntry::Asserted),
ClassifyDiagnostic(A));
break;
}
@ -1943,14 +1946,16 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Generic, CapDiagKind);
// Add locks.
FactEntry::SourceKind Source =
isScopedVar ? FactEntry::Managed : FactEntry::Acquired;
for (const auto &M : ExclusiveLocksToAdd)
Analyzer->addLock(FSet, std::make_unique<LockableFactEntry>(
M, LK_Exclusive, Loc, isScopedVar),
CapDiagKind);
Analyzer->addLock(
FSet, std::make_unique<LockableFactEntry>(M, LK_Exclusive, Loc, Source),
CapDiagKind);
for (const auto &M : SharedLocksToAdd)
Analyzer->addLock(FSet, std::make_unique<LockableFactEntry>(
M, LK_Shared, Loc, isScopedVar),
CapDiagKind);
Analyzer->addLock(
FSet, std::make_unique<LockableFactEntry>(M, LK_Shared, Loc, Source),
CapDiagKind);
if (isScopedVar) {
// Add the managing object as a dummy mutex, mapped to the underlying mutex.
@ -2362,13 +2367,13 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
// FIXME -- Loc can be wrong here.
for (const auto &Mu : ExclusiveLocksToAdd) {
auto Entry = std::make_unique<LockableFactEntry>(Mu, LK_Exclusive, Loc);
Entry->setDeclared(true);
auto Entry = std::make_unique<LockableFactEntry>(Mu, LK_Exclusive, Loc,
FactEntry::Declared);
addLock(InitialLockset, std::move(Entry), CapDiagKind, true);
}
for (const auto &Mu : SharedLocksToAdd) {
auto Entry = std::make_unique<LockableFactEntry>(Mu, LK_Shared, Loc);
Entry->setDeclared(true);
auto Entry = std::make_unique<LockableFactEntry>(Mu, LK_Shared, Loc,
FactEntry::Declared);
addLock(InitialLockset, std::move(Entry), CapDiagKind, true);
}
}