From a0c3f50c1c8ed3c78a0a808ce2230a0e16231a27 Mon Sep 17 00:00:00 2001 From: DeLesley Hutchins Date: Thu, 29 May 2014 21:24:16 +0000 Subject: [PATCH] Thread Safety Analysis: implement review suggestions from Aaron Ballman. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@209847 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../clang/Analysis/Analyses/ThreadSafetyTIL.h | 49 +++++++++++-------- .../Analysis/Analyses/ThreadSafetyTraverse.h | 4 +- .../Analysis/Analyses/ThreadSafetyUtil.h | 10 ++-- lib/Analysis/ThreadSafetyCommon.cpp | 2 +- 4 files changed, 38 insertions(+), 27 deletions(-) diff --git a/include/clang/Analysis/Analyses/ThreadSafetyTIL.h b/include/clang/Analysis/Analyses/ThreadSafetyTIL.h index 34e70552a6..a876e65392 100644 --- a/include/clang/Analysis/Analyses/ThreadSafetyTIL.h +++ b/include/clang/Analysis/Analyses/ThreadSafetyTIL.h @@ -52,6 +52,7 @@ #include "ThreadSafetyUtil.h" #include +#include #include #include #include @@ -598,6 +599,8 @@ public: }; +template class LiteralT; + // Base class for literal values. class Literal : public SExpr { public: @@ -614,6 +617,13 @@ public: ValueType valueType() const { return ValType; } + template const LiteralT& as() const { + return *static_cast*>(this); + } + template LiteralT& as() { + return *static_cast*>(this); + } + template typename V::R_SExpr traverse(V &Vs, typename V::R_Ctx Ctx); template typename C::CType compare(Literal* E, C& Cmp) { @@ -621,7 +631,7 @@ public: return Cmp.comparePointers(Cexpr, E->Cexpr); } -protected: +private: const ValueType ValType; const clang::Expr *Cexpr; }; @@ -642,6 +652,7 @@ private: }; + template typename V::R_SExpr Literal::traverse(V &Vs, typename V::R_Ctx Ctx) { if (Cexpr) @@ -651,29 +662,29 @@ typename V::R_SExpr Literal::traverse(V &Vs, typename V::R_Ctx Ctx) { case ValueType::BT_Void: break; case ValueType::BT_Bool: - return Vs.reduceLiteralT(*static_cast*>(this)); + return Vs.reduceLiteralT(as()); case ValueType::BT_Int: { switch (ValType.Size) { case ValueType::ST_8: if (ValType.Signed) - return Vs.reduceLiteralT(*static_cast*>(this)); + return Vs.reduceLiteralT(as()); else - return Vs.reduceLiteralT(*static_cast*>(this)); + return Vs.reduceLiteralT(as()); case ValueType::ST_16: if (ValType.Signed) - return Vs.reduceLiteralT(*static_cast*>(this)); + return Vs.reduceLiteralT(as()); else - return Vs.reduceLiteralT(*static_cast*>(this)); + return Vs.reduceLiteralT(as()); case ValueType::ST_32: if (ValType.Signed) - return Vs.reduceLiteralT(*static_cast*>(this)); + return Vs.reduceLiteralT(as()); else - return Vs.reduceLiteralT(*static_cast*>(this)); + return Vs.reduceLiteralT(as()); case ValueType::ST_64: if (ValType.Signed) - return Vs.reduceLiteralT(*static_cast*>(this)); + return Vs.reduceLiteralT(as()); else - return Vs.reduceLiteralT(*static_cast*>(this)); + return Vs.reduceLiteralT(as()); default: break; } @@ -681,17 +692,17 @@ typename V::R_SExpr Literal::traverse(V &Vs, typename V::R_Ctx Ctx) { case ValueType::BT_Float: { switch (ValType.Size) { case ValueType::ST_32: - return Vs.reduceLiteralT(*static_cast*>(this)); + return Vs.reduceLiteralT(as()); case ValueType::ST_64: - return Vs.reduceLiteralT(*static_cast*>(this)); + return Vs.reduceLiteralT(as()); default: break; } } case ValueType::BT_String: - return Vs.reduceLiteralT(*static_cast*>(this)); + return Vs.reduceLiteralT(as()); case ValueType::BT_Pointer: - return Vs.reduceLiteralT(*static_cast*>(this)); + return Vs.reduceLiteralT(as()); case ValueType::BT_ValueRef: break; } @@ -1458,13 +1469,9 @@ public: void reservePredecessors(unsigned NumPreds); // Return the index of BB, or Predecessors.size if BB is not a predecessor. - unsigned findPredecessorIndex(BasicBlock *BB) { - unsigned I = 0; - for (BasicBlock *B : Predecessors) { - if (B == BB) return I; - ++I; - } - return Predecessors.size(); + unsigned findPredecessorIndex(const BasicBlock *BB) const { + auto I = std::find(Predecessors.cbegin(), Predecessors.cend(), BB); + return std::distance(Predecessors.cbegin(), I); } // Set id numbers for variables. diff --git a/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h b/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h index 8d3ec5b423..344f9d6814 100644 --- a/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h +++ b/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h @@ -843,7 +843,7 @@ protected: if (E->parent()) SS << " BB_" << E->parent()->blockID(); newline(SS); - for (auto A : E->arguments()) { + for (auto *A : E->arguments()) { SS << "let "; self()->printVariable(A, SS, true); SS << " = "; @@ -851,7 +851,7 @@ protected: SS << ";"; newline(SS); } - for (auto I : E->instructions()) { + for (auto *I : E->instructions()) { if (I->definition()->opcode() != COP_Store) { SS << "let "; self()->printVariable(I, SS, true); diff --git a/include/clang/Analysis/Analyses/ThreadSafetyUtil.h b/include/clang/Analysis/Analyses/ThreadSafetyUtil.h index 3c254264cd..31200a3a72 100644 --- a/include/clang/Analysis/Analyses/ThreadSafetyUtil.h +++ b/include/clang/Analysis/Analyses/ThreadSafetyUtil.h @@ -123,9 +123,9 @@ public: // Reserve space for at least N more items. void reserveCheck(size_t N, MemRegionRef A) { if (Capacity == 0) - reserve(InitialCapacity, A); + reserve(u_max(InitialCapacity, N), A); else if (Size + N < Capacity) - reserve(Capacity*2, A); + reserve(u_max(Size + N, Capacity * 2), A); } typedef T *iterator; @@ -172,7 +172,11 @@ public: } private: - static const unsigned InitialCapacity = 4; + // std::max is annoying here, because it requires a reference, + // thus forcing InitialCapacity to be initialized outside the .h file. + size_t u_max(size_t i, size_t j) { return (i < j) ? j : i; } + + static const size_t InitialCapacity = 4; SimpleArray(const SimpleArray &A) LLVM_DELETED_FUNCTION; diff --git a/lib/Analysis/ThreadSafetyCommon.cpp b/lib/Analysis/ThreadSafetyCommon.cpp index deb9ba96fb..da88b78126 100644 --- a/lib/Analysis/ThreadSafetyCommon.cpp +++ b/lib/Analysis/ThreadSafetyCommon.cpp @@ -725,7 +725,7 @@ void SExprBuilder::exitCFGBlockBody(const CFGBlock *B) { if (N == 1) { til::BasicBlock *BB = *It ? lookupBlock(*It) : nullptr; // TODO: set index - unsigned Idx = BB->findPredecessorIndex(CurrentBB); + unsigned Idx = BB ? BB->findPredecessorIndex(CurrentBB) : 0; til::SExpr *Tm = new (Arena) til::Goto(BB, Idx); CurrentBB->setTerminator(Tm); } -- 2.40.0