From ea1f83385fab96b5274fb60402e7ce6d0925a2f8 Mon Sep 17 00:00:00 2001 From: DeLesley Hutchins Date: Mon, 28 Jul 2014 15:57:27 +0000 Subject: [PATCH] Thread Safety Analysis: Replace the old and broken SExpr with the new til::SExpr. This is a large patch, with many small changes to pretty printing and expression lowering to make the new SExpr representation equivalent in functionality to the old. llvm-svn: 214089 --- .../clang/Analysis/Analyses/ThreadSafety.h | 4 +- .../Analysis/Analyses/ThreadSafetyCommon.h | 33 +- .../clang/Analysis/Analyses/ThreadSafetyTIL.h | 116 ++- .../Analysis/Analyses/ThreadSafetyTraverse.h | 198 +++- .../Analysis/Analyses/ThreadSafetyUtil.h | 2 + clang/lib/Analysis/ThreadSafety.cpp | 891 ++++-------------- clang/lib/Analysis/ThreadSafetyCommon.cpp | 248 ++++- clang/lib/Analysis/ThreadSafetyTIL.cpp | 39 +- clang/lib/Sema/AnalysisBasedWarnings.cpp | 12 +- .../SemaCXX/warn-thread-safety-analysis.cpp | 151 ++- 10 files changed, 817 insertions(+), 877 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h b/clang/include/clang/Analysis/Analyses/ThreadSafety.h index b533c1db492e..fadc8b0bcec3 100644 --- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h +++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h @@ -24,7 +24,7 @@ #include "llvm/ADT/StringRef.h" namespace clang { -namespace thread_safety { +namespace threadSafety { /// This enum distinguishes between different kinds of operations that may /// need to be protected by locks. We use this enum in error handling. @@ -190,5 +190,5 @@ void runThreadSafetyAnalysis(AnalysisDeclContext &AC, /// of access. LockKind getLockKindFromAccessKind(AccessKind AK); -}} // end namespace clang::thread_safety +}} // end namespace clang::threadSafety #endif diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h index 09c614ca3e36..9dd2547248d5 100644 --- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h +++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h @@ -219,18 +219,16 @@ public: /// should be evaluated; multiple calling contexts can be chained together /// by the lock_returned attribute. struct CallingContext { + CallingContext *Prev; // The previous context; or 0 if none. const NamedDecl *AttrDecl; // The decl to which the attr is attached. const Expr *SelfArg; // Implicit object argument -- e.g. 'this' unsigned NumArgs; // Number of funArgs const Expr *const *FunArgs; // Function arguments - CallingContext *Prev; // The previous context; or 0 if none. bool SelfArrow; // is Self referred to with -> or .? - CallingContext(const NamedDecl *D = nullptr, const Expr *S = nullptr, - unsigned N = 0, const Expr *const *A = nullptr, - CallingContext *P = nullptr) - : AttrDecl(D), SelfArg(S), NumArgs(N), FunArgs(A), Prev(P), - SelfArrow(false) + CallingContext(CallingContext *P, const NamedDecl *D = nullptr) + : Prev(P), AttrDecl(D), SelfArg(nullptr), + NumArgs(0), FunArgs(nullptr), SelfArrow(false) {} }; @@ -242,6 +240,13 @@ public: SelfVar->setKind(til::Variable::VK_SFun); } + // Translate a clang expression in an attribute to a til::SExpr. + // Constructs the context from D, DeclExp, and SelfDecl. + til::SExpr *translateAttrExpr(const Expr *AttrExp, const NamedDecl *D, + const Expr *DeclExp, VarDecl *SelfDecl=nullptr); + + til::SExpr *translateAttrExpr(const Expr *AttrExp, CallingContext *Ctx); + // 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. @@ -262,7 +267,8 @@ private: CallingContext *Ctx) ; til::SExpr *translateCXXThisExpr(const CXXThisExpr *TE, CallingContext *Ctx); til::SExpr *translateMemberExpr(const MemberExpr *ME, CallingContext *Ctx); - til::SExpr *translateCallExpr(const CallExpr *CE, CallingContext *Ctx); + til::SExpr *translateCallExpr(const CallExpr *CE, CallingContext *Ctx, + const Expr *SelfE = nullptr); til::SExpr *translateCXXMemberCallExpr(const CXXMemberCallExpr *ME, CallingContext *Ctx); til::SExpr *translateCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE, @@ -280,10 +286,8 @@ private: til::SExpr *translateCastExpr(const CastExpr *CE, CallingContext *Ctx); til::SExpr *translateArraySubscriptExpr(const ArraySubscriptExpr *E, CallingContext *Ctx); - til::SExpr *translateConditionalOperator(const ConditionalOperator *C, - CallingContext *Ctx); - til::SExpr *translateBinaryConditionalOperator( - const BinaryConditionalOperator *C, CallingContext *Ctx); + til::SExpr *translateAbstractConditionalOperator( + const AbstractConditionalOperator *C, CallingContext *Ctx); til::SExpr *translateDeclStmt(const DeclStmt *S, CallingContext *Ctx); @@ -362,16 +366,19 @@ private: void mergePhiNodesBackEdge(const CFGBlock *Blk); private: + // Set to true when parsing capability expressions, which get translated + // inaccurately in order to hack around smart pointers etc. + static const bool CapabilityExprMode = true; + til::MemRegionRef Arena; til::Variable *SelfVar; // Variable to use for 'this'. May be null. - til::SCFG *Scfg; + til::SCFG *Scfg; StatementMap SMap; // Map from Stmt to TIL Variables LVarIndexMap LVarIdxMap; // Indices of clang local vars. std::vector BlockMap; // Map from clang to til BBs. std::vector BBInfo; // Extra information per BB. // Indexed by clang BlockID. - std::unique_ptr CallCtx; // Root calling context LVarDefinitionMap CurrentLVarMap; std::vector CurrentArguments; diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h index 8e4299ea70e8..d6d9a902e5bf 100644 --- a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h +++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h @@ -100,6 +100,7 @@ enum TIL_CastOpcode : unsigned char { CAST_truncNum, // truncate precision of numeric type CAST_toFloat, // convert to floating point type CAST_toInt, // convert to integer type + CAST_objToPtr // convert smart pointer to pointer (C++ only) }; const TIL_Opcode COP_Min = COP_Future; @@ -405,7 +406,8 @@ public: return Vs.reduceVariableRef(this); } - template typename C::CType compare(Variable* E, C& Cmp) { + template + typename C::CType compare(const Variable* E, C& Cmp) const { return Cmp.compareVariableRefs(this, E); } @@ -455,7 +457,7 @@ public: virtual SExpr *create() { return nullptr; } // Return the result of this future if it exists, otherwise return null. - SExpr *maybeGetResult() { + SExpr *maybeGetResult() const { return Result; } @@ -478,7 +480,8 @@ public: return Vs.traverse(Result, Ctx); } - template typename C::CType compare(Future* E, C& Cmp) { + template + typename C::CType compare(const Future* E, C& Cmp) const { if (!Result || !E->Result) return Cmp.comparePointers(this, E); return Cmp.compare(Result, E->Result); @@ -572,8 +575,9 @@ public: return Vs.reduceUndefined(*this); } - template typename C::CType compare(Undefined* E, C& Cmp) { - return Cmp.comparePointers(Cstmt, E->Cstmt); + template + typename C::CType compare(const Undefined* E, C& Cmp) const { + return Cmp.trueResult(); } private: @@ -593,7 +597,8 @@ public: return Vs.reduceWildcard(*this); } - template typename C::CType compare(Wildcard* E, C& Cmp) { + template + typename C::CType compare(const Wildcard* E, C& Cmp) const { return Cmp.trueResult(); } }; @@ -626,9 +631,10 @@ public: template typename V::R_SExpr traverse(V &Vs, typename V::R_Ctx Ctx); - template typename C::CType compare(Literal* E, C& Cmp) { - // TODO -- use value, not pointer equality - return Cmp.comparePointers(Cexpr, E->Cexpr); + template + typename C::CType compare(const Literal* E, C& Cmp) const { + // TODO: defer actual comparison to LiteralT + return Cmp.trueResult(); } private: @@ -727,7 +733,8 @@ public: return Vs.reduceLiteralPtr(*this); } - template typename C::CType compare(LiteralPtr* E, C& Cmp) { + template + typename C::CType compare(const LiteralPtr* E, C& Cmp) const { return Cmp.comparePointers(Cvdecl, E->Cvdecl); } @@ -769,7 +776,8 @@ public: return Vs.reduceFunction(*this, Nvd, E1); } - template typename C::CType compare(Function* E, C& Cmp) { + template + typename C::CType compare(const Function* E, C& Cmp) const { typename C::CType Ct = Cmp.compare(VarDecl->definition(), E->VarDecl->definition()); if (Cmp.notTrue(Ct)) @@ -824,7 +832,8 @@ public: return Vs.reduceSFunction(*this, Nvd, E1); } - template typename C::CType compare(SFunction* E, C& Cmp) { + template + typename C::CType compare(const SFunction* E, C& Cmp) const { Cmp.enterScope(variableDecl(), E->variableDecl()); typename C::CType Ct = Cmp.compare(body(), E->body()); Cmp.leaveScope(); @@ -859,7 +868,8 @@ public: return Vs.reduceCode(*this, Nt, Nb); } - template typename C::CType compare(Code* E, C& Cmp) { + template + typename C::CType compare(const Code* E, C& Cmp) const { typename C::CType Ct = Cmp.compare(returnType(), E->returnType()); if (Cmp.notTrue(Ct)) return Ct; @@ -894,7 +904,8 @@ public: return Vs.reduceField(*this, Nr, Nb); } - template typename C::CType compare(Field* E, C& Cmp) { + template + typename C::CType compare(const Field* E, C& Cmp) const { typename C::CType Ct = Cmp.compare(range(), E->range()); if (Cmp.notTrue(Ct)) return Ct; @@ -930,7 +941,8 @@ public: return Vs.reduceApply(*this, Nf, Na); } - template typename C::CType compare(Apply* E, C& Cmp) { + template + typename C::CType compare(const Apply* E, C& Cmp) const { typename C::CType Ct = Cmp.compare(fun(), E->fun()); if (Cmp.notTrue(Ct)) return Ct; @@ -958,7 +970,7 @@ public: SExpr *arg() { return Arg.get() ? Arg.get() : Sfun.get(); } const SExpr *arg() const { return Arg.get() ? Arg.get() : Sfun.get(); } - bool isDelegation() const { return Arg == nullptr; } + bool isDelegation() const { return Arg != nullptr; } template typename V::R_SExpr traverse(V &Vs, typename V::R_Ctx Ctx) { @@ -968,7 +980,8 @@ public: return Vs.reduceSApply(*this, Nf, Na); } - template typename C::CType compare(SApply* E, C& Cmp) { + template + typename C::CType compare(const SApply* E, C& Cmp) const { typename C::CType Ct = Cmp.compare(sfun(), E->sfun()); if (Cmp.notTrue(Ct) || (!arg() && !E->arg())) return Ct; @@ -989,7 +1002,7 @@ public: Project(SExpr *R, StringRef SName) : SExpr(COP_Project), Rec(R), SlotName(SName), Cvdecl(nullptr) { } - Project(SExpr *R, clang::ValueDecl *Cvd) + Project(SExpr *R, const clang::ValueDecl *Cvd) : SExpr(COP_Project), Rec(R), SlotName(Cvd->getName()), Cvdecl(Cvd) { } Project(const Project &P, SExpr *R) @@ -999,7 +1012,13 @@ public: SExpr *record() { return Rec.get(); } const SExpr *record() const { return Rec.get(); } - const clang::ValueDecl *clangValueDecl() const { return Cvdecl; } + const clang::ValueDecl *clangDecl() const { return Cvdecl; } + + bool isArrow() const { return (Flags & 0x01) != 0; } + void setArrow(bool b) { + if (b) Flags |= 0x01; + else Flags &= 0xFFFE; + } StringRef slotName() const { if (Cvdecl) @@ -1014,7 +1033,8 @@ public: return Vs.reduceProject(*this, Nr); } - template typename C::CType compare(Project* E, C& Cmp) { + template + typename C::CType compare(const Project* E, C& Cmp) const { typename C::CType Ct = Cmp.compare(record(), E->record()); if (Cmp.notTrue(Ct)) return Ct; @@ -1024,7 +1044,7 @@ public: private: SExprRef Rec; StringRef SlotName; - clang::ValueDecl *Cvdecl; + const clang::ValueDecl *Cvdecl; }; @@ -1048,7 +1068,8 @@ public: return Vs.reduceCall(*this, Nt); } - template typename C::CType compare(Call* E, C& Cmp) { + template + typename C::CType compare(const Call* E, C& Cmp) const { return Cmp.compare(target(), E->target()); } @@ -1082,7 +1103,8 @@ public: return Vs.reduceAlloc(*this, Nd); } - template typename C::CType compare(Alloc* E, C& Cmp) { + template + typename C::CType compare(const Alloc* E, C& Cmp) const { typename C::CType Ct = Cmp.compareIntegers(kind(), E->kind()); if (Cmp.notTrue(Ct)) return Ct; @@ -1111,7 +1133,8 @@ public: return Vs.reduceLoad(*this, Np); } - template typename C::CType compare(Load* E, C& Cmp) { + template + typename C::CType compare(const Load* E, C& Cmp) const { return Cmp.compare(pointer(), E->pointer()); } @@ -1142,7 +1165,8 @@ public: return Vs.reduceStore(*this, Np, Nv); } - template typename C::CType compare(Store* E, C& Cmp) { + template + typename C::CType compare(const Store* E, C& Cmp) const { typename C::CType Ct = Cmp.compare(destination(), E->destination()); if (Cmp.notTrue(Ct)) return Ct; @@ -1178,7 +1202,8 @@ public: return Vs.reduceArrayIndex(*this, Na, Ni); } - template typename C::CType compare(ArrayIndex* E, C& Cmp) { + template + typename C::CType compare(const ArrayIndex* E, C& Cmp) const { typename C::CType Ct = Cmp.compare(array(), E->array()); if (Cmp.notTrue(Ct)) return Ct; @@ -1215,7 +1240,8 @@ public: return Vs.reduceArrayAdd(*this, Na, Ni); } - template typename C::CType compare(ArrayAdd* E, C& Cmp) { + template + typename C::CType compare(const ArrayAdd* E, C& Cmp) const { typename C::CType Ct = Cmp.compare(array(), E->array()); if (Cmp.notTrue(Ct)) return Ct; @@ -1251,7 +1277,8 @@ public: return Vs.reduceUnaryOp(*this, Ne); } - template typename C::CType compare(UnaryOp* E, C& Cmp) { + template + typename C::CType compare(const UnaryOp* E, C& Cmp) const { typename C::CType Ct = Cmp.compareIntegers(unaryOpcode(), E->unaryOpcode()); if (Cmp.notTrue(Ct)) @@ -1295,7 +1322,8 @@ public: return Vs.reduceBinaryOp(*this, Ne0, Ne1); } - template typename C::CType compare(BinaryOp* E, C& Cmp) { + template + typename C::CType compare(const BinaryOp* E, C& Cmp) const { typename C::CType Ct = Cmp.compareIntegers(binaryOpcode(), E->binaryOpcode()); if (Cmp.notTrue(Ct)) @@ -1333,7 +1361,8 @@ public: return Vs.reduceCast(*this, Ne); } - template typename C::CType compare(Cast* E, C& Cmp) { + template + typename C::CType compare(const Cast* E, C& Cmp) const { typename C::CType Ct = Cmp.compareIntegers(castOpcode(), E->castOpcode()); if (Cmp.notTrue(Ct)) @@ -1386,7 +1415,8 @@ public: return Vs.reducePhi(*this, Nvs); } - template typename C::CType compare(Phi *E, C &Cmp) { + template + typename C::CType compare(const Phi *E, C &Cmp) const { // TODO: implement CFG comparisons return Cmp.comparePointers(this, E); } @@ -1503,7 +1533,8 @@ public: return Vs.reduceBasicBlock(*this, Nas, Nis, Nt); } - template typename C::CType compare(BasicBlock *E, C &Cmp) { + template + typename C::CType compare(const BasicBlock *E, C &Cmp) const { // TODO: implement CFG comparisons return Cmp.comparePointers(this, E); } @@ -1590,7 +1621,8 @@ public: return Vs.reduceSCFG(*this, Bbs); } - template typename C::CType compare(SCFG *E, C &Cmp) { + template + typename C::CType compare(const SCFG *E, C &Cmp) const { // TODO -- implement CFG comparisons return Cmp.comparePointers(this, E); } @@ -1623,7 +1655,8 @@ public: return Vs.reduceGoto(*this, Ntb); } - template typename C::CType compare(Goto *E, C &Cmp) { + template + typename C::CType compare(const Goto *E, C &Cmp) const { // TODO -- implement CFG comparisons return Cmp.comparePointers(this, E); } @@ -1668,7 +1701,8 @@ public: return Vs.reduceBranch(*this, Nc, Ntb, Nte); } - template typename C::CType compare(Branch *E, C &Cmp) { + template + typename C::CType compare(const Branch *E, C &Cmp) const { // TODO -- implement CFG comparisons return Cmp.comparePointers(this, E); } @@ -1698,7 +1732,8 @@ public: return Vs.reduceIdentifier(*this); } - template typename C::CType compare(Identifier* E, C& Cmp) { + template + typename C::CType compare(const Identifier* E, C& Cmp) const { return Cmp.compareStrings(name(), E->name()); } @@ -1737,7 +1772,8 @@ public: return Vs.reduceIfThenElse(*this, Nc, Nt, Ne); } - template typename C::CType compare(IfThenElse* E, C& Cmp) { + template + typename C::CType compare(const IfThenElse* E, C& Cmp) const { typename C::CType Ct = Cmp.compare(condition(), E->condition()); if (Cmp.notTrue(Ct)) return Ct; @@ -1784,7 +1820,8 @@ public: return Vs.reduceLet(*this, Nvd, E1); } - template typename C::CType compare(Let* E, C& Cmp) { + template + typename C::CType compare(const Let* E, C& Cmp) const { typename C::CType Ct = Cmp.compare(VarDecl->definition(), E->VarDecl->definition()); if (Cmp.notTrue(Ct)) @@ -1802,7 +1839,8 @@ private: -SExpr *getCanonicalVal(SExpr *E); +const SExpr *getCanonicalVal(const SExpr *E); +SExpr* simplifyToCanonicalVal(SExpr *E); void simplifyIncompleteArg(Variable *V, til::Phi *Ph); diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h index bc1490b4a448..811feaca016b 100644 --- a/clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h +++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h @@ -19,6 +19,8 @@ #include "ThreadSafetyTIL.h" +#include + namespace clang { namespace threadSafety { namespace til { @@ -423,7 +425,7 @@ protected: Self *self() { return reinterpret_cast(this); } public: - bool compareByCase(SExpr *E1, SExpr* E2) { + bool compareByCase(const SExpr *E1, const SExpr* E2) { switch (E1->opcode()) { #define TIL_OPCODE_DEF(X) \ case COP_##X: \ @@ -449,38 +451,86 @@ public: bool compareStrings (StringRef s, StringRef r) { return s == r; } bool comparePointers(const void* P, const void* Q) { return P == Q; } - bool compare(SExpr *E1, SExpr* E2) { + bool compare(const SExpr *E1, const SExpr* E2) { if (E1->opcode() != E2->opcode()) return false; return compareByCase(E1, E2); } // TODO -- handle alpha-renaming of variables - void enterScope(Variable* V1, Variable* V2) { } + void enterScope(const Variable* V1, const Variable* V2) { } void leaveScope() { } - bool compareVariableRefs(Variable* V1, Variable* V2) { + bool compareVariableRefs(const Variable* V1, const Variable* V2) { return V1 == V2; } - static bool compareExprs(SExpr *E1, SExpr* E2) { + static bool compareExprs(const SExpr *E1, const SExpr* E2) { EqualsComparator Eq; return Eq.compare(E1, E2); } }; + +class MatchComparator : public Comparator { +public: + // Result type for the comparison, e.g. bool for simple equality, + // or int for lexigraphic comparison (-1, 0, 1). Must have one value which + // denotes "true". + typedef bool CType; + + CType trueResult() { return true; } + bool notTrue(CType ct) { return !ct; } + + bool compareIntegers(unsigned i, unsigned j) { return i == j; } + bool compareStrings (StringRef s, StringRef r) { return s == r; } + bool comparePointers(const void* P, const void* Q) { return P == Q; } + + bool compare(const SExpr *E1, const SExpr* E2) { + // Wildcards match anything. + if (E1->opcode() == COP_Wildcard || E2->opcode() == COP_Wildcard) + return true; + // otherwise normal equality. + if (E1->opcode() != E2->opcode()) + return false; + return compareByCase(E1, E2); + } + + // TODO -- handle alpha-renaming of variables + void enterScope(const Variable* V1, const Variable* V2) { } + void leaveScope() { } + + bool compareVariableRefs(const Variable* V1, const Variable* V2) { + return V1 == V2; + } + + static bool compareExprs(const SExpr *E1, const SExpr* E2) { + MatchComparator Matcher; + return Matcher.compare(E1, E2); + } +}; + + + +inline std::ostream& operator<<(std::ostream& SS, llvm::StringRef R) { + return SS.write(R.data(), R.size()); +} + // Pretty printer for TIL expressions template class PrettyPrinter { private: bool Verbose; // Print out additional information bool Cleanup; // Omit redundant decls. + bool CStyle; // Print exprs in C-like syntax. public: - PrettyPrinter(bool V = false, bool C = true) : Verbose(V), Cleanup(C) { } + PrettyPrinter(bool V = false, bool C = true, bool CS = true) + : Verbose(V), Cleanup(C), CStyle(CS) + {} - static void print(SExpr *E, StreamType &SS) { + static void print(const SExpr *E, StreamType &SS) { Self printer; printer.printSExpr(E, SS, Prec_MAX); } @@ -502,7 +552,7 @@ protected: static const unsigned Prec_MAX = 6; // Return the precedence of a given node, for use in pretty printing. - unsigned precedence(SExpr *E) { + unsigned precedence(const SExpr *E) { switch (E->opcode()) { case COP_Future: return Prec_Atom; case COP_Undefined: return Prec_Atom; @@ -529,7 +579,7 @@ protected: case COP_UnaryOp: return Prec_Unary; case COP_BinaryOp: return Prec_Binary; - case COP_Cast: return Prec_Unary; + case COP_Cast: return Prec_Atom; case COP_SCFG: return Prec_Decl; case COP_BasicBlock: return Prec_MAX; @@ -544,7 +594,7 @@ protected: return Prec_MAX; } - void printBlockLabel(StreamType & SS, BasicBlock *BB, unsigned index) { + void printBlockLabel(StreamType & SS, const BasicBlock *BB, unsigned index) { if (!BB) { SS << "BB_null"; return; @@ -555,7 +605,7 @@ protected: SS << index; } - void printSExpr(SExpr *E, StreamType &SS, unsigned P) { + void printSExpr(const SExpr *E, StreamType &SS, unsigned P) { if (!E) { self()->printNull(SS); return; @@ -582,28 +632,28 @@ protected: SS << "#null"; } - void printFuture(Future *E, StreamType &SS) { + void printFuture(const Future *E, StreamType &SS) { self()->printSExpr(E->maybeGetResult(), SS, Prec_Atom); } - void printUndefined(Undefined *E, StreamType &SS) { + void printUndefined(const Undefined *E, StreamType &SS) { SS << "#undefined"; } - void printWildcard(Wildcard *E, StreamType &SS) { - SS << "_"; + void printWildcard(const Wildcard *E, StreamType &SS) { + SS << "*"; } template - void printLiteralT(LiteralT *E, StreamType &SS) { + void printLiteralT(const LiteralT *E, StreamType &SS) { SS << E->value(); } - void printLiteralT(LiteralT *E, StreamType &SS) { + void printLiteralT(const LiteralT *E, StreamType &SS) { SS << "'" << E->value() << "'"; } - void printLiteral(Literal *E, StreamType &SS) { + void printLiteral(const Literal *E, StreamType &SS) { if (E->clangExpr()) { SS << getSourceLiteralString(E->clangExpr()); return; @@ -685,13 +735,13 @@ protected: SS << "#lit"; } - void printLiteralPtr(LiteralPtr *E, StreamType &SS) { + void printLiteralPtr(const LiteralPtr *E, StreamType &SS) { SS << E->clangDecl()->getNameAsString(); } - void printVariable(Variable *V, StreamType &SS, bool IsVarDecl = false) { + void printVariable(const Variable *V, StreamType &SS, bool IsVarDecl = false) { if (!IsVarDecl && Cleanup) { - SExpr* E = getCanonicalVal(V); + const SExpr* E = getCanonicalVal(V); if (E != V) { printSExpr(E, SS, Prec_Atom); return; @@ -699,11 +749,13 @@ protected: } if (V->kind() == Variable::VK_LetBB) SS << V->name() << V->getBlockID() << "_" << V->getID(); + else if (CStyle && V->kind() == Variable::VK_SFun) + SS << "this"; else SS << V->name() << V->getID(); } - void printFunction(Function *E, StreamType &SS, unsigned sugared = 0) { + void printFunction(const Function *E, StreamType &SS, unsigned sugared = 0) { switch (sugared) { default: SS << "\\("; // Lambda @@ -719,7 +771,7 @@ protected: SS << ": "; self()->printSExpr(E->variableDecl()->definition(), SS, Prec_MAX); - SExpr *B = E->body(); + const SExpr *B = E->body(); if (B && B->opcode() == COP_Function) self()->printFunction(cast(B), SS, 2); else { @@ -728,29 +780,29 @@ protected: } } - void printSFunction(SFunction *E, StreamType &SS) { + void printSFunction(const SFunction *E, StreamType &SS) { SS << "@"; self()->printVariable(E->variableDecl(), SS, true); SS << " "; self()->printSExpr(E->body(), SS, Prec_Decl); } - void printCode(Code *E, StreamType &SS) { + void printCode(const Code *E, StreamType &SS) { SS << ": "; self()->printSExpr(E->returnType(), SS, Prec_Decl-1); SS << " -> "; self()->printSExpr(E->body(), SS, Prec_Decl); } - void printField(Field *E, StreamType &SS) { + void printField(const Field *E, StreamType &SS) { SS << ": "; self()->printSExpr(E->range(), SS, Prec_Decl-1); SS << " = "; self()->printSExpr(E->body(), SS, Prec_Decl); } - void printApply(Apply *E, StreamType &SS, bool sugared = false) { - SExpr *F = E->fun(); + void printApply(const Apply *E, StreamType &SS, bool sugared = false) { + const SExpr *F = E->fun(); if (F->opcode() == COP_Apply) { printApply(cast(F), SS, true); SS << ", "; @@ -763,7 +815,7 @@ protected: SS << ")$"; } - void printSApply(SApply *E, StreamType &SS) { + void printSApply(const SApply *E, StreamType &SS) { self()->printSExpr(E->sfun(), SS, Prec_Postfix); if (E->isDelegation()) { SS << "@("; @@ -772,14 +824,36 @@ protected: } } - void printProject(Project *E, StreamType &SS) { + void printProject(const Project *E, StreamType &SS) { + if (CStyle) { + // Omit the this-> + if (const SApply *SAP = dyn_cast(E->record())) { + if (const Variable *V = dyn_cast(SAP->sfun())) { + if (!SAP->isDelegation() && V->kind() == Variable::VK_SFun) { + SS << E->slotName(); + return; + } + } + } + if (isa(E->record())) { + // handle existentials + SS << "&"; + SS << E->clangDecl()->getQualifiedNameAsString(); + return; + } + } self()->printSExpr(E->record(), SS, Prec_Postfix); - SS << "."; + if (CStyle && E->isArrow()) { + SS << "->"; + } + else { + SS << "."; + } SS << E->slotName(); } - void printCall(Call *E, StreamType &SS) { - SExpr *T = E->target(); + void printCall(const Call *E, StreamType &SS) { + const SExpr *T = E->target(); if (T->opcode() == COP_Apply) { self()->printApply(cast(T), SS, true); SS << ")"; @@ -790,52 +864,60 @@ protected: } } - void printAlloc(Alloc *E, StreamType &SS) { + void printAlloc(const Alloc *E, StreamType &SS) { SS << "new "; self()->printSExpr(E->dataType(), SS, Prec_Other-1); } - void printLoad(Load *E, StreamType &SS) { + void printLoad(const Load *E, StreamType &SS) { self()->printSExpr(E->pointer(), SS, Prec_Postfix); - SS << "^"; + if (!CStyle) + SS << "^"; } - void printStore(Store *E, StreamType &SS) { + void printStore(const Store *E, StreamType &SS) { self()->printSExpr(E->destination(), SS, Prec_Other-1); SS << " := "; self()->printSExpr(E->source(), SS, Prec_Other-1); } - void printArrayIndex(ArrayIndex *E, StreamType &SS) { + void printArrayIndex(const ArrayIndex *E, StreamType &SS) { self()->printSExpr(E->array(), SS, Prec_Postfix); SS << "["; self()->printSExpr(E->index(), SS, Prec_MAX); SS << "]"; } - void printArrayAdd(ArrayAdd *E, StreamType &SS) { + void printArrayAdd(const ArrayAdd *E, StreamType &SS) { self()->printSExpr(E->array(), SS, Prec_Postfix); SS << " + "; self()->printSExpr(E->index(), SS, Prec_Atom); } - void printUnaryOp(UnaryOp *E, StreamType &SS) { + void printUnaryOp(const UnaryOp *E, StreamType &SS) { SS << getUnaryOpcodeString(E->unaryOpcode()); self()->printSExpr(E->expr(), SS, Prec_Unary); } - void printBinaryOp(BinaryOp *E, StreamType &SS) { + void printBinaryOp(const BinaryOp *E, StreamType &SS) { self()->printSExpr(E->expr0(), SS, Prec_Binary-1); SS << " " << getBinaryOpcodeString(E->binaryOpcode()) << " "; self()->printSExpr(E->expr1(), SS, Prec_Binary-1); } - void printCast(Cast *E, StreamType &SS) { - SS << "%"; + void printCast(const Cast *E, StreamType &SS) { + if (!CStyle) { + SS << "cast["; + SS << E->castOpcode(); + SS << "]("; + self()->printSExpr(E->expr(), SS, Prec_Unary); + SS << ")"; + return; + } self()->printSExpr(E->expr(), SS, Prec_Unary); } - void printSCFG(SCFG *E, StreamType &SS) { + void printSCFG(const SCFG *E, StreamType &SS) { SS << "CFG {\n"; for (auto BBI : *E) { printBasicBlock(BBI, SS); @@ -844,7 +926,7 @@ protected: newline(SS); } - void printBasicBlock(BasicBlock *E, StreamType &SS) { + void printBasicBlock(const BasicBlock *E, StreamType &SS) { SS << "BB_" << E->blockID() << ":"; if (E->parent()) SS << " BB_" << E->parent()->blockID(); @@ -867,7 +949,7 @@ protected: SS << ";"; newline(SS); } - SExpr *T = E->terminator(); + const SExpr *T = E->terminator(); if (T) { self()->printSExpr(T, SS, Prec_MAX); SS << ";"; @@ -876,7 +958,7 @@ protected: newline(SS); } - void printPhi(Phi *E, StreamType &SS) { + void printPhi(const Phi *E, StreamType &SS) { SS << "phi("; if (E->status() == Phi::PH_SingleVal) self()->printSExpr(E->values()[0], SS, Prec_MAX); @@ -891,12 +973,12 @@ protected: SS << ")"; } - void printGoto(Goto *E, StreamType &SS) { + void printGoto(const Goto *E, StreamType &SS) { SS << "goto "; printBlockLabel(SS, E->targetBlock(), E->index()); } - void printBranch(Branch *E, StreamType &SS) { + void printBranch(const Branch *E, StreamType &SS) { SS << "branch ("; self()->printSExpr(E->condition(), SS, Prec_MAX); SS << ") "; @@ -905,11 +987,19 @@ protected: printBlockLabel(SS, E->elseBlock(), E->elseIndex()); } - void printIdentifier(Identifier *E, StreamType &SS) { + void printIdentifier(const Identifier *E, StreamType &SS) { SS << E->name(); } - void printIfThenElse(IfThenElse *E, StreamType &SS) { + void printIfThenElse(const IfThenElse *E, StreamType &SS) { + if (CStyle) { + printSExpr(E->condition(), SS, Prec_Unary); + SS << " ? "; + printSExpr(E->thenExpr(), SS, Prec_Unary); + SS << " : "; + printSExpr(E->elseExpr(), SS, Prec_Unary); + return; + } SS << "if ("; printSExpr(E->condition(), SS, Prec_MAX); SS << ") then "; @@ -918,7 +1008,7 @@ protected: printSExpr(E->elseExpr(), SS, Prec_Other); } - void printLet(Let *E, StreamType &SS) { + void printLet(const Let *E, StreamType &SS) { SS << "let "; printVariable(E->variableDecl(), SS, true); SS << " = "; @@ -929,6 +1019,10 @@ protected: }; +class StdPrinter : public PrettyPrinter { }; + + + } // end namespace til } // end namespace threadSafety } // end namespace clang diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyUtil.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyUtil.h index 31200a3a7253..0861eade76b8 100644 --- a/clang/include/clang/Analysis/Analyses/ThreadSafetyUtil.h +++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyUtil.h @@ -144,7 +144,9 @@ public: } iterator begin() { return Data; } + const_iterator begin() const { return Data; } iterator end() { return Data + Size; } + const_iterator end() const { return Data + Size; } const_iterator cbegin() const { return Data; } const_iterator cend() const { return Data + Size; } diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp index 11df61f80fa0..f4f174d59bc7 100644 --- a/clang/lib/Analysis/ThreadSafety.cpp +++ b/clang/lib/Analysis/ThreadSafety.cpp @@ -40,682 +40,107 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/raw_ostream.h" #include +#include +#include #include #include -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 TILPrinter : + public til::PrettyPrinter {}; - 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 +/// 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(); - public: - SExprNode(ExprOp O, unsigned F, const void* D) - : Op(static_cast(O)), - Flags(static_cast(F)), Sz(1), Data(D) - { } - - unsigned size() const { return Sz; } - void setSize(unsigned S) { Sz = S; } - - ExprOp kind() const { return static_cast(Op); } - - const NamedDecl* getNamedDecl() const { - assert(Op == EOP_NVar || Op == EOP_LVar || Op == EOP_Dot); - return reinterpret_cast(Data); - } - - const NamedDecl* getFunctionDecl() const { - assert(Op == EOP_Call || Op == EOP_MCall); - return reinterpret_cast(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); - } - }; + // FIXME: add a note about the attribute location in MutexExp or D + if (Loc.isValid()) + Handler.handleInvalidLockExp(Kind, Loc); +} - /// \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. +// Various helper functions on til::SExpr +namespace sx { - CallingContext(const NamedDecl *D) - : AttrDecl(D), SelfArg(nullptr), SelfArrow(false), NumArgs(0), - FunArgs(nullptr), PrevCtx(nullptr) {} - }; +bool isUniversal(const til::SExpr *E) { + return isa(E); +} - typedef SmallVector NodeVector; +bool equals(const til::SExpr *E1, const til::SExpr *E2) { + return til::EqualsComparator::compareExprs(E1, E2); +} -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; +const til::SExpr* ignorePtrCasts(const til::SExpr *E) { + if (auto *CE = dyn_cast(E)) { + if (CE->castOpcode() == til::CAST_objToPtr) + return CE->expr(); } + return E; +} - unsigned makeNop() { - return make(EOP_Nop); - } +bool matches(const til::SExpr *E1, const til::SExpr *E2) { + // We treat a top-level wildcard as the "univsersal" lock. + // It matches everything for the purpose of checking locks, but not + // for unlocking them. + if (isa(E1)) + return isa(E2); + if (isa(E2)) + return isa(E1); - unsigned makeWildcard() { - return make(EOP_Wildcard); - } + return til::MatchComparator::compareExprs(E1, E2); +} - 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(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(Exp)) { - const NamedDecl *ND = cast(DRE->getDecl()->getCanonicalDecl()); - const ParmVarDecl *PV = dyn_cast_or_null(ND); - if (PV) { - const FunctionDecl *FD = - cast(PV->getDeclContext())->getCanonicalDecl(); - unsigned i = PV->getFunctionScopeIndex(); - - 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(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(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(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()) { - 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(Exp)) { - const FunctionDecl *FD = CE->getDirectCallee()->getMostRecentDecl(); - if (LockReturnedAttr* At = FD->getAttr()) { - 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(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(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(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(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(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(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(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(Exp)) { - return buildSExpr(CE->getSubExpr(), CallCtx, NDeref); - } else if (const ParenExpr *PE = dyn_cast(Exp)) { - return buildSExpr(PE->getSubExpr(), CallCtx, NDeref); - } else if (const ExprWithCleanups *EWC = dyn_cast(Exp)) { - return buildSExpr(EWC->getSubExpr(), CallCtx, NDeref); - } else if (const CXXBindTemporaryExpr *E = dyn_cast(Exp)) { - return buildSExpr(E->getSubExpr(), CallCtx, NDeref); - } else if (isa(Exp) || - isa(Exp) || - isa(Exp) || - isa(Exp) || - isa(Exp) || - isa(Exp) || - isa(Exp) || - isa(Exp) || - isa(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(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(DeclExp)) { - CallCtx.SelfArg = ME->getBase(); - CallCtx.SelfArrow = ME->isArrow(); - } else if (const CXXMemberCallExpr *CE = - dyn_cast(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(DeclExp)) { - CallCtx.NumArgs = CE->getNumArgs(); - CallCtx.FunArgs = CE->getArgs(); - } else if (const CXXConstructExpr *CE = - dyn_cast(DeclExp)) { - CallCtx.SelfArg = nullptr; // Will be set below - CallCtx.NumArgs = CE->getNumArgs(); - CallCtx.FunArgs = CE->getArgs(); - } else if (D && isa(D)) { - // There's no such thing as a "destructor call" in the AST. - CallCtx.SelfArg = DeclExp; - } - - // 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; - } +bool partiallyMatches(const til::SExpr *E1, const til::SExpr *E2) { + auto *PE1 = dyn_cast_or_null(E1); + if (!PE1) 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]); + auto *PE2 = dyn_cast_or_null(E2); + if (!PE2) return false; - } + return PE1->clangDecl() == PE2->clangDecl(); +} - /// \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 ""; +std::string toString(const til::SExpr *E) { + std::stringstream ss; + til::StdPrinter::print(E, ss); + return ss.str(); +} + +bool shouldIgnore(const til::SExpr *E) { + if (!E) + return true; + // Trap mutex expressions like nullptr, or 0. + // Any literal value is nonsense. + if (isa(E)) + return true; + return false; +} + +} // end namespace sx - 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; - 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; kgetFunctionDecl()) - S += D->getNameAsString() + "("; - else - S += "#("; - unsigned NumArgs = N->arity()-1; - unsigned ci = getNextSibling(i+1); - for (unsigned k=0; karity(); - 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 { +class MutexIDList : public SmallVector { 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 til::SExpr *E) { + iterator It = std::find_if(begin(), end(), [=](const til::SExpr *E2) { + return sx::equals(E, E2); + }); + if (It == end()) + push_back(E); } }; @@ -735,15 +160,15 @@ struct LockData { LockKind LKind; bool Asserted; // for asserted locks bool Managed; // for ScopedLockable objects - SExpr UnderlyingMutex; // for ScopedLockable objects + const til::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()) + UnderlyingMutex(nullptr) {} - LockData(SourceLocation AcquireLoc, LockKind LKind, const SExpr &Mu) + LockData(SourceLocation AcquireLoc, LockKind LKind, const til::SExpr *Mu) : AcquireLoc(AcquireLoc), LKind(LKind), Asserted(false), Managed(false), UnderlyingMutex(Mu) {} @@ -771,10 +196,10 @@ struct LockData { /// in the program execution. Currently, this is information regarding a lock /// that is held at that point. struct FactEntry { - SExpr MutID; + const til::SExpr *MutID; LockData LDat; - FactEntry(const SExpr& M, const LockData& L) + FactEntry(const til::SExpr* M, const LockData& L) : MutID(M), LDat(L) { } }; @@ -789,8 +214,8 @@ private: std::vector Facts; public: - FactID newLock(const SExpr& M, const LockData& L) { - Facts.push_back(FactEntry(M,L)); + FactID newLock(const til::SExpr *M, const LockData& L) { + Facts.push_back(FactEntry(M, L)); return static_cast(Facts.size() - 1); } @@ -824,66 +249,67 @@ public: bool isEmpty() const { return FactIDs.size() == 0; } - FactID addLock(FactManager& FM, const SExpr& M, const LockData& L) { + FactID addLock(FactManager& FM, const til::SExpr *M, const LockData& L) { FactID F = FM.newLock(M, L); FactIDs.push_back(F); return F; } - bool removeLock(FactManager& FM, const SExpr& M) { + bool removeLock(FactManager& FM, const til::SExpr *M) { 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 (sx::matches(FM[FactIDs[i]].MutID, M)) { FactIDs[i] = FactIDs[n-1]; FactIDs.pop_back(); return true; } } - if (FM[FactIDs[n-1]].MutID.matches(M)) { + if (sx::matches(FM[FactIDs[n-1]].MutID, M)) { FactIDs.pop_back(); return true; } return false; } - iterator findLockIter(FactManager &FM, const SExpr &M) { + iterator findLockIter(FactManager &FM, const til::SExpr *M) { return std::find_if(begin(), end(), [&](FactID ID) { - return FM[ID].MutID.matches(M); + return sx::matches(FM[ID].MutID, M); }); } - LockData *findLock(FactManager &FM, const SExpr &M) const { + LockData *findLock(FactManager &FM, const til::SExpr *M) const { auto I = std::find_if(begin(), end(), [&](FactID ID) { - return FM[ID].MutID.matches(M); + return sx::matches(FM[ID].MutID, M); }); return I != end() ? &FM[*I].LDat : nullptr; } - LockData *findLockUniv(FactManager &FM, const SExpr &M) const { + LockData *findLockUniv(FactManager &FM, const til::SExpr *M) const { auto I = std::find_if(begin(), end(), [&](FactID ID) -> bool { - const SExpr &Expr = FM[ID].MutID; - return Expr.isUniversal() || Expr.matches(M); + const til::SExpr *E = FM[ID].MutID; + return sx::isUniversal(E) || sx::matches(E, M); }); return I != end() ? &FM[*I].LDat : nullptr; } - FactEntry *findPartialMatch(FactManager &FM, const SExpr &M) const { + FactEntry *findPartialMatch(FactManager &FM, const til::SExpr *M) const { auto I = std::find_if(begin(), end(), [&](FactID ID) { - return FM[ID].MutID.partiallyMatches(M); + return sx::partiallyMatches(FM[ID].MutID, M); }); return I != end() ? &FM[*I] : nullptr; } }; + /// A Lockset maps each SExpr (defined above) to information about how it has /// been locked. -typedef llvm::ImmutableMap Lockset; +typedef llvm::ImmutableMap Lockset; typedef llvm::ImmutableMap LocalVarContext; class LocalVariableMap; @@ -1408,18 +834,24 @@ static void findBlockLocations(CFG *CFGraph, class ThreadSafetyAnalyzer { friend class BuildLockset; + llvm::BumpPtrAllocator Bpa; + threadSafety::til::MemRegionRef Arena; + threadSafety::SExprBuilder SxBuilder; + ThreadSafetyHandler &Handler; LocalVariableMap LocalVarMap; FactManager FactMan; std::vector BlockInfo; public: - ThreadSafetyAnalyzer(ThreadSafetyHandler &H) : Handler(H) {} + ThreadSafetyAnalyzer(ThreadSafetyHandler &H) + : Arena(&Bpa), SxBuilder(Arena), Handler(H) {} - void addLock(FactSet &FSet, const SExpr &Mutex, const LockData &LDat, + void addLock(FactSet &FSet, const til::SExpr *Mutex, const LockData &LDat, StringRef DiagKind); - void removeLock(FactSet &FSet, const SExpr &Mutex, SourceLocation UnlockLoc, - bool FullyRemove, LockKind Kind, StringRef DiagKind); + void removeLock(FactSet &FSet, const til::SExpr *Mutex, + SourceLocation UnlockLoc, bool FullyRemove, LockKind Kind, + StringRef DiagKind); template void getMutexIDs(MutexIDList &Mtxs, AttrType *Attr, Expr *Exp, @@ -1533,16 +965,16 @@ ClassifyDiagnostic(const AttrTy *A) { /// \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, +void ThreadSafetyAnalyzer::addLock(FactSet &FSet, const til::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()) + if (sx::shouldIgnore(Mutex)) return; if (FSet.findLock(FactMan, Mutex)) { if (!LDat.Asserted) - Handler.handleDoubleLock(DiagKind, Mutex.toString(), LDat.AcquireLoc); + Handler.handleDoubleLock(DiagKind, sx::toString(Mutex), LDat.AcquireLoc); } else { FSet.addLock(FactMan, Mutex, LDat); } @@ -1552,28 +984,28 @@ void ThreadSafetyAnalyzer::addLock(FactSet &FSet, const SExpr &Mutex, /// \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 til::SExpr *Mutex, SourceLocation UnlockLoc, bool FullyRemove, LockKind ReceivedKind, StringRef DiagKind) { - if (Mutex.shouldIgnore()) + if (sx::shouldIgnore(Mutex)) return; const LockData *LDat = FSet.findLock(FactMan, Mutex); if (!LDat) { - Handler.handleUnmatchedUnlock(DiagKind, Mutex.toString(), UnlockLoc); + Handler.handleUnmatchedUnlock(DiagKind, sx::toString(Mutex), 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); + Handler.handleIncorrectUnlockKind(DiagKind, sx::toString(Mutex), + LDat->LKind, ReceivedKind, UnlockLoc); return; } - if (LDat->UnderlyingMutex.isValid()) { + if (LDat->UnderlyingMutex) { // This is scoped lockable object, which manages the real mutex. if (FullyRemove) { // We're destroying the managing object. @@ -1585,7 +1017,7 @@ void ThreadSafetyAnalyzer::removeLock(FactSet &FSet, const SExpr &Mutex, // managing object. Warn on dual release. if (!FSet.findLock(FactMan, LDat->UnderlyingMutex)) { Handler.handleUnmatchedUnlock( - DiagKind, LDat->UnderlyingMutex.toString(), UnlockLoc); + DiagKind, sx::toString(LDat->UnderlyingMutex), UnlockLoc); } FSet.removeLock(FactMan, LDat->UnderlyingMutex); return; @@ -1603,20 +1035,25 @@ void ThreadSafetyAnalyzer::getMutexIDs(MutexIDList &Mtxs, AttrType *Attr, 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 + til::SExpr *Mu = SxBuilder.translateAttrExpr(nullptr, D, Exp, SelfDecl); + if (Mu && isa(Mu)) { + warnInvalidLock(Handler, nullptr, D, Exp, ClassifyDiagnostic(Attr)); + return; + } + //else + if (Mu) Mtxs.push_back_nodup(Mu); 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 + til::SExpr *Mu = SxBuilder.translateAttrExpr(Arg, D, Exp, SelfDecl); + if (Mu && isa(Mu)) { + warnInvalidLock(Handler, nullptr, D, Exp, ClassifyDiagnostic(Attr)); + return; + } + //else + if (Mu) Mtxs.push_back_nodup(Mu); } } @@ -1845,11 +1282,12 @@ void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, StringRef DiagKind) { LockKind LK = getLockKindFromAccessKind(AK); - SExpr Mutex(MutexExp, Exp, D); - if (!Mutex.isValid()) { - SExpr::warnInvalidLock(Analyzer->Handler, MutexExp, Exp, D, DiagKind); + til::SExpr *Mutex = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp); + if (!Mutex) { + // TODO: invalid locks? + // warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, DiagKind); return; - } else if (Mutex.shouldIgnore()) { + } else if (sx::shouldIgnore(Mutex)) { return; } @@ -1861,38 +1299,42 @@ void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, if (FEntry) { // Warn that there's no precise match. LDat = &FEntry->LDat; - std::string PartMatchStr = FEntry->MutID.toString(); + std::string PartMatchStr = sx::toString(FEntry->MutID); StringRef PartMatchName(PartMatchStr); - Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Mutex.toString(), + Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, + sx::toString(Mutex), LK, Exp->getExprLoc(), &PartMatchName); } else { // Warn that there's no match at all. - Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Mutex.toString(), + Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, + sx::toString(Mutex), LK, Exp->getExprLoc()); } 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()); + Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, + sx::toString(Mutex), + LK, Exp->getExprLoc()); } /// \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); + til::SExpr *Mutex = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp); + if (!Mutex) { + // TODO: invalid locks? + // warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, DiagKind); return; } LockData* LDat = FSet.findLock(Analyzer->FactMan, Mutex); if (LDat) Analyzer->Handler.handleFunExcludesLock( - DiagKind, D->getNameAsString(), Mutex.toString(), Exp->getExprLoc()); + DiagKind, D->getNameAsString(), sx::toString(Mutex), Exp->getExprLoc()); } /// \brief Checks guarded_by and pt_guarded_by attributes. @@ -2085,7 +1527,7 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) { if (isScopedVar) { SourceLocation MLoc = VD->getLocation(); DeclRefExpr DRE(VD, false, VD->getType(), VK_LValue, VD->getLocation()); - SExpr SMutex(&DRE, nullptr, nullptr); + til::SExpr *SMutex = Analyzer->SxBuilder.translateAttrExpr(&DRE, nullptr); for (const auto &M : ExclusiveLocksToAdd) Analyzer->addLock(FSet, SMutex, LockData(MLoc, LK_Exclusive, M), @@ -2093,6 +1535,12 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) { for (const auto &M : SharedLocksToAdd) Analyzer->addLock(FSet, SMutex, LockData(MLoc, LK_Shared, M), CapDiagKind); + + // handle corner case where the underlying mutex is invalid + if (ExclusiveLocksToAdd.size() == 0 && SharedLocksToAdd.size() == 0) { + Analyzer->addLock(FSet, SMutex, LockData(MLoc, LK_Exclusive), + CapDiagKind); + } } // Remove locks. @@ -2254,14 +1702,14 @@ 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 til::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(), + Handler.handleExclusiveAndShared("mutex", sx::toString(FSet2Mutex), LDat2.AcquireLoc, LDat1->AcquireLoc); if (Modify && LDat1->LKind != LK_Exclusive) { // Take the exclusive lock, which is the one in FSet2. @@ -2273,40 +1721,42 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &FSet1, *I1 = Fact; } } else { - if (LDat2.UnderlyingMutex.isValid()) { + if (LDat2.UnderlyingMutex) { 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(), + sx::toString(LDat2.UnderlyingMutex), LDat2.AcquireLoc, JoinLoc, LEK1); } } - else if (!LDat2.Managed && !FSet2Mutex.isUniversal() && !LDat2.Asserted) - Handler.handleMutexHeldEndOfScope("mutex", FSet2Mutex.toString(), + else if (!LDat2.Managed && !sx::isUniversal(FSet2Mutex) && + !LDat2.Asserted) + Handler.handleMutexHeldEndOfScope("mutex", sx::toString(FSet2Mutex), LDat2.AcquireLoc, JoinLoc, LEK1); } } // Find locks in FSet1 that are not in FSet2, and remove them. for (const auto &Fact : FSet1Orig) { - const SExpr &FSet1Mutex = FactMan[Fact].MutID; + const til::SExpr *FSet1Mutex = FactMan[Fact].MutID; const LockData &LDat1 = FactMan[Fact].LDat; if (!FSet2.findLock(FactMan, FSet1Mutex)) { - if (LDat1.UnderlyingMutex.isValid()) { + if (LDat1.UnderlyingMutex) { 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(), + sx::toString(LDat1.UnderlyingMutex), LDat1.AcquireLoc, JoinLoc, LEK1); } } - else if (!LDat1.Managed && !FSet1Mutex.isUniversal() && !LDat1.Asserted) - Handler.handleMutexHeldEndOfScope("mutex", FSet1Mutex.toString(), + else if (!LDat1.Managed && !sx::isUniversal(FSet1Mutex) && + !LDat1.Asserted) + Handler.handleMutexHeldEndOfScope("mutex", sx::toString(FSet1Mutex), LDat1.AcquireLoc, JoinLoc, LEK2); if (Modify) FSet1.removeLock(FactMan, FSet1Mutex); @@ -2618,11 +2068,6 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { false); } -} // end anonymous namespace - - -namespace clang { -namespace thread_safety { /// \brief Check a function's CFG for thread-safety violations. /// @@ -2647,4 +2092,4 @@ LockKind getLockKindFromAccessKind(AccessKind AK) { llvm_unreachable("Unknown AccessKind"); } -}} // end namespace clang::thread_safety +}} // end namespace clang::threadSafety diff --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp b/clang/lib/Analysis/ThreadSafetyCommon.cpp index da88b78126fa..56feba706010 100644 --- a/clang/lib/Analysis/ThreadSafetyCommon.cpp +++ b/clang/lib/Analysis/ThreadSafetyCommon.cpp @@ -91,6 +91,102 @@ til::SCFG *SExprBuilder::buildCFG(CFGWalker &Walker) { } + +inline bool isCalleeArrow(const Expr *E) { + const MemberExpr *ME = dyn_cast(E->IgnoreParenCasts()); + return ME ? ME->isArrow() : false; +} + + +/// \brief Translate a clang expression in an attribute to a til::SExpr. +/// Constructs the context from D, DeclExp, and SelfDecl. +/// +/// \param AttrExp The expression to translate. +/// \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. +til::SExpr *SExprBuilder::translateAttrExpr(const Expr *AttrExp, + const NamedDecl *D, + const Expr *DeclExp, + VarDecl *SelfDecl) { + // If we are processing a raw attribute expression, with no substitutions. + 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 (const MemberExpr *ME = dyn_cast(DeclExp)) { + Ctx.SelfArg = ME->getBase(); + Ctx.SelfArrow = ME->isArrow(); + } else if (const CXXMemberCallExpr *CE = + dyn_cast(DeclExp)) { + Ctx.SelfArg = CE->getImplicitObjectArgument(); + Ctx.SelfArrow = isCalleeArrow(CE->getCallee()); + Ctx.NumArgs = CE->getNumArgs(); + Ctx.FunArgs = CE->getArgs(); + } else if (const CallExpr *CE = dyn_cast(DeclExp)) { + Ctx.NumArgs = CE->getNumArgs(); + Ctx.FunArgs = CE->getArgs(); + } else if (const CXXConstructExpr *CE = + dyn_cast(DeclExp)) { + Ctx.SelfArg = nullptr; // Will be set below + Ctx.NumArgs = CE->getNumArgs(); + Ctx.FunArgs = CE->getArgs(); + } else if (D && isa(D)) { + // There's no such thing as a "destructor call" in the AST. + Ctx.SelfArg = DeclExp; + } + + // Hack to handle constructors, where self cannot be recovered from + // the expression. + if (SelfDecl && !Ctx.SelfArg) { + DeclRefExpr SelfDRE(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 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(Ctx.SelfArg, nullptr); + else // For most attributes. + return translateAttrExpr(AttrExp, &Ctx); +} + + +/// \brief Translate a clang expression in an attribute to a til::SExpr. +// This assumes a CallingContext has already been created. +til::SExpr *SExprBuilder::translateAttrExpr(const Expr *AttrExp, + CallingContext *Ctx) { + if (const StringLiteral* SLit = dyn_cast_or_null(AttrExp)) { + if (SLit->getString() == StringRef("*")) + // The "*" expr is a universal lock, which essentially turns off + // checks until it is removed from the lockset. + return new (Arena) til::Wildcard(); + else + // Ignore other string literals for now. + return nullptr; + } + + til::SExpr *E = translate(AttrExp, Ctx); + + // Hack to deal with smart pointers -- strip off top-level pointer casts. + if (auto *CE = dyn_cast_or_null(E)) { + if (CE->castOpcode() == til::CAST_objToPtr) + return CE->expr(); + } + return E; +} + + + // 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. @@ -125,9 +221,10 @@ til::SExpr *SExprBuilder::translate(const Stmt *S, CallingContext *Ctx) { case Stmt::ArraySubscriptExprClass: return translateArraySubscriptExpr(cast(S), Ctx); case Stmt::ConditionalOperatorClass: - return translateConditionalOperator(cast(S), Ctx); + return translateAbstractConditionalOperator( + cast(S), Ctx); case Stmt::BinaryConditionalOperatorClass: - return translateBinaryConditionalOperator( + return translateAbstractConditionalOperator( cast(S), Ctx); // We treat these as no-ops @@ -162,6 +259,7 @@ til::SExpr *SExprBuilder::translate(const Stmt *S, CallingContext *Ctx) { } + til::SExpr *SExprBuilder::translateDeclRefExpr(const DeclRefExpr *DRE, CallingContext *Ctx) { const ValueDecl *VD = cast(DRE->getDecl()->getCanonicalDecl()); @@ -197,17 +295,72 @@ til::SExpr *SExprBuilder::translateCXXThisExpr(const CXXThisExpr *TE, } +const ValueDecl *getValueDeclFromSExpr(const til::SExpr *E) { + if (auto *V = dyn_cast(E)) + return V->clangDecl(); + if (auto *P = dyn_cast(E)) + return P->clangDecl(); + if (auto *L = dyn_cast(E)) + return L->clangDecl(); + return 0; +} + +bool hasCppPointerType(const til::SExpr *E) { + auto *VD = getValueDeclFromSExpr(E); + if (VD && VD->getType()->isPointerType()) + return true; + if (auto *C = dyn_cast(E)) + return C->castOpcode() == til::CAST_objToPtr; + + return false; +} + + +// 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; +} + til::SExpr *SExprBuilder::translateMemberExpr(const MemberExpr *ME, CallingContext *Ctx) { - til::SExpr *E = translate(ME->getBase(), Ctx); - E = new (Arena) til::SApply(E); - return new (Arena) til::Project(E, ME->getMemberDecl()); + til::SExpr *BE = translate(ME->getBase(), Ctx); + til::SExpr *E = new (Arena) til::SApply(BE); + + const ValueDecl *D = ME->getMemberDecl(); + if (auto *VD = dyn_cast(D)) + D = getFirstVirtualDecl(VD); + + til::Project *P = new (Arena) til::Project(E, D); + if (hasCppPointerType(BE)) + P->setArrow(true); + return P; } til::SExpr *SExprBuilder::translateCallExpr(const CallExpr *CE, - CallingContext *Ctx) { - // TODO -- Lock returned + CallingContext *Ctx, + const Expr *SelfE) { + if (CapabilityExprMode) { + // Handle LOCK_RETURNED + const FunctionDecl *FD = CE->getDirectCallee()->getMostRecentDecl(); + if (LockReturnedAttr* At = FD->getAttr()) { + CallingContext LRCallCtx(Ctx); + LRCallCtx.AttrDecl = CE->getDirectCallee(); + LRCallCtx.SelfArg = SelfE; + LRCallCtx.NumArgs = CE->getNumArgs(); + LRCallCtx.FunArgs = CE->getArgs(); + return translateAttrExpr(At->getArg(), &LRCallCtx); + } + } + til::SExpr *E = translate(CE->getCallee(), Ctx); for (const auto *Arg : CE->arguments()) { til::SExpr *A = translate(Arg, Ctx); @@ -219,12 +372,31 @@ til::SExpr *SExprBuilder::translateCallExpr(const CallExpr *CE, til::SExpr *SExprBuilder::translateCXXMemberCallExpr( const CXXMemberCallExpr *ME, CallingContext *Ctx) { - return translateCallExpr(cast(ME), Ctx); + if (CapabilityExprMode) { + // Ignore calls to get() on smart pointers. + if (ME->getMethodDecl()->getNameAsString() == "get" && + ME->getNumArgs() == 0) { + auto *E = translate(ME->getImplicitObjectArgument(), Ctx); + return new (Arena) til::Cast(til::CAST_objToPtr, E); + // return E; + } + } + return translateCallExpr(cast(ME), Ctx, + ME->getImplicitObjectArgument()); } til::SExpr *SExprBuilder::translateCXXOperatorCallExpr( const CXXOperatorCallExpr *OCE, CallingContext *Ctx) { + if (CapabilityExprMode) { + // Ignore operator * and operator -> on smart pointers. + OverloadedOperatorKind k = OCE->getOperator(); + if (k == OO_Star || k == OO_Arrow) { + auto *E = translate(OCE->getArg(0), Ctx); + return new (Arena) til::Cast(til::CAST_objToPtr, E); + // return E; + } + } return translateCallExpr(cast(OCE), Ctx); } @@ -238,8 +410,23 @@ til::SExpr *SExprBuilder::translateUnaryOperator(const UnaryOperator *UO, case UO_PreDec: return new (Arena) til::Undefined(UO); + case UO_AddrOf: { + if (CapabilityExprMode) { + // interpret &Graph::mu_ as an existential. + if (DeclRefExpr* DRE = dyn_cast(UO->getSubExpr())) { + if (DRE->getDecl()->isCXXInstanceMember()) { + // This is a pointer-to-member expression, e.g. &MyClass::mu_. + // We interpret this syntax specially, as a wildcard. + auto *W = new (Arena) til::Wildcard(); + return new (Arena) til::Project(W, DRE->getDecl()); + } + } + } + // otherwise, & is a no-op + return translate(UO->getSubExpr(), Ctx); + } + // We treat these as no-ops - case UO_AddrOf: case UO_Deref: case UO_Plus: return translate(UO->getSubExpr(), Ctx); @@ -360,7 +547,9 @@ til::SExpr *SExprBuilder::translateCastExpr(const CastExpr *CE, return E0; } til::SExpr *E0 = translate(CE->getSubExpr(), Ctx); - return new (Arena) til::Load(E0); + return E0; + // FIXME!! -- get Load working properly + // return new (Arena) til::Load(E0); } case CK_NoOp: case CK_DerivedToBase: @@ -373,6 +562,8 @@ til::SExpr *SExprBuilder::translateCastExpr(const CastExpr *CE, default: { // FIXME: handle different kinds of casts. til::SExpr *E0 = translate(CE->getSubExpr(), Ctx); + if (CapabilityExprMode) + return E0; return new (Arena) til::Cast(til::CAST_none, E0); } } @@ -389,15 +580,12 @@ SExprBuilder::translateArraySubscriptExpr(const ArraySubscriptExpr *E, til::SExpr * -SExprBuilder::translateConditionalOperator(const ConditionalOperator *C, - CallingContext *Ctx) { - return new (Arena) til::Undefined(C); -} - - -til::SExpr *SExprBuilder::translateBinaryConditionalOperator( - const BinaryConditionalOperator *C, CallingContext *Ctx) { - return new (Arena) til::Undefined(C); +SExprBuilder::translateAbstractConditionalOperator( + const AbstractConditionalOperator *CO, CallingContext *Ctx) { + auto *C = translate(CO->getCond(), Ctx); + auto *T = translate(CO->getTrueExpr(), Ctx); + auto *E = translate(CO->getFalseExpr(), Ctx); + return new (Arena) til::IfThenElse(C, T, E); } @@ -430,9 +618,7 @@ SExprBuilder::translateDeclStmt(const DeclStmt *S, CallingContext *Ctx) { // If E is trivial returns E. til::SExpr *SExprBuilder::addStatement(til::SExpr* E, const Stmt *S, const ValueDecl *VD) { - if (!E) - return nullptr; - if (til::ThreadSafetyTIL::isTrivial(E)) + if (!E || !CurrentBB || til::ThreadSafetyTIL::isTrivial(E)) return E; til::Variable *V = new (Arena) til::Variable(E, VD); @@ -631,7 +817,6 @@ void SExprBuilder::enterCFG(CFG *Cfg, const NamedDecl *D, BB->reserveInstructions(B->size()); BlockMap[B->getBlockID()] = BB; } - CallCtx.reset(new SExprBuilder::CallingContext(D)); CurrentBB = lookupBlock(&Cfg->getEntry()); auto Parms = isa(D) ? cast(D)->parameters() @@ -697,7 +882,7 @@ void SExprBuilder::enterCFGBlockBody(const CFGBlock *B) { void SExprBuilder::handleStatement(const Stmt *S) { - til::SExpr *E = translate(S, CallCtx.get()); + til::SExpr *E = translate(S, nullptr); addStatement(E, S); } @@ -730,7 +915,7 @@ void SExprBuilder::exitCFGBlockBody(const CFGBlock *B) { CurrentBB->setTerminator(Tm); } else if (N == 2) { - til::SExpr *C = translate(B->getTerminatorCondition(true), CallCtx.get()); + til::SExpr *C = translate(B->getTerminatorCondition(true), nullptr); til::BasicBlock *BB1 = *It ? lookupBlock(*It) : nullptr; ++It; til::BasicBlock *BB2 = *It ? lookupBlock(*It) : nullptr; @@ -775,18 +960,15 @@ void SExprBuilder::exitCFG(const CFGBlock *Last) { } - -class TILPrinter : public til::PrettyPrinter {}; - - +/* void printSCFG(CFGWalker &Walker) { llvm::BumpPtrAllocator Bpa; til::MemRegionRef Arena(&Bpa); - SExprBuilder builder(Arena); - til::SCFG *Cfg = builder.buildCFG(Walker); - TILPrinter::print(Cfg, llvm::errs()); + SExprBuilder SxBuilder(Arena); + til::SCFG *Scfg = SxBuilder.buildCFG(Walker); + TILPrinter::print(Scfg, llvm::errs()); } - +*/ } // end namespace threadSafety diff --git a/clang/lib/Analysis/ThreadSafetyTIL.cpp b/clang/lib/Analysis/ThreadSafetyTIL.cpp index f67cbb90e5ab..0bb7d4c2dbb1 100644 --- a/clang/lib/Analysis/ThreadSafetyTIL.cpp +++ b/clang/lib/Analysis/ThreadSafetyTIL.cpp @@ -88,10 +88,42 @@ void SCFG::renumberVars() { +// If E is a variable, then trace back through any aliases or redundant +// Phi nodes to find the canonical definition. +const SExpr *getCanonicalVal(const SExpr *E) { + while (auto *V = dyn_cast(E)) { + const SExpr *D; + do { + if (V->kind() != Variable::VK_Let) + return V; + D = V->definition(); + auto *V2 = dyn_cast(D); + if (V2) + V = V2; + else + break; + } while (true); + + if (ThreadSafetyTIL::isTrivial(D)) + return D; + + if (const Phi *Ph = dyn_cast(D)) { + if (Ph->status() == Phi::PH_SingleVal) { + E = Ph->values()[0]; + continue; + } + } + return V; + } + return E; +} + + // If E is a variable, then trace back through any aliases or redundant // Phi nodes to find the canonical definition. -SExpr *getCanonicalVal(SExpr *E) { +// The non-const version will simplify incomplete Phi nodes. +SExpr *simplifyToCanonicalVal(SExpr *E) { while (auto *V = dyn_cast(E)) { SExpr *D; do { @@ -123,6 +155,7 @@ SExpr *getCanonicalVal(SExpr *E) { } + // Trace the arguments of an incomplete Phi node to see if they have the same // canonical definition. If so, mark the Phi node as redundant. // getCanonicalVal() will recursively call simplifyIncompletePhi(). @@ -132,9 +165,9 @@ void simplifyIncompleteArg(Variable *V, til::Phi *Ph) { // eliminate infinite recursion -- assume that this node is not redundant. Ph->setStatus(Phi::PH_MultiVal); - SExpr *E0 = getCanonicalVal(Ph->values()[0]); + SExpr *E0 = simplifyToCanonicalVal(Ph->values()[0]); for (unsigned i=1, n=Ph->values().size(); ivalues()[i]); + SExpr *Ei = simplifyToCanonicalVal(Ph->values()[i]); if (Ei == V) continue; // Recursive reference to itself. Don't count. if (Ei != E0) { diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 213d6fbffffc..d1d4c272e584 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -1444,9 +1444,9 @@ struct SortDiagBySourceLocation { // -Wthread-safety //===----------------------------------------------------------------------===// namespace clang { -namespace thread_safety { -namespace { -class ThreadSafetyReporter : public clang::thread_safety::ThreadSafetyHandler { +namespace threadSafety { + +class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { Sema &S; DiagList Warnings; SourceLocation FunLocation, FunEndLocation; @@ -1608,7 +1608,7 @@ class ThreadSafetyReporter : public clang::thread_safety::ThreadSafetyHandler { Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); } }; -} + } } @@ -1896,11 +1896,11 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P, if (P.enableThreadSafetyAnalysis) { SourceLocation FL = AC.getDecl()->getLocation(); SourceLocation FEL = AC.getDecl()->getLocEnd(); - thread_safety::ThreadSafetyReporter Reporter(S, FL, FEL); + threadSafety::ThreadSafetyReporter Reporter(S, FL, FEL); if (!Diags.isIgnored(diag::warn_thread_safety_beta, D->getLocStart())) Reporter.setIssueBetaWarnings(true); - thread_safety::runThreadSafetyAnalysis(AC, Reporter); + threadSafety::runThreadSafetyAnalysis(AC, Reporter); Reporter.emitDiagnostics(); } diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index 34a33aab42d2..17ab4c9afdb0 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -95,6 +95,13 @@ public: }; +template +class MyMap { +public: + T& operator[](const K& k); +}; + + Mutex sls_mu; @@ -2280,6 +2287,15 @@ void test() { (a > 0 ? fooArray[1] : fooArray[b]).mu_.Lock(); (a > 0 ? fooArray[1] : fooArray[b]).a = 0; (a > 0 ? fooArray[1] : fooArray[b]).mu_.Unlock(); +} + + +void test2() { + Foo *fooArray; + Bar bar; + int a; + int b; + int c; bar.getFoo().mu_.Lock(); bar.getFooey().a = 0; // \ @@ -2295,20 +2311,20 @@ void test() { bar.getFoo3(a, b).mu_.Lock(); bar.getFoo3(a, c).a = 0; // \ - // expected-warning {{writing variable 'a' requires holding mutex 'bar.getFoo3(a,c).mu_' exclusively}} \ - // expected-note {{'bar.getFoo3(a,b).mu_'}} + // expected-warning {{writing variable 'a' requires holding mutex 'bar.getFoo3(a, c).mu_' exclusively}} \ + // expected-note {{found near match 'bar.getFoo3(a, b).mu_'}} bar.getFoo3(a, b).mu_.Unlock(); getBarFoo(bar, a).mu_.Lock(); getBarFoo(bar, b).a = 0; // \ - // expected-warning {{writing variable 'a' requires holding mutex 'getBarFoo(bar,b).mu_' exclusively}} \ - // expected-note {{'getBarFoo(bar,a).mu_'}} + // expected-warning {{writing variable 'a' requires holding mutex 'getBarFoo(bar, b).mu_' exclusively}} \ + // expected-note {{found near match 'getBarFoo(bar, a).mu_'}} getBarFoo(bar, a).mu_.Unlock(); (a > 0 ? fooArray[1] : fooArray[b]).mu_.Lock(); (a > 0 ? fooArray[b] : fooArray[c]).a = 0; // \ - // expected-warning {{writing variable 'a' requires holding mutex '((a#_)#_#fooArray[b]).mu_' exclusively}} \ - // expected-note {{'((a#_)#_#fooArray[_]).mu_'}} + // expected-warning {{writing variable 'a' requires holding mutex '((0 < a) ? fooArray[b] : fooArray[c]).mu_' exclusively}} \ + // expected-note {{found near match '((0 < a) ? fooArray[1] : fooArray[b]).mu_'}} (a > 0 ? fooArray[1] : fooArray[b]).mu_.Unlock(); } @@ -4378,3 +4394,126 @@ class Foo { }; } // end namespace ThreadAttributesOnLambdas + + + +namespace AttributeExpressionCornerCases { + +class Foo { + int a GUARDED_BY(getMu()); + + Mutex* getMu() LOCK_RETURNED(""); + Mutex* getUniv() LOCK_RETURNED("*"); + + void test1() { + a = 0; + } + + void test2() EXCLUSIVE_LOCKS_REQUIRED(getUniv()) { + a = 0; + } + + void foo(Mutex* mu) EXCLUSIVE_LOCKS_REQUIRED(mu); + + void test3() { + foo(nullptr); + } +}; + + +class MapTest { + struct MuCell { Mutex* mu; }; + + MyMap map; + MyMap mapCell; + + int a GUARDED_BY(map["foo"]); + int b GUARDED_BY(mapCell["foo"].mu); + + void test() { + map["foo"]->Lock(); + a = 0; + map["foo"]->Unlock(); + } + + void test2() { + mapCell["foo"].mu->Lock(); + b = 0; + mapCell["foo"].mu->Unlock(); + } +}; + + +class PreciseSmartPtr { + SmartPtr mu; + int val GUARDED_BY(mu); + + static bool compare(PreciseSmartPtr& a, PreciseSmartPtr &b) { + a.mu->Lock(); + bool result = (a.val == b.val); // expected-warning {{reading variable 'val' requires holding mutex 'b.mu'}} \ + // expected-note {{found near match 'a.mu'}} + a.mu->Unlock(); + return result; + } +}; + + +class SmartRedeclare { + SmartPtr mu; + int val GUARDED_BY(mu); + + void test() EXCLUSIVE_LOCKS_REQUIRED(mu); + void test2() EXCLUSIVE_LOCKS_REQUIRED(mu.get()); + void test3() EXCLUSIVE_LOCKS_REQUIRED(mu.get()); +}; + + +void SmartRedeclare::test() EXCLUSIVE_LOCKS_REQUIRED(mu.get()) { + val = 0; +} + +void SmartRedeclare::test2() EXCLUSIVE_LOCKS_REQUIRED(mu) { + val = 0; +} + +void SmartRedeclare::test3() { + val = 0; +} + + +namespace CustomMutex { + + +class LOCKABLE BaseMutex { }; +class DerivedMutex : public BaseMutex { }; + +void customLock(const BaseMutex *m) EXCLUSIVE_LOCK_FUNCTION(m); +void customUnlock(const BaseMutex *m) UNLOCK_FUNCTION(m); + +static struct DerivedMutex custMu; + +static void doSomethingRequiringLock() EXCLUSIVE_LOCKS_REQUIRED(custMu) { } + +void customTest() { + customLock(reinterpret_cast(&custMu)); // ignore casts + doSomethingRequiringLock(); + customUnlock(reinterpret_cast(&custMu)); +} + +} // end namespace CustomMutex + +} // end AttributeExpressionCornerCases + + +namespace ScopedLockReturnedInvalid { + +class Opaque; + +Mutex* getMutex(Opaque* o) LOCK_RETURNED(""); + +void test(Opaque* o) { + MutexLock lock(getMutex(o)); +} + +} // end namespace ScopedLockReturnedInvalid +