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,
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()))
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;
}
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;
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;
void warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, AccessKind AK,
Expr *MutexExp, ProtectedOperationKind POK,
SourceLocation Loc);
- void warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, Expr *MutexExp,
- 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,
void VisitCallExpr(const CallExpr *Exp);
void VisitCXXConstructExpr(const CXXConstructExpr *Exp);
void VisitDeclStmt(const DeclStmt *S);
- void VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *Exp);
};
} // namespace
/// Warn if the LSet contains the given lock.
void BuildLockset::warnIfMutexHeld(const NamedDecl *D, const Expr *Exp,
- Expr *MutexExp, SourceLocation Loc) {
+ Expr *MutexExp) {
CapabilityExpr Cp = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp);
if (Cp.isInvalid()) {
warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, Cp.getKind());
const FactEntry *LDat = FSet.findLock(Analyzer->FactMan, Cp);
if (LDat) {
Analyzer->Handler.handleFunExcludesLock(Cp.getKind(), D->getNameAsString(),
- Cp.toString(), Loc);
+ Cp.toString(), Exp->getExprLoc());
}
}
/// 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);
- auto inserted = ConstructedObjects.insert({Exp, Placeholder.first});
- (void)inserted;
- 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()) {
const auto *A = cast<AcquireCapabilityAttr>(At);
Analyzer->getMutexIDs(A->isShared() ? SharedLocksToAdd
: ExclusiveLocksToAdd,
- A, Exp, D, Self);
+ A, Exp, D, VD);
break;
}
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>(
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>(
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,
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;
}
const auto *A = cast<RequiresCapabilityAttr>(At);
for (auto *Arg : A->args()) {
warnIfMutexNotHeld(D, Exp, A->isShared() ? AK_Read : AK_Written, Arg,
- POK_FunctionCall, 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;
}
case attr::LocksExcluded: {
const auto *A = cast<LocksExcludedAttr>(At);
for (auto *Arg : A->args()) {
- warnIfMutexHeld(D, Exp, Arg, 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;
}
// 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));
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)
} 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();
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();
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.
///
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:
/// \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)) {
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);
}
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.
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;
}