Merge branch 'master' into BigRefactor

Author: Machiry

clang/docs/checkedc/Bounds-Widening-for-Null-Terminated-Arrays.md

@@ -31,19 +31,20 @@ in a basic block is taken into consideration when performing the analysis.
 4. **Intra-procedural:** The analysis is done on one function at a time.
 
 ## Dataflow Analysis Details
-For every basic block we compute the following sets: `In` and `Kill`. The `In`
+For every basic block, we compute the following sets: `In` and `Kill`. The `In`
 set for basic block `B` is denoted as `In[B]` and the `Kill` set is denoted as
 `Kill[B]`.
 
-For every edge we compute the following sets: `Out` and `Gen`. The `Out` set on
-edge `Bi->Bj` is denoted as `Out[Bi][Bj]` and the Gen set is denoted as
+For every edge, we compute the following sets: `Out` and `Gen`. The `Out` set on
+edge `Bi->Bj` is denoted as `Out[Bi][Bj]` and the `Gen` set is denoted as
 `Gen[Bi][Bj]`.
 
 ### In[B]
 `In[B]` stores the mapping between an `_Nt_array_ptr` and its widened bounds
 inside block `B`. For example, given `_Nt_array_ptr V` with declared bounds
-`(low, high)`, `In[B]` would store the mapping `{V:i}`, where `i` is an unsigned
-integer and the bounds of `V` should be widened to `(low, high + i)`.
+`(V + low, V + high)`, `In[B]` would store the mapping `{V:i}`, where `i` is an
+unsigned integer implying that the bounds of `V` should be widened to
+`(V + low, V + high + i)`.
 
 Dataflow equation:
 `In[B] = ∩ Out[B*][B], where B* ∈ pred(B)`.
@@ -57,43 +58,43 @@ Thus, `Kill[B]` stores the mapping between a statement `S` and `_Nt_array_ptr's`
 whose bounds are killed in `S`.
 
 ### Gen[Bi][Bj]
-Given `_Nt_array_ptr V` with declared bounds `(low, high)`, the bounds of `V`
-can be widened by 1 if `V` is dereferenced at the upper bound. This means that
-if there is an edge `Bi->Bj` whose edge condition is of the form `if (*(V +
-high + i))`, where `i` is an unsigned integer offset, the widened bounds
-`{V:i+1}` can be added to `Gen[Bi][Bj]`, provided we have already tested for
-pointer access of the form `if (*(V + high + i - 1))`.
+Given `_Nt_array_ptr V` with declared bounds `(V + low, V + high)`, the bounds
+of `V` can be widened by 1 if `V` is dereferenced at its current upper bound.
+This means that if there is an edge `Bi->Bj` whose edge condition is of the
+form `if (*(V + high + i))`, where `i` is an unsigned integer offset, the
+widened bounds `{V:i+1}` can be added to `Gen[Bi][Bj]`, provided we have
+already tested for pointer access of the form `if (*(V + high + (i - 1)))`.
 
 For example:
 ```
-_Nt_array_ptr<T> V : bounds (low, high);
+_Nt_array_ptr<T> V : bounds (V + low, V + high);
 if (*V) { // Ptr dereference is NOT at the current upper bound. No bounds widening.
-  if (*(V + high)) { // Ptr dereference is at the current upper bound. Widen bounds by 1. New bounds for V are (low, high + 1).
-    if (*(V + high + 1)) { // Ptr dereference is at the current upper bound. Widen bounds by 1. New bounds for V are (low, high + 2).
-      if (*(V + high + 3)) { // Ptr dereference is *not* at the current upper bound. No bounds widening. Flag an error!.
+  if (*(V + high)) { // Ptr dereference is at the current upper bound. Widen bounds by 1. New bounds for V are (V + low, V + high + 1).
+    if (*(V + high + 1)) { // Ptr dereference is at the current upper bound. Widen bounds by 1. New bounds for V are (V + low, V + high + 2).
+      if (*(V + high + 3)) { // Ptr dereference is NOT at the current upper bound. No bounds widening. Flag an error!
 ```
 
 ### Out[Bi][Bj]
 `Out[Bi][Bj]` denotes the bounds widened by block `Bi` on edge `Bi->Bj`.
 
 Dataflow equation:
-`Out[Bi][Bj] = (In[Bi] - Kill[Bi]) ∪ Gen[Bi][Bj]`
+`Out[Bi][Bj] = (In[Bi] - Kill[Bi]) ∪ Gen[Bi][Bj], where Bj ∈ succ(Bi)`.
 
 ### Initial values of In and Out sets
 
 To compute `In[B]`, we compute the intersection of `Out[B*][B]`, where `B*` are
 all preds of block `B`. When there is a back edge from block `B'` to `B` (for
-example in the case of loops), the Out set for block `B'` will be empty. As a
+example in the case of loops), the `Out` set for block `B'` will be empty. As a
 result, the intersection operation would always result in an empty set `In[B]`.
 
-So to handle this, we initialize the In and Out sets for all blocks to `Top`.
-`Top` represents the union of the Gen sets of all edges. We have chosen the
-offsets of ptr variables in `Top` to be the max unsigned int. The reason behind
-this is that in order to compute the actual In sets for blocks we are going to
-intersect the Out sets on all the incoming edges of the block. And in that case
-we would always pick the ptr with the smaller offset. Choosing max unsigned int
-also makes handling `Top` much easier as we do not need to explicitly store edge
-info.
+So to handle this, we initialize the `In` and `Out` sets for all blocks to
+`Top`. `Top` represents the union of the `Gen` sets of all edges. We have
+chosen the offsets of ptr variables in `Top` to be `UINT_MAX`. The reason
+behind this is that in order to compute the actual `In` sets for blocks we are
+going to intersect the `Out` sets on all the incoming edges of the block. And
+in that case we would always pick the ptr with the smaller offset. Choosing
+`UINT_MAX` also makes handling `Top` much easier as we do not need to
+explicitly store edge info.
 
 Thus, we have the following two equations for `Top`:
 ```
@@ -107,16 +108,16 @@ In[B] = Top
 Out[Bi][Bj] = Top, where Bj ∈ succ(Bi)
 ```
 
-Now, we also need to handle the case where there is an unconditional jump into a
-block (for example, as a result of a `goto`). In this case, we cannot widen the
-bounds because we would not have tested the ptr dereference on the
-unconditional edge. So in this case we want the intersection (and hence the In
-set) to result in an empty set.
+Now, we also need to handle the case where there is an unconditional jump into
+a block (for example, as a result of a `goto`). In this case, we cannot widen
+the bounds because we would not have tested the ptr dereference on the
+unconditional edge. So in this case we want the intersection (and hence the
+`In` set) to result in an empty set.
 
-So we initialize the In and Out sets of all blocks to `Top`, except the Entry
-block.
+So we initialize the `In` and `Out` sets of all blocks to `Top`, except the
+`Entry` block.
 
-Thus, we have the following initial value for the Entry block:
+Thus, we have the following initial values for the `Entry` block:
 ```
 In[Entry] = ∅
 Out[Entry][B*] = ∅, where B* ∈ succ(Entry)
@@ -128,12 +129,12 @@ The main class that implements the analysis is
 and the main function is `BoundsAnalysis::WidenBounds()`.
 
 `WidenBounds` will perform the bounds widening for the entire function. We can
-then we can call `BoundsAnalysis::GetWidenedBounds` to retrieve the
-widened bounds for the current basic block.
+then call `BoundsAnalysis::GetWidenedBounds` to retrieve the widened bounds for
+the current basic block.
 
 The approach used for implementing the analysis is the iterative worklist
 algorithm in which we keep adding blocks to a worklist as long as we do not
-reach a fixed point i.e.: as long as the Out sets for the blocks keep changing.
+reach a fixed point i.e.: as long as the `Out` sets for the blocks keep changing.
 
 ### Algorithm
 ```

clang/include/clang/AST/PreorderAST.h

@@ -21,38 +21,54 @@
 #include "clang/AST/Expr.h"
 
 namespace clang {
-
   using Result = Lexicographic::Result;
 
-  // Each binary operator of an expression results in a new node of the
-  // PreorderAST. Each node contains the following fields:
+  class Node {
+  public:
+    enum class NodeKind { BinaryNode, LeafExprNode };
 
-  // Opc: The opcode of the operator.
-  // Vars: A list of variables in the sub expression.
-  // Const: Constants of the sub expression are folded.
-  // HasConst: Indicates whether there is a constant in the node. It is used to
-  // differentiate between the absence of a constant and a constant value of 0.
-  // Parent: A link to the parent node of the current node.
-  // Children: The preorder AST is an n-ary tree. Children is a list of all the
-  // child nodes of the current node.
+    NodeKind Kind;
+    Node *Parent;
+
+    Node(NodeKind Kind, Node *Parent) :
+      Kind(Kind), Parent(Parent) {}
+  };
 
-  struct Node {
+  class BinaryNode : public Node {
+  public:
     BinaryOperator::Opcode Opc;
-    std::vector<const VarDecl *> Vars;
-    llvm::APSInt Const;
-    bool HasConst;
-    Node *Parent;
     llvm::SmallVector<Node *, 2> Children;
 
-    Node(Node *Parent) :
-      Opc(BO_Add), HasConst(false), Parent(Parent) {}
+    BinaryNode(BinaryOperator::Opcode Opc, Node *Parent) :
+      Node(NodeKind::BinaryNode, Parent),
+      Opc(Opc) {}
+
+    static bool classof(const Node *N) {
+      return N->Kind == NodeKind::BinaryNode;
+    }
 
     // Is the operator commutative and associative?
     bool IsOpCommutativeAndAssociative() {
       return Opc == BO_Add || Opc == BO_Mul;
     }
   };
 
+  class LeafExprNode : public Node {
+  public:
+    Expr *E;
+
+    LeafExprNode(Expr *E, Node *Parent) :
+      Node(NodeKind::LeafExprNode, Parent),
+      E(E) {}
+
+    static bool classof(const Node *N) {
+      return N->Kind == NodeKind::LeafExprNode;
+    }
+  };
+
+} // end namespace clang
+
+namespace clang {
   class PreorderAST {
   private:
     ASTContext &Ctx;
@@ -62,13 +78,29 @@ namespace clang {
     Node *Root;
 
     // Create a PreorderAST for the expression E.
-    // @param[in] E is the sub expression which needs to be added to N.
-    // @param[in] N is the current node of the AST.
-    // @param[in] Parent is the parent node for N.
-    void Create(Expr *E, Node *N = nullptr, Node *Parent = nullptr);
-
-    // Sort the variables in a node of the AST.
-    // @param[in] N is current node of the AST.
+    // @param[in] E is the sub expression to be added to a new node.
+    // @param[in] Parent is the parent of the new node.
+    void Create(Expr *E, Node *Parent = nullptr);
+
+    // Add a new node to the AST.
+    // @param[in] Node is the current node to be added.
+    // @param[in] Parent is the parent of the node to be added.
+    void AddNode(Node *N, Node *Parent);
+
+    // Coalesce the BinaryNode with its parent.
+    // @param[in] B is the current BinaryNode.
+    // @param[in] Parent is the parent of the node to be coalesced.
+    void CoalesceNode(BinaryNode *B, BinaryNode *Parent);
+
+    // Recursively coalesce binary nodes having the same commutative and
+    // associative operator.
+    // @param[in] N is current node of the AST. Initial value is Root.
+    // @param[in] Changed indicates whether a node was coalesced. We need this
+    // to control when to stop recursive coalescing.
+    void Coalesce(Node *N, bool &Changed);
+
+    // Sort the children expressions in a binary node of the AST.
+    // @param[in] N is current node of the AST. Initial value is Root.
     void Sort(Node *N);
 
     // Check if the two AST nodes N1 and N2 are equal.
@@ -81,20 +113,13 @@ namespace clang {
     void SetError() { Error = true; }
 
     // Print the PreorderAST.
-    // @param[in] N is the current node of the AST.
+    // @param[in] N is the current node of the AST. Initial value is Root.
     void PrettyPrint(Node *N);
 
     // Cleanup the memory consumed by node N.
-    // @param[in] N is the current node of the AST.
+    // @param[in] N is the current node of the AST. Initial value is Root.
     void Cleanup(Node *N);
 
-    // A DeclRefExpr can be a reference either to an array subscript (in which
-    // case it is wrapped around a ArrayToPointerDecay cast) or to a pointer
-    // dereference (in which case it is wrapped around an LValueToRValue cast).
-    // @param[in] An expression E.
-    // @return Returns a DeclRefExpr if E is a DeclRefExpr, otherwise nullptr.
-    DeclRefExpr *GetDeclOperand(Expr *E);
-
   public:
     PreorderAST(ASTContext &Ctx, Expr *E) :
       Ctx(Ctx), Lex(Lexicographic(Ctx, nullptr)), OS(llvm::outs()),

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -10164,20 +10164,21 @@ def err_bounds_type_annotation_lost_checking : Error<
 
   def warn_bounds_declaration_invalid : Warning<
     "cannot prove declared bounds for %1 are valid after "
-    "%select{assignment|initialization|statement}0">,
+    "%select{assignment|decrement|increment|initialization|statement}0">,
     InGroup<CheckBoundsDeclsUnchecked>;
 
   def warn_checked_scope_bounds_declaration_invalid : Warning<
     "cannot prove declared bounds for %1 are valid after "
-    "%select{assignment|initialization|statement}0">,
+    "%select{assignment|decrement|increment|initialization|statement}0">,
     InGroup<CheckBoundsDeclsChecked>;
 
   def error_bounds_declaration_invalid : Error<
     "declared bounds for %1 are invalid after "
-    "%select{assignment|initialization|statement}0">;
+    "%select{assignment|decrement|increment|initialization|statement}0">;
 
   def err_unknown_inferred_bounds : Error<
-    "inferred bounds for %0 are unknown after statement">;
+    "inferred bounds for %1 are unknown after "
+    "%select{assignment|decrement|increment|initialization|statement}0">;
 
   def note_declared_bounds : Note<
     "(expanded) declared bounds are '%0'">;

clang/include/clang/Sema/BoundsAnalysis.h

@@ -116,9 +116,6 @@ namespace clang {
       EdgeBoundsTy Gen, Out;
       // The Kill set for the block.
       StmtDeclSetTy Kill;
-      // The set of all variables used in bounds expr for each ntptr in the
-      // block.
-      BoundsVarTy BoundsVars;
 
       // To compute In[B] we compute the intersection of Out[B*->B], where B*
       // are all preds of B. When there is a back edge from block B' to B (for
@@ -156,9 +153,9 @@ namespace clang {
     // to lookup ElevatedCFGBlock from CFGBlock.
     BlockMapTy BlockMap;
 
-    // A set of all ntptrs in scope. Currently, we simply collect all ntptrs
-    // defined in the function.
-    DeclSetTy NtPtrsInScope;
+    // The mapping of all ntptrs in the function and all variables occurring in
+    // the bounds expr for each ntptr.
+    BoundsVarTy NtPtrsInScope;
 
     // To compute In[B] we compute the intersection of Out[B*->B], where B* are
     // all preds of B. When there is a back edge from block B' to B (for
@@ -246,14 +243,13 @@ namespace clang {
     // @param[in] Dest block for the edge for which the Gen set is updated.
     void FillGenSet(Expr *E, ElevatedCFGBlock *EB, ElevatedCFGBlock *SuccEB);
 
-    // Uniformize the expr, fill Gen set and get variables used in bounds expr
-    // for the ntptr.
+    // Uniformize the expr, fill Gen set for the edge EB->SuccEB.
     // @param[in] E is an ntptr dereference or array subscript expr.
     // @param[in] Source block for the edge for which the Gen set is updated.
     // @param[in] Dest block for the edge for which the Gen set is updated.
-    void FillGenSetAndGetBoundsVars(const Expr *E,
-                                    ElevatedCFGBlock *EB,
-                                    ElevatedCFGBlock *SuccEB);
+    void FillGenSetForEdge(const Expr *E,
+                           ElevatedCFGBlock *EB,
+                           ElevatedCFGBlock *SuccEB);
 
     // Collect all variables used in bounds expr E.
     // @param[in] E represents the bounds expr for an ntptr.
@@ -264,6 +260,11 @@ namespace clang {
     // Assign the widened bounds from the ElevatedBlock to the CFG Block.
     void CollectWidenedBounds();
 
+    // Extract the terminating sub-expression from the expression E.
+    // @param[in] E is the expression from which we need to extract the terminating sub-expression.
+    // @return The terminating sub-expression from the expression E.
+    Expr *GetTerminatorCondition(const Expr *E) const;
+
     // Get the terminating condition for a block. This could be an if condition
     // of the form "if(*(p + i))".
     // @param[in] B is the block for which we need the terminating condition.
@@ -297,17 +298,9 @@ namespace clang {
     // Get the DeclRefExpr from an expression E.
     // @param[in] An expression E which is known to be either an LValueToRValue
     // cast or an ArrayToPointerDecay cast.
-    // @return The DeclRefExpr from the expression E.
+    // @return The DeclRefExpr from the expression E or nullptr.
     DeclRefExpr *GetDeclOperand(const Expr *E);
 
-    // A DeclRefExpr can be a reference either to an array subscript (in which
-    // case it is wrapped around a ArrayToPointerDecay cast) or to a pointer
-    // dereference (in which case it is wrapped around an LValueToRValue cast).
-    // @param[in] An expression E.
-    // @return Whether E is an expression containing a reference to an array
-    // subscript or a pointer dereference.
-    bool IsDeclOperand(const Expr *E);
-
     // Make an expression uniform by moving all DeclRefExpr to the LHS and all
     // IntegerLiterals to the RHS.
     // @param[in] E is the expression which should be made uniform.
@@ -343,10 +336,10 @@ namespace clang {
     // @return The intersection of sets A and B.
     template<class T> T Intersect(T &A, T &B) const;
 
-    // Compute the union of sets A and B.
+    // Compute the union of sets A and B and widen the bounds where applicable.
     // @param[in] A is a set.
     // @param[in] B is a set.
-    // @return The union of sets A and B.
+    // @return The union of sets A and B containing the widened bounds.
     template<class T> T Union(T &A, T &B) const;
 
     // Compute the set difference of sets A and B.

clang/include/clang/Sema/Sema.h

@@ -5186,8 +5186,10 @@ class Sema {
 
   enum BoundsDeclarationCheck {
       BDC_Assignment,
+      BDC_Decrement,
+      BDC_Increment,
       BDC_Initialization,
-      BDC_Statement
+      BDC_Statement,
   };
 
   /// \brief Check that address=of operation is not taking the