Index: lib/Analysis/ThreadSafety.cpp =================================================================== --- lib/Analysis/ThreadSafety.cpp +++ lib/Analysis/ThreadSafety.cpp @@ -1689,7 +1689,7 @@ CapExprSet ScopedExclusiveReqs, ScopedSharedReqs; StringRef CapDiagKind = "mutex"; - // Figure out if we're calling the constructor of scoped lockable class + // Figure out if we're constructing an object of scoped lockable class bool isScopedVar = false; if (VD) { if (const CXXConstructorDecl *CD = dyn_cast(D)) { @@ -1991,6 +1991,33 @@ // FIXME -- only handles constructors in DeclStmt below. } +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 (CXXConstructorDecl *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 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)); +} + void BuildLockset::VisitDeclStmt(DeclStmt *S) { // adjust the context LVarCtx = Analyzer->LocalVarMap.getNextContext(CtxIndex, S, LVarCtx); @@ -1998,15 +2025,34 @@ for (auto *D : S->getDeclGroup()) { if (VarDecl *VD = dyn_cast_or_null(D)) { Expr *E = VD->getInit(); + if (!E) + continue; + E = E->IgnoreParens(); + // handle constructors that involve temporaries - if (ExprWithCleanups *EWC = dyn_cast_or_null(E)) + if (auto *EWC = dyn_cast(E)) E = EWC->getSubExpr(); + if (auto *BTE = dyn_cast(E)) + E = BTE->getSubExpr(); - if (CXXConstructExpr *CE = dyn_cast_or_null(E)) { + if (CXXConstructExpr *CE = dyn_cast(E)) { NamedDecl *CtorD = dyn_cast_or_null(CE->getConstructor()); if (!CtorD || !CtorD->hasAttrs()) - return; - handleCall(CE, CtorD, VD); + continue; + handleCall(E, CtorD, VD); + } else if (isa(E) && E->isRValue()) { + // 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()) + continue; + CXXConstructorDecl *CtorD = findConstructorForByValueReturn(RD); + if (!CtorD || !CtorD->hasAttrs()) + continue; + handleCall(buildFakeCtorCall(CtorD, {E}, E->getLocStart()), CtorD, VD); } } } Index: test/SemaCXX/warn-thread-safety-analysis.cpp =================================================================== --- test/SemaCXX/warn-thread-safety-analysis.cpp +++ test/SemaCXX/warn-thread-safety-analysis.cpp @@ -1,5 +1,6 @@ // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=0 %s // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=1 %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=0 %s // FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 -Wc++98-compat %s // FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety %s @@ -5248,3 +5249,28 @@ C c; void f() { c[A()]->g(); } } // namespace PR34800 + +namespace ReturnScopedLockable { + template class SCOPED_LOCKABLE ReadLockedPtr { + public: + ReadLockedPtr(Object *ptr) SHARED_LOCK_FUNCTION((*this)->mutex); + ReadLockedPtr(ReadLockedPtr &&) SHARED_LOCK_FUNCTION((*this)->mutex); + ~ReadLockedPtr() UNLOCK_FUNCTION(); + + Object *operator->() const { return object; } + + private: + Object *object; + }; + + struct Object { + int f() SHARED_LOCKS_REQUIRED(mutex); + Mutex mutex; + }; + + ReadLockedPtr get(); + int use() { + auto ptr = get(); + return ptr->f(); + } +}