From 5df82f215599a9844270f1a97490fe8653035fdd Mon Sep 17 00:00:00 2001 From: DeLesley Hutchins Date: Wed, 5 Dec 2012 00:52:33 +0000 Subject: [PATCH] Thread Safety Analysis: refactor to make more methods accept const pointers, adjust checkAccess. No change in functionality. llvm-svn: 169348 --- clang/lib/Analysis/ThreadSafety.cpp | 180 +++++++++++++++++++----------------- 1 file changed, 94 insertions(+), 86 deletions(-) diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp index 3079536..bc3f85c 100644 --- a/clang/lib/Analysis/ThreadSafety.cpp +++ b/clang/lib/Analysis/ThreadSafety.cpp @@ -165,15 +165,16 @@ private: /// 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. - Expr* SelfArg; // Implicit object argument -- e.g. 'this' - bool SelfArrow; // is Self referred to with -> or .? - unsigned NumArgs; // Number of funArgs - Expr** FunArgs; // Function arguments - CallingContext* PrevCtx; // The previous context; or 0 if none. - - CallingContext(const NamedDecl *D = 0, Expr *S = 0, - unsigned N = 0, Expr **A = 0, CallingContext *P = 0) + 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 = 0, const Expr *S = 0, + unsigned N = 0, const Expr* const *A = 0, + CallingContext *P = 0) : AttrDecl(D), SelfArg(S), SelfArrow(false), NumArgs(N), FunArgs(A), PrevCtx(P) { } @@ -273,15 +274,16 @@ private: /// NDeref returns the number of Derefence and AddressOf operations /// preceeding the Expr; this is used to decide whether to pretty-print /// SExprs with . or ->. - unsigned buildSExpr(Expr *Exp, CallingContext* CallCtx, int* NDeref = 0) { + unsigned buildSExpr(const Expr *Exp, CallingContext* CallCtx, + int* NDeref = 0) { if (!Exp) return 0; - if (DeclRefExpr *DRE = dyn_cast(Exp)) { - NamedDecl *ND = cast(DRE->getDecl()->getCanonicalDecl()); - ParmVarDecl *PV = dyn_cast_or_null(ND); + if (const DeclRefExpr *DRE = dyn_cast(Exp)) { + const NamedDecl *ND = cast(DRE->getDecl()->getCanonicalDecl()); + const ParmVarDecl *PV = dyn_cast_or_null(ND); if (PV) { - FunctionDecl *FD = + const FunctionDecl *FD = cast(PV->getDeclContext())->getCanonicalDecl(); unsigned i = PV->getFunctionScopeIndex(); @@ -310,18 +312,18 @@ private: makeThis(); return 1; } - } else if (MemberExpr *ME = dyn_cast(Exp)) { - NamedDecl *ND = ME->getMemberDecl(); + } 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 (CXXMemberCallExpr *CMCE = dyn_cast(Exp)) { + } 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. - CXXMethodDecl* MD = + const CXXMethodDecl* MD = cast(CMCE->getMethodDecl()->getMostRecentDecl()); if (LockReturnedAttr* At = MD->getAttr()) { CallingContext LRCallCtx(CMCE->getMethodDecl()); @@ -344,14 +346,14 @@ private: unsigned NumCallArgs = CMCE->getNumArgs(); unsigned Root = makeMCall(NumCallArgs, CMCE->getMethodDecl()); unsigned Sz = buildSExpr(CMCE->getImplicitObjectArgument(), CallCtx); - Expr** CallArgs = CMCE->getArgs(); + 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 (CallExpr *CE = dyn_cast(Exp)) { - FunctionDecl* FD = + } else if (const CallExpr *CE = dyn_cast(Exp)) { + const FunctionDecl* FD = cast(CE->getDirectCallee()->getMostRecentDecl()); if (LockReturnedAttr* At = FD->getAttr()) { CallingContext LRCallCtx(CE->getDirectCallee()); @@ -362,7 +364,7 @@ private: } // Treat smart pointers and iterators as pointers; // ignore the * and -> operators. - if (CXXOperatorCallExpr *OE = dyn_cast(CE)) { + if (const CXXOperatorCallExpr *OE = dyn_cast(CE)) { OverloadedOperatorKind k = OE->getOperator(); if (k == OO_Star) { if (NDeref) ++(*NDeref); @@ -375,19 +377,19 @@ private: unsigned NumCallArgs = CE->getNumArgs(); unsigned Root = makeCall(NumCallArgs, 0); unsigned Sz = buildSExpr(CE->getCallee(), CallCtx); - Expr** CallArgs = CE->getArgs(); + 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 (BinaryOperator *BOE = dyn_cast(Exp)) { + } 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 (UnaryOperator *UOE = dyn_cast(Exp)) { + } 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. @@ -413,13 +415,14 @@ private: unsigned Sz = buildSExpr(UOE->getSubExpr(), CallCtx); NodeVec[Root].setSize(Sz); return Sz; - } else if (ArraySubscriptExpr *ASE = dyn_cast(Exp)) { + } 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 (AbstractConditionalOperator *CE = + } else if (const AbstractConditionalOperator *CE = dyn_cast(Exp)) { unsigned Root = makeUnknown(3); unsigned Sz = buildSExpr(CE->getCond(), CallCtx); @@ -427,20 +430,20 @@ private: Sz += buildSExpr(CE->getFalseExpr(), CallCtx); NodeVec[Root].setSize(Sz); return Sz; - } else if (ChooseExpr *CE = dyn_cast(Exp)) { + } 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 (CastExpr *CE = dyn_cast(Exp)) { + } else if (const CastExpr *CE = dyn_cast(Exp)) { return buildSExpr(CE->getSubExpr(), CallCtx, NDeref); - } else if (ParenExpr *PE = dyn_cast(Exp)) { + } else if (const ParenExpr *PE = dyn_cast(Exp)) { return buildSExpr(PE->getSubExpr(), CallCtx, NDeref); - } else if (ExprWithCleanups *EWC = dyn_cast(Exp)) { + } else if (const ExprWithCleanups *EWC = dyn_cast(Exp)) { return buildSExpr(EWC->getSubExpr(), CallCtx, NDeref); - } else if (CXXBindTemporaryExpr *E = dyn_cast(Exp)) { + } else if (const CXXBindTemporaryExpr *E = dyn_cast(Exp)) { return buildSExpr(E->getSubExpr(), CallCtx, NDeref); } else if (isa(Exp) || isa(Exp) || @@ -464,12 +467,12 @@ private: /// \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(Expr *MutexExp, Expr *DeclExp, const NamedDecl *D, - VarDecl *SelfDecl = 0) { + void buildSExprFromExpr(const Expr *MutexExp, const Expr *DeclExp, + const NamedDecl *D, VarDecl *SelfDecl = 0) { CallingContext CallCtx(D); if (MutexExp) { - if (StringLiteral* SLit = dyn_cast(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. @@ -489,18 +492,21 @@ private: // Examine DeclExp to find SelfArg and FunArgs, which are used to substitute // for formal parameters when we call buildMutexID later. - if (MemberExpr *ME = dyn_cast(DeclExp)) { + if (const MemberExpr *ME = dyn_cast(DeclExp)) { CallCtx.SelfArg = ME->getBase(); CallCtx.SelfArrow = ME->isArrow(); - } else if (CXXMemberCallExpr *CE = dyn_cast(DeclExp)) { + } else if (const CXXMemberCallExpr *CE = + dyn_cast(DeclExp)) { CallCtx.SelfArg = CE->getImplicitObjectArgument(); CallCtx.SelfArrow = dyn_cast(CE->getCallee())->isArrow(); CallCtx.NumArgs = CE->getNumArgs(); CallCtx.FunArgs = CE->getArgs(); - } else if (CallExpr *CE = dyn_cast(DeclExp)) { + } else if (const CallExpr *CE = + dyn_cast(DeclExp)) { CallCtx.NumArgs = CE->getNumArgs(); CallCtx.FunArgs = CE->getArgs(); - } else if (CXXConstructExpr *CE = dyn_cast(DeclExp)) { + } else if (const CXXConstructExpr *CE = + dyn_cast(DeclExp)) { CallCtx.SelfArg = 0; // Will be set below CallCtx.NumArgs = CE->getNumArgs(); CallCtx.FunArgs = CE->getArgs(); @@ -544,7 +550,7 @@ public: /// occurs. /// \param D The declaration to which the lock/unlock attribute is attached. /// Caller must check isValid() after construction. - SExpr(Expr* MutexExp, Expr *DeclExp, const NamedDecl* D, + SExpr(const Expr* MutexExp, const Expr *DeclExp, const NamedDecl* D, VarDecl *SelfDecl=0) { buildSExprFromExpr(MutexExp, DeclExp, D, SelfDecl); } @@ -567,8 +573,9 @@ public: } /// Issue a warning about an invalid lock expression - static void warnInvalidLock(ThreadSafetyHandler &Handler, Expr* MutexExp, - Expr *DeclExp, const NamedDecl* D) { + static void warnInvalidLock(ThreadSafetyHandler &Handler, + const Expr *MutexExp, + const Expr *DeclExp, const NamedDecl* D) { SourceLocation Loc; if (DeclExp) Loc = DeclExp->getExprLoc(); @@ -1734,14 +1741,15 @@ class BuildLockset : public StmtVisitor { unsigned CtxIndex; // Helper functions - const ValueDecl *getValueDecl(Expr *Exp); + const ValueDecl *getValueDecl(const Expr *Exp); - void warnIfMutexNotHeld(const NamedDecl *D, Expr *Exp, AccessKind AK, + void warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, AccessKind AK, Expr *MutexExp, ProtectedOperationKind POK); - void warnIfMutexHeld(const NamedDecl *D, Expr *Exp, Expr *MutexExp); + void warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, Expr *MutexExp); + + void checkAccess(const Expr *Exp, AccessKind AK); + void checkPtAccess(const Expr *Exp, AccessKind AK); - void checkAccess(Expr *Exp, AccessKind AK); - void checkDereference(Expr *Exp, AccessKind AK); void handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD = 0); public: @@ -1763,7 +1771,10 @@ public: /// \brief Gets the value decl pointer from DeclRefExprs or MemberExprs -const ValueDecl *BuildLockset::getValueDecl(Expr *Exp) { +const ValueDecl *BuildLockset::getValueDecl(const Expr *Exp) { + if (const ImplicitCastExpr *CE = dyn_cast(Exp)) + return getValueDecl(CE->getSubExpr()); + if (const DeclRefExpr *DR = dyn_cast(Exp)) return DR->getDecl(); @@ -1775,7 +1786,7 @@ const ValueDecl *BuildLockset::getValueDecl(Expr *Exp) { /// \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, Expr *Exp, +void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, AccessKind AK, Expr *MutexExp, ProtectedOperationKind POK) { LockKind LK = getLockKindFromAccessKind(AK); @@ -1814,7 +1825,7 @@ void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, Expr *Exp, } /// \brief Warn if the LSet contains the given lock. -void BuildLockset::warnIfMutexHeld(const NamedDecl *D, Expr* Exp, +void BuildLockset::warnIfMutexHeld(const NamedDecl *D, const Expr* Exp, Expr *MutexExp) { SExpr Mutex(MutexExp, Exp, D); if (!Mutex.isValid()) { @@ -1832,51 +1843,54 @@ void BuildLockset::warnIfMutexHeld(const NamedDecl *D, Expr* Exp, } -/// \brief This method identifies variable dereferences and checks pt_guarded_by -/// and pt_guarded_var annotations. Note that we only check these annotations -/// at the time a pointer is dereferenced. -/// FIXME: We need to check for other types of pointer dereferences -/// (e.g. [], ->) and deal with them here. -/// \param Exp An expression that has been read or written. -void BuildLockset::checkDereference(Expr *Exp, AccessKind AK) { - UnaryOperator *UO = dyn_cast(Exp); - if (!UO || UO->getOpcode() != clang::UO_Deref) +/// \brief Checks guarded_by and pt_guarded_by attributes. +/// Whenever we identify an access (read or write) to a DeclRefExpr that is +/// 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) { + Exp = Exp->IgnoreParenCasts(); + + if (const UnaryOperator *UO = dyn_cast(Exp)) { + // For dereferences + if (UO->getOpcode() == clang::UO_Deref) + checkPtAccess(UO->getSubExpr(), AK); return; - Exp = UO->getSubExpr()->IgnoreParenCasts(); + } const ValueDecl *D = getValueDecl(Exp); - if(!D || !D->hasAttrs()) + if (!D || !D->hasAttrs()) return; - if (D->getAttr() && FSet.isEmpty()) - Analyzer->Handler.handleNoMutexHeld(D, POK_VarDereference, AK, + if (D->getAttr() && FSet.isEmpty()) + Analyzer->Handler.handleNoMutexHeld(D, POK_VarAccess, AK, Exp->getExprLoc()); const AttrVec &ArgAttrs = D->getAttrs(); - for(unsigned i = 0, Size = ArgAttrs.size(); i < Size; ++i) - if (PtGuardedByAttr *PGBAttr = dyn_cast(ArgAttrs[i])) - warnIfMutexNotHeld(D, Exp, AK, PGBAttr->getArg(), POK_VarDereference); + for (unsigned i = 0, Size = ArgAttrs.size(); i < Size; ++i) + if (GuardedByAttr *GBAttr = dyn_cast(ArgAttrs[i])) + warnIfMutexNotHeld(D, Exp, AK, GBAttr->getArg(), POK_VarAccess); } -/// \brief Checks guarded_by and guarded_var attributes. -/// Whenever we identify an access (read or write) of a DeclRefExpr or -/// MemberExpr, we need to check whether there are any guarded_by or -/// guarded_var attributes, and make sure we hold the appropriate mutexes. -void BuildLockset::checkAccess(Expr *Exp, AccessKind AK) { +/// \brief Checks pt_guarded_by and pt_guarded_var attributes. +void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK) { + Exp = Exp->IgnoreParenCasts(); + const ValueDecl *D = getValueDecl(Exp); - if(!D || !D->hasAttrs()) + if (!D || !D->hasAttrs()) return; - if (D->getAttr() && FSet.isEmpty()) - Analyzer->Handler.handleNoMutexHeld(D, POK_VarAccess, AK, + if (D->getAttr() && FSet.isEmpty()) + Analyzer->Handler.handleNoMutexHeld(D, POK_VarDereference, AK, Exp->getExprLoc()); const AttrVec &ArgAttrs = D->getAttrs(); - for(unsigned i = 0, Size = ArgAttrs.size(); i < Size; ++i) - if (GuardedByAttr *GBAttr = dyn_cast(ArgAttrs[i])) - warnIfMutexNotHeld(D, Exp, AK, GBAttr->getArg(), POK_VarAccess); + for (unsigned i = 0, Size = ArgAttrs.size(); i < Size; ++i) + if (PtGuardedByAttr *GBAttr = dyn_cast(ArgAttrs[i])) + warnIfMutexNotHeld(D, Exp, AK, GBAttr->getArg(), POK_VarDereference); } + /// \brief Process a function call, method call, constructor call, /// or destructor call. This involves looking at the attributes on the /// corresponding function/method/constructor/destructor, issuing warnings, @@ -2010,9 +2024,7 @@ void BuildLockset::VisitUnaryOperator(UnaryOperator *UO) { case clang::UO_PostInc: case clang::UO_PreDec: case clang::UO_PreInc: { - Expr *SubExp = UO->getSubExpr()->IgnoreParenCasts(); - checkAccess(SubExp, AK_Written); - checkDereference(SubExp, AK_Written); + checkAccess(UO->getSubExpr(), AK_Written); break; } default: @@ -2030,9 +2042,7 @@ void BuildLockset::VisitBinaryOperator(BinaryOperator *BO) { // adjust the context LVarCtx = Analyzer->LocalVarMap.getNextContext(CtxIndex, BO, LVarCtx); - Expr *LHSExp = BO->getLHS()->IgnoreParenCasts(); - checkAccess(LHSExp, AK_Written); - checkDereference(LHSExp, AK_Written); + checkAccess(BO->getLHS(), AK_Written); } /// Whenever we do an LValue to Rvalue cast, we are reading a variable and @@ -2041,9 +2051,7 @@ void BuildLockset::VisitBinaryOperator(BinaryOperator *BO) { void BuildLockset::VisitCastExpr(CastExpr *CE) { if (CE->getCastKind() != CK_LValueToRValue) return; - Expr *SubExp = CE->getSubExpr()->IgnoreParenCasts(); - checkAccess(SubExp, AK_Read); - checkDereference(SubExp, AK_Read); + checkAccess(CE->getSubExpr(), AK_Read); } -- 2.7.4