Author: Aaron Puchert Date: 2021-05-03T14:03:17+02:00 New Revision: 530e074faafe995a560e80815f5af8306670ea7b
URL: https://github.com/llvm/llvm-project/commit/530e074faafe995a560e80815f5af8306670ea7b DIFF: https://github.com/llvm/llvm-project/commit/530e074faafe995a560e80815f5af8306670ea7b.diff LOG: 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 Added: Modified: clang/lib/Analysis/ThreadSafety.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp index cbb992f40c32b..bb3a4f1616fca 100644 --- a/clang/lib/Analysis/ThreadSafety.cpp +++ b/clang/lib/Analysis/ThreadSafety.cpp @@ -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 @@ class ScopedLockableFactEntry : public FactEntry { 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 @@ class ScopedLockableFactEntry : public FactEntry { } 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); } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
