Author: rsmith Date: Thu Jan 11 14:13:57 2018 New Revision: 322316 URL: http://llvm.org/viewvc/llvm-project?rev=322316&view=rev Log: Handle scoped_lockable objects being returned by value in C++17.
In C++17, guaranteed copy elision means that there isn't necessarily a constructor call when a local variable is initialized by a function call that returns a scoped_lockable by value. In order to model the effects of initializing a local variable with a function call returning a scoped_lockable, pretend that the move constructor was invoked within the caller at the point of return. Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=322316&r1=322315&r2=322316&view=diff ============================================================================== --- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original) +++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Thu Jan 11 14:13:57 2018 @@ -1689,7 +1689,7 @@ void BuildLockset::handleCall(Expr *Exp, 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<const CXXConstructorDecl>(D)) { @@ -1991,6 +1991,33 @@ void BuildLockset::VisitCXXConstructExpr // 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<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)); +} + void BuildLockset::VisitDeclStmt(DeclStmt *S) { // adjust the context LVarCtx = Analyzer->LocalVarMap.getNextContext(CtxIndex, S, LVarCtx); @@ -1998,15 +2025,34 @@ void BuildLockset::VisitDeclStmt(DeclStm for (auto *D : S->getDeclGroup()) { if (VarDecl *VD = dyn_cast_or_null<VarDecl>(D)) { Expr *E = VD->getInit(); + if (!E) + continue; + E = E->IgnoreParens(); + // handle constructors that involve temporaries - if (ExprWithCleanups *EWC = dyn_cast_or_null<ExprWithCleanups>(E)) + if (auto *EWC = dyn_cast<ExprWithCleanups>(E)) E = EWC->getSubExpr(); + if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E)) + E = BTE->getSubExpr(); - if (CXXConstructExpr *CE = dyn_cast_or_null<CXXConstructExpr>(E)) { + if (CXXConstructExpr *CE = dyn_cast<CXXConstructExpr>(E)) { NamedDecl *CtorD = dyn_cast_or_null<NamedDecl>(CE->getConstructor()); if (!CtorD || !CtorD->hasAttrs()) - return; - handleCall(CE, CtorD, VD); + continue; + handleCall(E, CtorD, VD); + } else if (isa<CallExpr>(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<ScopedLockableAttr>()) + continue; + CXXConstructorDecl *CtorD = findConstructorForByValueReturn(RD); + if (!CtorD || !CtorD->hasAttrs()) + continue; + handleCall(buildFakeCtorCall(CtorD, {E}, E->getLocStart()), CtorD, VD); } } } Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=322316&r1=322315&r2=322316&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original) +++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Thu Jan 11 14:13:57 2018 @@ -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 @@ struct C { C c; void f() { c[A()]->g(); } } // namespace PR34800 + +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(); + + Object *operator->() const { return object; } + + private: + Object *object; + }; + + struct Object { + int f() SHARED_LOCKS_REQUIRED(mutex); + Mutex mutex; + }; + + ReadLockedPtr<Object> get(); + int use() { + auto ptr = get(); + return ptr->f(); + } +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits