Revert "[analyzer] Handle [[assume(cond)]] as __builtin_assume(cond) (#116462)"

This reverts commit 89da344.

Reason: buildbot breakages e.g., https://lab.llvm.org/buildbot/#/builders/55/builds/4556 (for which the reverted patch is the only code change)

Author: thurstond

clang/include/clang/AST/AttrIterator.h

@@ -16,7 +16,6 @@
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/ADL.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Casting.h"
 #include <cassert>
 #include <cstddef>
@@ -125,17 +124,6 @@ inline auto *getSpecificAttr(const Container &container) {
   return It != specific_attr_end<IterTy>(container) ? *It : nullptr;
 }
 
-template <typename SpecificAttr, typename Container>
-inline auto getSpecificAttrs(const Container &container) {
-  using ValueTy = llvm::detail::ValueOfRange<Container>;
-  using ValuePointeeTy = std::remove_pointer_t<ValueTy>;
-  using IterTy = std::conditional_t<std::is_const_v<ValuePointeeTy>,
-                                    const SpecificAttr, SpecificAttr>;
-  auto Begin = specific_attr_begin<IterTy>(container);
-  auto End = specific_attr_end<IterTy>(container);
-  return llvm::make_range(Begin, End);
-}
-
 } // namespace clang
 
 #endif // LLVM_CLANG_AST_ATTRITERATOR_H

clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

@@ -498,10 +498,6 @@ class ExprEngine {
   void VisitInitListExpr(const InitListExpr *E, ExplodedNode *Pred,
                          ExplodedNodeSet &Dst);
 
-  /// VisitAttributedStmt - Transfer function logic for AttributedStmt
-  void VisitAttributedStmt(const AttributedStmt *A, ExplodedNode *Pred,
-                           ExplodedNodeSet &Dst);
-
   /// VisitLogicalExpr - Transfer function logic for '&&', '||'
   void VisitLogicalExpr(const BinaryOperator* B, ExplodedNode *Pred,
                         ExplodedNodeSet &Dst);

clang/lib/Analysis/CFG.cpp

@@ -433,7 +433,7 @@ class reverse_children {
   ArrayRef<Stmt *> children;
 
 public:
-  reverse_children(Stmt *S, ASTContext &Ctx);
+  reverse_children(Stmt *S);
 
   using iterator = ArrayRef<Stmt *>::reverse_iterator;
 
@@ -443,47 +443,28 @@ class reverse_children {
 
 } // namespace
 
-reverse_children::reverse_children(Stmt *S, ASTContext &Ctx) {
-  switch (S->getStmtClass()) {
-  case Stmt::CallExprClass: {
-    children = cast<CallExpr>(S)->getRawSubExprs();
+reverse_children::reverse_children(Stmt *S) {
+  if (CallExpr *CE = dyn_cast<CallExpr>(S)) {
+    children = CE->getRawSubExprs();
     return;
   }
-
-  // Note: Fill in this switch with more cases we want to optimize.
-  case Stmt::InitListExprClass: {
-    InitListExpr *IE = cast<InitListExpr>(S);
-    children = llvm::ArrayRef(reinterpret_cast<Stmt **>(IE->getInits()),
-                              IE->getNumInits());
-    return;
+  switch (S->getStmtClass()) {
+    // Note: Fill in this switch with more cases we want to optimize.
+    case Stmt::InitListExprClass: {
+      InitListExpr *IE = cast<InitListExpr>(S);
+      children = llvm::ArrayRef(reinterpret_cast<Stmt **>(IE->getInits()),
+                                IE->getNumInits());
+      return;
+    }
+    default:
+      break;
   }
-  case Stmt::AttributedStmtClass: {
-    auto *AS = cast<AttributedStmt>(S);
 
-    // for an attributed stmt, the "children()" returns only the NullStmt
-    // (;) but semantically the "children" are supposed to be the
-    // expressions _within_ i.e. the two square brackets i.e. [[ HERE ]]
-    // so we add the subexpressions first, _then_ add the "children"
+  // Default case for all other statements.
+  llvm::append_range(childrenBuf, S->children());
 
-    for (const auto *Attr : AS->getAttrs()) {
-      if (const auto *AssumeAttr = dyn_cast<CXXAssumeAttr>(Attr)) {
-        Expr *AssumeExpr = AssumeAttr->getAssumption();
-        if (!AssumeExpr->HasSideEffects(Ctx)) {
-          childrenBuf.push_back(AssumeExpr);
-        }
-      }
-      // Visit the actual children AST nodes.
-      // For CXXAssumeAttrs, this is always a NullStmt.
-      llvm::append_range(childrenBuf, AS->children());
-      children = childrenBuf;
-    }
-    return;
-  }
-  default:
-    // Default case for all other statements.
-    llvm::append_range(childrenBuf, S->children());
-    children = childrenBuf;
-  }
+  // This needs to be done *after* childrenBuf has been populated.
+  children = childrenBuf;
 }
 
 namespace {
@@ -2450,7 +2431,7 @@ CFGBlock *CFGBuilder::VisitChildren(Stmt *S) {
 
   // Visit the children in their reverse order so that they appear in
   // left-to-right (natural) order in the CFG.
-  reverse_children RChildren(S, *Context);
+  reverse_children RChildren(S);
   for (Stmt *Child : RChildren) {
     if (Child)
       if (CFGBlock *R = Visit(Child))
@@ -2466,7 +2447,7 @@ CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) {
   }
   CFGBlock *B = Block;
 
-  reverse_children RChildren(ILE, *Context);
+  reverse_children RChildren(ILE);
   for (Stmt *Child : RChildren) {
     if (!Child)
       continue;
@@ -2501,14 +2482,6 @@ static bool isFallthroughStatement(const AttributedStmt *A) {
   return isFallthrough;
 }
 
-static bool isCXXAssumeAttr(const AttributedStmt *A) {
-  bool hasAssumeAttr = hasSpecificAttr<CXXAssumeAttr>(A->getAttrs());
-
-  assert((!hasAssumeAttr || isa<NullStmt>(A->getSubStmt())) &&
-         "expected [[assume]] not to have children");
-  return hasAssumeAttr;
-}
-
 CFGBlock *CFGBuilder::VisitAttributedStmt(AttributedStmt *A,
                                           AddStmtChoice asc) {
   // AttributedStmts for [[likely]] can have arbitrary statements as children,
@@ -2524,11 +2497,6 @@ CFGBlock *CFGBuilder::VisitAttributedStmt(AttributedStmt *A,
     appendStmt(Block, A);
   }
 
-  if (isCXXAssumeAttr(A) && asc.alwaysAdd(*this, A)) {
-    autoCreateBlock();
-    appendStmt(Block, A);
-  }
-
   return VisitChildren(A);
 }
 

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

@@ -1941,6 +1941,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
     // to be explicitly evaluated.
     case Stmt::PredefinedExprClass:
     case Stmt::AddrLabelExprClass:
+    case Stmt::AttributedStmtClass:
     case Stmt::IntegerLiteralClass:
     case Stmt::FixedPointLiteralClass:
     case Stmt::CharacterLiteralClass:
@@ -1971,13 +1972,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       break;
     }
 
-    case Stmt::AttributedStmtClass: {
-      Bldr.takeNodes(Pred);
-      VisitAttributedStmt(cast<AttributedStmt>(S), Pred, Dst);
-      Bldr.addNodes(Dst);
-      break;
-    }
-
     case Stmt::CXXDefaultArgExprClass:
     case Stmt::CXXDefaultInitExprClass: {
       Bldr.takeNodes(Pred);

clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp

@@ -794,18 +794,17 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex,
 
   // Find the predecessor block.
   ProgramStateRef SrcState = state;
-
   for (const ExplodedNode *N = Pred ; N ; N = *N->pred_begin()) {
-    auto Edge = N->getLocationAs<BlockEdge>();
-    if (!Edge.has_value()) {
+    ProgramPoint PP = N->getLocation();
+    if (PP.getAs<PreStmtPurgeDeadSymbols>() || PP.getAs<BlockEntrance>()) {
       // If the state N has multiple predecessors P, it means that successors
       // of P are all equivalent.
       // In turn, that means that all nodes at P are equivalent in terms
       // of observable behavior at N, and we can follow any of them.
       // FIXME: a more robust solution which does not walk up the tree.
       continue;
     }
-    SrcBlock = Edge->getSrc();
+    SrcBlock = PP.castAs<BlockEdge>().getSrc();
     SrcState = N->getState();
     break;
   }

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

@@ -10,7 +10,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/AST/AttrIterator.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/ParentMap.h"
 #include "clang/AST/StmtCXX.h"
@@ -1201,20 +1200,3 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE, ExplodedNode *Pred,
   // FIXME: Move all post/pre visits to ::Visit().
   getCheckerManager().runCheckersForPostStmt(Dst, Tmp, LE, *this);
 }
-
-void ExprEngine::VisitAttributedStmt(const AttributedStmt *A,
-                                     ExplodedNode *Pred, ExplodedNodeSet &Dst) {
-  ExplodedNodeSet CheckerPreStmt;
-  getCheckerManager().runCheckersForPreStmt(CheckerPreStmt, Pred, A, *this);
-
-  ExplodedNodeSet EvalSet;
-  StmtNodeBuilder Bldr(CheckerPreStmt, EvalSet, *currBldrCtx);
-
-  for (const auto *Attr : getSpecificAttrs<CXXAssumeAttr>(A->getAttrs())) {
-    for (ExplodedNode *N : CheckerPreStmt) {
-      Visit(Attr->getAssumption(), N, EvalSet);
-    }
-  }
-
-  getCheckerManager().runCheckersForPostStmt(Dst, EvalSet, A, *this);
-}

clang/test/Analysis/cxx23-assume-attribute.cpp

@@ -1,11 +1,4 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -verify %s \
-// RUN:   -analyzer-checker=unix,core,alpha.security.ArrayBoundV2,debug.ExprInspection
-
-template <typename T> void clang_analyzer_dump(T);
-template <typename T> void clang_analyzer_value(T);
-void clang_analyzer_eval(bool);
-template <typename T>
-void clang_analyzer_explain(T);
+// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -analyzer-checker=unix,core,alpha.security.ArrayBoundV2 -verify %s
 
 // Tests doing an out-of-bounds access after the end of an array using:
 // - constant integer index
@@ -187,58 +180,3 @@ int test_reference_that_might_be_after_the_end(int idx) {
   return ref;
 }
 
-// From: https://github.com/llvm/llvm-project/issues/100762
-extern int arrOf10[10];
-void using_builtin(int x) {
-  __builtin_assume(x > 101); // CallExpr
-  arrOf10[x] = 404; // expected-warning {{Out of bound access to memory}}
-}
-
-void using_assume_attr(int ax) {
-  [[assume(ax > 100)]]; // NullStmt with an "assume" attribute.
-  arrOf10[ax] = 405; // expected-warning {{Out of bound access to memory}}
-}
-
-void using_many_assume_attr(int yx) {
-  [[assume(yx > 104), assume(yx > 200), assume(yx < 300)]]; // NullStmt with an attribute
-  arrOf10[yx] = 406; // expected-warning{{Out of bound access to memory}}
-}
-
-
-int using_builtin_assume_has_no_sideeffects(int y) {
-  // We should not apply sideeffects of the argument of [[assume(...)]].
-  // "y" should not get incremented;
-  __builtin_assume(++y == 43); // expected-warning {{assumption is ignored because it contains (potential) side-effects}}
-  clang_analyzer_eval(y == 42); // expected-warning {{FALSE}}
-  return y;
-}
-
-
-
-int using_assume_attr_has_no_sideeffects(int y) {
-
-  // We should not apply sideeffects of the argument of [[assume(...)]].
-  // "y" should not get incremented;
-  [[assume(++y == 43)]]; // expected-warning {{assumption is ignored because it contains (potential) side-effects}}
- 
-  clang_analyzer_eval(y == 42); // expected-warning {{TRUE}} expected-warning {{FALSE}} FIXME: This should be only TRUE.
-
-  clang_analyzer_eval(y == 43); // expected-warning {{FALSE}} expected-warning {{TRUE}} FIXME: This should be only FALSE.
-
-  return y;
-}
-
-
-int using_builtinassume_has_no_sideeffects(int u) {
-  // We should not apply sideeffects of the argument of __builtin_assume(...)
-  // "u" should not get incremented;
-  __builtin_assume(++u == 43); // expected-warning {{assumption is ignored because it contains (potential) side-effects}}
- 
-  // FIXME: evaluate this to true
-  clang_analyzer_eval(u == 42); // expected-warning {{FALSE}}  current behavior 
-
-  // FIXME: evaluate this to false
-  clang_analyzer_eval(u == 43); // expected-warning {{TRUE}}  current behavior 
-
-  return u;
-}