| blob | 683d87b7a2125b0fb7adc870693864ca8b4e1fa1 |
1 The patch changed the way the no_thread_safety_analysis works in a way
2 that I think actually makes sense, but we have exceptions in the code
3 that aren't enough to accomodate that change.
5 Until our code is adjusted, revert this change.
7 ---
8 clang/docs/ThreadSafetyAnalysis.rst | 10 +-
9 .../Analysis/Analyses/ThreadSafetyCommon.h | 13 +-
10 .../clang/Analysis/Analyses/ThreadSafetyTIL.h | 7 +-
11 .../Analysis/Analyses/ThreadSafetyTraverse.h | 5 +-
12 clang/lib/Analysis/ThreadSafety.cpp | 202 +++++++++---------
13 clang/lib/Analysis/ThreadSafetyCommon.cpp | 46 ++--
14 .../SemaCXX/warn-thread-safety-analysis.cpp | 155 +++-----------
15 8 files changed, 155 insertions(+), 290 deletions(-)
17 diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst
18 index dcde0c706c70..23f460b248e1 100644
19 --- a/clang/docs/ThreadSafetyAnalysis.rst
20 +++ b/clang/docs/ThreadSafetyAnalysis.rst
21 @@ -408,8 +408,7 @@ and destructor refer to the capability via different names; see the
22 Scoped capabilities are treated as capabilities that are implicitly acquired
23 on construction and released on destruction. They are associated with
24 the set of (regular) capabilities named in thread safety attributes on the
25 -constructor or function returning them by value (using C++17 guaranteed copy
26 -elision). Acquire-type attributes on other member functions are treated as
27 +constructor. Acquire-type attributes on other member functions are treated as
28 applying to that set of associated capabilities, while ``RELEASE`` implies that
29 a function releases all associated capabilities in whatever mode they're held.
31 @@ -931,13 +930,6 @@ implementation.
32 // Assume mu is not held, implicitly acquire *this and associate it with mu.
33 MutexLocker(Mutex *mu, defer_lock_t) EXCLUDES(mu) : mut(mu), locked(false) {}
35 - // Same as constructors, but without tag types. (Requires C++17 copy elision.)
36 - static MutexLocker Lock(Mutex *mu) ACQUIRE(mu);
37 - static MutexLocker Adopt(Mutex *mu) REQUIRES(mu);
38 - static MutexLocker ReaderLock(Mutex *mu) ACQUIRE_SHARED(mu);
39 - static MutexLocker AdoptReaderLock(Mutex *mu) REQUIRES_SHARED(mu);
40 - static MutexLocker DeferLock(Mutex *mu) EXCLUDES(mu);
41 -
42 // Release *this and all associated mutexes, if they are still held.
43 // There is no warning if the scope was already unlocked before.
44 ~MutexLocker() RELEASE() {
45 diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
46 index 9c73d65db266..da69348ea938 100644
47 --- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
48 +++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
49 @@ -31,7 +31,6 @@
50 #include "clang/Basic/LLVM.h"
51 #include "llvm/ADT/DenseMap.h"
52 #include "llvm/ADT/PointerIntPair.h"
53 -#include "llvm/ADT/PointerUnion.h"
54 #include "llvm/ADT/SmallVector.h"
55 #include "llvm/Support/Casting.h"
56 #include <sstream>
57 @@ -355,7 +354,7 @@ public:
58 const NamedDecl *AttrDecl;
60 // Implicit object argument -- e.g. 'this'
61 - llvm::PointerUnion<const Expr *, til::SExpr *> SelfArg = nullptr;
62 + const Expr *SelfArg = nullptr;
64 // Number of funArgs
65 unsigned NumArgs = 0;
66 @@ -379,18 +378,10 @@ public:
67 // Translate a clang expression in an attribute to a til::SExpr.
68 // Constructs the context from D, DeclExp, and SelfDecl.
69 CapabilityExpr translateAttrExpr(const Expr *AttrExp, const NamedDecl *D,
70 - const Expr *DeclExp,
71 - til::SExpr *Self = nullptr);
72 + const Expr *DeclExp, VarDecl *SelfD=nullptr);
74 CapabilityExpr translateAttrExpr(const Expr *AttrExp, CallingContext *Ctx);
76 - // Translate a variable reference.
77 - til::LiteralPtr *createVariable(const VarDecl *VD);
78 -
79 - // Create placeholder for this: we don't know the VarDecl on construction yet.
80 - std::pair<til::LiteralPtr *, StringRef>
81 - createThisPlaceholder(const Expr *Exp);
82 -
83 // Translate a clang statement or expression to a TIL expression.
84 // Also performs substitution of variables; Ctx provides the context.
85 // Dispatches on the type of S.
86 diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
87 index 48593516d853..65556c8d584c 100644
88 --- a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
89 +++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
90 @@ -634,14 +634,15 @@ typename V::R_SExpr Literal::traverse(V &Vs, typename V::R_Ctx Ctx) {
91 /// At compile time, pointer literals are represented by symbolic names.
92 class LiteralPtr : public SExpr {
93 public:
94 - LiteralPtr(const ValueDecl *D) : SExpr(COP_LiteralPtr), Cvdecl(D) {}
95 + LiteralPtr(const ValueDecl *D) : SExpr(COP_LiteralPtr), Cvdecl(D) {
96 + assert(D && "ValueDecl must not be null");
97 + }
98 LiteralPtr(const LiteralPtr &) = default;
100 static bool classof(const SExpr *E) { return E->opcode() == COP_LiteralPtr; }
102 // The clang declaration for the value that this pointer points to.
103 const ValueDecl *clangDecl() const { return Cvdecl; }
104 - void setClangDecl(const ValueDecl *VD) { Cvdecl = VD; }
106 template <class V>
107 typename V::R_SExpr traverse(V &Vs, typename V::R_Ctx Ctx) {
108 @@ -650,8 +651,6 @@ public:
110 template <class C>
111 typename C::CType compare(const LiteralPtr* E, C& Cmp) const {
112 - if (!Cvdecl || !E->Cvdecl)
113 - return Cmp.comparePointers(this, E);
114 return Cmp.comparePointers(Cvdecl, E->Cvdecl);
115 }
117 diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
118 index 6fc55130655a..e81c00d3dddb 100644
119 --- a/clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
120 +++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
121 @@ -623,10 +623,7 @@ protected:
122 }
124 void printLiteralPtr(const LiteralPtr *E, StreamType &SS) {
125 - if (const NamedDecl *D = E->clangDecl())
126 - SS << D->getNameAsString();
127 - else
128 - SS << "<temporary>";
129 + SS << E->clangDecl()->getNameAsString();
130 }
132 void printVariable(const Variable *V, StreamType &SS, bool IsVarDecl=false) {
133 diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
134 index 76747561a9a3..a29134c6a5e7 100644
135 --- a/clang/lib/Analysis/ThreadSafety.cpp
136 +++ b/clang/lib/Analysis/ThreadSafety.cpp
137 @@ -1029,7 +1029,7 @@ public:
139 template <typename AttrType>
140 void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, const Expr *Exp,
141 - const NamedDecl *D, til::SExpr *Self = nullptr);
142 + const NamedDecl *D, VarDecl *SelfDecl = nullptr);
144 template <class AttrType>
145 void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, const Expr *Exp,
146 @@ -1220,7 +1220,7 @@ bool ThreadSafetyAnalyzer::inCurrentScope(const CapabilityExpr &CapE) {
147 if (const auto *LP = dyn_cast<til::LiteralPtr>(SExp)) {
148 const ValueDecl *VD = LP->clangDecl();
149 // Variables defined in a function are always inaccessible.
150 - if (!VD || !VD->isDefinedOutsideFunctionOrMethod())
151 + if (!VD->isDefinedOutsideFunctionOrMethod())
152 return false;
153 // For now we consider static class members to be inaccessible.
154 if (isa<CXXRecordDecl>(VD->getDeclContext()))
155 @@ -1311,10 +1311,10 @@ void ThreadSafetyAnalyzer::removeLock(FactSet &FSet, const CapabilityExpr &Cp,
156 template <typename AttrType>
157 void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet &Mtxs, AttrType *Attr,
158 const Expr *Exp, const NamedDecl *D,
159 - til::SExpr *Self) {
160 + VarDecl *SelfDecl) {
161 if (Attr->args_size() == 0) {
162 // The mutex held is the "this" object.
163 - CapabilityExpr Cp = SxBuilder.translateAttrExpr(nullptr, D, Exp, Self);
164 + CapabilityExpr Cp = SxBuilder.translateAttrExpr(nullptr, D, Exp, SelfDecl);
165 if (Cp.isInvalid()) {
166 warnInvalidLock(Handler, nullptr, D, Exp, Cp.getKind());
167 return;
168 @@ -1326,7 +1326,7 @@ void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet &Mtxs, AttrType *Attr,
169 }
171 for (const auto *Arg : Attr->args()) {
172 - CapabilityExpr Cp = SxBuilder.translateAttrExpr(Arg, D, Exp, Self);
173 + CapabilityExpr Cp = SxBuilder.translateAttrExpr(Arg, D, Exp, SelfDecl);
174 if (Cp.isInvalid()) {
175 warnInvalidLock(Handler, nullptr, D, Exp, Cp.getKind());
176 continue;
177 @@ -1529,26 +1529,21 @@ class BuildLockset : public ConstStmtVisitor<BuildLockset> {
179 ThreadSafetyAnalyzer *Analyzer;
180 FactSet FSet;
181 - /// Maps constructed objects to `this` placeholder prior to initialization.
182 - llvm::SmallDenseMap<const Expr *, til::LiteralPtr *> ConstructedObjects;
183 LocalVariableMap::Context LVarCtx;
184 unsigned CtxIndex;
186 // helper functions
187 void warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, AccessKind AK,
188 Expr *MutexExp, ProtectedOperationKind POK,
189 - til::LiteralPtr *Self, SourceLocation Loc);
190 - void warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, Expr *MutexExp,
191 - til::LiteralPtr *Self, SourceLocation Loc);
192 + SourceLocation Loc);
193 + void warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, Expr *MutexExp);
195 void checkAccess(const Expr *Exp, AccessKind AK,
196 ProtectedOperationKind POK = POK_VarAccess);
197 void checkPtAccess(const Expr *Exp, AccessKind AK,
198 ProtectedOperationKind POK = POK_VarAccess);
200 - void handleCall(const Expr *Exp, const NamedDecl *D,
201 - til::LiteralPtr *Self = nullptr,
202 - SourceLocation Loc = SourceLocation());
203 + void handleCall(const Expr *Exp, const NamedDecl *D, VarDecl *VD = nullptr);
204 void examineArguments(const FunctionDecl *FD,
205 CallExpr::const_arg_iterator ArgBegin,
206 CallExpr::const_arg_iterator ArgEnd,
207 @@ -1565,7 +1560,6 @@ public:
208 void VisitCallExpr(const CallExpr *Exp);
209 void VisitCXXConstructExpr(const CXXConstructExpr *Exp);
210 void VisitDeclStmt(const DeclStmt *S);
211 - void VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *Exp);
212 };
214 } // namespace
215 @@ -1575,12 +1569,10 @@ public:
216 void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp,
217 AccessKind AK, Expr *MutexExp,
218 ProtectedOperationKind POK,
219 - til::LiteralPtr *Self,
220 SourceLocation Loc) {
221 LockKind LK = getLockKindFromAccessKind(AK);
223 - CapabilityExpr Cp =
224 - Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp, Self);
225 + CapabilityExpr Cp = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp);
226 if (Cp.isInvalid()) {
227 warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, Cp.getKind());
228 return;
229 @@ -1637,10 +1629,8 @@ void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp,
231 /// Warn if the LSet contains the given lock.
232 void BuildLockset::warnIfMutexHeld(const NamedDecl *D, const Expr *Exp,
233 - Expr *MutexExp, til::LiteralPtr *Self,
234 - SourceLocation Loc) {
235 - CapabilityExpr Cp =
236 - Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp, Self);
237 + Expr *MutexExp) {
238 + CapabilityExpr Cp = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp);
239 if (Cp.isInvalid()) {
240 warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, Cp.getKind());
241 return;
242 @@ -1651,7 +1641,7 @@ void BuildLockset::warnIfMutexHeld(const NamedDecl *D, const Expr *Exp,
243 const FactEntry *LDat = FSet.findLock(Analyzer->FactMan, Cp);
244 if (LDat) {
245 Analyzer->Handler.handleFunExcludesLock(Cp.getKind(), D->getNameAsString(),
246 - Cp.toString(), Loc);
247 + Cp.toString(), Exp->getExprLoc());
248 }
249 }
251 @@ -1721,7 +1711,7 @@ void BuildLockset::checkAccess(const Expr *Exp, AccessKind AK,
252 }
254 for (const auto *I : D->specific_attrs<GuardedByAttr>())
255 - warnIfMutexNotHeld(D, Exp, AK, I->getArg(), POK, nullptr, Loc);
256 + warnIfMutexNotHeld(D, Exp, AK, I->getArg(), POK, Loc);
257 }
259 /// Checks pt_guarded_by and pt_guarded_var attributes.
260 @@ -1758,8 +1748,7 @@ void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK,
261 Analyzer->Handler.handleNoMutexHeld(D, PtPOK, AK, Exp->getExprLoc());
263 for (auto const *I : D->specific_attrs<PtGuardedByAttr>())
264 - warnIfMutexNotHeld(D, Exp, AK, I->getArg(), PtPOK, nullptr,
265 - Exp->getExprLoc());
266 + warnIfMutexNotHeld(D, Exp, AK, I->getArg(), PtPOK, Exp->getExprLoc());
267 }
269 /// Process a function call, method call, constructor call,
270 @@ -1772,35 +1761,21 @@ void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK,
271 /// and check that the appropriate locks are held. Non-const method calls with
272 /// the same signature as const method calls can be also treated as reads.
273 ///
274 -/// \param Exp The call expression.
275 -/// \param D The callee declaration.
276 -/// \param Self If \p Exp = nullptr, the implicit this argument.
277 -/// \param Loc If \p Exp = nullptr, the location.
278 void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
279 - til::LiteralPtr *Self, SourceLocation Loc) {
280 + VarDecl *VD) {
281 + SourceLocation Loc = Exp->getExprLoc();
282 CapExprSet ExclusiveLocksToAdd, SharedLocksToAdd;
283 CapExprSet ExclusiveLocksToRemove, SharedLocksToRemove, GenericLocksToRemove;
284 CapExprSet ScopedReqsAndExcludes;
286 // Figure out if we're constructing an object of scoped lockable class
287 - CapabilityExpr Scp;
288 - if (Exp) {
289 - assert(!Self);
290 - const auto *TagT = Exp->getType()->getAs<TagType>();
291 - if (TagT && Exp->isPRValue()) {
292 - std::pair<til::LiteralPtr *, StringRef> Placeholder =
293 - Analyzer->SxBuilder.createThisPlaceholder(Exp);
294 - [[maybe_unused]] auto inserted =
295 - ConstructedObjects.insert({Exp, Placeholder.first});
296 - assert(inserted.second && "Are we visiting the same expression again?");
297 - if (isa<CXXConstructExpr>(Exp))
298 - Self = Placeholder.first;
299 - if (TagT->getDecl()->hasAttr<ScopedLockableAttr>())
300 - Scp = CapabilityExpr(Placeholder.first, Placeholder.second, false);
301 + bool isScopedVar = false;
302 + if (VD) {
303 + if (const auto *CD = dyn_cast<const CXXConstructorDecl>(D)) {
304 + const CXXRecordDecl* PD = CD->getParent();
305 + if (PD && PD->hasAttr<ScopedLockableAttr>())
306 + isScopedVar = true;
307 }
308 -
309 - assert(Loc.isInvalid());
310 - Loc = Exp->getExprLoc();
311 }
313 for(const Attr *At : D->attrs()) {
314 @@ -1811,7 +1786,7 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
315 const auto *A = cast<AcquireCapabilityAttr>(At);
316 Analyzer->getMutexIDs(A->isShared() ? SharedLocksToAdd
317 : ExclusiveLocksToAdd,
318 - A, Exp, D, Self);
319 + A, Exp, D, VD);
320 break;
321 }
323 @@ -1822,7 +1797,7 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
324 const auto *A = cast<AssertExclusiveLockAttr>(At);
326 CapExprSet AssertLocks;
327 - Analyzer->getMutexIDs(AssertLocks, A, Exp, D, Self);
328 + Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
329 for (const auto &AssertLock : AssertLocks)
330 Analyzer->addLock(
331 FSet, std::make_unique<LockableFactEntry>(
332 @@ -1833,7 +1808,7 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
333 const auto *A = cast<AssertSharedLockAttr>(At);
335 CapExprSet AssertLocks;
336 - Analyzer->getMutexIDs(AssertLocks, A, Exp, D, Self);
337 + Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
338 for (const auto &AssertLock : AssertLocks)
339 Analyzer->addLock(
340 FSet, std::make_unique<LockableFactEntry>(
341 @@ -1844,7 +1819,7 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
342 case attr::AssertCapability: {
343 const auto *A = cast<AssertCapabilityAttr>(At);
344 CapExprSet AssertLocks;
345 - Analyzer->getMutexIDs(AssertLocks, A, Exp, D, Self);
346 + Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
347 for (const auto &AssertLock : AssertLocks)
348 Analyzer->addLock(FSet, std::make_unique<LockableFactEntry>(
349 AssertLock,
350 @@ -1858,11 +1833,11 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
351 case attr::ReleaseCapability: {
352 const auto *A = cast<ReleaseCapabilityAttr>(At);
353 if (A->isGeneric())
354 - Analyzer->getMutexIDs(GenericLocksToRemove, A, Exp, D, Self);
355 + Analyzer->getMutexIDs(GenericLocksToRemove, A, Exp, D, VD);
356 else if (A->isShared())
357 - Analyzer->getMutexIDs(SharedLocksToRemove, A, Exp, D, Self);
358 + Analyzer->getMutexIDs(SharedLocksToRemove, A, Exp, D, VD);
359 else
360 - Analyzer->getMutexIDs(ExclusiveLocksToRemove, A, Exp, D, Self);
361 + Analyzer->getMutexIDs(ExclusiveLocksToRemove, A, Exp, D, VD);
362 break;
363 }
365 @@ -1870,10 +1845,10 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
366 const auto *A = cast<RequiresCapabilityAttr>(At);
367 for (auto *Arg : A->args()) {
368 warnIfMutexNotHeld(D, Exp, A->isShared() ? AK_Read : AK_Written, Arg,
369 - POK_FunctionCall, Self, Loc);
370 + POK_FunctionCall, Exp->getExprLoc());
371 // use for adopting a lock
372 - if (!Scp.shouldIgnore())
373 - Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, Self);
374 + if (isScopedVar)
375 + Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, VD);
376 }
377 break;
378 }
379 @@ -1881,10 +1856,10 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
380 case attr::LocksExcluded: {
381 const auto *A = cast<LocksExcludedAttr>(At);
382 for (auto *Arg : A->args()) {
383 - warnIfMutexHeld(D, Exp, Arg, Self, Loc);
384 + warnIfMutexHeld(D, Exp, Arg);
385 // use for deferring a lock
386 - if (!Scp.shouldIgnore())
387 - Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, Self);
388 + if (isScopedVar)
389 + Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, VD);
390 }
391 break;
392 }
393 @@ -1907,7 +1882,7 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
395 // Add locks.
396 FactEntry::SourceKind Source =
397 - !Scp.shouldIgnore() ? FactEntry::Managed : FactEntry::Acquired;
398 + isScopedVar ? FactEntry::Managed : FactEntry::Acquired;
399 for (const auto &M : ExclusiveLocksToAdd)
400 Analyzer->addLock(FSet, std::make_unique<LockableFactEntry>(M, LK_Exclusive,
401 Loc, Source));
402 @@ -1915,9 +1890,15 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
403 Analyzer->addLock(
404 FSet, std::make_unique<LockableFactEntry>(M, LK_Shared, Loc, Source));
406 - if (!Scp.shouldIgnore()) {
407 + if (isScopedVar) {
408 // Add the managing object as a dummy mutex, mapped to the underlying mutex.
409 - auto ScopedEntry = std::make_unique<ScopedLockableFactEntry>(Scp, Loc);
410 + SourceLocation MLoc = VD->getLocation();
411 + DeclRefExpr DRE(VD->getASTContext(), VD, false, VD->getType(), VK_LValue,
412 + VD->getLocation());
413 + // FIXME: does this store a pointer to DRE?
414 + CapabilityExpr Scp = Analyzer->SxBuilder.translateAttrExpr(&DRE, nullptr);
415 +
416 + auto ScopedEntry = std::make_unique<ScopedLockableFactEntry>(Scp, MLoc);
417 for (const auto &M : ExclusiveLocksToAdd)
418 ScopedEntry->addLock(M);
419 for (const auto &M : SharedLocksToAdd)
420 @@ -2077,11 +2058,36 @@ void BuildLockset::VisitCXXConstructExpr(const CXXConstructExpr *Exp) {
421 } else {
422 examineArguments(D, Exp->arg_begin(), Exp->arg_end());
423 }
424 - if (D && D->hasAttrs())
425 - handleCall(Exp, D);
426 }
428 -static const Expr *UnpackConstruction(const Expr *E) {
429 +static CXXConstructorDecl *
430 +findConstructorForByValueReturn(const CXXRecordDecl *RD) {
431 + // Prefer a move constructor over a copy constructor. If there's more than
432 + // one copy constructor or more than one move constructor, we arbitrarily
433 + // pick the first declared such constructor rather than trying to guess which
434 + // one is more appropriate.
435 + CXXConstructorDecl *CopyCtor = nullptr;
436 + for (auto *Ctor : RD->ctors()) {
437 + if (Ctor->isDeleted())
438 + continue;
439 + if (Ctor->isMoveConstructor())
440 + return Ctor;
441 + if (!CopyCtor && Ctor->isCopyConstructor())
442 + CopyCtor = Ctor;
443 + }
444 + return CopyCtor;
445 +}
446 +
447 +static Expr *buildFakeCtorCall(CXXConstructorDecl *CD, ArrayRef<Expr *> Args,
448 + SourceLocation Loc) {
449 + ASTContext &Ctx = CD->getASTContext();
450 + return CXXConstructExpr::Create(Ctx, Ctx.getRecordType(CD->getParent()), Loc,
451 + CD, true, Args, false, false, false, false,
452 + CXXConstructExpr::CK_Complete,
453 + SourceRange(Loc, Loc));
454 +}
455 +
456 +static Expr *UnpackConstruction(Expr *E) {
457 if (auto *CE = dyn_cast<CastExpr>(E))
458 if (CE->getCastKind() == CK_NoOp)
459 E = CE->getSubExpr()->IgnoreParens();
460 @@ -2100,7 +2106,7 @@ void BuildLockset::VisitDeclStmt(const DeclStmt *S) {
462 for (auto *D : S->getDeclGroup()) {
463 if (auto *VD = dyn_cast_or_null<VarDecl>(D)) {
464 - const Expr *E = VD->getInit();
465 + Expr *E = VD->getInit();
466 if (!E)
467 continue;
468 E = E->IgnoreParens();
469 @@ -2110,27 +2116,29 @@ void BuildLockset::VisitDeclStmt(const DeclStmt *S) {
470 E = EWC->getSubExpr()->IgnoreParens();
471 E = UnpackConstruction(E);
473 - if (auto Object = ConstructedObjects.find(E);
474 - Object != ConstructedObjects.end()) {
475 - Object->second->setClangDecl(VD);
476 - ConstructedObjects.erase(Object);
477 + if (const auto *CE = dyn_cast<CXXConstructExpr>(E)) {
478 + const auto *CtorD = dyn_cast_or_null<NamedDecl>(CE->getConstructor());
479 + if (!CtorD || !CtorD->hasAttrs())
480 + continue;
481 + handleCall(E, CtorD, VD);
482 + } else if (isa<CallExpr>(E) && E->isPRValue()) {
483 + // If the object is initialized by a function call that returns a
484 + // scoped lockable by value, use the attributes on the copy or move
485 + // constructor to figure out what effect that should have on the
486 + // lockset.
487 + // FIXME: Is this really the best way to handle this situation?
488 + auto *RD = E->getType()->getAsCXXRecordDecl();
489 + if (!RD || !RD->hasAttr<ScopedLockableAttr>())
490 + continue;
491 + CXXConstructorDecl *CtorD = findConstructorForByValueReturn(RD);
492 + if (!CtorD || !CtorD->hasAttrs())
493 + continue;
494 + handleCall(buildFakeCtorCall(CtorD, {E}, E->getBeginLoc()), CtorD, VD);
495 }
496 }
497 }
498 }
500 -void BuildLockset::VisitMaterializeTemporaryExpr(
501 - const MaterializeTemporaryExpr *Exp) {
502 - if (const ValueDecl *ExtD = Exp->getExtendingDecl()) {
503 - if (auto Object =
504 - ConstructedObjects.find(UnpackConstruction(Exp->getSubExpr()));
505 - Object != ConstructedObjects.end()) {
506 - Object->second->setClangDecl(ExtD);
507 - ConstructedObjects.erase(Object);
508 - }
509 - }
510 -}
511 -
512 /// Given two facts merging on a join point, possibly warn and decide whether to
513 /// keep or replace.
514 ///
515 @@ -2403,33 +2411,19 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
516 LocksetBuilder.Visit(CS.getStmt());
517 break;
518 }
519 - // Ignore BaseDtor and MemberDtor for now.
520 + // Ignore BaseDtor, MemberDtor, and TemporaryDtor for now.
521 case CFGElement::AutomaticObjectDtor: {
522 CFGAutomaticObjDtor AD = BI.castAs<CFGAutomaticObjDtor>();
523 const auto *DD = AD.getDestructorDecl(AC.getASTContext());
524 if (!DD->hasAttrs())
525 break;
527 - LocksetBuilder.handleCall(nullptr, DD,
528 - SxBuilder.createVariable(AD.getVarDecl()),
529 - AD.getTriggerStmt()->getEndLoc());
530 - break;
531 - }
532 - case CFGElement::TemporaryDtor: {
533 - auto TD = BI.castAs<CFGTemporaryDtor>();
534 -
535 - // Clean up constructed object even if there are no attributes to
536 - // keep the number of objects in limbo as small as possible.
537 - if (auto Object = LocksetBuilder.ConstructedObjects.find(
538 - TD.getBindTemporaryExpr()->getSubExpr());
539 - Object != LocksetBuilder.ConstructedObjects.end()) {
540 - const auto *DD = TD.getDestructorDecl(AC.getASTContext());
541 - if (DD->hasAttrs())
542 - // TODO: the location here isn't quite correct.
543 - LocksetBuilder.handleCall(nullptr, DD, Object->second,
544 - TD.getBindTemporaryExpr()->getEndLoc());
545 - LocksetBuilder.ConstructedObjects.erase(Object);
546 - }
547 + // Create a dummy expression,
548 + auto *VD = const_cast<VarDecl *>(AD.getVarDecl());
549 + DeclRefExpr DRE(VD->getASTContext(), VD, false,
550 + VD->getType().getNonReferenceType(), VK_LValue,
551 + AD.getTriggerStmt()->getEndLoc());
552 + LocksetBuilder.handleCall(&DRE, DD);
553 break;
554 }
555 default:
556 diff --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp b/clang/lib/Analysis/ThreadSafetyCommon.cpp
557 index a771149f1591..06b61b4de92f 100644
558 --- a/clang/lib/Analysis/ThreadSafetyCommon.cpp
559 +++ b/clang/lib/Analysis/ThreadSafetyCommon.cpp
560 @@ -115,22 +115,19 @@ static StringRef ClassifyDiagnostic(QualType VDT) {
561 /// \param D The declaration to which the attribute is attached.
562 /// \param DeclExp An expression involving the Decl to which the attribute
563 /// is attached. E.g. the call to a function.
564 -/// \param Self S-expression to substitute for a \ref CXXThisExpr.
565 CapabilityExpr SExprBuilder::translateAttrExpr(const Expr *AttrExp,
566 const NamedDecl *D,
567 const Expr *DeclExp,
568 - til::SExpr *Self) {
569 + VarDecl *SelfDecl) {
570 // If we are processing a raw attribute expression, with no substitutions.
571 - if (!DeclExp && !Self)
572 + if (!DeclExp)
573 return translateAttrExpr(AttrExp, nullptr);
575 CallingContext Ctx(nullptr, D);
577 // Examine DeclExp to find SelfArg and FunArgs, which are used to substitute
578 // for formal parameters when we call buildMutexID later.
579 - if (!DeclExp)
580 - /* We'll use Self. */;
581 - else if (const auto *ME = dyn_cast<MemberExpr>(DeclExp)) {
582 + if (const auto *ME = dyn_cast<MemberExpr>(DeclExp)) {
583 Ctx.SelfArg = ME->getBase();
584 Ctx.SelfArrow = ME->isArrow();
585 } else if (const auto *CE = dyn_cast<CXXMemberCallExpr>(DeclExp)) {
586 @@ -145,24 +142,29 @@ CapabilityExpr SExprBuilder::translateAttrExpr(const Expr *AttrExp,
587 Ctx.SelfArg = nullptr; // Will be set below
588 Ctx.NumArgs = CE->getNumArgs();
589 Ctx.FunArgs = CE->getArgs();
590 + } else if (D && isa<CXXDestructorDecl>(D)) {
591 + // There's no such thing as a "destructor call" in the AST.
592 + Ctx.SelfArg = DeclExp;
593 }
595 - if (Self) {
596 - assert(!Ctx.SelfArg && "Ambiguous self argument");
597 - Ctx.SelfArg = Self;
598 + // Hack to handle constructors, where self cannot be recovered from
599 + // the expression.
600 + if (SelfDecl && !Ctx.SelfArg) {
601 + DeclRefExpr SelfDRE(SelfDecl->getASTContext(), SelfDecl, false,
602 + SelfDecl->getType(), VK_LValue,
603 + SelfDecl->getLocation());
604 + Ctx.SelfArg = &SelfDRE;
606 // If the attribute has no arguments, then assume the argument is "this".
607 if (!AttrExp)
608 - return CapabilityExpr(
609 - Self, ClassifyDiagnostic(cast<CXXMethodDecl>(D)->getThisObjectType()),
610 - false);
611 + return translateAttrExpr(Ctx.SelfArg, nullptr);
612 else // For most attributes.
613 return translateAttrExpr(AttrExp, &Ctx);
614 }
616 // If the attribute has no arguments, then assume the argument is "this".
617 if (!AttrExp)
618 - return translateAttrExpr(cast<const Expr *>(Ctx.SelfArg), nullptr);
619 + return translateAttrExpr(Ctx.SelfArg, nullptr);
620 else // For most attributes.
621 return translateAttrExpr(AttrExp, &Ctx);
622 }
623 @@ -216,16 +218,6 @@ CapabilityExpr SExprBuilder::translateAttrExpr(const Expr *AttrExp,
624 return CapabilityExpr(E, Kind, Neg);
625 }
627 -til::LiteralPtr *SExprBuilder::createVariable(const VarDecl *VD) {
628 - return new (Arena) til::LiteralPtr(VD);
629 -}
630 -
631 -std::pair<til::LiteralPtr *, StringRef>
632 -SExprBuilder::createThisPlaceholder(const Expr *Exp) {
633 - return {new (Arena) til::LiteralPtr(nullptr),
634 - ClassifyDiagnostic(Exp->getType())};
635 -}
636 -
637 // Translate a clang statement or expression to a TIL expression.
638 // Also performs substitution of variables; Ctx provides the context.
639 // Dispatches on the type of S.
640 @@ -335,12 +327,8 @@ til::SExpr *SExprBuilder::translateDeclRefExpr(const DeclRefExpr *DRE,
641 til::SExpr *SExprBuilder::translateCXXThisExpr(const CXXThisExpr *TE,
642 CallingContext *Ctx) {
643 // Substitute for 'this'
644 - if (Ctx && Ctx->SelfArg) {
645 - if (const auto *SelfArg = dyn_cast<const Expr *>(Ctx->SelfArg))
646 - return translate(SelfArg, Ctx->Prev);
647 - else
648 - return cast<til::SExpr *>(Ctx->SelfArg);
649 - }
650 + if (Ctx && Ctx->SelfArg)
651 + return translate(Ctx->SelfArg, Ctx->Prev);
652 assert(SelfVar && "We have no variable for 'this'!");
653 return SelfVar;
654 }
655 diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
656 index 8e312e589d81..e1cfa1f3fd17 100644
657 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
658 +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
659 @@ -1606,30 +1606,6 @@ namespace substitution_test {
660 dlr.unlockData(d1);
661 }
662 };
663 -
664 - // Automatic object destructor calls don't appear as expressions in the CFG,
665 - // so we have to handle them separately whenever substitutions are required.
666 - struct DestructorRequires {
667 - Mutex mu;
668 - ~DestructorRequires() EXCLUSIVE_LOCKS_REQUIRED(mu);
669 - };
670 -
671 - void destructorRequires() {
672 - DestructorRequires rd;
673 - rd.mu.AssertHeld();
674 - }
675 -
676 - struct DestructorExcludes {
677 - Mutex mu;
678 - ~DestructorExcludes() LOCKS_EXCLUDED(mu);
679 - };
680 -
681 - void destructorExcludes() {
682 - DestructorExcludes ed;
683 - ed.mu.Lock(); // expected-note {{mutex acquired here}}
684 - } // expected-warning {{cannot call function '~DestructorExcludes' while mutex 'ed.mu' is held}}
685 - // expected-warning@-1 {{mutex 'ed.mu' is still held at the end of function}}
686 -
687 } // end namespace substituation_test
690 @@ -1714,15 +1690,6 @@ struct TestScopedLockable {
691 }
692 #endif
694 - void temporary() {
695 - MutexLock{&mu1}, a = 5;
696 - }
697 -
698 - void lifetime_extension() {
699 - const MutexLock &mulock = MutexLock(&mu1);
700 - a = 5;
701 - }
702 -
703 void foo2() {
704 ReaderMutexLock mulock1(&mu1);
705 if (getBool()) {
706 @@ -1741,12 +1708,6 @@ struct TestScopedLockable {
707 // expected-warning {{acquiring mutex 'mu1' that is already held}}
708 }
710 - void temporary_double_lock() {
711 - MutexLock mulock_a(&mu1); // expected-note{{mutex acquired here}}
712 - MutexLock{&mu1}; // \
713 - // expected-warning {{acquiring mutex 'mu1' that is already held}}
714 - }
715 -
716 void foo4() {
717 MutexLock mulock1(&mu1), mulock2(&mu2);
718 a = b+1;
719 @@ -4226,20 +4187,6 @@ public:
720 void foo() EXCLUSIVE_LOCKS_REQUIRED(this);
721 };
723 -class SelfLockDeferred {
724 -public:
725 - SelfLockDeferred() LOCKS_EXCLUDED(mu_);
726 - ~SelfLockDeferred() UNLOCK_FUNCTION(mu_);
727 -
728 - Mutex mu_;
729 -};
730 -
731 -class LOCKABLE SelfLockDeferred2 {
732 -public:
733 - SelfLockDeferred2() LOCKS_EXCLUDED(this);
734 - ~SelfLockDeferred2() UNLOCK_FUNCTION();
735 -};
736 -
738 void test() {
739 SelfLock s;
740 @@ -4251,14 +4198,6 @@ void test2() {
741 s2.foo();
742 }
744 -void testDeferredTemporary() {
745 - SelfLockDeferred(); // expected-warning {{releasing mutex '<temporary>.mu_' that was not held}}
746 -}
747 -
748 -void testDeferredTemporary2() {
749 - SelfLockDeferred2(); // expected-warning {{releasing mutex '<temporary>' that was not held}}
750 -}
751 -
752 } // end namespace SelfConstructorTest
755 @@ -5953,75 +5892,47 @@ C c;
756 void f() { c[A()]->g(); }
757 } // namespace PR34800
759 -#ifdef __cpp_guaranteed_copy_elision
760 -
761 namespace ReturnScopedLockable {
762 + template<typename Object> class SCOPED_LOCKABLE ReadLockedPtr {
763 + public:
764 + ReadLockedPtr(Object *ptr) SHARED_LOCK_FUNCTION((*this)->mutex);
765 + ReadLockedPtr(ReadLockedPtr &&) SHARED_LOCK_FUNCTION((*this)->mutex);
766 + ~ReadLockedPtr() UNLOCK_FUNCTION();
768 -class Object {
769 -public:
770 - MutexLock lock() EXCLUSIVE_LOCK_FUNCTION(mutex) {
771 - // TODO: False positive because scoped lock isn't destructed.
772 - return MutexLock(&mutex); // expected-note {{mutex acquired here}}
773 - } // expected-warning {{mutex 'mutex' is still held at the end of function}}
774 -
775 - ReaderMutexLock lockShared() SHARED_LOCK_FUNCTION(mutex) {
776 - // TODO: False positive because scoped lock isn't destructed.
777 - return ReaderMutexLock(&mutex); // expected-note {{mutex acquired here}}
778 - } // expected-warning {{mutex 'mutex' is still held at the end of function}}
779 -
780 - MutexLock adopt() EXCLUSIVE_LOCKS_REQUIRED(mutex) {
781 - // TODO: False positive because scoped lock isn't destructed.
782 - return MutexLock(&mutex, true); // expected-note {{mutex acquired here}}
783 - } // expected-warning {{mutex 'mutex' is still held at the end of function}}
784 + Object *operator->() const { return object; }
786 - ReaderMutexLock adoptShared() SHARED_LOCKS_REQUIRED(mutex) {
787 - // TODO: False positive because scoped lock isn't destructed.
788 - return ReaderMutexLock(&mutex, true); // expected-note {{mutex acquired here}}
789 - } // expected-warning {{mutex 'mutex' is still held at the end of function}}
790 + private:
791 + Object *object;
792 + };
794 - int x GUARDED_BY(mutex);
795 - void needsLock() EXCLUSIVE_LOCKS_REQUIRED(mutex);
796 + struct Object {
797 + int f() SHARED_LOCKS_REQUIRED(mutex);
798 + Mutex mutex;
799 + };
801 - void testInside() {
802 - MutexLock scope = lock();
803 - x = 1;
804 - needsLock();
805 + ReadLockedPtr<Object> get();
806 + int use() {
807 + auto ptr = get();
808 + return ptr->f();
809 + }
810 + void use_constructor() {
811 + auto ptr = ReadLockedPtr<Object>(nullptr);
812 + ptr->f();
813 + auto ptr2 = ReadLockedPtr<Object>{nullptr};
814 + ptr2->f();
815 + auto ptr3 = (ReadLockedPtr<Object>{nullptr});
816 + ptr3->f();
817 + }
818 + struct Convertible {
819 + Convertible();
820 + operator ReadLockedPtr<Object>();
821 + };
822 + void use_conversion() {
823 + ReadLockedPtr<Object> ptr = Convertible();
824 + ptr->f();
825 }
826 -
827 - Mutex mutex;
828 -};
829 -
830 -Object obj;
831 -
832 -void testLock() {
833 - MutexLock scope = obj.lock();
834 - obj.x = 1;
835 - obj.needsLock();
836 }
838 -int testSharedLock() {
839 - ReaderMutexLock scope = obj.lockShared();
840 - obj.x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'obj.mutex' exclusively}}
841 - return obj.x;
842 -}
843 -
844 -void testAdopt() {
845 - obj.mutex.Lock();
846 - MutexLock scope = obj.adopt();
847 - obj.x = 1;
848 -}
849 -
850 -int testAdoptShared() {
851 - obj.mutex.Lock();
852 - ReaderMutexLock scope = obj.adoptShared();
853 - obj.x = 1;
854 - return obj.x;
855 -}
856 -
857 -} // namespace ReturnScopedLockable
858 -
859 -#endif
860 -
861 namespace PR38640 {
862 void f() {
863 // Self-referencing assignment previously caused an infinite loop when thread
864 --
865 2.37.1.1.g659da70093