Thread safety analysis: misc updates to SExpr handling. Fix to minimal SSA,

function parameters, and compound assignment.

llvm-svn: 206827

clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h

@@ -192,7 +192,9 @@ public:
   const CFG *getGraph() const { return CFGraph; }
   CFG *getGraph() { return CFGraph; }
 
-  const NamedDecl *getDecl() const { return cast<NamedDecl>(ACtx->getDecl()); }
+  const FunctionDecl *getDecl() const {
+    return dyn_cast<FunctionDecl>(ACtx->getDecl());
+  }
 
   const PostOrderCFGView *getSortedGraph() const { return SortedGraph; }
 
@@ -237,6 +239,10 @@ public:
     // FIXME: we don't always have a self-variable.
     SelfVar = new (Arena) til::Variable(til::Variable::VK_SFun);
   }
+  ~SExprBuilder() {
+    if (CallCtx)
+      delete CallCtx;
+  }
 
   // Translate a clang statement or expression to a TIL expression.
   // Also performs substitution of variables; Ctx provides the context.
@@ -251,7 +257,7 @@ public:
   }
 
   const til::SCFG *getCFG() const { return Scfg; }
-  til::SCFG *getCFF() { return Scfg; }
+  til::SCFG *getCFG() { return Scfg; }
 
 private:
   til::SExpr *translateDeclRefExpr(const DeclRefExpr *DRE,
@@ -265,6 +271,9 @@ private:
                                            CallingContext *Ctx);
   til::SExpr *translateUnaryOperator(const UnaryOperator *UO,
                                      CallingContext *Ctx);
+  til::SExpr *translateBinAssign(til::TIL_BinaryOpcode Op,
+                                 const BinaryOperator *BO,
+                                 CallingContext *Ctx);
   til::SExpr *translateBinaryOperator(const BinaryOperator *BO,
                                       CallingContext *Ctx);
   til::SExpr *translateCastExpr(const CastExpr *CE, CallingContext *Ctx);
@@ -320,7 +329,7 @@ private:
   // We implement the CFGVisitor API
   friend class CFGWalker;
 
-  void enterCFG(CFG *Cfg, const NamedDecl *D, const CFGBlock *First);
+  void enterCFG(CFG *Cfg, const FunctionDecl *D, const CFGBlock *First);
   void enterCFGBlock(const CFGBlock *B);
   bool visitPredecessors() { return true; }
   void handlePredecessor(const CFGBlock *Pred);

clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h

@@ -222,6 +222,7 @@ public:
     Id = static_cast<unsigned short>(I);
   }
   void setClangDecl(const clang::ValueDecl *VD) { Cvdecl = VD; }
+  void setDefinition(SExpr *E);
 
   template <class V> typename V::R_SExpr traverse(V &Visitor) {
     // This routine is only called for variable references.
@@ -316,7 +317,8 @@ private:
   SExprRef *Location;
 };
 
-void SExprRef::attach() {
+
+inline void SExprRef::attach() {
   if (!Ptr)
     return;
 
@@ -328,45 +330,49 @@ void SExprRef::attach() {
   }
 }
 
-void SExprRef::detach() {
+inline void SExprRef::detach() {
   if (Ptr && Ptr->opcode() == COP_Variable) {
     cast<Variable>(Ptr)->detachVar();
   }
 }
 
-SExprRef::SExprRef(SExpr *P) : Ptr(P) {
+inline SExprRef::SExprRef(SExpr *P) : Ptr(P) {
   attach();
 }
 
-SExprRef::~SExprRef() {
+inline SExprRef::~SExprRef() {
   detach();
 }
 
-void SExprRef::reset(SExpr *P) {
+inline void SExprRef::reset(SExpr *P) {
   detach();
   Ptr = P;
   attach();
 }
 
 
-Variable::Variable(VariableKind K, SExpr *D, const clang::ValueDecl *Cvd)
+inline Variable::Variable(VariableKind K, SExpr *D, const clang::ValueDecl *Cvd)
     : SExpr(COP_Variable), Definition(D), Cvdecl(Cvd),
       BlockID(0), Id(0),  NumUses(0) {
   Flags = K;
 }
 
-Variable::Variable(SExpr *D, const clang::ValueDecl *Cvd)
+inline Variable::Variable(SExpr *D, const clang::ValueDecl *Cvd)
     : SExpr(COP_Variable), Definition(D), Cvdecl(Cvd),
       BlockID(0), Id(0),  NumUses(0) {
   Flags = VK_Let;
 }
 
-Variable::Variable(const Variable &Vd, SExpr *D) // rewrite constructor
+inline Variable::Variable(const Variable &Vd, SExpr *D) // rewrite constructor
     : SExpr(Vd), Definition(D), Cvdecl(Vd.Cvdecl),
       BlockID(0), Id(0), NumUses(0) {
   Flags = Vd.kind();
 }
 
+inline void Variable::setDefinition(SExpr *E) {
+  Definition.reset(E);
+}
+
 void Future::force() {
   Status = FS_evaluating;
   SExpr *R = create();
@@ -376,6 +382,7 @@ void Future::force() {
   Status = FS_done;
 }
 
+
 // Placeholder for C++ expressions that cannot be represented in the TIL.
 class Undefined : public SExpr {
 public:

clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h

@@ -396,7 +396,12 @@ public:
 // Pretty printer for TIL expressions
 template <typename Self, typename StreamType>
 class PrettyPrinter {
+private:
+  bool Verbose;  // Print out additional information
+
 public:
+  PrettyPrinter(bool V = false) : Verbose(V) { }
+
   static void print(SExpr *E, StreamType &SS) {
     Self printer;
     printer.printSExpr(E, SS, Prec_MAX);
@@ -530,17 +535,21 @@ protected:
     SS << E->clangDecl()->getNameAsString();
   }
 
-  void printVariable(Variable *E, StreamType &SS, bool IsVarDecl = false) {
+  void printVariable(Variable *V, StreamType &SS, bool IsVarDecl = false) {
-    SS << E->name() << E->getBlockID() << "_" << E->getID();
+    SExpr* E = nullptr;
-    if (IsVarDecl)
+    if (!IsVarDecl) {
-      return;
+      E = getCanonicalVal(V);
-
+      if (E != V) {
-    SExpr *V = getCanonicalVal(E);
+        printSExpr(E, SS, Prec_Atom);
-    if (V != E) {
+        if (Verbose) {
-      SS << "{";
+          SS << " /*";
-      printSExpr(V, SS, Prec_MAX);
+          SS << V->name() << V->getBlockID() << "_" << V->getID();
-      SS << "}";
+          SS << "*/";
+        }
+        return;
+      }
     }
+    SS << V->name() << V->getBlockID() << "_" << V->getID();
   }
 
   void printFunction(Function *E, StreamType &SS, unsigned sugared = 0) {

clang/lib/Analysis/ThreadSafetyCommon.cpp

@@ -47,11 +47,12 @@ SExpr *getCanonicalVal(SExpr *E) {
       if (V->kind() != Variable::VK_Let)
         return V;
       D = V->definition();
-      if (auto *V2 = dyn_cast<Variable>(D)) {
+      auto *V2 = dyn_cast<Variable>(D);
+      if (V2)
         V = V2;
-        continue;
+      else
-      }
+        break;
-    } while(false);
+    } while (true);
 
     if (ThreadSafetyTIL::isTrivial(D))
       return D;
@@ -75,7 +76,7 @@ SExpr *getCanonicalVal(SExpr *E) {
 // canonical definition.  If so, mark the Phi node as redundant.
 // getCanonicalVal() will recursively call simplifyIncompletePhi().
 void simplifyIncompleteArg(Variable *V, til::Phi *Ph) {
-  assert(!Ph && Ph->status() == Phi::PH_Incomplete);
+  assert(Ph && Ph->status() == Phi::PH_Incomplete);
 
   // eliminate infinite recursion -- assume that this node is not redundant.
   Ph->setStatus(Phi::PH_MultiVal);
@@ -90,8 +91,21 @@ void simplifyIncompleteArg(Variable *V, til::Phi *Ph) {
     }
   }
   Ph->setStatus(Phi::PH_SingleVal);
+  // Eliminate Redundant Phi node.
+  V->setDefinition(Ph->values()[0]);
 }
 
+
+// Return true if E is a variable that points to an incomplete Phi node.
+inline bool isIncompleteVar(SExpr *E) {
+  if (Variable *V = dyn_cast<Variable>(E)) {
+    if (Phi *Ph = dyn_cast<Phi>(V->definition()))
+      return Ph->status() == Phi::PH_Incomplete;
+  }
+  return false;
+}
+
+
 }  // end namespace til
 
 
@@ -140,6 +154,7 @@ til::SExpr *SExprBuilder::translate(const Stmt *S, CallingContext *Ctx) {
   case Stmt::UnaryOperatorClass:
     return translateUnaryOperator(cast<UnaryOperator>(S), Ctx);
   case Stmt::BinaryOperatorClass:
+  case Stmt::CompoundAssignOperatorClass:
     return translateBinaryOperator(cast<BinaryOperator>(S), Ctx);
 
   case Stmt::ArraySubscriptExprClass:
@@ -277,6 +292,32 @@ til::SExpr *SExprBuilder::translateUnaryOperator(const UnaryOperator *UO,
 }
 
 
+til::SExpr *SExprBuilder::translateBinAssign(til::TIL_BinaryOpcode Op,
+                                             const BinaryOperator *BO,
+                                             CallingContext *Ctx) {
+  const Expr *LHS = BO->getLHS();
+  const Expr *RHS = BO->getRHS();
+  til::SExpr *E0 = translate(LHS, Ctx);
+  til::SExpr *E1 = translate(RHS, Ctx);
+
+  const ValueDecl *VD = nullptr;
+  til::SExpr *CV = nullptr;
+  if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(LHS)) {
+    VD = DRE->getDecl();
+    CV = lookupVarDecl(VD);
+  }
+
+  if (Op != BO_Assign) {
+    til::SExpr *Arg = CV ? CV : new (Arena) til::Load(E0);
+    E1 = new (Arena) til::BinaryOp(Op, Arg, E1);
+    E1 = addStatement(E1, nullptr, VD);
+  }
+  if (VD && CV)
+    return updateVarDecl(VD, E1);
+  return new (Arena) til::Store(E0, E1);
+}
+
+
 til::SExpr *SExprBuilder::translateBinaryOperator(const BinaryOperator *BO,
                                                   CallingContext *Ctx) {
   switch (BO->getOpcode()) {
@@ -306,35 +347,24 @@ til::SExpr *SExprBuilder::translateBinaryOperator(const BinaryOperator *BO,
         til::BinaryOp(BO->getOpcode(), translate(BO->getLHS(), Ctx),
                       translate(BO->getRHS(), Ctx));
 
-  case BO_Assign: {
+  case BO_Assign:    return translateBinAssign(BO_Assign, BO, Ctx);
-    const Expr *LHS = BO->getLHS();
+  case BO_MulAssign: return translateBinAssign(BO_Mul, BO, Ctx);
-    if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(LHS)) {
+  case BO_DivAssign: return translateBinAssign(BO_Div, BO, Ctx);
-      const Expr *RHS = BO->getRHS();
+  case BO_RemAssign: return translateBinAssign(BO_Rem, BO, Ctx);
-      til::SExpr *E1 = translate(RHS, Ctx);
+  case BO_AddAssign: return translateBinAssign(BO_Add, BO, Ctx);
-      return updateVarDecl(DRE->getDecl(), E1);
+  case BO_SubAssign: return translateBinAssign(BO_Sub, BO, Ctx);
-    }
+  case BO_ShlAssign: return translateBinAssign(BO_Shl, BO, Ctx);
-    til::SExpr *E0 = translate(LHS, Ctx);
+  case BO_ShrAssign: return translateBinAssign(BO_Shr, BO, Ctx);
-    til::SExpr *E1 = translate(BO->getRHS(), Ctx);
+  case BO_AndAssign: return translateBinAssign(BO_And, BO, Ctx);
-    return new (Arena) til::Store(E0, E1);
+  case BO_XorAssign: return translateBinAssign(BO_Xor, BO, Ctx);
-  }
+  case BO_OrAssign:  return translateBinAssign(BO_Or,  BO, Ctx);
-  case BO_MulAssign:
-  case BO_DivAssign:
-  case BO_RemAssign:
-  case BO_AddAssign:
-  case BO_SubAssign:
-  case BO_ShlAssign:
-  case BO_ShrAssign:
-  case BO_AndAssign:
-  case BO_XorAssign:
-  case BO_OrAssign:
-    return new (Arena) til::Undefined(BO);
 
   case BO_Comma:
-    // TODO: handle LHS
+    // The clang CFG should have already processed both sides.
     return translate(BO->getRHS(), Ctx);
-  }
 
   return new (Arena) til::Undefined(BO);
+  }
 }
 
 
@@ -495,14 +525,16 @@ void SExprBuilder::makePhiNodeVar(unsigned i, unsigned NPreds, til::SExpr *E) {
 
   // Make a new phi node: phi(..., E)
   // All phi args up to the current index are set to the current value.
+  til::SExpr *CurrE = CurrentLVarMap[i].second;
   til::Phi *Ph = new (Arena) til::Phi(Arena, NPreds);
   Ph->values().setValues(NPreds, nullptr);
   for (unsigned PIdx = 0; PIdx < ArgIndex; ++PIdx)
-    Ph->values()[PIdx] = CurrentLVarMap[i].second;
+    Ph->values()[PIdx] = CurrE;
   if (E)
     Ph->values()[ArgIndex] = E;
-  if (!E) {
+  // If E is from a back-edge, or either E or CurrE are incomplete, then
-    // This is a non-minimal SSA node, which may be removed later.
+  // mark this node as incomplete; we may need to remove it later.
+  if (!E || isIncompleteVar(E) || isIncompleteVar(CurrE)) {
     Ph->setStatus(til::Phi::PH_Incomplete);
   }
 
@@ -601,8 +633,7 @@ void SExprBuilder::mergePhiNodesBackEdge(const CFGBlock *Blk) {
 }
 
 
-
+void SExprBuilder::enterCFG(CFG *Cfg, const FunctionDecl *FD,
-void SExprBuilder::enterCFG(CFG *Cfg, const NamedDecl *D,
                             const CFGBlock *First) {
   // Perform initial setup operations.
   unsigned NBlocks = Cfg->getNumBlockIDs();
@@ -616,22 +647,35 @@ void SExprBuilder::enterCFG(CFG *Cfg, const NamedDecl *D,
     auto *BB = new (Arena) til::BasicBlock(Arena, 0, B->size());
     BlockMap[B->getBlockID()] = BB;
   }
-  CallCtx = new SExprBuilder::CallingContext(D);
+  CallCtx = new SExprBuilder::CallingContext(FD);
+
+  CurrentBB = lookupBlock(&Cfg->getEntry());
+  for (auto *Pm : FD->parameters()) {
+    QualType T = Pm->getType();
+    if (!T.isTrivialType(Pm->getASTContext()))
+      continue;
+
+    // Add parameters to local variable map.
+    // FIXME: right now we emulate params with loads; that should be fixed.
+    til::SExpr *Lp = new (Arena) til::LiteralPtr(Pm);
+    til::SExpr *Ld = new (Arena) til::Load(Lp);
+    til::SExpr *V  = addStatement(Ld, nullptr, Pm);
+    addVarDecl(Pm, V);
+  }
 }
 
 
 void SExprBuilder::enterCFGBlock(const CFGBlock *B) {
   // Intialize TIL basic block and add it to the CFG.
-  CurrentBB = BlockMap[B->getBlockID()];
+  CurrentBB = lookupBlock(B);
   CurrentBB->setNumPredecessors(B->pred_size());
   Scfg->add(CurrentBB);
 
   CurrentBlockInfo = &BBInfo[B->getBlockID()];
-  CurrentArguments.clear();
-  CurrentInstructions.clear();
 
   // CurrentLVarMap is moved to ExitMap on block exit.
-  assert(!CurrentLVarMap.valid() && "CurrentLVarMap already initialized.");
+  // FIXME: the entry block will hold function parameters.
+  // assert(!CurrentLVarMap.valid() && "CurrentLVarMap already initialized.");
 }
 
 
@@ -721,6 +765,8 @@ void SExprBuilder::handleSuccessorBackEdge(const CFGBlock *Succ) {
 
 
 void SExprBuilder::exitCFGBlock(const CFGBlock *B) {
+  CurrentArguments.clear();
+  CurrentInstructions.clear();
   CurrentBlockInfo->ExitMap = std::move(CurrentLVarMap);
   CurrentBB = nullptr;
   CurrentBlockInfo = nullptr;