diff options
Diffstat (limited to 'lib/Analysis/ThreadSafety.cpp')
| -rw-r--r-- | lib/Analysis/ThreadSafety.cpp | 1436 |
1 files changed, 493 insertions, 943 deletions
diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp index 11df61f80fa0..a986c587f869 100644 --- a/lib/Analysis/ThreadSafety.cpp +++ b/lib/Analysis/ThreadSafety.cpp @@ -22,10 +22,10 @@ #include "clang/AST/StmtVisitor.h" #include "clang/Analysis/Analyses/PostOrderCFGView.h" #include "clang/Analysis/Analyses/ThreadSafety.h" +#include "clang/Analysis/Analyses/ThreadSafetyCommon.h" #include "clang/Analysis/Analyses/ThreadSafetyLogical.h" #include "clang/Analysis/Analyses/ThreadSafetyTIL.h" #include "clang/Analysis/Analyses/ThreadSafetyTraverse.h" -#include "clang/Analysis/Analyses/ThreadSafetyCommon.h" #include "clang/Analysis/AnalysisContext.h" #include "clang/Analysis/CFG.h" #include "clang/Analysis/CFGStmtMap.h" @@ -40,762 +40,111 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/raw_ostream.h" #include <algorithm> +#include <ostream> +#include <sstream> #include <utility> #include <vector> -using namespace clang; -using namespace thread_safety; + +namespace clang { +namespace threadSafety { // Key method definition ThreadSafetyHandler::~ThreadSafetyHandler() {} -namespace { - -/// SExpr implements a simple expression language that is used to store, -/// compare, and pretty-print C++ expressions. Unlike a clang Expr, a SExpr -/// does not capture surface syntax, and it does not distinguish between -/// C++ concepts, like pointers and references, that have no real semantic -/// differences. This simplicity allows SExprs to be meaningfully compared, -/// e.g. -/// (x) = x -/// (*this).foo = this->foo -/// *&a = a -/// -/// Thread-safety analysis works by comparing lock expressions. Within the -/// body of a function, an expression such as "x->foo->bar.mu" will resolve to -/// a particular mutex object at run-time. Subsequent occurrences of the same -/// expression (where "same" means syntactic equality) will refer to the same -/// run-time object if three conditions hold: -/// (1) Local variables in the expression, such as "x" have not changed. -/// (2) Values on the heap that affect the expression have not changed. -/// (3) The expression involves only pure function calls. -/// -/// The current implementation assumes, but does not verify, that multiple uses -/// of the same lock expression satisfies these criteria. -class SExpr { -private: - enum ExprOp { - EOP_Nop, ///< No-op - EOP_Wildcard, ///< Matches anything. - EOP_Universal, ///< Universal lock. - EOP_This, ///< This keyword. - EOP_NVar, ///< Named variable. - EOP_LVar, ///< Local variable. - EOP_Dot, ///< Field access - EOP_Call, ///< Function call - EOP_MCall, ///< Method call - EOP_Index, ///< Array index - EOP_Unary, ///< Unary operation - EOP_Binary, ///< Binary operation - EOP_Unknown ///< Catchall for everything else - }; - - - class SExprNode { - private: - unsigned char Op; ///< Opcode of the root node - unsigned char Flags; ///< Additional opcode-specific data - unsigned short Sz; ///< Number of child nodes - const void* Data; ///< Additional opcode-specific data - - public: - SExprNode(ExprOp O, unsigned F, const void* D) - : Op(static_cast<unsigned char>(O)), - Flags(static_cast<unsigned char>(F)), Sz(1), Data(D) - { } - - unsigned size() const { return Sz; } - void setSize(unsigned S) { Sz = S; } - - ExprOp kind() const { return static_cast<ExprOp>(Op); } - - const NamedDecl* getNamedDecl() const { - assert(Op == EOP_NVar || Op == EOP_LVar || Op == EOP_Dot); - return reinterpret_cast<const NamedDecl*>(Data); - } - - const NamedDecl* getFunctionDecl() const { - assert(Op == EOP_Call || Op == EOP_MCall); - return reinterpret_cast<const NamedDecl*>(Data); - } - - bool isArrow() const { return Op == EOP_Dot && Flags == 1; } - void setArrow(bool A) { Flags = A ? 1 : 0; } - - unsigned arity() const { - switch (Op) { - case EOP_Nop: return 0; - case EOP_Wildcard: return 0; - case EOP_Universal: return 0; - case EOP_NVar: return 0; - case EOP_LVar: return 0; - case EOP_This: return 0; - case EOP_Dot: return 1; - case EOP_Call: return Flags+1; // First arg is function. - case EOP_MCall: return Flags+1; // First arg is implicit obj. - case EOP_Index: return 2; - case EOP_Unary: return 1; - case EOP_Binary: return 2; - case EOP_Unknown: return Flags; - } - return 0; - } - - bool operator==(const SExprNode& Other) const { - // Ignore flags and size -- they don't matter. - return (Op == Other.Op && - Data == Other.Data); - } - - bool operator!=(const SExprNode& Other) const { - return !(*this == Other); - } - - bool matches(const SExprNode& Other) const { - return (*this == Other) || - (Op == EOP_Wildcard) || - (Other.Op == EOP_Wildcard); - } - }; - - - /// \brief Encapsulates the lexical context of a function call. The lexical - /// context includes the arguments to the call, including the implicit object - /// argument. When an attribute containing a mutex expression is attached to - /// a method, the expression may refer to formal parameters of the method. - /// Actual arguments must be substituted for formal parameters to derive - /// the appropriate mutex expression in the lexical context where the function - /// is called. PrevCtx holds the context in which the arguments themselves - /// should be evaluated; multiple calling contexts can be chained together - /// by the lock_returned attribute. - struct CallingContext { - const NamedDecl* AttrDecl; // The decl to which the attribute is attached. - const Expr* SelfArg; // Implicit object argument -- e.g. 'this' - bool SelfArrow; // is Self referred to with -> or .? - unsigned NumArgs; // Number of funArgs - const Expr* const* FunArgs; // Function arguments - CallingContext* PrevCtx; // The previous context; or 0 if none. - - CallingContext(const NamedDecl *D) - : AttrDecl(D), SelfArg(nullptr), SelfArrow(false), NumArgs(0), - FunArgs(nullptr), PrevCtx(nullptr) {} - }; - - typedef SmallVector<SExprNode, 4> NodeVector; - -private: - // A SExpr is a list of SExprNodes in prefix order. The Size field allows - // the list to be traversed as a tree. - NodeVector NodeVec; - -private: - unsigned make(ExprOp O, unsigned F = 0, const void *D = nullptr) { - NodeVec.push_back(SExprNode(O, F, D)); - return NodeVec.size() - 1; - } - - unsigned makeNop() { - return make(EOP_Nop); - } - - unsigned makeWildcard() { - return make(EOP_Wildcard); - } - - unsigned makeUniversal() { - return make(EOP_Universal); - } - - unsigned makeNamedVar(const NamedDecl *D) { - return make(EOP_NVar, 0, D); - } - - unsigned makeLocalVar(const NamedDecl *D) { - return make(EOP_LVar, 0, D); - } - - unsigned makeThis() { - return make(EOP_This); - } - - unsigned makeDot(const NamedDecl *D, bool Arrow) { - return make(EOP_Dot, Arrow ? 1 : 0, D); - } - - unsigned makeCall(unsigned NumArgs, const NamedDecl *D) { - return make(EOP_Call, NumArgs, D); - } - - // Grab the very first declaration of virtual method D - const CXXMethodDecl* getFirstVirtualDecl(const CXXMethodDecl *D) { - while (true) { - D = D->getCanonicalDecl(); - CXXMethodDecl::method_iterator I = D->begin_overridden_methods(), - E = D->end_overridden_methods(); - if (I == E) - return D; // Method does not override anything - D = *I; // FIXME: this does not work with multiple inheritance. - } - return nullptr; - } - - unsigned makeMCall(unsigned NumArgs, const CXXMethodDecl *D) { - return make(EOP_MCall, NumArgs, getFirstVirtualDecl(D)); - } - - unsigned makeIndex() { - return make(EOP_Index); - } - - unsigned makeUnary() { - return make(EOP_Unary); - } - - unsigned makeBinary() { - return make(EOP_Binary); - } - - unsigned makeUnknown(unsigned Arity) { - return make(EOP_Unknown, Arity); - } - - inline bool isCalleeArrow(const Expr *E) { - const MemberExpr *ME = dyn_cast<MemberExpr>(E->IgnoreParenCasts()); - return ME ? ME->isArrow() : false; - } - - /// Build an SExpr from the given C++ expression. - /// Recursive function that terminates on DeclRefExpr. - /// Note: this function merely creates a SExpr; it does not check to - /// ensure that the original expression is a valid mutex expression. - /// - /// NDeref returns the number of Derefence and AddressOf operations - /// preceding the Expr; this is used to decide whether to pretty-print - /// SExprs with . or ->. - unsigned buildSExpr(const Expr *Exp, CallingContext *CallCtx, - int *NDeref = nullptr) { - if (!Exp) - return 0; - - if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Exp)) { - const NamedDecl *ND = cast<NamedDecl>(DRE->getDecl()->getCanonicalDecl()); - const ParmVarDecl *PV = dyn_cast_or_null<ParmVarDecl>(ND); - if (PV) { - const FunctionDecl *FD = - cast<FunctionDecl>(PV->getDeclContext())->getCanonicalDecl(); - unsigned i = PV->getFunctionScopeIndex(); +class TILPrinter : + public til::PrettyPrinter<TILPrinter, llvm::raw_ostream> {}; - if (CallCtx && CallCtx->FunArgs && - FD == CallCtx->AttrDecl->getCanonicalDecl()) { - // Substitute call arguments for references to function parameters - assert(i < CallCtx->NumArgs); - return buildSExpr(CallCtx->FunArgs[i], CallCtx->PrevCtx, NDeref); - } - // Map the param back to the param of the original function declaration. - makeNamedVar(FD->getParamDecl(i)); - return 1; - } - // Not a function parameter -- just store the reference. - makeNamedVar(ND); - return 1; - } else if (isa<CXXThisExpr>(Exp)) { - // Substitute parent for 'this' - if (CallCtx && CallCtx->SelfArg) { - if (!CallCtx->SelfArrow && NDeref) - // 'this' is a pointer, but self is not, so need to take address. - --(*NDeref); - return buildSExpr(CallCtx->SelfArg, CallCtx->PrevCtx, NDeref); - } - else { - makeThis(); - return 1; - } - } else if (const MemberExpr *ME = dyn_cast<MemberExpr>(Exp)) { - const NamedDecl *ND = ME->getMemberDecl(); - int ImplicitDeref = ME->isArrow() ? 1 : 0; - unsigned Root = makeDot(ND, false); - unsigned Sz = buildSExpr(ME->getBase(), CallCtx, &ImplicitDeref); - NodeVec[Root].setArrow(ImplicitDeref > 0); - NodeVec[Root].setSize(Sz + 1); - return Sz + 1; - } else if (const CXXMemberCallExpr *CMCE = dyn_cast<CXXMemberCallExpr>(Exp)) { - // When calling a function with a lock_returned attribute, replace - // the function call with the expression in lock_returned. - const CXXMethodDecl *MD = CMCE->getMethodDecl()->getMostRecentDecl(); - if (LockReturnedAttr* At = MD->getAttr<LockReturnedAttr>()) { - CallingContext LRCallCtx(CMCE->getMethodDecl()); - LRCallCtx.SelfArg = CMCE->getImplicitObjectArgument(); - LRCallCtx.SelfArrow = isCalleeArrow(CMCE->getCallee()); - LRCallCtx.NumArgs = CMCE->getNumArgs(); - LRCallCtx.FunArgs = CMCE->getArgs(); - LRCallCtx.PrevCtx = CallCtx; - return buildSExpr(At->getArg(), &LRCallCtx); - } - // Hack to treat smart pointers and iterators as pointers; - // ignore any method named get(). - if (CMCE->getMethodDecl()->getNameAsString() == "get" && - CMCE->getNumArgs() == 0) { - if (NDeref && isCalleeArrow(CMCE->getCallee())) - ++(*NDeref); - return buildSExpr(CMCE->getImplicitObjectArgument(), CallCtx, NDeref); - } - unsigned NumCallArgs = CMCE->getNumArgs(); - unsigned Root = makeMCall(NumCallArgs, CMCE->getMethodDecl()); - unsigned Sz = buildSExpr(CMCE->getImplicitObjectArgument(), CallCtx); - const Expr* const* CallArgs = CMCE->getArgs(); - for (unsigned i = 0; i < NumCallArgs; ++i) { - Sz += buildSExpr(CallArgs[i], CallCtx); - } - NodeVec[Root].setSize(Sz + 1); - return Sz + 1; - } else if (const CallExpr *CE = dyn_cast<CallExpr>(Exp)) { - const FunctionDecl *FD = CE->getDirectCallee()->getMostRecentDecl(); - if (LockReturnedAttr* At = FD->getAttr<LockReturnedAttr>()) { - CallingContext LRCallCtx(CE->getDirectCallee()); - LRCallCtx.NumArgs = CE->getNumArgs(); - LRCallCtx.FunArgs = CE->getArgs(); - LRCallCtx.PrevCtx = CallCtx; - return buildSExpr(At->getArg(), &LRCallCtx); - } - // Treat smart pointers and iterators as pointers; - // ignore the * and -> operators. - if (const CXXOperatorCallExpr *OE = dyn_cast<CXXOperatorCallExpr>(CE)) { - OverloadedOperatorKind k = OE->getOperator(); - if (k == OO_Star) { - if (NDeref) ++(*NDeref); - return buildSExpr(OE->getArg(0), CallCtx, NDeref); - } - else if (k == OO_Arrow) { - return buildSExpr(OE->getArg(0), CallCtx, NDeref); - } - } - unsigned NumCallArgs = CE->getNumArgs(); - unsigned Root = makeCall(NumCallArgs, nullptr); - unsigned Sz = buildSExpr(CE->getCallee(), CallCtx); - const Expr* const* CallArgs = CE->getArgs(); - for (unsigned i = 0; i < NumCallArgs; ++i) { - Sz += buildSExpr(CallArgs[i], CallCtx); - } - NodeVec[Root].setSize(Sz+1); - return Sz+1; - } else if (const BinaryOperator *BOE = dyn_cast<BinaryOperator>(Exp)) { - unsigned Root = makeBinary(); - unsigned Sz = buildSExpr(BOE->getLHS(), CallCtx); - Sz += buildSExpr(BOE->getRHS(), CallCtx); - NodeVec[Root].setSize(Sz); - return Sz; - } else if (const UnaryOperator *UOE = dyn_cast<UnaryOperator>(Exp)) { - // Ignore & and * operators -- they're no-ops. - // However, we try to figure out whether the expression is a pointer, - // so we can use . and -> appropriately in error messages. - if (UOE->getOpcode() == UO_Deref) { - if (NDeref) ++(*NDeref); - return buildSExpr(UOE->getSubExpr(), CallCtx, NDeref); - } - if (UOE->getOpcode() == UO_AddrOf) { - if (DeclRefExpr* DRE = dyn_cast<DeclRefExpr>(UOE->getSubExpr())) { - if (DRE->getDecl()->isCXXInstanceMember()) { - // This is a pointer-to-member expression, e.g. &MyClass::mu_. - // We interpret this syntax specially, as a wildcard. - unsigned Root = makeDot(DRE->getDecl(), false); - makeWildcard(); - NodeVec[Root].setSize(2); - return 2; - } - } - if (NDeref) --(*NDeref); - return buildSExpr(UOE->getSubExpr(), CallCtx, NDeref); - } - unsigned Root = makeUnary(); - unsigned Sz = buildSExpr(UOE->getSubExpr(), CallCtx); - NodeVec[Root].setSize(Sz); - return Sz; - } else if (const ArraySubscriptExpr *ASE = - dyn_cast<ArraySubscriptExpr>(Exp)) { - unsigned Root = makeIndex(); - unsigned Sz = buildSExpr(ASE->getBase(), CallCtx); - Sz += buildSExpr(ASE->getIdx(), CallCtx); - NodeVec[Root].setSize(Sz); - return Sz; - } else if (const AbstractConditionalOperator *CE = - dyn_cast<AbstractConditionalOperator>(Exp)) { - unsigned Root = makeUnknown(3); - unsigned Sz = buildSExpr(CE->getCond(), CallCtx); - Sz += buildSExpr(CE->getTrueExpr(), CallCtx); - Sz += buildSExpr(CE->getFalseExpr(), CallCtx); - NodeVec[Root].setSize(Sz); - return Sz; - } else if (const ChooseExpr *CE = dyn_cast<ChooseExpr>(Exp)) { - unsigned Root = makeUnknown(3); - unsigned Sz = buildSExpr(CE->getCond(), CallCtx); - Sz += buildSExpr(CE->getLHS(), CallCtx); - Sz += buildSExpr(CE->getRHS(), CallCtx); - NodeVec[Root].setSize(Sz); - return Sz; - } else if (const CastExpr *CE = dyn_cast<CastExpr>(Exp)) { - return buildSExpr(CE->getSubExpr(), CallCtx, NDeref); - } else if (const ParenExpr *PE = dyn_cast<ParenExpr>(Exp)) { - return buildSExpr(PE->getSubExpr(), CallCtx, NDeref); - } else if (const ExprWithCleanups *EWC = dyn_cast<ExprWithCleanups>(Exp)) { - return buildSExpr(EWC->getSubExpr(), CallCtx, NDeref); - } else if (const CXXBindTemporaryExpr *E = dyn_cast<CXXBindTemporaryExpr>(Exp)) { - return buildSExpr(E->getSubExpr(), CallCtx, NDeref); - } else if (isa<CharacterLiteral>(Exp) || - isa<CXXNullPtrLiteralExpr>(Exp) || - isa<GNUNullExpr>(Exp) || - isa<CXXBoolLiteralExpr>(Exp) || - isa<FloatingLiteral>(Exp) || - isa<ImaginaryLiteral>(Exp) || - isa<IntegerLiteral>(Exp) || - isa<StringLiteral>(Exp) || - isa<ObjCStringLiteral>(Exp)) { - makeNop(); - return 1; // FIXME: Ignore literals for now - } else { - makeNop(); - return 1; // Ignore. FIXME: mark as invalid expression? - } - } - - /// \brief Construct a SExpr from an expression. - /// \param MutexExp The original mutex expression within an attribute - /// \param DeclExp An expression involving the Decl on which the attribute - /// occurs. - /// \param D The declaration to which the lock/unlock attribute is attached. - void buildSExprFromExpr(const Expr *MutexExp, const Expr *DeclExp, - const NamedDecl *D, VarDecl *SelfDecl = nullptr) { - CallingContext CallCtx(D); - - if (MutexExp) { - if (const StringLiteral* SLit = dyn_cast<StringLiteral>(MutexExp)) { - if (SLit->getString() == StringRef("*")) - // The "*" expr is a universal lock, which essentially turns off - // checks until it is removed from the lockset. - makeUniversal(); - else - // Ignore other string literals for now. - makeNop(); - return; - } - } - - // If we are processing a raw attribute expression, with no substitutions. - if (!DeclExp) { - buildSExpr(MutexExp, nullptr); - return; - } - // Examine DeclExp to find SelfArg and FunArgs, which are used to substitute - // for formal parameters when we call buildMutexID later. - if (const MemberExpr *ME = dyn_cast<MemberExpr>(DeclExp)) { - CallCtx.SelfArg = ME->getBase(); - CallCtx.SelfArrow = ME->isArrow(); - } else if (const CXXMemberCallExpr *CE = - dyn_cast<CXXMemberCallExpr>(DeclExp)) { - CallCtx.SelfArg = CE->getImplicitObjectArgument(); - CallCtx.SelfArrow = isCalleeArrow(CE->getCallee()); - CallCtx.NumArgs = CE->getNumArgs(); - CallCtx.FunArgs = CE->getArgs(); - } else if (const CallExpr *CE = dyn_cast<CallExpr>(DeclExp)) { - CallCtx.NumArgs = CE->getNumArgs(); - CallCtx.FunArgs = CE->getArgs(); - } else if (const CXXConstructExpr *CE = - dyn_cast<CXXConstructExpr>(DeclExp)) { - CallCtx.SelfArg = nullptr; // Will be set below - CallCtx.NumArgs = CE->getNumArgs(); - CallCtx.FunArgs = CE->getArgs(); - } else if (D && isa<CXXDestructorDecl>(D)) { - // There's no such thing as a "destructor call" in the AST. - CallCtx.SelfArg = DeclExp; - } +/// Issue a warning about an invalid lock expression +static void warnInvalidLock(ThreadSafetyHandler &Handler, + const Expr *MutexExp, const NamedDecl *D, + const Expr *DeclExp, StringRef Kind) { + SourceLocation Loc; + if (DeclExp) + Loc = DeclExp->getExprLoc(); - // Hack to handle constructors, where self cannot be recovered from - // the expression. - if (SelfDecl && !CallCtx.SelfArg) { - DeclRefExpr SelfDRE(SelfDecl, false, SelfDecl->getType(), VK_LValue, - SelfDecl->getLocation()); - CallCtx.SelfArg = &SelfDRE; - - // If the attribute has no arguments, then assume the argument is "this". - if (!MutexExp) - buildSExpr(CallCtx.SelfArg, nullptr); - else // For most attributes. - buildSExpr(MutexExp, &CallCtx); - return; - } - - // If the attribute has no arguments, then assume the argument is "this". - if (!MutexExp) - buildSExpr(CallCtx.SelfArg, nullptr); - else // For most attributes. - buildSExpr(MutexExp, &CallCtx); - } - - /// \brief Get index of next sibling of node i. - unsigned getNextSibling(unsigned i) const { - return i + NodeVec[i].size(); - } - -public: - explicit SExpr(clang::Decl::EmptyShell e) { NodeVec.clear(); } - - /// \param MutexExp The original mutex expression within an attribute - /// \param DeclExp An expression involving the Decl on which the attribute - /// occurs. - /// \param D The declaration to which the lock/unlock attribute is attached. - /// Caller must check isValid() after construction. - SExpr(const Expr *MutexExp, const Expr *DeclExp, const NamedDecl *D, - VarDecl *SelfDecl = nullptr) { - buildSExprFromExpr(MutexExp, DeclExp, D, SelfDecl); - } - - /// Return true if this is a valid decl sequence. - /// Caller must call this by hand after construction to handle errors. - bool isValid() const { - return !NodeVec.empty(); - } - - bool shouldIgnore() const { - // Nop is a mutex that we have decided to deliberately ignore. - assert(NodeVec.size() > 0 && "Invalid Mutex"); - return NodeVec[0].kind() == EOP_Nop; - } - - bool isUniversal() const { - assert(NodeVec.size() > 0 && "Invalid Mutex"); - return NodeVec[0].kind() == EOP_Universal; - } - - /// Issue a warning about an invalid lock expression - static void warnInvalidLock(ThreadSafetyHandler &Handler, - const Expr *MutexExp, const Expr *DeclExp, - const NamedDecl *D, StringRef Kind) { - SourceLocation Loc; - if (DeclExp) - Loc = DeclExp->getExprLoc(); - - // FIXME: add a note about the attribute location in MutexExp or D - if (Loc.isValid()) - Handler.handleInvalidLockExp(Kind, Loc); - } - - bool operator==(const SExpr &other) const { - return NodeVec == other.NodeVec; - } - - bool operator!=(const SExpr &other) const { - return !(*this == other); - } - - bool matches(const SExpr &Other, unsigned i = 0, unsigned j = 0) const { - if (NodeVec[i].matches(Other.NodeVec[j])) { - unsigned ni = NodeVec[i].arity(); - unsigned nj = Other.NodeVec[j].arity(); - unsigned n = (ni < nj) ? ni : nj; - bool Result = true; - unsigned ci = i+1; // first child of i - unsigned cj = j+1; // first child of j - for (unsigned k = 0; k < n; - ++k, ci=getNextSibling(ci), cj = Other.getNextSibling(cj)) { - Result = Result && matches(Other, ci, cj); - } - return Result; - } - return false; - } - - // A partial match between a.mu and b.mu returns true a and b have the same - // type (and thus mu refers to the same mutex declaration), regardless of - // whether a and b are different objects or not. - bool partiallyMatches(const SExpr &Other) const { - if (NodeVec[0].kind() == EOP_Dot) - return NodeVec[0].matches(Other.NodeVec[0]); - return false; - } - - /// \brief Pretty print a lock expression for use in error messages. - std::string toString(unsigned i = 0) const { - assert(isValid()); - if (i >= NodeVec.size()) - return ""; - - const SExprNode* N = &NodeVec[i]; - switch (N->kind()) { - case EOP_Nop: - return "_"; - case EOP_Wildcard: - return "(?)"; - case EOP_Universal: - return "*"; - case EOP_This: - return "this"; - case EOP_NVar: - case EOP_LVar: { - return N->getNamedDecl()->getNameAsString(); - } - case EOP_Dot: { - if (NodeVec[i+1].kind() == EOP_Wildcard) { - std::string S = "&"; - S += N->getNamedDecl()->getQualifiedNameAsString(); - return S; - } - std::string FieldName = N->getNamedDecl()->getNameAsString(); - if (NodeVec[i+1].kind() == EOP_This) - return FieldName; + // FIXME: add a note about the attribute location in MutexExp or D + if (Loc.isValid()) + Handler.handleInvalidLockExp(Kind, Loc); +} - std::string S = toString(i+1); - if (N->isArrow()) - return S + "->" + FieldName; - else - return S + "." + FieldName; - } - case EOP_Call: { - std::string S = toString(i+1) + "("; - unsigned NumArgs = N->arity()-1; - unsigned ci = getNextSibling(i+1); - for (unsigned k=0; k<NumArgs; ++k, ci = getNextSibling(ci)) { - S += toString(ci); - if (k+1 < NumArgs) S += ","; - } - S += ")"; - return S; - } - case EOP_MCall: { - std::string S = ""; - if (NodeVec[i+1].kind() != EOP_This) - S = toString(i+1) + "."; - if (const NamedDecl *D = N->getFunctionDecl()) - S += D->getNameAsString() + "("; - else - S += "#("; - unsigned NumArgs = N->arity()-1; - unsigned ci = getNextSibling(i+1); - for (unsigned k=0; k<NumArgs; ++k, ci = getNextSibling(ci)) { - S += toString(ci); - if (k+1 < NumArgs) S += ","; - } - S += ")"; - return S; - } - case EOP_Index: { - std::string S1 = toString(i+1); - std::string S2 = toString(i+1 + NodeVec[i+1].size()); - return S1 + "[" + S2 + "]"; - } - case EOP_Unary: { - std::string S = toString(i+1); - return "#" + S; - } - case EOP_Binary: { - std::string S1 = toString(i+1); - std::string S2 = toString(i+1 + NodeVec[i+1].size()); - return "(" + S1 + "#" + S2 + ")"; - } - case EOP_Unknown: { - unsigned NumChildren = N->arity(); - if (NumChildren == 0) - return "(...)"; - std::string S = "("; - unsigned ci = i+1; - for (unsigned j = 0; j < NumChildren; ++j, ci = getNextSibling(ci)) { - S += toString(ci); - if (j+1 < NumChildren) S += "#"; - } - S += ")"; - return S; - } - } - return ""; - } -}; -/// \brief A short list of SExprs -class MutexIDList : public SmallVector<SExpr, 3> { +/// \brief A set of CapabilityInfo objects, which are compiled from the +/// requires attributes on a function. +class CapExprSet : public SmallVector<CapabilityExpr, 4> { public: /// \brief Push M onto list, but discard duplicates. - void push_back_nodup(const SExpr& M) { - if (end() == std::find(begin(), end(), M)) - push_back(M); + void push_back_nodup(const CapabilityExpr &CapE) { + iterator It = std::find_if(begin(), end(), + [=](const CapabilityExpr &CapE2) { + return CapE.equals(CapE2); + }); + if (It == end()) + push_back(CapE); } }; -/// \brief This is a helper class that stores info about the most recent -/// accquire of a Lock. -/// -/// The main body of the analysis maps MutexIDs to LockDatas. -struct LockData { - SourceLocation AcquireLoc; - - /// \brief LKind stores whether a lock is held shared or exclusively. - /// Note that this analysis does not currently support either re-entrant - /// locking or lock "upgrading" and "downgrading" between exclusive and - /// shared. - /// - /// FIXME: add support for re-entrant locking and lock up/downgrading - LockKind LKind; - bool Asserted; // for asserted locks - bool Managed; // for ScopedLockable objects - SExpr UnderlyingMutex; // for ScopedLockable objects - - LockData(SourceLocation AcquireLoc, LockKind LKind, bool M=false, - bool Asrt=false) - : AcquireLoc(AcquireLoc), LKind(LKind), Asserted(Asrt), Managed(M), - UnderlyingMutex(Decl::EmptyShell()) - {} +class FactManager; +class FactSet; - LockData(SourceLocation AcquireLoc, LockKind LKind, const SExpr &Mu) - : AcquireLoc(AcquireLoc), LKind(LKind), Asserted(false), Managed(false), - UnderlyingMutex(Mu) - {} - - bool operator==(const LockData &other) const { - return AcquireLoc == other.AcquireLoc && LKind == other.LKind; - } - - bool operator!=(const LockData &other) const { - return !(*this == other); - } - - void Profile(llvm::FoldingSetNodeID &ID) const { - ID.AddInteger(AcquireLoc.getRawEncoding()); - ID.AddInteger(LKind); - } +/// \brief This is a helper class that stores a fact that is known at a +/// particular point in program execution. Currently, a fact is a capability, +/// along with additional information, such as where it was acquired, whether +/// it is exclusive or shared, etc. +/// +/// FIXME: this analysis does not currently support either re-entrant +/// locking or lock "upgrading" and "downgrading" between exclusive and +/// shared. +class FactEntry : public CapabilityExpr { +private: + LockKind LKind; ///< exclusive or shared + SourceLocation AcquireLoc; ///< where it was acquired. + bool Asserted; ///< true if the lock was asserted +public: + FactEntry(const CapabilityExpr &CE, LockKind LK, SourceLocation Loc, + bool Asrt) + : CapabilityExpr(CE), LKind(LK), AcquireLoc(Loc), Asserted(Asrt) {} + + virtual ~FactEntry() {} + + LockKind kind() const { return LKind; } + SourceLocation loc() const { return AcquireLoc; } + bool asserted() const { return Asserted; } + + virtual void + handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan, + SourceLocation JoinLoc, LockErrorKind LEK, + ThreadSafetyHandler &Handler) const = 0; + virtual void handleUnlock(FactSet &FSet, FactManager &FactMan, + const CapabilityExpr &Cp, SourceLocation UnlockLoc, + bool FullyRemove, ThreadSafetyHandler &Handler, + StringRef DiagKind) const = 0; + + // Return true if LKind >= LK, where exclusive > shared bool isAtLeast(LockKind LK) { - return (LK == LK_Shared) || (LKind == LK_Exclusive); + return (LKind == LK_Exclusive) || (LK == LK_Shared); } }; -/// \brief A FactEntry stores a single fact that is known at a particular point -/// in the program execution. Currently, this is information regarding a lock -/// that is held at that point. -struct FactEntry { - SExpr MutID; - LockData LDat; - - FactEntry(const SExpr& M, const LockData& L) - : MutID(M), LDat(L) - { } -}; - - typedef unsigned short FactID; /// \brief FactManager manages the memory for all facts that are created during /// the analysis of a single routine. class FactManager { private: - std::vector<FactEntry> Facts; + std::vector<std::unique_ptr<FactEntry>> Facts; public: - FactID newLock(const SExpr& M, const LockData& L) { - Facts.push_back(FactEntry(M,L)); + FactID newFact(std::unique_ptr<FactEntry> Entry) { + Facts.push_back(std::move(Entry)); return static_cast<unsigned short>(Facts.size() - 1); } - const FactEntry& operator[](FactID F) const { return Facts[F]; } - FactEntry& operator[](FactID F) { return Facts[F]; } + const FactEntry &operator[](FactID F) const { return *Facts[F]; } + FactEntry &operator[](FactID F) { return *Facts[F]; } }; @@ -824,68 +173,73 @@ public: bool isEmpty() const { return FactIDs.size() == 0; } - FactID addLock(FactManager& FM, const SExpr& M, const LockData& L) { - FactID F = FM.newLock(M, L); + // Return true if the set contains only negative facts + bool isEmpty(FactManager &FactMan) const { + for (FactID FID : *this) { + if (!FactMan[FID].negative()) + return false; + } + return true; + } + + void addLockByID(FactID ID) { FactIDs.push_back(ID); } + + FactID addLock(FactManager &FM, std::unique_ptr<FactEntry> Entry) { + FactID F = FM.newFact(std::move(Entry)); FactIDs.push_back(F); return F; } - bool removeLock(FactManager& FM, const SExpr& M) { + bool removeLock(FactManager& FM, const CapabilityExpr &CapE) { unsigned n = FactIDs.size(); if (n == 0) return false; for (unsigned i = 0; i < n-1; ++i) { - if (FM[FactIDs[i]].MutID.matches(M)) { + if (FM[FactIDs[i]].matches(CapE)) { FactIDs[i] = FactIDs[n-1]; FactIDs.pop_back(); return true; } } - if (FM[FactIDs[n-1]].MutID.matches(M)) { + if (FM[FactIDs[n-1]].matches(CapE)) { FactIDs.pop_back(); return true; } return false; } - iterator findLockIter(FactManager &FM, const SExpr &M) { + iterator findLockIter(FactManager &FM, const CapabilityExpr &CapE) { return std::find_if(begin(), end(), [&](FactID ID) { - return FM[ID].MutID.matches(M); + return FM[ID].matches(CapE); }); } - LockData *findLock(FactManager &FM, const SExpr &M) const { + FactEntry *findLock(FactManager &FM, const CapabilityExpr &CapE) const { auto I = std::find_if(begin(), end(), [&](FactID ID) { - return FM[ID].MutID.matches(M); + return FM[ID].matches(CapE); }); - - return I != end() ? &FM[*I].LDat : nullptr; + return I != end() ? &FM[*I] : nullptr; } - LockData *findLockUniv(FactManager &FM, const SExpr &M) const { + FactEntry *findLockUniv(FactManager &FM, const CapabilityExpr &CapE) const { auto I = std::find_if(begin(), end(), [&](FactID ID) -> bool { - const SExpr &Expr = FM[ID].MutID; - return Expr.isUniversal() || Expr.matches(M); + return FM[ID].matchesUniv(CapE); }); - - return I != end() ? &FM[*I].LDat : nullptr; + return I != end() ? &FM[*I] : nullptr; } - FactEntry *findPartialMatch(FactManager &FM, const SExpr &M) const { + FactEntry *findPartialMatch(FactManager &FM, + const CapabilityExpr &CapE) const { auto I = std::find_if(begin(), end(), [&](FactID ID) { - return FM[ID].MutID.partiallyMatches(M); + return FM[ID].partiallyMatches(CapE); }); - return I != end() ? &FM[*I] : nullptr; } }; -/// A Lockset maps each SExpr (defined above) to information about how it has -/// been locked. -typedef llvm::ImmutableMap<SExpr, LockData> Lockset; -typedef llvm::ImmutableMap<const NamedDecl*, unsigned> LocalVarContext; +typedef llvm::ImmutableMap<const NamedDecl*, unsigned> LocalVarContext; class LocalVariableMap; /// A side (entry or exit) of a CFG node. @@ -1404,29 +758,130 @@ static void findBlockLocations(CFG *CFGraph, } } +class LockableFactEntry : public FactEntry { +private: + bool Managed; ///< managed by ScopedLockable object + +public: + LockableFactEntry(const CapabilityExpr &CE, LockKind LK, SourceLocation Loc, + bool Mng = false, bool Asrt = false) + : FactEntry(CE, LK, Loc, Asrt), Managed(Mng) {} + + void + handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan, + SourceLocation JoinLoc, LockErrorKind LEK, + ThreadSafetyHandler &Handler) const override { + if (!Managed && !asserted() && !negative() && !isUniversal()) { + Handler.handleMutexHeldEndOfScope("mutex", toString(), loc(), JoinLoc, + LEK); + } + } + + void handleUnlock(FactSet &FSet, FactManager &FactMan, + const CapabilityExpr &Cp, SourceLocation UnlockLoc, + bool FullyRemove, ThreadSafetyHandler &Handler, + StringRef DiagKind) const override { + FSet.removeLock(FactMan, Cp); + if (!Cp.negative()) { + FSet.addLock(FactMan, llvm::make_unique<LockableFactEntry>( + !Cp, LK_Exclusive, UnlockLoc)); + } + } +}; + +class ScopedLockableFactEntry : public FactEntry { +private: + SmallVector<const til::SExpr *, 4> UnderlyingMutexes; + +public: + ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc, + const CapExprSet &Excl, const CapExprSet &Shrd) + : FactEntry(CE, LK_Exclusive, Loc, false) { + for (const auto &M : Excl) + UnderlyingMutexes.push_back(M.sexpr()); + for (const auto &M : Shrd) + UnderlyingMutexes.push_back(M.sexpr()); + } + + void + handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan, + SourceLocation JoinLoc, LockErrorKind LEK, + ThreadSafetyHandler &Handler) const override { + for (const til::SExpr *UnderlyingMutex : UnderlyingMutexes) { + if (FSet.findLock(FactMan, CapabilityExpr(UnderlyingMutex, false))) { + // If this scoped lock manages another mutex, and if the underlying + // mutex is still held, then warn about the underlying mutex. + Handler.handleMutexHeldEndOfScope( + "mutex", sx::toString(UnderlyingMutex), loc(), JoinLoc, LEK); + } + } + } + + void handleUnlock(FactSet &FSet, FactManager &FactMan, + const CapabilityExpr &Cp, SourceLocation UnlockLoc, + bool FullyRemove, ThreadSafetyHandler &Handler, + StringRef DiagKind) const override { + assert(!Cp.negative() && "Managing object cannot be negative."); + for (const til::SExpr *UnderlyingMutex : UnderlyingMutexes) { + CapabilityExpr UnderCp(UnderlyingMutex, false); + auto UnderEntry = llvm::make_unique<LockableFactEntry>( + !UnderCp, LK_Exclusive, UnlockLoc); + + if (FullyRemove) { + // We're destroying the managing object. + // Remove the underlying mutex if it exists; but don't warn. + if (FSet.findLock(FactMan, UnderCp)) { + FSet.removeLock(FactMan, UnderCp); + FSet.addLock(FactMan, std::move(UnderEntry)); + } + } else { + // We're releasing the underlying mutex, but not destroying the + // managing object. Warn on dual release. + if (!FSet.findLock(FactMan, UnderCp)) { + Handler.handleUnmatchedUnlock(DiagKind, UnderCp.toString(), + UnlockLoc); + } + FSet.removeLock(FactMan, UnderCp); + FSet.addLock(FactMan, std::move(UnderEntry)); + } + } + if (FullyRemove) + FSet.removeLock(FactMan, Cp); + } +}; + /// \brief Class which implements the core thread safety analysis routines. class ThreadSafetyAnalyzer { friend class BuildLockset; + llvm::BumpPtrAllocator Bpa; + threadSafety::til::MemRegionRef Arena; + threadSafety::SExprBuilder SxBuilder; + ThreadSafetyHandler &Handler; + const CXXMethodDecl *CurrentMethod; LocalVariableMap LocalVarMap; FactManager FactMan; std::vector<CFGBlockInfo> BlockInfo; public: - ThreadSafetyAnalyzer(ThreadSafetyHandler &H) : Handler(H) {} + ThreadSafetyAnalyzer(ThreadSafetyHandler &H) + : Arena(&Bpa), SxBuilder(Arena), Handler(H) {} + + bool inCurrentScope(const CapabilityExpr &CapE); - void addLock(FactSet &FSet, const SExpr &Mutex, const LockData &LDat, - StringRef DiagKind); - void removeLock(FactSet &FSet, const SExpr &Mutex, SourceLocation UnlockLoc, - bool FullyRemove, LockKind Kind, StringRef DiagKind); + void addLock(FactSet &FSet, std::unique_ptr<FactEntry> Entry, + StringRef DiagKind, bool ReqAttr = false); + void removeLock(FactSet &FSet, const CapabilityExpr &CapE, + SourceLocation UnlockLoc, bool FullyRemove, LockKind Kind, + StringRef DiagKind); template <typename AttrType> - void getMutexIDs(MutexIDList &Mtxs, AttrType *Attr, Expr *Exp, + void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, Expr *Exp, const NamedDecl *D, VarDecl *SelfDecl = nullptr); template <class AttrType> - void getMutexIDs(MutexIDList &Mtxs, AttrType *Attr, Expr *Exp, + void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, Expr *Exp, const NamedDecl *D, const CFGBlock *PredBlock, const CFGBlock *CurrBlock, Expr *BrE, bool Neg); @@ -1530,94 +985,107 @@ ClassifyDiagnostic(const AttrTy *A) { return "mutex"; } + +inline bool ThreadSafetyAnalyzer::inCurrentScope(const CapabilityExpr &CapE) { + if (!CurrentMethod) + return false; + if (auto *P = dyn_cast_or_null<til::Project>(CapE.sexpr())) { + auto *VD = P->clangDecl(); + if (VD) + return VD->getDeclContext() == CurrentMethod->getDeclContext(); + } + return false; +} + + /// \brief Add a new lock to the lockset, warning if the lock is already there. -/// \param Mutex -- the Mutex expression for the lock -/// \param LDat -- the LockData for the lock -void ThreadSafetyAnalyzer::addLock(FactSet &FSet, const SExpr &Mutex, - const LockData &LDat, StringRef DiagKind) { - // FIXME: deal with acquired before/after annotations. - // FIXME: Don't always warn when we have support for reentrant locks. - if (Mutex.shouldIgnore()) +/// \param ReqAttr -- true if this is part of an initial Requires attribute. +void ThreadSafetyAnalyzer::addLock(FactSet &FSet, + std::unique_ptr<FactEntry> Entry, + StringRef DiagKind, bool ReqAttr) { + if (Entry->shouldIgnore()) return; - if (FSet.findLock(FactMan, Mutex)) { - if (!LDat.Asserted) - Handler.handleDoubleLock(DiagKind, Mutex.toString(), LDat.AcquireLoc); + if (!ReqAttr && !Entry->negative()) { + // look for the negative capability, and remove it from the fact set. + CapabilityExpr NegC = !*Entry; + FactEntry *Nen = FSet.findLock(FactMan, NegC); + if (Nen) { + FSet.removeLock(FactMan, NegC); + } + else { + if (inCurrentScope(*Entry) && !Entry->asserted()) + Handler.handleNegativeNotHeld(DiagKind, Entry->toString(), + NegC.toString(), Entry->loc()); + } + } + + // FIXME: deal with acquired before/after annotations. + // FIXME: Don't always warn when we have support for reentrant locks. + if (FSet.findLock(FactMan, *Entry)) { + if (!Entry->asserted()) + Handler.handleDoubleLock(DiagKind, Entry->toString(), Entry->loc()); } else { - FSet.addLock(FactMan, Mutex, LDat); + FSet.addLock(FactMan, std::move(Entry)); } } /// \brief Remove a lock from the lockset, warning if the lock is not there. -/// \param Mutex The lock expression corresponding to the lock to be removed /// \param UnlockLoc The source location of the unlock (only used in error msg) -void ThreadSafetyAnalyzer::removeLock(FactSet &FSet, const SExpr &Mutex, +void ThreadSafetyAnalyzer::removeLock(FactSet &FSet, const CapabilityExpr &Cp, SourceLocation UnlockLoc, bool FullyRemove, LockKind ReceivedKind, StringRef DiagKind) { - if (Mutex.shouldIgnore()) + if (Cp.shouldIgnore()) return; - const LockData *LDat = FSet.findLock(FactMan, Mutex); + const FactEntry *LDat = FSet.findLock(FactMan, Cp); if (!LDat) { - Handler.handleUnmatchedUnlock(DiagKind, Mutex.toString(), UnlockLoc); + Handler.handleUnmatchedUnlock(DiagKind, Cp.toString(), UnlockLoc); return; } // Generic lock removal doesn't care about lock kind mismatches, but // otherwise diagnose when the lock kinds are mismatched. - if (ReceivedKind != LK_Generic && LDat->LKind != ReceivedKind) { - Handler.handleIncorrectUnlockKind(DiagKind, Mutex.toString(), LDat->LKind, - ReceivedKind, UnlockLoc); - return; + if (ReceivedKind != LK_Generic && LDat->kind() != ReceivedKind) { + Handler.handleIncorrectUnlockKind(DiagKind, Cp.toString(), + LDat->kind(), ReceivedKind, UnlockLoc); } - if (LDat->UnderlyingMutex.isValid()) { - // This is scoped lockable object, which manages the real mutex. - if (FullyRemove) { - // We're destroying the managing object. - // Remove the underlying mutex if it exists; but don't warn. - if (FSet.findLock(FactMan, LDat->UnderlyingMutex)) - FSet.removeLock(FactMan, LDat->UnderlyingMutex); - } else { - // We're releasing the underlying mutex, but not destroying the - // managing object. Warn on dual release. - if (!FSet.findLock(FactMan, LDat->UnderlyingMutex)) { - Handler.handleUnmatchedUnlock( - DiagKind, LDat->UnderlyingMutex.toString(), UnlockLoc); - } - FSet.removeLock(FactMan, LDat->UnderlyingMutex); - return; - } - } - FSet.removeLock(FactMan, Mutex); + LDat->handleUnlock(FSet, FactMan, Cp, UnlockLoc, FullyRemove, Handler, + DiagKind); } /// \brief Extract the list of mutexIDs from the attribute on an expression, /// and push them onto Mtxs, discarding any duplicates. template <typename AttrType> -void ThreadSafetyAnalyzer::getMutexIDs(MutexIDList &Mtxs, AttrType *Attr, +void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, Expr *Exp, const NamedDecl *D, VarDecl *SelfDecl) { if (Attr->args_size() == 0) { // The mutex held is the "this" object. - SExpr Mu(nullptr, Exp, D, SelfDecl); - if (!Mu.isValid()) - SExpr::warnInvalidLock(Handler, nullptr, Exp, D, - ClassifyDiagnostic(Attr)); - else - Mtxs.push_back_nodup(Mu); + CapabilityExpr Cp = SxBuilder.translateAttrExpr(nullptr, D, Exp, SelfDecl); + if (Cp.isInvalid()) { + warnInvalidLock(Handler, nullptr, D, Exp, ClassifyDiagnostic(Attr)); + return; + } + //else + if (!Cp.shouldIgnore()) + Mtxs.push_back_nodup(Cp); return; } for (const auto *Arg : Attr->args()) { - SExpr Mu(Arg, Exp, D, SelfDecl); - if (!Mu.isValid()) - SExpr::warnInvalidLock(Handler, Arg, Exp, D, ClassifyDiagnostic(Attr)); - else - Mtxs.push_back_nodup(Mu); + CapabilityExpr Cp = SxBuilder.translateAttrExpr(Arg, D, Exp, SelfDecl); + if (Cp.isInvalid()) { + warnInvalidLock(Handler, nullptr, D, Exp, ClassifyDiagnostic(Attr)); + continue; + } + //else + if (!Cp.shouldIgnore()) + Mtxs.push_back_nodup(Cp); } } @@ -1626,7 +1094,7 @@ void ThreadSafetyAnalyzer::getMutexIDs(MutexIDList &Mtxs, AttrType *Attr, /// trylock applies to the given edge, then push them onto Mtxs, discarding /// any duplicates. template <class AttrType> -void ThreadSafetyAnalyzer::getMutexIDs(MutexIDList &Mtxs, AttrType *Attr, +void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, Expr *Exp, const NamedDecl *D, const CFGBlock *PredBlock, const CFGBlock *CurrBlock, @@ -1758,8 +1226,8 @@ void ThreadSafetyAnalyzer::getEdgeLockset(FactSet& Result, if(!FunDecl || !FunDecl->hasAttrs()) return; - MutexIDList ExclusiveLocksToAdd; - MutexIDList SharedLocksToAdd; + CapExprSet ExclusiveLocksToAdd; + CapExprSet SharedLocksToAdd; // If the condition is a call to a Trylock function, then grab the attributes for (auto *Attr : FunDecl->getAttrs()) { @@ -1788,10 +1256,13 @@ void ThreadSafetyAnalyzer::getEdgeLockset(FactSet& Result, // Add and remove locks. SourceLocation Loc = Exp->getExprLoc(); for (const auto &ExclusiveLockToAdd : ExclusiveLocksToAdd) - addLock(Result, ExclusiveLockToAdd, LockData(Loc, LK_Exclusive), + addLock(Result, llvm::make_unique<LockableFactEntry>(ExclusiveLockToAdd, + LK_Exclusive, Loc), CapDiagKind); for (const auto &SharedLockToAdd : SharedLocksToAdd) - addLock(Result, SharedLockToAdd, LockData(Loc, LK_Shared), CapDiagKind); + addLock(Result, llvm::make_unique<LockableFactEntry>(SharedLockToAdd, + LK_Shared, Loc), + CapDiagKind); } /// \brief We use this class to visit different types of expressions in @@ -1807,16 +1278,17 @@ class BuildLockset : public StmtVisitor<BuildLockset> { LocalVariableMap::Context LVarCtx; unsigned CtxIndex; - // Helper functions - + // helper functions void warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, AccessKind AK, Expr *MutexExp, ProtectedOperationKind POK, - StringRef DiagKind); + StringRef DiagKind, SourceLocation Loc); void warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, Expr *MutexExp, StringRef DiagKind); - void checkAccess(const Expr *Exp, AccessKind AK); - void checkPtAccess(const Expr *Exp, AccessKind AK); + void checkAccess(const Expr *Exp, AccessKind AK, + ProtectedOperationKind POK = POK_VarAccess); + void checkPtAccess(const Expr *Exp, AccessKind AK, + ProtectedOperationKind POK = POK_VarAccess); void handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD = nullptr); @@ -1837,62 +1309,87 @@ public: void VisitDeclStmt(DeclStmt *S); }; + /// \brief Warn if the LSet does not contain a lock sufficient to protect access /// of at least the passed in AccessKind. void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, AccessKind AK, Expr *MutexExp, ProtectedOperationKind POK, - StringRef DiagKind) { + StringRef DiagKind, SourceLocation Loc) { LockKind LK = getLockKindFromAccessKind(AK); - SExpr Mutex(MutexExp, Exp, D); - if (!Mutex.isValid()) { - SExpr::warnInvalidLock(Analyzer->Handler, MutexExp, Exp, D, DiagKind); + CapabilityExpr Cp = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp); + if (Cp.isInvalid()) { + warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, DiagKind); return; - } else if (Mutex.shouldIgnore()) { + } else if (Cp.shouldIgnore()) { + return; + } + + if (Cp.negative()) { + // Negative capabilities act like locks excluded + FactEntry *LDat = FSet.findLock(Analyzer->FactMan, !Cp); + if (LDat) { + Analyzer->Handler.handleFunExcludesLock( + DiagKind, D->getNameAsString(), (!Cp).toString(), Loc); + return; + } + + // If this does not refer to a negative capability in the same class, + // then stop here. + if (!Analyzer->inCurrentScope(Cp)) + return; + + // Otherwise the negative requirement must be propagated to the caller. + LDat = FSet.findLock(Analyzer->FactMan, Cp); + if (!LDat) { + Analyzer->Handler.handleMutexNotHeld("", D, POK, Cp.toString(), + LK_Shared, Loc); + } return; } - LockData* LDat = FSet.findLockUniv(Analyzer->FactMan, Mutex); + FactEntry* LDat = FSet.findLockUniv(Analyzer->FactMan, Cp); bool NoError = true; if (!LDat) { // No exact match found. Look for a partial match. - FactEntry* FEntry = FSet.findPartialMatch(Analyzer->FactMan, Mutex); - if (FEntry) { + LDat = FSet.findPartialMatch(Analyzer->FactMan, Cp); + if (LDat) { // Warn that there's no precise match. - LDat = &FEntry->LDat; - std::string PartMatchStr = FEntry->MutID.toString(); + std::string PartMatchStr = LDat->toString(); StringRef PartMatchName(PartMatchStr); - Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Mutex.toString(), - LK, Exp->getExprLoc(), - &PartMatchName); + Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Cp.toString(), + LK, Loc, &PartMatchName); } else { // Warn that there's no match at all. - Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Mutex.toString(), - LK, Exp->getExprLoc()); + Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Cp.toString(), + LK, Loc); } NoError = false; } // Make sure the mutex we found is the right kind. - if (NoError && LDat && !LDat->isAtLeast(LK)) - Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Mutex.toString(), LK, - Exp->getExprLoc()); + if (NoError && LDat && !LDat->isAtLeast(LK)) { + Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Cp.toString(), + LK, Loc); + } } /// \brief Warn if the LSet contains the given lock. void BuildLockset::warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, - Expr *MutexExp, - StringRef DiagKind) { - SExpr Mutex(MutexExp, Exp, D); - if (!Mutex.isValid()) { - SExpr::warnInvalidLock(Analyzer->Handler, MutexExp, Exp, D, DiagKind); + Expr *MutexExp, StringRef DiagKind) { + CapabilityExpr Cp = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp); + if (Cp.isInvalid()) { + warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, DiagKind); + return; + } else if (Cp.shouldIgnore()) { return; } - LockData* LDat = FSet.findLock(Analyzer->FactMan, Mutex); - if (LDat) + FactEntry* LDat = FSet.findLock(Analyzer->FactMan, Cp); + if (LDat) { Analyzer->Handler.handleFunExcludesLock( - DiagKind, D->getNameAsString(), Mutex.toString(), Exp->getExprLoc()); + DiagKind, D->getNameAsString(), Cp.toString(), Exp->getExprLoc()); + } } /// \brief Checks guarded_by and pt_guarded_by attributes. @@ -1900,43 +1397,62 @@ void BuildLockset::warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, /// marked with guarded_by, we must ensure the appropriate mutexes are held. /// Similarly, we check if the access is to an expression that dereferences /// a pointer marked with pt_guarded_by. -void BuildLockset::checkAccess(const Expr *Exp, AccessKind AK) { +void BuildLockset::checkAccess(const Expr *Exp, AccessKind AK, + ProtectedOperationKind POK) { Exp = Exp->IgnoreParenCasts(); + SourceLocation Loc = Exp->getExprLoc(); + + // Local variables of reference type cannot be re-assigned; + // map them to their initializer. + while (const auto *DRE = dyn_cast<DeclRefExpr>(Exp)) { + const auto *VD = dyn_cast<VarDecl>(DRE->getDecl()->getCanonicalDecl()); + if (VD && VD->isLocalVarDecl() && VD->getType()->isReferenceType()) { + if (const auto *E = VD->getInit()) { + Exp = E; + continue; + } + } + break; + } + if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(Exp)) { // For dereferences if (UO->getOpcode() == clang::UO_Deref) - checkPtAccess(UO->getSubExpr(), AK); + checkPtAccess(UO->getSubExpr(), AK, POK); return; } if (const ArraySubscriptExpr *AE = dyn_cast<ArraySubscriptExpr>(Exp)) { - checkPtAccess(AE->getLHS(), AK); + checkPtAccess(AE->getLHS(), AK, POK); return; } if (const MemberExpr *ME = dyn_cast<MemberExpr>(Exp)) { if (ME->isArrow()) - checkPtAccess(ME->getBase(), AK); + checkPtAccess(ME->getBase(), AK, POK); else - checkAccess(ME->getBase(), AK); + checkAccess(ME->getBase(), AK, POK); } const ValueDecl *D = getValueDecl(Exp); if (!D || !D->hasAttrs()) return; - if (D->hasAttr<GuardedVarAttr>() && FSet.isEmpty()) - Analyzer->Handler.handleNoMutexHeld("mutex", D, POK_VarAccess, AK, - Exp->getExprLoc()); + if (D->hasAttr<GuardedVarAttr>() && FSet.isEmpty(Analyzer->FactMan)) { + Analyzer->Handler.handleNoMutexHeld("mutex", D, POK, AK, Loc); + } for (const auto *I : D->specific_attrs<GuardedByAttr>()) - warnIfMutexNotHeld(D, Exp, AK, I->getArg(), POK_VarAccess, - ClassifyDiagnostic(I)); + warnIfMutexNotHeld(D, Exp, AK, I->getArg(), POK, + ClassifyDiagnostic(I), Loc); } + /// \brief Checks pt_guarded_by and pt_guarded_var attributes. -void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK) { +/// POK is the same operationKind that was passed to checkAccess. +void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK, + ProtectedOperationKind POK) { while (true) { if (const ParenExpr *PE = dyn_cast<ParenExpr>(Exp)) { Exp = PE->getSubExpr(); @@ -1946,7 +1462,7 @@ void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK) { if (CE->getCastKind() == CK_ArrayToPointerDecay) { // If it's an actual array, and not a pointer, then it's elements // are protected by GUARDED_BY, not PT_GUARDED_BY; - checkAccess(CE->getSubExpr(), AK); + checkAccess(CE->getSubExpr(), AK, POK); return; } Exp = CE->getSubExpr(); @@ -1955,17 +1471,21 @@ void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK) { break; } + // Pass by reference warnings are under a different flag. + ProtectedOperationKind PtPOK = POK_VarDereference; + if (POK == POK_PassByRef) PtPOK = POK_PtPassByRef; + const ValueDecl *D = getValueDecl(Exp); if (!D || !D->hasAttrs()) return; - if (D->hasAttr<PtGuardedVarAttr>() && FSet.isEmpty()) - Analyzer->Handler.handleNoMutexHeld("mutex", D, POK_VarDereference, AK, + if (D->hasAttr<PtGuardedVarAttr>() && FSet.isEmpty(Analyzer->FactMan)) + Analyzer->Handler.handleNoMutexHeld("mutex", D, PtPOK, AK, Exp->getExprLoc()); for (auto const *I : D->specific_attrs<PtGuardedByAttr>()) - warnIfMutexNotHeld(D, Exp, AK, I->getArg(), POK_VarDereference, - ClassifyDiagnostic(I)); + warnIfMutexNotHeld(D, Exp, AK, I->getArg(), PtPOK, + ClassifyDiagnostic(I), Exp->getExprLoc()); } /// \brief Process a function call, method call, constructor call, @@ -1981,8 +1501,8 @@ void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK) { void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) { SourceLocation Loc = Exp->getExprLoc(); const AttrVec &ArgAttrs = D->getAttrs(); - MutexIDList ExclusiveLocksToAdd, SharedLocksToAdd; - MutexIDList ExclusiveLocksToRemove, SharedLocksToRemove, GenericLocksToRemove; + CapExprSet ExclusiveLocksToAdd, SharedLocksToAdd; + CapExprSet ExclusiveLocksToRemove, SharedLocksToRemove, GenericLocksToRemove; StringRef CapDiagKind = "mutex"; for(unsigned i = 0; i < ArgAttrs.size(); ++i) { @@ -2006,22 +1526,23 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) { case attr::AssertExclusiveLock: { AssertExclusiveLockAttr *A = cast<AssertExclusiveLockAttr>(At); - MutexIDList AssertLocks; + CapExprSet AssertLocks; Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD); for (const auto &AssertLock : AssertLocks) - Analyzer->addLock(FSet, AssertLock, - LockData(Loc, LK_Exclusive, false, true), + Analyzer->addLock(FSet, + llvm::make_unique<LockableFactEntry>( + AssertLock, LK_Exclusive, Loc, false, true), ClassifyDiagnostic(A)); break; } case attr::AssertSharedLock: { AssertSharedLockAttr *A = cast<AssertSharedLockAttr>(At); - MutexIDList AssertLocks; + CapExprSet AssertLocks; Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD); for (const auto &AssertLock : AssertLocks) - Analyzer->addLock(FSet, AssertLock, - LockData(Loc, LK_Shared, false, true), + Analyzer->addLock(FSet, llvm::make_unique<LockableFactEntry>( + AssertLock, LK_Shared, Loc, false, true), ClassifyDiagnostic(A)); break; } @@ -2045,7 +1566,8 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) { RequiresCapabilityAttr *A = cast<RequiresCapabilityAttr>(At); for (auto *Arg : A->args()) warnIfMutexNotHeld(D, Exp, A->isShared() ? AK_Read : AK_Written, Arg, - POK_FunctionCall, ClassifyDiagnostic(A)); + POK_FunctionCall, ClassifyDiagnostic(A), + Exp->getExprLoc()); break; } @@ -2074,25 +1596,28 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) { // Add locks. for (const auto &M : ExclusiveLocksToAdd) - Analyzer->addLock(FSet, M, LockData(Loc, LK_Exclusive, isScopedVar), + Analyzer->addLock(FSet, llvm::make_unique<LockableFactEntry>( + M, LK_Exclusive, Loc, isScopedVar), CapDiagKind); for (const auto &M : SharedLocksToAdd) - Analyzer->addLock(FSet, M, LockData(Loc, LK_Shared, isScopedVar), + Analyzer->addLock(FSet, llvm::make_unique<LockableFactEntry>( + M, LK_Shared, Loc, isScopedVar), CapDiagKind); - // Add the managing object as a dummy mutex, mapped to the underlying mutex. - // FIXME -- this doesn't work if we acquire multiple locks. if (isScopedVar) { + // Add the managing object as a dummy mutex, mapped to the underlying mutex. SourceLocation MLoc = VD->getLocation(); DeclRefExpr DRE(VD, false, VD->getType(), VK_LValue, VD->getLocation()); - SExpr SMutex(&DRE, nullptr, nullptr); - - for (const auto &M : ExclusiveLocksToAdd) - Analyzer->addLock(FSet, SMutex, LockData(MLoc, LK_Exclusive, M), - CapDiagKind); - for (const auto &M : SharedLocksToAdd) - Analyzer->addLock(FSet, SMutex, LockData(MLoc, LK_Shared, M), - CapDiagKind); + // FIXME: does this store a pointer to DRE? + CapabilityExpr Scp = Analyzer->SxBuilder.translateAttrExpr(&DRE, nullptr); + + CapExprSet UnderlyingMutexes(ExclusiveLocksToAdd); + std::copy(SharedLocksToAdd.begin(), SharedLocksToAdd.end(), + std::back_inserter(UnderlyingMutexes)); + Analyzer->addLock(FSet, + llvm::make_unique<ScopedLockableFactEntry>( + Scp, MLoc, ExclusiveLocksToAdd, SharedLocksToAdd), + CapDiagKind); } // Remove locks. @@ -2149,6 +1674,9 @@ void BuildLockset::VisitCastExpr(CastExpr *CE) { void BuildLockset::VisitCallExpr(CallExpr *Exp) { + bool ExamineArgs = true; + bool OperatorFun = false; + if (CXXMemberCallExpr *CE = dyn_cast<CXXMemberCallExpr>(Exp)) { MemberExpr *ME = dyn_cast<MemberExpr>(CE->getCallee()); // ME can be null when calling a method pointer @@ -2169,8 +1697,12 @@ void BuildLockset::VisitCallExpr(CallExpr *Exp) { } } } else if (CXXOperatorCallExpr *OE = dyn_cast<CXXOperatorCallExpr>(Exp)) { - switch (OE->getOperator()) { + OperatorFun = true; + + auto OEop = OE->getOperator(); + switch (OEop) { case OO_Equal: { + ExamineArgs = false; const Expr *Target = OE->getArg(0); const Expr *Source = OE->getArg(1); checkAccess(Target, AK_Written); @@ -2182,16 +1714,53 @@ void BuildLockset::VisitCallExpr(CallExpr *Exp) { case OO_Subscript: { const Expr *Obj = OE->getArg(0); checkAccess(Obj, AK_Read); - checkPtAccess(Obj, AK_Read); + if (!(OEop == OO_Star && OE->getNumArgs() > 1)) { + // Grrr. operator* can be multiplication... + checkPtAccess(Obj, AK_Read); + } break; } default: { + // TODO: get rid of this, and rely on pass-by-ref instead. const Expr *Obj = OE->getArg(0); checkAccess(Obj, AK_Read); break; } } } + + + if (ExamineArgs) { + if (FunctionDecl *FD = Exp->getDirectCallee()) { + unsigned Fn = FD->getNumParams(); + unsigned Cn = Exp->getNumArgs(); + unsigned Skip = 0; + + unsigned i = 0; + if (OperatorFun) { + if (isa<CXXMethodDecl>(FD)) { + // First arg in operator call is implicit self argument, + // and doesn't appear in the FunctionDecl. + Skip = 1; + Cn--; + } else { + // Ignore the first argument of operators; it's been checked above. + i = 1; + } + } + // Ignore default arguments + unsigned n = (Fn < Cn) ? Fn : Cn; + + for (; i < n; ++i) { + ParmVarDecl* Pvd = FD->getParamDecl(i); + Expr* Arg = Exp->getArg(i+Skip); + QualType Qt = Pvd->getType(); + if (Qt->isReferenceType()) + checkAccess(Arg, AK_Read, POK_PassByRef); + } + } + } + NamedDecl *D = dyn_cast_or_null<NamedDecl>(Exp->getCalleeDecl()); if(!D || !D->hasAttrs()) return; @@ -2254,62 +1823,40 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &FSet1, // Find locks in FSet2 that conflict or are not in FSet1, and warn. for (const auto &Fact : FSet2) { - const SExpr &FSet2Mutex = FactMan[Fact].MutID; - const LockData &LDat2 = FactMan[Fact].LDat; - FactSet::iterator I1 = FSet1.findLockIter(FactMan, FSet2Mutex); - - if (I1 != FSet1.end()) { - const LockData* LDat1 = &FactMan[*I1].LDat; - if (LDat1->LKind != LDat2.LKind) { - Handler.handleExclusiveAndShared("mutex", FSet2Mutex.toString(), - LDat2.AcquireLoc, LDat1->AcquireLoc); - if (Modify && LDat1->LKind != LK_Exclusive) { + const FactEntry *LDat1 = nullptr; + const FactEntry *LDat2 = &FactMan[Fact]; + FactSet::iterator Iter1 = FSet1.findLockIter(FactMan, *LDat2); + if (Iter1 != FSet1.end()) LDat1 = &FactMan[*Iter1]; + + if (LDat1) { + if (LDat1->kind() != LDat2->kind()) { + Handler.handleExclusiveAndShared("mutex", LDat2->toString(), + LDat2->loc(), LDat1->loc()); + if (Modify && LDat1->kind() != LK_Exclusive) { // Take the exclusive lock, which is the one in FSet2. - *I1 = Fact; + *Iter1 = Fact; } } - else if (LDat1->Asserted && !LDat2.Asserted) { + else if (Modify && LDat1->asserted() && !LDat2->asserted()) { // The non-asserted lock in FSet2 is the one we want to track. - *I1 = Fact; + *Iter1 = Fact; } } else { - if (LDat2.UnderlyingMutex.isValid()) { - if (FSet2.findLock(FactMan, LDat2.UnderlyingMutex)) { - // If this is a scoped lock that manages another mutex, and if the - // underlying mutex is still held, then warn about the underlying - // mutex. - Handler.handleMutexHeldEndOfScope("mutex", - LDat2.UnderlyingMutex.toString(), - LDat2.AcquireLoc, JoinLoc, LEK1); - } - } - else if (!LDat2.Managed && !FSet2Mutex.isUniversal() && !LDat2.Asserted) - Handler.handleMutexHeldEndOfScope("mutex", FSet2Mutex.toString(), - LDat2.AcquireLoc, JoinLoc, LEK1); + LDat2->handleRemovalFromIntersection(FSet2, FactMan, JoinLoc, LEK1, + Handler); } } // Find locks in FSet1 that are not in FSet2, and remove them. for (const auto &Fact : FSet1Orig) { - const SExpr &FSet1Mutex = FactMan[Fact].MutID; - const LockData &LDat1 = FactMan[Fact].LDat; - - if (!FSet2.findLock(FactMan, FSet1Mutex)) { - if (LDat1.UnderlyingMutex.isValid()) { - if (FSet1Orig.findLock(FactMan, LDat1.UnderlyingMutex)) { - // If this is a scoped lock that manages another mutex, and if the - // underlying mutex is still held, then warn about the underlying - // mutex. - Handler.handleMutexHeldEndOfScope("mutex", - LDat1.UnderlyingMutex.toString(), - LDat1.AcquireLoc, JoinLoc, LEK1); - } - } - else if (!LDat1.Managed && !FSet1Mutex.isUniversal() && !LDat1.Asserted) - Handler.handleMutexHeldEndOfScope("mutex", FSet1Mutex.toString(), - LDat1.AcquireLoc, JoinLoc, LEK2); + const FactEntry *LDat1 = &FactMan[Fact]; + const FactEntry *LDat2 = FSet2.findLock(FactMan, *LDat1); + + if (!LDat2) { + LDat1->handleRemovalFromIntersection(FSet1Orig, FactMan, JoinLoc, LEK2, + Handler); if (Modify) - FSet1.removeLock(FactMan, FSet1Mutex); + FSet1.removeLock(FactMan, *LDat1); } } } @@ -2348,6 +1895,8 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { CFG *CFGraph = walker.getGraph(); const NamedDecl *D = walker.getDecl(); + const FunctionDecl *CurrentFunction = dyn_cast<FunctionDecl>(D); + CurrentMethod = dyn_cast<CXXMethodDecl>(D); if (D->hasAttr<NoThreadSafetyAnalysisAttr>()) return; @@ -2361,6 +1910,8 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { if (isa<CXXDestructorDecl>(D)) return; // Don't check inside destructors. + Handler.enterFunction(CurrentFunction); + BlockInfo.resize(CFGraph->getNumBlockIDs(), CFGBlockInfo::getEmptyBlockInfo(LocalVarMap)); @@ -2379,9 +1930,9 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { // Fill in source locations for all CFGBlocks. findBlockLocations(CFGraph, SortedGraph, BlockInfo); - MutexIDList ExclusiveLocksAcquired; - MutexIDList SharedLocksAcquired; - MutexIDList LocksReleased; + CapExprSet ExclusiveLocksAcquired; + CapExprSet SharedLocksAcquired; + CapExprSet LocksReleased; // Add locks from exclusive_locks_required and shared_locks_required // to initial lockset. Also turn off checking for lock and unlock functions. @@ -2391,8 +1942,8 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { FactSet &InitialLockset = BlockInfo[FirstBlock->getBlockID()].EntrySet; const AttrVec &ArgAttrs = D->getAttrs(); - MutexIDList ExclusiveLocksToAdd; - MutexIDList SharedLocksToAdd; + CapExprSet ExclusiveLocksToAdd; + CapExprSet SharedLocksToAdd; StringRef CapDiagKind = "mutex"; SourceLocation Loc = D->getLocation(); @@ -2428,12 +1979,14 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { } // FIXME -- Loc can be wrong here. - for (const auto &ExclusiveLockToAdd : ExclusiveLocksToAdd) - addLock(InitialLockset, ExclusiveLockToAdd, LockData(Loc, LK_Exclusive), - CapDiagKind); - for (const auto &SharedLockToAdd : SharedLocksToAdd) - addLock(InitialLockset, SharedLockToAdd, LockData(Loc, LK_Shared), - CapDiagKind); + for (const auto &Mu : ExclusiveLocksToAdd) + addLock(InitialLockset, + llvm::make_unique<LockableFactEntry>(Mu, LK_Exclusive, Loc), + CapDiagKind, true); + for (const auto &Mu : SharedLocksToAdd) + addLock(InitialLockset, + llvm::make_unique<LockableFactEntry>(Mu, LK_Shared, Loc), + CapDiagKind, true); } for (const auto *CurrBlock : *SortedGraph) { @@ -2602,11 +2155,11 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { // issue the appropriate warning. // FIXME: the location here is not quite right. for (const auto &Lock : ExclusiveLocksAcquired) - ExpectedExitSet.addLock(FactMan, Lock, - LockData(D->getLocation(), LK_Exclusive)); + ExpectedExitSet.addLock(FactMan, llvm::make_unique<LockableFactEntry>( + Lock, LK_Exclusive, D->getLocation())); for (const auto &Lock : SharedLocksAcquired) - ExpectedExitSet.addLock(FactMan, Lock, - LockData(D->getLocation(), LK_Shared)); + ExpectedExitSet.addLock(FactMan, llvm::make_unique<LockableFactEntry>( + Lock, LK_Shared, D->getLocation())); for (const auto &Lock : LocksReleased) ExpectedExitSet.removeLock(FactMan, Lock); @@ -2616,13 +2169,10 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { LEK_LockedAtEndOfFunction, LEK_NotLockedAtEndOfFunction, false); -} - -} // end anonymous namespace + Handler.leaveFunction(CurrentFunction); +} -namespace clang { -namespace thread_safety { /// \brief Check a function's CFG for thread-safety violations. /// @@ -2647,4 +2197,4 @@ LockKind getLockKindFromAccessKind(AccessKind AK) { llvm_unreachable("Unknown AccessKind"); } -}} // end namespace clang::thread_safety +}} // end namespace clang::threadSafety |