diff options
Diffstat (limited to '')
| -rw-r--r-- | build/build-clang/revert-llvmorg-16-init-7598-g54bfd0484615.patch | 866 |
1 files changed, 866 insertions, 0 deletions
diff --git a/build/build-clang/revert-llvmorg-16-init-7598-g54bfd0484615.patch b/build/build-clang/revert-llvmorg-16-init-7598-g54bfd0484615.patch new file mode 100644 index 0000000000..683d87b7a2 --- /dev/null +++ b/build/build-clang/revert-llvmorg-16-init-7598-g54bfd0484615.patch @@ -0,0 +1,866 @@ +The patch changed the way the no_thread_safety_analysis works in a way +that I think actually makes sense, but we have exceptions in the code +that aren't enough to accomodate that change. + +Until our code is adjusted, revert this change. + +--- + clang/docs/ThreadSafetyAnalysis.rst | 10 +- + .../Analysis/Analyses/ThreadSafetyCommon.h | 13 +- + .../clang/Analysis/Analyses/ThreadSafetyTIL.h | 7 +- + .../Analysis/Analyses/ThreadSafetyTraverse.h | 5 +- + clang/lib/Analysis/ThreadSafety.cpp | 202 +++++++++--------- + clang/lib/Analysis/ThreadSafetyCommon.cpp | 46 ++-- + .../SemaCXX/warn-thread-safety-analysis.cpp | 155 +++----------- + 8 files changed, 155 insertions(+), 290 deletions(-) + +diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst +index dcde0c706c70..23f460b248e1 100644 +--- a/clang/docs/ThreadSafetyAnalysis.rst ++++ b/clang/docs/ThreadSafetyAnalysis.rst +@@ -408,8 +408,7 @@ and destructor refer to the capability via different names; see the + Scoped capabilities are treated as capabilities that are implicitly acquired + on construction and released on destruction. They are associated with + the set of (regular) capabilities named in thread safety attributes on the +-constructor or function returning them by value (using C++17 guaranteed copy +-elision). Acquire-type attributes on other member functions are treated as ++constructor. Acquire-type attributes on other member functions are treated as + applying to that set of associated capabilities, while ``RELEASE`` implies that + a function releases all associated capabilities in whatever mode they're held. + +@@ -931,13 +930,6 @@ implementation. + // Assume mu is not held, implicitly acquire *this and associate it with mu. + MutexLocker(Mutex *mu, defer_lock_t) EXCLUDES(mu) : mut(mu), locked(false) {} + +- // Same as constructors, but without tag types. (Requires C++17 copy elision.) +- static MutexLocker Lock(Mutex *mu) ACQUIRE(mu); +- static MutexLocker Adopt(Mutex *mu) REQUIRES(mu); +- static MutexLocker ReaderLock(Mutex *mu) ACQUIRE_SHARED(mu); +- static MutexLocker AdoptReaderLock(Mutex *mu) REQUIRES_SHARED(mu); +- static MutexLocker DeferLock(Mutex *mu) EXCLUDES(mu); +- + // Release *this and all associated mutexes, if they are still held. + // There is no warning if the scope was already unlocked before. + ~MutexLocker() RELEASE() { +diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h +index 9c73d65db266..da69348ea938 100644 +--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h ++++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h +@@ -31,7 +31,6 @@ + #include "clang/Basic/LLVM.h" + #include "llvm/ADT/DenseMap.h" + #include "llvm/ADT/PointerIntPair.h" +-#include "llvm/ADT/PointerUnion.h" + #include "llvm/ADT/SmallVector.h" + #include "llvm/Support/Casting.h" + #include <sstream> +@@ -355,7 +354,7 @@ public: + const NamedDecl *AttrDecl; + + // Implicit object argument -- e.g. 'this' +- llvm::PointerUnion<const Expr *, til::SExpr *> SelfArg = nullptr; ++ const Expr *SelfArg = nullptr; + + // Number of funArgs + unsigned NumArgs = 0; +@@ -379,18 +378,10 @@ public: + // Translate a clang expression in an attribute to a til::SExpr. + // Constructs the context from D, DeclExp, and SelfDecl. + CapabilityExpr translateAttrExpr(const Expr *AttrExp, const NamedDecl *D, +- const Expr *DeclExp, +- til::SExpr *Self = nullptr); ++ const Expr *DeclExp, VarDecl *SelfD=nullptr); + + CapabilityExpr translateAttrExpr(const Expr *AttrExp, CallingContext *Ctx); + +- // Translate a variable reference. +- til::LiteralPtr *createVariable(const VarDecl *VD); +- +- // Create placeholder for this: we don't know the VarDecl on construction yet. +- std::pair<til::LiteralPtr *, StringRef> +- createThisPlaceholder(const Expr *Exp); +- + // Translate a clang statement or expression to a TIL expression. + // Also performs substitution of variables; Ctx provides the context. + // Dispatches on the type of S. +diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h +index 48593516d853..65556c8d584c 100644 +--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h ++++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h +@@ -634,14 +634,15 @@ typename V::R_SExpr Literal::traverse(V &Vs, typename V::R_Ctx Ctx) { + /// At compile time, pointer literals are represented by symbolic names. + class LiteralPtr : public SExpr { + public: +- LiteralPtr(const ValueDecl *D) : SExpr(COP_LiteralPtr), Cvdecl(D) {} ++ LiteralPtr(const ValueDecl *D) : SExpr(COP_LiteralPtr), Cvdecl(D) { ++ assert(D && "ValueDecl must not be null"); ++ } + LiteralPtr(const LiteralPtr &) = default; + + static bool classof(const SExpr *E) { return E->opcode() == COP_LiteralPtr; } + + // The clang declaration for the value that this pointer points to. + const ValueDecl *clangDecl() const { return Cvdecl; } +- void setClangDecl(const ValueDecl *VD) { Cvdecl = VD; } + + template <class V> + typename V::R_SExpr traverse(V &Vs, typename V::R_Ctx Ctx) { +@@ -650,8 +651,6 @@ public: + + template <class C> + typename C::CType compare(const LiteralPtr* E, C& Cmp) const { +- if (!Cvdecl || !E->Cvdecl) +- return Cmp.comparePointers(this, E); + return Cmp.comparePointers(Cvdecl, E->Cvdecl); + } + +diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h +index 6fc55130655a..e81c00d3dddb 100644 +--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h ++++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h +@@ -623,10 +623,7 @@ protected: + } + + void printLiteralPtr(const LiteralPtr *E, StreamType &SS) { +- if (const NamedDecl *D = E->clangDecl()) +- SS << D->getNameAsString(); +- else +- SS << "<temporary>"; ++ SS << E->clangDecl()->getNameAsString(); + } + + void printVariable(const Variable *V, StreamType &SS, bool IsVarDecl=false) { +diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp +index 76747561a9a3..a29134c6a5e7 100644 +--- a/clang/lib/Analysis/ThreadSafety.cpp ++++ b/clang/lib/Analysis/ThreadSafety.cpp +@@ -1029,7 +1029,7 @@ public: + + template <typename AttrType> + void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, const Expr *Exp, +- const NamedDecl *D, til::SExpr *Self = nullptr); ++ const NamedDecl *D, VarDecl *SelfDecl = nullptr); + + template <class AttrType> + void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, const Expr *Exp, +@@ -1220,7 +1220,7 @@ bool ThreadSafetyAnalyzer::inCurrentScope(const CapabilityExpr &CapE) { + if (const auto *LP = dyn_cast<til::LiteralPtr>(SExp)) { + const ValueDecl *VD = LP->clangDecl(); + // Variables defined in a function are always inaccessible. +- if (!VD || !VD->isDefinedOutsideFunctionOrMethod()) ++ if (!VD->isDefinedOutsideFunctionOrMethod()) + return false; + // For now we consider static class members to be inaccessible. + if (isa<CXXRecordDecl>(VD->getDeclContext())) +@@ -1311,10 +1311,10 @@ void ThreadSafetyAnalyzer::removeLock(FactSet &FSet, const CapabilityExpr &Cp, + template <typename AttrType> + void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, + const Expr *Exp, const NamedDecl *D, +- til::SExpr *Self) { ++ VarDecl *SelfDecl) { + if (Attr->args_size() == 0) { + // The mutex held is the "this" object. +- CapabilityExpr Cp = SxBuilder.translateAttrExpr(nullptr, D, Exp, Self); ++ CapabilityExpr Cp = SxBuilder.translateAttrExpr(nullptr, D, Exp, SelfDecl); + if (Cp.isInvalid()) { + warnInvalidLock(Handler, nullptr, D, Exp, Cp.getKind()); + return; +@@ -1326,7 +1326,7 @@ void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, + } + + for (const auto *Arg : Attr->args()) { +- CapabilityExpr Cp = SxBuilder.translateAttrExpr(Arg, D, Exp, Self); ++ CapabilityExpr Cp = SxBuilder.translateAttrExpr(Arg, D, Exp, SelfDecl); + if (Cp.isInvalid()) { + warnInvalidLock(Handler, nullptr, D, Exp, Cp.getKind()); + continue; +@@ -1529,26 +1529,21 @@ class BuildLockset : public ConstStmtVisitor<BuildLockset> { + + ThreadSafetyAnalyzer *Analyzer; + FactSet FSet; +- /// Maps constructed objects to `this` placeholder prior to initialization. +- llvm::SmallDenseMap<const Expr *, til::LiteralPtr *> ConstructedObjects; + LocalVariableMap::Context LVarCtx; + unsigned CtxIndex; + + // helper functions + void warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, AccessKind AK, + Expr *MutexExp, ProtectedOperationKind POK, +- til::LiteralPtr *Self, SourceLocation Loc); +- void warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, Expr *MutexExp, +- til::LiteralPtr *Self, SourceLocation Loc); ++ SourceLocation Loc); ++ void warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, Expr *MutexExp); + + void checkAccess(const Expr *Exp, AccessKind AK, + ProtectedOperationKind POK = POK_VarAccess); + void checkPtAccess(const Expr *Exp, AccessKind AK, + ProtectedOperationKind POK = POK_VarAccess); + +- void handleCall(const Expr *Exp, const NamedDecl *D, +- til::LiteralPtr *Self = nullptr, +- SourceLocation Loc = SourceLocation()); ++ void handleCall(const Expr *Exp, const NamedDecl *D, VarDecl *VD = nullptr); + void examineArguments(const FunctionDecl *FD, + CallExpr::const_arg_iterator ArgBegin, + CallExpr::const_arg_iterator ArgEnd, +@@ -1565,7 +1560,6 @@ public: + void VisitCallExpr(const CallExpr *Exp); + void VisitCXXConstructExpr(const CXXConstructExpr *Exp); + void VisitDeclStmt(const DeclStmt *S); +- void VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *Exp); + }; + + } // namespace +@@ -1575,12 +1569,10 @@ public: + void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, + AccessKind AK, Expr *MutexExp, + ProtectedOperationKind POK, +- til::LiteralPtr *Self, + SourceLocation Loc) { + LockKind LK = getLockKindFromAccessKind(AK); + +- CapabilityExpr Cp = +- Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp, Self); ++ CapabilityExpr Cp = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp); + if (Cp.isInvalid()) { + warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, Cp.getKind()); + return; +@@ -1637,10 +1629,8 @@ void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, + + /// Warn if the LSet contains the given lock. + void BuildLockset::warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, +- Expr *MutexExp, til::LiteralPtr *Self, +- SourceLocation Loc) { +- CapabilityExpr Cp = +- Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp, Self); ++ Expr *MutexExp) { ++ CapabilityExpr Cp = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp); + if (Cp.isInvalid()) { + warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, Cp.getKind()); + return; +@@ -1651,7 +1641,7 @@ void BuildLockset::warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, + const FactEntry *LDat = FSet.findLock(Analyzer->FactMan, Cp); + if (LDat) { + Analyzer->Handler.handleFunExcludesLock(Cp.getKind(), D->getNameAsString(), +- Cp.toString(), Loc); ++ Cp.toString(), Exp->getExprLoc()); + } + } + +@@ -1721,7 +1711,7 @@ void BuildLockset::checkAccess(const Expr *Exp, AccessKind AK, + } + + for (const auto *I : D->specific_attrs<GuardedByAttr>()) +- warnIfMutexNotHeld(D, Exp, AK, I->getArg(), POK, nullptr, Loc); ++ warnIfMutexNotHeld(D, Exp, AK, I->getArg(), POK, Loc); + } + + /// Checks pt_guarded_by and pt_guarded_var attributes. +@@ -1758,8 +1748,7 @@ void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK, + Analyzer->Handler.handleNoMutexHeld(D, PtPOK, AK, Exp->getExprLoc()); + + for (auto const *I : D->specific_attrs<PtGuardedByAttr>()) +- warnIfMutexNotHeld(D, Exp, AK, I->getArg(), PtPOK, nullptr, +- Exp->getExprLoc()); ++ warnIfMutexNotHeld(D, Exp, AK, I->getArg(), PtPOK, Exp->getExprLoc()); + } + + /// Process a function call, method call, constructor call, +@@ -1772,35 +1761,21 @@ void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK, + /// and check that the appropriate locks are held. Non-const method calls with + /// the same signature as const method calls can be also treated as reads. + /// +-/// \param Exp The call expression. +-/// \param D The callee declaration. +-/// \param Self If \p Exp = nullptr, the implicit this argument. +-/// \param Loc If \p Exp = nullptr, the location. + void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D, +- til::LiteralPtr *Self, SourceLocation Loc) { ++ VarDecl *VD) { ++ SourceLocation Loc = Exp->getExprLoc(); + CapExprSet ExclusiveLocksToAdd, SharedLocksToAdd; + CapExprSet ExclusiveLocksToRemove, SharedLocksToRemove, GenericLocksToRemove; + CapExprSet ScopedReqsAndExcludes; + + // Figure out if we're constructing an object of scoped lockable class +- CapabilityExpr Scp; +- if (Exp) { +- assert(!Self); +- const auto *TagT = Exp->getType()->getAs<TagType>(); +- if (TagT && Exp->isPRValue()) { +- std::pair<til::LiteralPtr *, StringRef> Placeholder = +- Analyzer->SxBuilder.createThisPlaceholder(Exp); +- [[maybe_unused]] auto inserted = +- ConstructedObjects.insert({Exp, Placeholder.first}); +- assert(inserted.second && "Are we visiting the same expression again?"); +- if (isa<CXXConstructExpr>(Exp)) +- Self = Placeholder.first; +- if (TagT->getDecl()->hasAttr<ScopedLockableAttr>()) +- Scp = CapabilityExpr(Placeholder.first, Placeholder.second, false); ++ bool isScopedVar = false; ++ if (VD) { ++ if (const auto *CD = dyn_cast<const CXXConstructorDecl>(D)) { ++ const CXXRecordDecl* PD = CD->getParent(); ++ if (PD && PD->hasAttr<ScopedLockableAttr>()) ++ isScopedVar = true; + } +- +- assert(Loc.isInvalid()); +- Loc = Exp->getExprLoc(); + } + + for(const Attr *At : D->attrs()) { +@@ -1811,7 +1786,7 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D, + const auto *A = cast<AcquireCapabilityAttr>(At); + Analyzer->getMutexIDs(A->isShared() ? SharedLocksToAdd + : ExclusiveLocksToAdd, +- A, Exp, D, Self); ++ A, Exp, D, VD); + break; + } + +@@ -1822,7 +1797,7 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D, + const auto *A = cast<AssertExclusiveLockAttr>(At); + + CapExprSet AssertLocks; +- Analyzer->getMutexIDs(AssertLocks, A, Exp, D, Self); ++ Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD); + for (const auto &AssertLock : AssertLocks) + Analyzer->addLock( + FSet, std::make_unique<LockableFactEntry>( +@@ -1833,7 +1808,7 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D, + const auto *A = cast<AssertSharedLockAttr>(At); + + CapExprSet AssertLocks; +- Analyzer->getMutexIDs(AssertLocks, A, Exp, D, Self); ++ Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD); + for (const auto &AssertLock : AssertLocks) + Analyzer->addLock( + FSet, std::make_unique<LockableFactEntry>( +@@ -1844,7 +1819,7 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D, + case attr::AssertCapability: { + const auto *A = cast<AssertCapabilityAttr>(At); + CapExprSet AssertLocks; +- Analyzer->getMutexIDs(AssertLocks, A, Exp, D, Self); ++ Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD); + for (const auto &AssertLock : AssertLocks) + Analyzer->addLock(FSet, std::make_unique<LockableFactEntry>( + AssertLock, +@@ -1858,11 +1833,11 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D, + case attr::ReleaseCapability: { + const auto *A = cast<ReleaseCapabilityAttr>(At); + if (A->isGeneric()) +- Analyzer->getMutexIDs(GenericLocksToRemove, A, Exp, D, Self); ++ Analyzer->getMutexIDs(GenericLocksToRemove, A, Exp, D, VD); + else if (A->isShared()) +- Analyzer->getMutexIDs(SharedLocksToRemove, A, Exp, D, Self); ++ Analyzer->getMutexIDs(SharedLocksToRemove, A, Exp, D, VD); + else +- Analyzer->getMutexIDs(ExclusiveLocksToRemove, A, Exp, D, Self); ++ Analyzer->getMutexIDs(ExclusiveLocksToRemove, A, Exp, D, VD); + break; + } + +@@ -1870,10 +1845,10 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D, + const auto *A = cast<RequiresCapabilityAttr>(At); + for (auto *Arg : A->args()) { + warnIfMutexNotHeld(D, Exp, A->isShared() ? AK_Read : AK_Written, Arg, +- POK_FunctionCall, Self, Loc); ++ POK_FunctionCall, Exp->getExprLoc()); + // use for adopting a lock +- if (!Scp.shouldIgnore()) +- Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, Self); ++ if (isScopedVar) ++ Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, VD); + } + break; + } +@@ -1881,10 +1856,10 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D, + case attr::LocksExcluded: { + const auto *A = cast<LocksExcludedAttr>(At); + for (auto *Arg : A->args()) { +- warnIfMutexHeld(D, Exp, Arg, Self, Loc); ++ warnIfMutexHeld(D, Exp, Arg); + // use for deferring a lock +- if (!Scp.shouldIgnore()) +- Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, Self); ++ if (isScopedVar) ++ Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, VD); + } + break; + } +@@ -1907,7 +1882,7 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D, + + // Add locks. + FactEntry::SourceKind Source = +- !Scp.shouldIgnore() ? FactEntry::Managed : FactEntry::Acquired; ++ isScopedVar ? FactEntry::Managed : FactEntry::Acquired; + for (const auto &M : ExclusiveLocksToAdd) + Analyzer->addLock(FSet, std::make_unique<LockableFactEntry>(M, LK_Exclusive, + Loc, Source)); +@@ -1915,9 +1890,15 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D, + Analyzer->addLock( + FSet, std::make_unique<LockableFactEntry>(M, LK_Shared, Loc, Source)); + +- if (!Scp.shouldIgnore()) { ++ if (isScopedVar) { + // Add the managing object as a dummy mutex, mapped to the underlying mutex. +- auto ScopedEntry = std::make_unique<ScopedLockableFactEntry>(Scp, Loc); ++ SourceLocation MLoc = VD->getLocation(); ++ DeclRefExpr DRE(VD->getASTContext(), VD, false, VD->getType(), VK_LValue, ++ VD->getLocation()); ++ // FIXME: does this store a pointer to DRE? ++ CapabilityExpr Scp = Analyzer->SxBuilder.translateAttrExpr(&DRE, nullptr); ++ ++ auto ScopedEntry = std::make_unique<ScopedLockableFactEntry>(Scp, MLoc); + for (const auto &M : ExclusiveLocksToAdd) + ScopedEntry->addLock(M); + for (const auto &M : SharedLocksToAdd) +@@ -2077,11 +2058,36 @@ void BuildLockset::VisitCXXConstructExpr(const CXXConstructExpr *Exp) { + } else { + examineArguments(D, Exp->arg_begin(), Exp->arg_end()); + } +- if (D && D->hasAttrs()) +- handleCall(Exp, D); + } + +-static const Expr *UnpackConstruction(const Expr *E) { ++static CXXConstructorDecl * ++findConstructorForByValueReturn(const CXXRecordDecl *RD) { ++ // Prefer a move constructor over a copy constructor. If there's more than ++ // one copy constructor or more than one move constructor, we arbitrarily ++ // pick the first declared such constructor rather than trying to guess which ++ // one is more appropriate. ++ CXXConstructorDecl *CopyCtor = nullptr; ++ for (auto *Ctor : RD->ctors()) { ++ if (Ctor->isDeleted()) ++ continue; ++ if (Ctor->isMoveConstructor()) ++ return Ctor; ++ if (!CopyCtor && Ctor->isCopyConstructor()) ++ CopyCtor = Ctor; ++ } ++ return CopyCtor; ++} ++ ++static Expr *buildFakeCtorCall(CXXConstructorDecl *CD, ArrayRef<Expr *> Args, ++ SourceLocation Loc) { ++ ASTContext &Ctx = CD->getASTContext(); ++ return CXXConstructExpr::Create(Ctx, Ctx.getRecordType(CD->getParent()), Loc, ++ CD, true, Args, false, false, false, false, ++ CXXConstructExpr::CK_Complete, ++ SourceRange(Loc, Loc)); ++} ++ ++static Expr *UnpackConstruction(Expr *E) { + if (auto *CE = dyn_cast<CastExpr>(E)) + if (CE->getCastKind() == CK_NoOp) + E = CE->getSubExpr()->IgnoreParens(); +@@ -2100,7 +2106,7 @@ void BuildLockset::VisitDeclStmt(const DeclStmt *S) { + + for (auto *D : S->getDeclGroup()) { + if (auto *VD = dyn_cast_or_null<VarDecl>(D)) { +- const Expr *E = VD->getInit(); ++ Expr *E = VD->getInit(); + if (!E) + continue; + E = E->IgnoreParens(); +@@ -2110,27 +2116,29 @@ void BuildLockset::VisitDeclStmt(const DeclStmt *S) { + E = EWC->getSubExpr()->IgnoreParens(); + E = UnpackConstruction(E); + +- if (auto Object = ConstructedObjects.find(E); +- Object != ConstructedObjects.end()) { +- Object->second->setClangDecl(VD); +- ConstructedObjects.erase(Object); ++ if (const auto *CE = dyn_cast<CXXConstructExpr>(E)) { ++ const auto *CtorD = dyn_cast_or_null<NamedDecl>(CE->getConstructor()); ++ if (!CtorD || !CtorD->hasAttrs()) ++ continue; ++ handleCall(E, CtorD, VD); ++ } else if (isa<CallExpr>(E) && E->isPRValue()) { ++ // If the object is initialized by a function call that returns a ++ // scoped lockable by value, use the attributes on the copy or move ++ // constructor to figure out what effect that should have on the ++ // lockset. ++ // FIXME: Is this really the best way to handle this situation? ++ auto *RD = E->getType()->getAsCXXRecordDecl(); ++ if (!RD || !RD->hasAttr<ScopedLockableAttr>()) ++ continue; ++ CXXConstructorDecl *CtorD = findConstructorForByValueReturn(RD); ++ if (!CtorD || !CtorD->hasAttrs()) ++ continue; ++ handleCall(buildFakeCtorCall(CtorD, {E}, E->getBeginLoc()), CtorD, VD); + } + } + } + } + +-void BuildLockset::VisitMaterializeTemporaryExpr( +- const MaterializeTemporaryExpr *Exp) { +- if (const ValueDecl *ExtD = Exp->getExtendingDecl()) { +- if (auto Object = +- ConstructedObjects.find(UnpackConstruction(Exp->getSubExpr())); +- Object != ConstructedObjects.end()) { +- Object->second->setClangDecl(ExtD); +- ConstructedObjects.erase(Object); +- } +- } +-} +- + /// Given two facts merging on a join point, possibly warn and decide whether to + /// keep or replace. + /// +@@ -2403,33 +2411,19 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { + LocksetBuilder.Visit(CS.getStmt()); + break; + } +- // Ignore BaseDtor and MemberDtor for now. ++ // Ignore BaseDtor, MemberDtor, and TemporaryDtor for now. + case CFGElement::AutomaticObjectDtor: { + CFGAutomaticObjDtor AD = BI.castAs<CFGAutomaticObjDtor>(); + const auto *DD = AD.getDestructorDecl(AC.getASTContext()); + if (!DD->hasAttrs()) + break; + +- LocksetBuilder.handleCall(nullptr, DD, +- SxBuilder.createVariable(AD.getVarDecl()), +- AD.getTriggerStmt()->getEndLoc()); +- break; +- } +- case CFGElement::TemporaryDtor: { +- auto TD = BI.castAs<CFGTemporaryDtor>(); +- +- // Clean up constructed object even if there are no attributes to +- // keep the number of objects in limbo as small as possible. +- if (auto Object = LocksetBuilder.ConstructedObjects.find( +- TD.getBindTemporaryExpr()->getSubExpr()); +- Object != LocksetBuilder.ConstructedObjects.end()) { +- const auto *DD = TD.getDestructorDecl(AC.getASTContext()); +- if (DD->hasAttrs()) +- // TODO: the location here isn't quite correct. +- LocksetBuilder.handleCall(nullptr, DD, Object->second, +- TD.getBindTemporaryExpr()->getEndLoc()); +- LocksetBuilder.ConstructedObjects.erase(Object); +- } ++ // Create a dummy expression, ++ auto *VD = const_cast<VarDecl *>(AD.getVarDecl()); ++ DeclRefExpr DRE(VD->getASTContext(), VD, false, ++ VD->getType().getNonReferenceType(), VK_LValue, ++ AD.getTriggerStmt()->getEndLoc()); ++ LocksetBuilder.handleCall(&DRE, DD); + break; + } + default: +diff --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp b/clang/lib/Analysis/ThreadSafetyCommon.cpp +index a771149f1591..06b61b4de92f 100644 +--- a/clang/lib/Analysis/ThreadSafetyCommon.cpp ++++ b/clang/lib/Analysis/ThreadSafetyCommon.cpp +@@ -115,22 +115,19 @@ static StringRef ClassifyDiagnostic(QualType VDT) { + /// \param D The declaration to which the attribute is attached. + /// \param DeclExp An expression involving the Decl to which the attribute + /// is attached. E.g. the call to a function. +-/// \param Self S-expression to substitute for a \ref CXXThisExpr. + CapabilityExpr SExprBuilder::translateAttrExpr(const Expr *AttrExp, + const NamedDecl *D, + const Expr *DeclExp, +- til::SExpr *Self) { ++ VarDecl *SelfDecl) { + // If we are processing a raw attribute expression, with no substitutions. +- if (!DeclExp && !Self) ++ if (!DeclExp) + return translateAttrExpr(AttrExp, nullptr); + + CallingContext Ctx(nullptr, D); + + // Examine DeclExp to find SelfArg and FunArgs, which are used to substitute + // for formal parameters when we call buildMutexID later. +- if (!DeclExp) +- /* We'll use Self. */; +- else if (const auto *ME = dyn_cast<MemberExpr>(DeclExp)) { ++ if (const auto *ME = dyn_cast<MemberExpr>(DeclExp)) { + Ctx.SelfArg = ME->getBase(); + Ctx.SelfArrow = ME->isArrow(); + } else if (const auto *CE = dyn_cast<CXXMemberCallExpr>(DeclExp)) { +@@ -145,24 +142,29 @@ CapabilityExpr SExprBuilder::translateAttrExpr(const Expr *AttrExp, + Ctx.SelfArg = nullptr; // Will be set below + Ctx.NumArgs = CE->getNumArgs(); + Ctx.FunArgs = CE->getArgs(); ++ } else if (D && isa<CXXDestructorDecl>(D)) { ++ // There's no such thing as a "destructor call" in the AST. ++ Ctx.SelfArg = DeclExp; + } + +- if (Self) { +- assert(!Ctx.SelfArg && "Ambiguous self argument"); +- Ctx.SelfArg = Self; ++ // Hack to handle constructors, where self cannot be recovered from ++ // the expression. ++ if (SelfDecl && !Ctx.SelfArg) { ++ DeclRefExpr SelfDRE(SelfDecl->getASTContext(), SelfDecl, false, ++ SelfDecl->getType(), VK_LValue, ++ SelfDecl->getLocation()); ++ Ctx.SelfArg = &SelfDRE; + + // If the attribute has no arguments, then assume the argument is "this". + if (!AttrExp) +- return CapabilityExpr( +- Self, ClassifyDiagnostic(cast<CXXMethodDecl>(D)->getThisObjectType()), +- false); ++ return translateAttrExpr(Ctx.SelfArg, nullptr); + else // For most attributes. + return translateAttrExpr(AttrExp, &Ctx); + } + + // If the attribute has no arguments, then assume the argument is "this". + if (!AttrExp) +- return translateAttrExpr(cast<const Expr *>(Ctx.SelfArg), nullptr); ++ return translateAttrExpr(Ctx.SelfArg, nullptr); + else // For most attributes. + return translateAttrExpr(AttrExp, &Ctx); + } +@@ -216,16 +218,6 @@ CapabilityExpr SExprBuilder::translateAttrExpr(const Expr *AttrExp, + return CapabilityExpr(E, Kind, Neg); + } + +-til::LiteralPtr *SExprBuilder::createVariable(const VarDecl *VD) { +- return new (Arena) til::LiteralPtr(VD); +-} +- +-std::pair<til::LiteralPtr *, StringRef> +-SExprBuilder::createThisPlaceholder(const Expr *Exp) { +- return {new (Arena) til::LiteralPtr(nullptr), +- ClassifyDiagnostic(Exp->getType())}; +-} +- + // Translate a clang statement or expression to a TIL expression. + // Also performs substitution of variables; Ctx provides the context. + // Dispatches on the type of S. +@@ -335,12 +327,8 @@ til::SExpr *SExprBuilder::translateDeclRefExpr(const DeclRefExpr *DRE, + til::SExpr *SExprBuilder::translateCXXThisExpr(const CXXThisExpr *TE, + CallingContext *Ctx) { + // Substitute for 'this' +- if (Ctx && Ctx->SelfArg) { +- if (const auto *SelfArg = dyn_cast<const Expr *>(Ctx->SelfArg)) +- return translate(SelfArg, Ctx->Prev); +- else +- return cast<til::SExpr *>(Ctx->SelfArg); +- } ++ if (Ctx && Ctx->SelfArg) ++ return translate(Ctx->SelfArg, Ctx->Prev); + assert(SelfVar && "We have no variable for 'this'!"); + return SelfVar; + } +diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +index 8e312e589d81..e1cfa1f3fd17 100644 +--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp ++++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +@@ -1606,30 +1606,6 @@ namespace substitution_test { + dlr.unlockData(d1); + } + }; +- +- // Automatic object destructor calls don't appear as expressions in the CFG, +- // so we have to handle them separately whenever substitutions are required. +- struct DestructorRequires { +- Mutex mu; +- ~DestructorRequires() EXCLUSIVE_LOCKS_REQUIRED(mu); +- }; +- +- void destructorRequires() { +- DestructorRequires rd; +- rd.mu.AssertHeld(); +- } +- +- struct DestructorExcludes { +- Mutex mu; +- ~DestructorExcludes() LOCKS_EXCLUDED(mu); +- }; +- +- void destructorExcludes() { +- DestructorExcludes ed; +- ed.mu.Lock(); // expected-note {{mutex acquired here}} +- } // expected-warning {{cannot call function '~DestructorExcludes' while mutex 'ed.mu' is held}} +- // expected-warning@-1 {{mutex 'ed.mu' is still held at the end of function}} +- + } // end namespace substituation_test + + +@@ -1714,15 +1690,6 @@ struct TestScopedLockable { + } + #endif + +- void temporary() { +- MutexLock{&mu1}, a = 5; +- } +- +- void lifetime_extension() { +- const MutexLock &mulock = MutexLock(&mu1); +- a = 5; +- } +- + void foo2() { + ReaderMutexLock mulock1(&mu1); + if (getBool()) { +@@ -1741,12 +1708,6 @@ struct TestScopedLockable { + // expected-warning {{acquiring mutex 'mu1' that is already held}} + } + +- void temporary_double_lock() { +- MutexLock mulock_a(&mu1); // expected-note{{mutex acquired here}} +- MutexLock{&mu1}; // \ +- // expected-warning {{acquiring mutex 'mu1' that is already held}} +- } +- + void foo4() { + MutexLock mulock1(&mu1), mulock2(&mu2); + a = b+1; +@@ -4226,20 +4187,6 @@ public: + void foo() EXCLUSIVE_LOCKS_REQUIRED(this); + }; + +-class SelfLockDeferred { +-public: +- SelfLockDeferred() LOCKS_EXCLUDED(mu_); +- ~SelfLockDeferred() UNLOCK_FUNCTION(mu_); +- +- Mutex mu_; +-}; +- +-class LOCKABLE SelfLockDeferred2 { +-public: +- SelfLockDeferred2() LOCKS_EXCLUDED(this); +- ~SelfLockDeferred2() UNLOCK_FUNCTION(); +-}; +- + + void test() { + SelfLock s; +@@ -4251,14 +4198,6 @@ void test2() { + s2.foo(); + } + +-void testDeferredTemporary() { +- SelfLockDeferred(); // expected-warning {{releasing mutex '<temporary>.mu_' that was not held}} +-} +- +-void testDeferredTemporary2() { +- SelfLockDeferred2(); // expected-warning {{releasing mutex '<temporary>' that was not held}} +-} +- + } // end namespace SelfConstructorTest + + +@@ -5953,75 +5892,47 @@ C c; + void f() { c[A()]->g(); } + } // namespace PR34800 + +-#ifdef __cpp_guaranteed_copy_elision +- + namespace ReturnScopedLockable { ++ template<typename Object> class SCOPED_LOCKABLE ReadLockedPtr { ++ public: ++ ReadLockedPtr(Object *ptr) SHARED_LOCK_FUNCTION((*this)->mutex); ++ ReadLockedPtr(ReadLockedPtr &&) SHARED_LOCK_FUNCTION((*this)->mutex); ++ ~ReadLockedPtr() UNLOCK_FUNCTION(); + +-class Object { +-public: +- MutexLock lock() EXCLUSIVE_LOCK_FUNCTION(mutex) { +- // TODO: False positive because scoped lock isn't destructed. +- return MutexLock(&mutex); // expected-note {{mutex acquired here}} +- } // expected-warning {{mutex 'mutex' is still held at the end of function}} +- +- ReaderMutexLock lockShared() SHARED_LOCK_FUNCTION(mutex) { +- // TODO: False positive because scoped lock isn't destructed. +- return ReaderMutexLock(&mutex); // expected-note {{mutex acquired here}} +- } // expected-warning {{mutex 'mutex' is still held at the end of function}} +- +- MutexLock adopt() EXCLUSIVE_LOCKS_REQUIRED(mutex) { +- // TODO: False positive because scoped lock isn't destructed. +- return MutexLock(&mutex, true); // expected-note {{mutex acquired here}} +- } // expected-warning {{mutex 'mutex' is still held at the end of function}} ++ Object *operator->() const { return object; } + +- ReaderMutexLock adoptShared() SHARED_LOCKS_REQUIRED(mutex) { +- // TODO: False positive because scoped lock isn't destructed. +- return ReaderMutexLock(&mutex, true); // expected-note {{mutex acquired here}} +- } // expected-warning {{mutex 'mutex' is still held at the end of function}} ++ private: ++ Object *object; ++ }; + +- int x GUARDED_BY(mutex); +- void needsLock() EXCLUSIVE_LOCKS_REQUIRED(mutex); ++ struct Object { ++ int f() SHARED_LOCKS_REQUIRED(mutex); ++ Mutex mutex; ++ }; + +- void testInside() { +- MutexLock scope = lock(); +- x = 1; +- needsLock(); ++ ReadLockedPtr<Object> get(); ++ int use() { ++ auto ptr = get(); ++ return ptr->f(); ++ } ++ void use_constructor() { ++ auto ptr = ReadLockedPtr<Object>(nullptr); ++ ptr->f(); ++ auto ptr2 = ReadLockedPtr<Object>{nullptr}; ++ ptr2->f(); ++ auto ptr3 = (ReadLockedPtr<Object>{nullptr}); ++ ptr3->f(); ++ } ++ struct Convertible { ++ Convertible(); ++ operator ReadLockedPtr<Object>(); ++ }; ++ void use_conversion() { ++ ReadLockedPtr<Object> ptr = Convertible(); ++ ptr->f(); + } +- +- Mutex mutex; +-}; +- +-Object obj; +- +-void testLock() { +- MutexLock scope = obj.lock(); +- obj.x = 1; +- obj.needsLock(); + } + +-int testSharedLock() { +- ReaderMutexLock scope = obj.lockShared(); +- obj.x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'obj.mutex' exclusively}} +- return obj.x; +-} +- +-void testAdopt() { +- obj.mutex.Lock(); +- MutexLock scope = obj.adopt(); +- obj.x = 1; +-} +- +-int testAdoptShared() { +- obj.mutex.Lock(); +- ReaderMutexLock scope = obj.adoptShared(); +- obj.x = 1; +- return obj.x; +-} +- +-} // namespace ReturnScopedLockable +- +-#endif +- + namespace PR38640 { + void f() { + // Self-referencing assignment previously caused an infinite loop when thread +-- +2.37.1.1.g659da70093 + |