caio/clang-p2996
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Handle scoped_lockable objects being returned by value in C++17.

Browse Source 
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.

llvm-svn: 322316
This commit is contained in:
Richard Smith
2018-01-11 22:13:57 +00:00
parent c43b7e61a2
commit e97654b2f2
2 changed files with 79 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

60
clang/lib/Analysis/ThreadSafety.cpp
View File

@@ -1689,7 +1689,7 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
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(CXXConstructExpr *Exp) {
// 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(DeclStmt *S) {
for (auto *D : S->getDeclGroup()) {
if (VarDecl *VD = dyn_cast_or_null<VarDecl>(D)) {
Expr *E = VD->getInit();
// handle constructors that involve temporaries
if (ExprWithCleanups *EWC = dyn_cast_or_null<ExprWithCleanups>(E))
E = EWC->getSubExpr();
if (!E)
continue;
E = E->IgnoreParens();
if (CXXConstructExpr *CE = dyn_cast_or_null<CXXConstructExpr>(E)) {
// handle constructors that involve temporaries
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<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);
}
}
}

26
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
View File

@@ -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();
}
}