aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, delesley.
aaronpuchert requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D100801
Files:
clang/lib/Analysis/ThreadSafety.cpp
Index: clang/lib/Analysis/ThreadSafety.cpp
===================================================================
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -105,32 +105,37 @@
///
/// 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 @@
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 @@
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 @@
} 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 @@
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 @@
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 @@
std::make_unique<LockableFactEntry>(
AssertLock,
A->isShared() ? LK_Shared : LK_Exclusive, Loc,
- false, true),
+ FactEntry::Asserted),
ClassifyDiagnostic(A));
break;
}
@@ -1943,14 +1946,16 @@
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.
@@ -2364,13 +2369,13 @@
// 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);
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
