From bf0b9637da7d60180fe0de12de64e467a354157c Mon Sep 17 00:00:00 2001 From: Aaron Ballman Date: Wed, 9 Apr 2014 17:45:44 +0000 Subject: [PATCH] Thread Safety Analysis: some minor cleanups to the latest thread safety changes. No functional changes intended. * Adds an iterator_range interface to CallExpr to get the arguments * Modifies SExpr such that it must be allocated in the Arena, and cannot be deleted * Minor const-correctness and nullptr updates * Adds some operator!= implementations to complement operator== * Removes unused functionality git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@205915 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/Expr.h | 7 ++ .../Analysis/Analyses/ThreadSafetyCommon.h | 59 +++++++------- .../clang/Analysis/Analyses/ThreadSafetyTIL.h | 56 ++++++------- lib/Analysis/ThreadSafetyCommon.cpp | 78 ++++++++----------- 4 files changed, 99 insertions(+), 101 deletions(-) diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h index f0226758f4..ee5f5b7020 100644 --- a/include/clang/AST/Expr.h +++ b/include/clang/AST/Expr.h @@ -2227,6 +2227,13 @@ public: typedef ExprIterator arg_iterator; typedef ConstExprIterator const_arg_iterator; + typedef llvm::iterator_range arg_range; + typedef llvm::iterator_range arg_const_range; + + arg_range arguments() { return arg_range(arg_begin(), arg_end()); } + arg_const_range arguments() const { + return arg_const_range(arg_begin(), arg_end()); + } arg_iterator arg_begin() { return SubExprs+PREARGS_START+getNumPreArgs(); } arg_iterator arg_end() { diff --git a/include/clang/Analysis/Analyses/ThreadSafetyCommon.h b/include/clang/Analysis/Analyses/ThreadSafetyCommon.h index 89a86ff2a0..1b5f817ee5 100644 --- a/include/clang/Analysis/Analyses/ThreadSafetyCommon.h +++ b/include/clang/Analysis/Analyses/ThreadSafetyCommon.h @@ -42,35 +42,32 @@ namespace clang { namespace threadSafety { - -// Simple Visitor class for traversing a clang CFG. -class CFGVisitor { -public: - // Enter the CFG for Decl D, and perform any initial setup operations. - void enterCFG(CFG *Cfg, const NamedDecl *D, const CFGBlock *First) {} - - // Enter a CFGBlock. - void enterCFGBlock(const CFGBlock *B) {} - - // Process an ordinary statement. - void handleStatement(const Stmt *S) {} - - // Process a destructor call - void handleDestructorCall(const VarDecl *VD, const CXXDestructorDecl *DD) {} - - // Process a successor edge. - void handleSuccessor(const CFGBlock *Succ) {} - - // Process a successor back edge to a previously visited block. - void handleSuccessorBackEdge(const CFGBlock *Succ) {} - - // Leave a CFGBlock. - void exitCFGBlock(const CFGBlock *B) {} - - // Leave the CFG, and perform any final cleanup operations. - void exitCFG(const CFGBlock *Last) {} -}; - +// CFG traversal uses templates instead of virtual function dispatch. Visitors +// must implement the following functions: +// +// Enter the CFG for Decl D, and perform any initial setup operations. +// void enterCFG(CFG *Cfg, const NamedDecl *D, const CFGBlock *First) {} +// +// Enter a CFGBlock. +// void enterCFGBlock(const CFGBlock *B) {} +// +// Process an ordinary statement. +// void handleStatement(const Stmt *S) {} +// +// Process a destructor call +// void handleDestructorCall(const VarDecl *VD, const CXXDestructorDecl *DD) {} +// +// Process a successor edge. +// void handleSuccessor(const CFGBlock *Succ) {} +// +// Process a successor back edge to a previously visited block. +// void handleSuccessorBackEdge(const CFGBlock *Succ) {} +// +// Leave a CFGBlock. +// void exitCFGBlock(const CFGBlock *B) {} +// +// Leave the CFG, and perform any final cleanup operations. +// void exitCFG(const CFGBlock *Last) {} // Walks the clang CFG, and invokes methods on a given CFGVisitor. class CFGWalker { @@ -104,13 +101,13 @@ public: V.enterCFG(CFGraph, FDecl, &CFGraph->getEntry()); - for (const CFGBlock* CurrBlock : *SortedGraph) { + for (const auto *CurrBlock : *SortedGraph) { VisitedBlocks.insert(CurrBlock); V.enterCFGBlock(CurrBlock); // Process statements - for (auto BI : *CurrBlock) { + for (const auto &BI : *CurrBlock) { switch (BI.getKind()) { case CFGElement::Statement: { V.handleStatement(BI.castAs().getStmt()); diff --git a/include/clang/Analysis/Analyses/ThreadSafetyTIL.h b/include/clang/Analysis/Analyses/ThreadSafetyTIL.h index 510bad8c4a..3938de66dc 100644 --- a/include/clang/Analysis/Analyses/ThreadSafetyTIL.h +++ b/include/clang/Analysis/Analyses/ThreadSafetyTIL.h @@ -207,6 +207,10 @@ public: // compare all subexpressions, following the comparator interface // } + void *operator new(size_t S, clang::threadSafety::til::MemRegionRef &R) { + return ::operator new(S, R); + } + protected: SExpr(TIL_Opcode Op) : Opcode(Op), Reserved(0), Flags(0) {} SExpr(const SExpr &E) : Opcode(E.Opcode), Reserved(0), Flags(E.Flags) {} @@ -216,7 +220,15 @@ protected: unsigned short Flags; private: - SExpr(); + // Note, this cannot be explicitly deleted due to initializers automatically + // referencing destructor declarations. However, it does not need to be + // defined because that reference does not require an definition. + ~SExpr(); + SExpr() = delete; + + // SExpr objects must be created in an arena and cannot be deleted. + void *operator new(size_t) = delete; + void operator delete(void *) = delete; }; @@ -227,7 +239,7 @@ class SExprRef { public: SExprRef() : Ptr(nullptr) { } SExprRef(std::nullptr_t P) : Ptr(nullptr) { } - SExprRef(SExprRef &&R) : Ptr(R.Ptr) { } + SExprRef(SExprRef &&R) : Ptr(R.Ptr) { R.Ptr = nullptr; } // Defined after Variable and Future, below. inline SExprRef(SExpr *P); @@ -242,9 +254,12 @@ public: SExpr &operator*() { return *Ptr; } const SExpr &operator*() const { return *Ptr; } - bool operator==(const SExprRef& R) const { return Ptr == R.Ptr; } - bool operator==(const SExpr* P) const { return Ptr == P; } - bool operator==(std::nullptr_t P) const { return Ptr == nullptr; } + bool operator==(const SExprRef &R) const { return Ptr == R.Ptr; } + bool operator!=(const SExprRef &R) const { return !operator==(R); } + bool operator==(const SExpr *P) const { return Ptr == P; } + bool operator!=(const SExpr *P) const { return !operator==(P); } + bool operator==(std::nullptr_t) const { return Ptr == nullptr; } + bool operator!=(std::nullptr_t) const { return Ptr != nullptr; } inline void reset(SExpr *E); @@ -252,28 +267,17 @@ private: inline void attach(); inline void detach(); - SExprRef(const SExprRef& R) : Ptr(R.Ptr) { } - SExpr *Ptr; }; // Contains various helper functions for SExprs. -class ThreadSafetyTIL { -public: - static const int MaxOpcode = COP_MAX; - - static inline bool isTrivial(SExpr *E) { - unsigned Op = E->opcode(); - return Op == COP_Variable || Op == COP_Literal || Op == COP_LiteralPtr; - } - - static inline bool isLargeValue(SExpr *E) { - unsigned Op = E->opcode(); - return (Op >= COP_Function && Op <= COP_Code); - } -}; - +namespace ThreadSafetyTIL { +static bool isTrivial(SExpr *E) { + unsigned Op = E->opcode(); + return Op == COP_Variable || Op == COP_Literal || Op == COP_LiteralPtr; +} +} class Function; class SFunction; @@ -311,7 +315,7 @@ public: VariableKind kind() const { return static_cast(Flags); } - StringRef name() const { return Cvdecl ? Cvdecl->getName() : "_x"; } + const StringRef name() const { return Cvdecl ? Cvdecl->getName() : "_x"; } const clang::ValueDecl *clangDecl() const { return Cvdecl; } // Returns the definition (for let vars) or type (for parameter & self vars) @@ -320,8 +324,8 @@ public: void attachVar() const { ++NumUses; } void detachVar() const { assert(NumUses > 0); --NumUses; } - unsigned getID() { return Id; } - unsigned getBlockID() { return BlockID; } + unsigned getID() const { return Id; } + unsigned getBlockID() const { return BlockID; } void setID(unsigned Bid, unsigned I) { BlockID = static_cast(Bid); @@ -369,7 +373,7 @@ public: Future() : SExpr(COP_Future), Status(FS_pending), Result(nullptr), Location(nullptr) {} - virtual ~Future() {} + virtual ~Future() = delete; // Registers the location in the AST where this future is stored. // Forcing the future will automatically update the AST. diff --git a/lib/Analysis/ThreadSafetyCommon.cpp b/lib/Analysis/ThreadSafetyCommon.cpp index 59a22313cd..156568c2ce 100644 --- a/lib/Analysis/ThreadSafetyCommon.cpp +++ b/lib/Analysis/ThreadSafetyCommon.cpp @@ -37,11 +37,11 @@ typedef SExprBuilder::CallingContext CallingContext; til::SExpr *SExprBuilder::lookupStmt(const Stmt *S) { if (!SMap) - return 0; + return nullptr; auto It = SMap->find(S); if (It != SMap->end()) return It->second; - return 0; + return nullptr; } void SExprBuilder::insertStmt(const Stmt *S, til::Variable *V) { @@ -160,8 +160,8 @@ til::SExpr *SExprBuilder::translateCallExpr(const CallExpr *CE, CallingContext *Ctx) { // TODO -- Lock returned til::SExpr *E = translate(CE->getCallee(), Ctx); - for (unsigned I = 0, N = CE->getNumArgs(); I < N; ++I) { - til::SExpr *A = translate(CE->getArg(I), Ctx); + for (const auto *Arg : CE->arguments()) { + til::SExpr *A = translate(Arg, Ctx); E = new (Arena) til::Apply(E, A); } return new (Arena) til::Call(E, CE); @@ -200,10 +200,9 @@ til::SExpr *SExprBuilder::translateUnaryOperator(const UnaryOperator *UO, case UO_LNot: case UO_Real: case UO_Imag: - case UO_Extension: { - til::SExpr *E0 = translate(UO->getSubExpr(), Ctx); - return new (Arena) til::UnaryOp(UO->getOpcode(), E0); - } + case UO_Extension: + return new (Arena) + til::UnaryOp(UO->getOpcode(), translate(UO->getSubExpr(), Ctx)); } return new (Arena) til::Undefined(UO); } @@ -232,16 +231,15 @@ til::SExpr *SExprBuilder::translateBinaryOperator(const BinaryOperator *BO, case BO_Xor: case BO_Or: case BO_LAnd: - case BO_LOr: { - til::SExpr *E0 = translate(BO->getLHS(), Ctx); - til::SExpr *E1 = translate(BO->getRHS(), Ctx); - return new (Arena) til::BinaryOp(BO->getOpcode(), E0, E1); - } - case BO_Assign: { - til::SExpr *E0 = translate(BO->getLHS(), Ctx); - til::SExpr *E1 = translate(BO->getRHS(), Ctx); - return new (Arena) til::Store(E0, E1); - } + case BO_LOr: + return new (Arena) + til::BinaryOp(BO->getOpcode(), translate(BO->getLHS(), Ctx), + translate(BO->getRHS(), Ctx)); + + case BO_Assign: + return new (Arena) + til::Store(translate(BO->getLHS(), Ctx), translate(BO->getRHS(), Ctx)); + case BO_MulAssign: case BO_DivAssign: case BO_RemAssign: @@ -284,42 +282,39 @@ til::SExpr *SExprBuilder::translateCastExpr(const CastExpr *CE, } } - -til::SExpr *SExprBuilder::translateArraySubscriptExpr( - const ArraySubscriptExpr *E, CallingContext *Ctx) { +til::SExpr * +SExprBuilder::translateArraySubscriptExpr(const ArraySubscriptExpr *E, + CallingContext *Ctx) { return new (Arena) til::Undefined(E); } - -til::SExpr *SExprBuilder::translateConditionalOperator( - const ConditionalOperator *C, CallingContext *Ctx) { +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); } - // Build a complete SCFG from a clang CFG. -class SCFGBuilder : public CFGVisitor { -public: - til::Variable *addStatement(til::SExpr* E, const Stmt *S) { +class SCFGBuilder { + void addStatement(til::SExpr* E, const Stmt *S) { if (!E) - return 0; + return; if (til::ThreadSafetyTIL::isTrivial(E)) - return 0; + return; til::Variable *V = new (Arena) til::Variable(til::Variable::VK_Let, E); V->setID(CurrentBlockID, CurrentVarID++); CurrentBB->addInstr(V); if (S) BuildEx.insertStmt(S, V); - return V; } +public: // Enter the CFG for Decl D, and perform any initial setup operations. void enterCFG(CFG *Cfg, const NamedDecl *D, const CFGBlock *First) { Scfg = new (Arena) til::SCFG(Arena, Cfg->getNumBlockIDs()); @@ -345,7 +340,7 @@ public: til::SExpr *Sf = new (Arena) til::LiteralPtr(VD); til::SExpr *Dr = new (Arena) til::LiteralPtr(DD); til::SExpr *Ap = new (Arena) til::Apply(Dr, Sf); - til::SExpr *E = new (Arena) til::Call(Ap, 0); + til::SExpr *E = new (Arena) til::Call(Ap); addStatement(E, nullptr); } @@ -363,20 +358,15 @@ public: // Leave the CFG, and perform any final cleanup operations. void exitCFG(const CFGBlock *Last) { - if (CallCtx) { - delete CallCtx; - CallCtx = nullptr; - } + delete CallCtx; + CallCtx = nullptr; } SCFGBuilder(til::MemRegionRef A) - : Arena(A), Scfg(0), CurrentBB(0), CurrentBlockID(0), - CallCtx(0), SMap(new SExprBuilder::StatementMap()), - BuildEx(A, SMap) - { } - ~SCFGBuilder() { - delete SMap; - } + : Arena(A), Scfg(nullptr), CurrentBB(nullptr), CurrentBlockID(0), + CurrentVarID(0), CallCtx(nullptr), + SMap(new SExprBuilder::StatementMap()), BuildEx(A, SMap) {} + ~SCFGBuilder() { delete SMap; } til::SCFG *getCFG() const { return Scfg; } -- 2.40.0