From daddb9e13db6ca8373dc7298d17aa36a03014aeb Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh@users.noreply.github.com>
Date: Fri, 15 Nov 2024 07:37:17 -0500
Subject: [PATCH 01/14] [analyzer] Handle `[[assume(cond)]]` as
 `__builtin_assume(cond)`

Resolves #100762
---
 .../Core/PathSensitive/ExprEngine.h           |  4 ++
 clang/lib/Analysis/CFG.cpp                    | 43 +++++++++++++++++++
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp  |  8 +++-
 .../lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 27 ++++++++++++
 clang/test/Analysis/out-of-bounds-new.cpp     | 16 +++++++
 5 files changed, 97 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 8c7493e27fcaa..078a1d840d051 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -498,6 +498,10 @@ 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);
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index f678ac6f2ff36..fab10f51cf5cf 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -456,6 +456,36 @@ reverse_children::reverse_children(Stmt *S) {
                                 IE->getNumInits());
       return;
     }
+    case Stmt::AttributedStmtClass: {
+      AttributedStmt *attrStmt = cast<AttributedStmt>(S);
+      assert(attrStmt);
+
+      {
+        // 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"
+
+        for (const Attr *attr : attrStmt->getAttrs()) {
+
+          // i.e. one `assume()`
+          CXXAssumeAttr const *assumeAttr = llvm::dyn_cast<CXXAssumeAttr>(attr);
+          if (!assumeAttr) {
+            continue;
+          }
+          // Only handles [[ assume(<assumption>) ]] right now
+          Expr *assumption = assumeAttr->getAssumption();
+          childrenBuf.push_back(assumption);
+        }
+
+        // children() for an AttributedStmt is NullStmt(;)
+        llvm::append_range(childrenBuf, attrStmt->children());
+
+        // This needs to be done *after* childrenBuf has been populated.
+        children = childrenBuf;
+      }
+      return;
+    }
     default:
       break;
   }
@@ -2475,6 +2505,14 @@ 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,
@@ -2490,6 +2528,11 @@ CFGBlock *CFGBuilder::VisitAttributedStmt(AttributedStmt *A,
     appendStmt(Block, A);
   }
 
+  if (isCXXAssumeAttr(A) && asc.alwaysAdd(*this, A)) {
+    autoCreateBlock();
+    appendStmt(Block, A);
+  }
+
   return VisitChildren(A);
 }
 
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 22eab9f66418d..cbc83f1dbda14 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1946,7 +1946,6 @@ 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:
@@ -1977,6 +1976,13 @@ 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);
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index f7020da2e6da2..1a211d1adc44f 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -1200,3 +1200,30 @@ 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 : A->getAttrs()) {
+
+      CXXAssumeAttr const *assumeAttr = llvm::dyn_cast<CXXAssumeAttr>(attr);
+      if (!assumeAttr) {
+        continue;
+      }
+      Expr *AssumeExpr = assumeAttr->getAssumption();
+
+      for (auto *node : CheckerPreStmt) {
+        Visit(AssumeExpr, node, EvalSet);
+      }
+    }
+  }
+
+  getCheckerManager().runCheckersForPostStmt(Dst, EvalSet, A, *this);
+}
diff --git a/clang/test/Analysis/out-of-bounds-new.cpp b/clang/test/Analysis/out-of-bounds-new.cpp
index f541bdf810d79..4db351b10055b 100644
--- a/clang/test/Analysis/out-of-bounds-new.cpp
+++ b/clang/test/Analysis/out-of-bounds-new.cpp
@@ -180,3 +180,19 @@ int test_reference_that_might_be_after_the_end(int idx) {
   return ref;
 }
 
+// From: https://github.com/llvm/llvm-project/issues/100762
+extern int arr[10];
+void using_builtin(int x) {
+    __builtin_assume(x > 101); // CallExpr
+    arr[x] = 404; // expected-warning{{Out of bound access to memory}}
+}
+
+void using_assume_attr(int ax) {
+    [[assume(ax > 100)]]; // NullStmt with an attribute
+    arr[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
+    arr[yx] = 406; // expected-warning{{Out of bound access to memory}}
+}

From 7f1e341e59a52017d9f73ab09e1be45444536731 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs@gmail.com>
Date: Mon, 18 Nov 2024 11:33:42 +0100
Subject: [PATCH 02/14] NFC Simplify ExprEngine::VisitAttributedStmt

---
 clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 1a211d1adc44f..86d57a8dee52e 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -1203,25 +1203,15 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE, ExplodedNode *Pred,
 
 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 : A->getAttrs()) {
-
-      CXXAssumeAttr const *assumeAttr = llvm::dyn_cast<CXXAssumeAttr>(attr);
-      if (!assumeAttr) {
-        continue;
-      }
-      Expr *AssumeExpr = assumeAttr->getAssumption();
-
-      for (auto *node : CheckerPreStmt) {
-        Visit(AssumeExpr, node, EvalSet);
-      }
+  if (const auto *AssumeAttr = getSpecificAttr<CXXAssumeAttr>(A->getAttrs())) {
+    for (ExplodedNode *N : CheckerPreStmt) {
+      Visit(AssumeAttr->getAssumption(), N, EvalSet);
     }
   }
 

From f4307e9a8b8d8390477a8951adda665b0dc35ba7 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs@gmail.com>
Date: Mon, 18 Nov 2024 11:34:25 +0100
Subject: [PATCH 03/14] NFC Simplify AttributedStmt handling in clang CFG

---
 clang/lib/Analysis/CFG.cpp | 32 +++++++-------------------------
 1 file changed, 7 insertions(+), 25 deletions(-)

diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index fab10f51cf5cf..dbdb08506bb62 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -457,34 +457,16 @@ reverse_children::reverse_children(Stmt *S) {
       return;
     }
     case Stmt::AttributedStmtClass: {
-      AttributedStmt *attrStmt = cast<AttributedStmt>(S);
-      assert(attrStmt);
+      auto Attrs = cast<AttributedStmt>(S)->getAttrs();
 
-      {
-        // 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"
-
-        for (const Attr *attr : attrStmt->getAttrs()) {
-
-          // i.e. one `assume()`
-          CXXAssumeAttr const *assumeAttr = llvm::dyn_cast<CXXAssumeAttr>(attr);
-          if (!assumeAttr) {
-            continue;
-          }
-          // Only handles [[ assume(<assumption>) ]] right now
-          Expr *assumption = assumeAttr->getAssumption();
-          childrenBuf.push_back(assumption);
-        }
-
-        // children() for an AttributedStmt is NullStmt(;)
-        llvm::append_range(childrenBuf, attrStmt->children());
-
-        // This needs to be done *after* childrenBuf has been populated.
+      // We only handle `[[assume(...)]]` attributes for now.
+      if (const auto *A = getSpecificAttr<CXXAssumeAttr>(Attrs)) {
+        childrenBuf.push_back(A->getAssumption());
+        llvm::append_range(childrenBuf, S->children());
         children = childrenBuf;
+        return;
       }
-      return;
+      break;
     }
     default:
       break;

From c2b5e71c2fbf0b2240914da068f1afe4dec790cc Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs@gmail.com>
Date: Mon, 18 Nov 2024 11:35:32 +0100
Subject: [PATCH 04/14] NFC Clarify test comment

---
 clang/test/Analysis/out-of-bounds-new.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/test/Analysis/out-of-bounds-new.cpp b/clang/test/Analysis/out-of-bounds-new.cpp
index 4db351b10055b..c2b70580a90c3 100644
--- a/clang/test/Analysis/out-of-bounds-new.cpp
+++ b/clang/test/Analysis/out-of-bounds-new.cpp
@@ -188,7 +188,7 @@ void using_builtin(int x) {
 }
 
 void using_assume_attr(int ax) {
-    [[assume(ax > 100)]]; // NullStmt with an attribute
+    [[assume(ax > 100)]]; // NullStmt with an "assume" attribute.
     arr[ax] = 405; // expected-warning{{Out of bound access to memory}}
 }
 

From 3d4953f8f2a9cc5cf8d47e9c728116911d5421ca Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs@gmail.com>
Date: Mon, 18 Nov 2024 11:35:51 +0100
Subject: [PATCH 05/14] NFC De-indent some tests

---
 clang/test/Analysis/out-of-bounds-new.cpp | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/clang/test/Analysis/out-of-bounds-new.cpp b/clang/test/Analysis/out-of-bounds-new.cpp
index c2b70580a90c3..4896893a6a231 100644
--- a/clang/test/Analysis/out-of-bounds-new.cpp
+++ b/clang/test/Analysis/out-of-bounds-new.cpp
@@ -183,16 +183,16 @@ int test_reference_that_might_be_after_the_end(int idx) {
 // From: https://github.com/llvm/llvm-project/issues/100762
 extern int arr[10];
 void using_builtin(int x) {
-    __builtin_assume(x > 101); // CallExpr
-    arr[x] = 404; // expected-warning{{Out of bound access to memory}}
+  __builtin_assume(x > 101); // CallExpr
+  arr[x] = 404; // expected-warning{{Out of bound access to memory}}
 }
 
 void using_assume_attr(int ax) {
-    [[assume(ax > 100)]]; // NullStmt with an "assume" attribute.
-    arr[ax] = 405; // expected-warning{{Out of bound access to memory}}
+  [[assume(ax > 100)]]; // NullStmt with an "assume" attribute.
+  arr[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
-    arr[yx] = 406; // expected-warning{{Out of bound access to memory}}
+  [[assume(yx > 104), assume(yx > 200), assume(yx < 300)]]; // NullStmt with an attribute
+  arr[yx] = 406; // expected-warning{{Out of bound access to memory}}
 }

From 57751d1bcaa1517033abf0da4f33bc06ffe1e8a7 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs@gmail.com>
Date: Mon, 18 Nov 2024 11:36:40 +0100
Subject: [PATCH 06/14] NFC Mention the size of the array in its name

---
 clang/test/Analysis/out-of-bounds-new.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/clang/test/Analysis/out-of-bounds-new.cpp b/clang/test/Analysis/out-of-bounds-new.cpp
index 4896893a6a231..c4abc70a2d953 100644
--- a/clang/test/Analysis/out-of-bounds-new.cpp
+++ b/clang/test/Analysis/out-of-bounds-new.cpp
@@ -181,18 +181,18 @@ int test_reference_that_might_be_after_the_end(int idx) {
 }
 
 // From: https://github.com/llvm/llvm-project/issues/100762
-extern int arr[10];
+extern int arrOf10[10];
 void using_builtin(int x) {
   __builtin_assume(x > 101); // CallExpr
-  arr[x] = 404; // expected-warning{{Out of bound access to memory}}
+  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.
-  arr[ax] = 405; // expected-warning{{Out of bound access to memory}}
+  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
-  arr[yx] = 406; // expected-warning{{Out of bound access to memory}}
+  arrOf10[yx] = 406; // expected-warning{{Out of bound access to memory}}
 }

From 75e964a4d45cfb0adf396f8b870ffa3bb3d65e35 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs@gmail.com>
Date: Mon, 18 Nov 2024 12:25:06 +0100
Subject: [PATCH 07/14] Add broken test about sideeffects

---
 clang/test/Analysis/out-of-bounds-new.cpp | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/clang/test/Analysis/out-of-bounds-new.cpp b/clang/test/Analysis/out-of-bounds-new.cpp
index c4abc70a2d953..200ee11d71442 100644
--- a/clang/test/Analysis/out-of-bounds-new.cpp
+++ b/clang/test/Analysis/out-of-bounds-new.cpp
@@ -1,4 +1,9 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -analyzer-checker=unix,core,alpha.security.ArrayBoundV2 -verify %s
+// 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);
 
 // Tests doing an out-of-bounds access after the end of an array using:
 // - constant integer index
@@ -194,5 +199,12 @@ void using_assume_attr(int ax) {
 
 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}}
+  arrOf10[yx] = 406; // expected-warning{{Out of bounsssd access to memory}}
+}
+
+int using_assume_attr_has_no_sideeffects(int y) {
+  // We should not apply sideeffects of the argument of [[assume(...)]].
+  [[assume(++y == 43)]]; // "y" should not get incremented
+  clang_analyzer_eval(y == 42); // expected-warning {{TRUE}} expected-warning {{FALSE}} FIXME: This should be only TRUE.
+  return y;
 }

From d55851b5af8967afb2edad2361f31081e8bd950f Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs@gmail.com>
Date: Mon, 18 Nov 2024 12:26:26 +0100
Subject: [PATCH 08/14] [clang] Generalize getSpecificAttr for const attributes

I'll upstream this commit separately to make getSpecificAttr work with
const attributes.
---
 clang/include/clang/AST/AttrIterator.h | 16 +++++++++-------
 clang/lib/CodeGen/CGLoopInfo.cpp       |  2 +-
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/clang/include/clang/AST/AttrIterator.h b/clang/include/clang/AST/AttrIterator.h
index 66571e1cf0b8e..7e2bb0381d4c8 100644
--- a/clang/include/clang/AST/AttrIterator.h
+++ b/clang/include/clang/AST/AttrIterator.h
@@ -14,11 +14,13 @@
 #define LLVM_CLANG_AST_ATTRITERATOR_H
 
 #include "clang/Basic/LLVM.h"
+#include "llvm/ADT/ADL.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
 #include <cassert>
 #include <cstddef>
 #include <iterator>
+#include <type_traits>
 
 namespace clang {
 
@@ -113,13 +115,13 @@ inline bool hasSpecificAttr(const Container& container) {
           specific_attr_end<SpecificAttr>(container);
 }
 template <typename SpecificAttr, typename Container>
-inline SpecificAttr *getSpecificAttr(const Container& container) {
-  specific_attr_iterator<SpecificAttr, Container> i =
-      specific_attr_begin<SpecificAttr>(container);
-  if (i != specific_attr_end<SpecificAttr>(container))
-    return *i;
-  else
-    return nullptr;
+inline auto *getSpecificAttr(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 It = specific_attr_begin<IterTy>(container);
+  return It != specific_attr_end<IterTy>(container) ? *It : nullptr;
 }
 
 } // namespace clang
diff --git a/clang/lib/CodeGen/CGLoopInfo.cpp b/clang/lib/CodeGen/CGLoopInfo.cpp
index cdff7e50c4ee7..448571221ef81 100644
--- a/clang/lib/CodeGen/CGLoopInfo.cpp
+++ b/clang/lib/CodeGen/CGLoopInfo.cpp
@@ -811,7 +811,7 @@ void LoopInfoStack::push(BasicBlock *Header, clang::ASTContext &Ctx,
   // Identify loop attribute 'code_align' from Attrs.
   // For attribute code_align:
   // n - 'llvm.loop.align i32 n' metadata will be emitted.
-  if (const auto *CodeAlign = getSpecificAttr<const CodeAlignAttr>(Attrs)) {
+  if (const auto *CodeAlign = getSpecificAttr<CodeAlignAttr>(Attrs)) {
     const auto *CE = cast<ConstantExpr>(CodeAlign->getAlignment());
     llvm::APSInt ArgVal = CE->getResultAsAPSInt();
     setCodeAlign(ArgVal.getSExtValue());

From c288bdc428a85262500a929ea787dc186a44ca10 Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh@users.noreply.github.com>
Date: Thu, 28 Nov 2024 22:51:46 -0500
Subject: [PATCH 09/14] Ignore assumptions with side-effects

---
 clang/lib/Analysis/CFG.cpp                    | 45 ++++++++++++++-----
 .../lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 16 ++++++-
 clang/test/Analysis/out-of-bounds-new.cpp     | 42 +++++++++++++++--
 3 files changed, 86 insertions(+), 17 deletions(-)

diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index dbdb08506bb62..8d1db015907eb 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -431,9 +431,10 @@ namespace {
 class reverse_children {
   llvm::SmallVector<Stmt *, 12> childrenBuf;
   ArrayRef<Stmt *> children;
+  ASTContext *astContext;
 
 public:
-  reverse_children(Stmt *S);
+  reverse_children(Stmt *S, ASTContext *astContext = nullptr);
 
   using iterator = ArrayRef<Stmt *>::reverse_iterator;
 
@@ -443,7 +444,8 @@ class reverse_children {
 
 } // namespace
 
-reverse_children::reverse_children(Stmt *S) {
+reverse_children::reverse_children(Stmt *S, ASTContext *AstC)
+    : astContext(AstC) {
   if (CallExpr *CE = dyn_cast<CallExpr>(S)) {
     children = CE->getRawSubExprs();
     return;
@@ -457,16 +459,37 @@ reverse_children::reverse_children(Stmt *S) {
       return;
     }
     case Stmt::AttributedStmtClass: {
-      auto Attrs = cast<AttributedStmt>(S)->getAttrs();
+      assert(S->getStmtClass() == Stmt::AttributedStmtClass);
+      assert(this->astContext &&
+             "Attributes need the ast context to determine side-effects");
+      AttributedStmt *AS = cast<AttributedStmt>(S);
+      assert(attrStmt);
+
+      // 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"
+      for (const Attr *Attr : AS->getAttrs()) {
+        // Only handles [[ assume(<assumption>) ]] right now
+        CXXAssumeAttr const *AssumeAttr = llvm::dyn_cast<CXXAssumeAttr>(Attr);
+        if (!AssumeAttr) {
+          continue;
+        }
+        Expr *AssumeExpr = AssumeAttr->getAssumption();
+        // If we skip adding the assumption expression to CFG,
+        // it doesn't get "branch"-ed by symbol analysis engine
+        // presumably because it's literally not in the CFG
 
-      // We only handle `[[assume(...)]]` attributes for now.
-      if (const auto *A = getSpecificAttr<CXXAssumeAttr>(Attrs)) {
-        childrenBuf.push_back(A->getAssumption());
-        llvm::append_range(childrenBuf, S->children());
-        children = childrenBuf;
-        return;
+        if (AssumeExpr->HasSideEffects(*astContext)) {
+          continue;
+        }
+        childrenBuf.push_back(AssumeExpr);
       }
-      break;
+      // children() for an CXXAssumeAttr is NullStmt(;)
+      // for others, it will have existing behavior
+      llvm::append_range(childrenBuf, AS->children());
+      children = childrenBuf;
+      return;
     }
     default:
       break;
@@ -2436,7 +2459,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);
+  reverse_children RChildren(S, Context);
   for (Stmt *Child : RChildren) {
     if (Child)
       if (CFGBlock *R = Visit(Child))
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 86d57a8dee52e..786820c61c41c 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -10,16 +10,23 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/ParentMap.h"
 #include "clang/AST/StmtCXX.h"
+#include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Analysis/ConstructionContext.h"
+#include "clang/Analysis/ProgramPoint.h"
 #include "clang/Basic/PrettyStackTrace.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Sequence.h"
 #include <optional>
@@ -1209,9 +1216,14 @@ void ExprEngine::VisitAttributedStmt(const AttributedStmt *A,
   ExplodedNodeSet EvalSet;
   StmtNodeBuilder Bldr(CheckerPreStmt, EvalSet, *currBldrCtx);
 
-  if (const auto *AssumeAttr = getSpecificAttr<CXXAssumeAttr>(A->getAttrs())) {
+  for (const auto *attr : A->getAttrs()) {
+    CXXAssumeAttr const *AssumeAttr = llvm::dyn_cast<CXXAssumeAttr>(attr);
+    if (!AssumeAttr) {
+      continue;
+    }
+    Expr *AssumeExpr = AssumeAttr->getAssumption();
     for (ExplodedNode *N : CheckerPreStmt) {
-      Visit(AssumeAttr->getAssumption(), N, EvalSet);
+      Visit(AssumeExpr, N, EvalSet);
     }
   }
 
diff --git a/clang/test/Analysis/out-of-bounds-new.cpp b/clang/test/Analysis/out-of-bounds-new.cpp
index 200ee11d71442..39a40eb10bea7 100644
--- a/clang/test/Analysis/out-of-bounds-new.cpp
+++ b/clang/test/Analysis/out-of-bounds-new.cpp
@@ -4,6 +4,8 @@
 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);
 
 // Tests doing an out-of-bounds access after the end of an array using:
 // - constant integer index
@@ -189,22 +191,54 @@ int test_reference_that_might_be_after_the_end(int idx) {
 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}}
+  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}}
+  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 bounsssd access to memory}}
+  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(...)]].
-  [[assume(++y == 43)]]; // "y" should not get incremented
+  // "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;
+}

From eabbef2be6a4f956171a21632d8cf07b4a48e162 Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh@users.noreply.github.com>
Date: Tue, 10 Dec 2024 16:45:20 +0530
Subject: [PATCH 10/14] Add ternary test for assumption

Also, remove unnecessary member for `reverse_children`
---
 clang/lib/Analysis/CFG.cpp                    |  8 ++-
 clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp | 23 ++++++++-
 .../lib/StaticAnalyzer/Core/ExprEngineCXX.cpp |  9 +++-
 .../test/Analysis/cxx23-assume-attribute.cpp  | 49 +++++++++++++++++++
 4 files changed, 81 insertions(+), 8 deletions(-)
 create mode 100644 clang/test/Analysis/cxx23-assume-attribute.cpp

diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 891c542aa5553..fb411043a8b1f 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -431,7 +431,6 @@ namespace {
 class reverse_children {
   llvm::SmallVector<Stmt *, 12> childrenBuf;
   ArrayRef<Stmt *> children;
-  ASTContext *astContext;
 
 public:
   reverse_children(Stmt *S, ASTContext *astContext = nullptr);
@@ -444,8 +443,7 @@ class reverse_children {
 
 } // namespace
 
-reverse_children::reverse_children(Stmt *S, ASTContext *AstC)
-    : astContext(AstC) {
+reverse_children::reverse_children(Stmt *S, ASTContext *AstC) {
   if (CallExpr *CE = dyn_cast<CallExpr>(S)) {
     children = CE->getRawSubExprs();
     return;
@@ -460,7 +458,7 @@ reverse_children::reverse_children(Stmt *S, ASTContext *AstC)
     }
     case Stmt::AttributedStmtClass: {
       assert(S->getStmtClass() == Stmt::AttributedStmtClass);
-      assert(this->astContext &&
+      assert(AstC &&
              "Attributes need the ast context to determine side-effects");
       AttributedStmt *AS = cast<AttributedStmt>(S);
       assert(attrStmt);
@@ -480,7 +478,7 @@ reverse_children::reverse_children(Stmt *S, ASTContext *AstC)
         // it doesn't get "branch"-ed by symbol analysis engine
         // presumably because it's literally not in the CFG
 
-        if (AssumeExpr->HasSideEffects(*astContext)) {
+        if (AssumeExpr->HasSideEffects(*AstC)) {
           continue;
         }
         childrenBuf.push_back(AssumeExpr);
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index 7a900780384a9..046d33164070b 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -785,7 +785,8 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex,
                                   const Expr *R,
                                   ExplodedNode *Pred,
                                   ExplodedNodeSet &Dst) {
-  assert(L && R);
+  assert(L && "L does not exist!");
+  assert(R && "R does not exist!");
 
   StmtNodeBuilder B(Pred, Dst, *currBldrCtx);
   ProgramStateRef state = Pred->getState();
@@ -794,9 +795,10 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex,
 
   // Find the predecessor block.
   ProgramStateRef SrcState = state;
+
   for (const ExplodedNode *N = Pred ; N ; N = *N->pred_begin()) {
     ProgramPoint PP = N->getLocation();
-    if (PP.getAs<PreStmtPurgeDeadSymbols>() || PP.getAs<BlockEntrance>()) {
+    if (PP.getAs<PreStmtPurgeDeadSymbols>() || PP.getAs<BlockEntrance>() || PP.getAs<StmtPoint>()) {
       // 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
@@ -804,6 +806,7 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex,
       // FIXME: a more robust solution which does not walk up the tree.
       continue;
     }
+    assert(PP.getAs<BlockEdge>());
     SrcBlock = PP.castAs<BlockEdge>().getSrc();
     SrcState = N->getState();
     break;
@@ -816,6 +819,22 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex,
   bool hasValue = false;
   SVal V;
 
+  llvm::errs() << "\nSrcBlock"; SrcBlock->dump();
+  llvm::errs() << "\nempty=" << SrcBlock->empty();
+  llvm::errs() << "\nsize=" << SrcBlock->size();
+  CFGBlock::const_iterator  bb = SrcBlock->begin();
+  llvm::errs() << "\nbegin=|" ;   bb->dump(); llvm::errs() << "|";
+  CFGElement const&  xx = *bb;
+  llvm::errs() << "\nbegin-deref=|" ;   xx.dump();llvm::errs() << "|";
+  // manual iteration
+  for(auto it = SrcBlock->begin(); it != SrcBlock->end() ; ++it) {
+    llvm::errs() << "\nit.kind=|" << it->getKind() << "|";
+  }
+
+  llvm::errs() << "\nend=" ; (SrcBlock->end()-1)->dump();
+  llvm::errs() << "\nrbegin=" << SrcBlock->rbegin();
+  llvm::errs() << "\nrend=" << SrcBlock->rend();
+
   for (CFGElement CE : llvm::reverse(*SrcBlock)) {
     if (std::optional<CFGStmt> CS = CE.getAs<CFGStmt>()) {
       const Expr *ValEx = cast<Expr>(CS->getStmt());
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 786820c61c41c..88a96f422f846 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -1223,7 +1223,14 @@ void ExprEngine::VisitAttributedStmt(const AttributedStmt *A,
     }
     Expr *AssumeExpr = AssumeAttr->getAssumption();
     for (ExplodedNode *N : CheckerPreStmt) {
-      Visit(AssumeExpr, N, EvalSet);
+      // Assume State to narrow down here?
+      // for now
+      // ?????? VINAY HERE, need to narrow it down here
+      ExplodedNode CloneWithAssume = *N;
+      auto stateWithAssume = CloneWithAssume.getState();
+      Bldr.generateNode(AssumeExpr, N, stateWithAssume);
+
+      Visit(AssumeExpr, &CloneWithAssume, EvalSet);
     }
   }
 
diff --git a/clang/test/Analysis/cxx23-assume-attribute.cpp b/clang/test/Analysis/cxx23-assume-attribute.cpp
new file mode 100644
index 0000000000000..c68a77aef42d8
--- /dev/null
+++ b/clang/test/Analysis/cxx23-assume-attribute.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -verify %s \
+// RUN:   -analyzer-checker=unix,core,alpha.security.ArrayBoundV2,debug.ExprInspection
+
+// TODO: fix the checker arguments
+
+template <typename T> void clang_analyzer_dump(T);
+template <typename T> void clang_analyzer_value(T);
+
+int ternary_in_builtin_assume(int a, int b) {
+
+  __builtin_assume(a > 10 ? b == 4 : b == 10);
+
+  clang_analyzer_value(a); // we should have 2 dumps here.  expected-warning{{[-2147483648, 10]}} expected-warning{{[11, 2147483647]}}
+  clang_analyzer_dump(b); // This should be either 4 or 10. expected-warning{{4}} expected-warning{{10}}
+  if (a > 20) {
+    clang_analyzer_dump(b + 100); // expecting 104 expected-warning{{104}}
+    return 2;
+  }
+  if (a > 10) {
+    clang_analyzer_dump(b + 200); // expecting 204 expected-warning{{204}}
+    return 1;
+  }
+  clang_analyzer_dump(b + 300); // expecting 310 expected-warning{{310}}
+  return 0;
+}
+
+// From: https://github.com/llvm/llvm-project/pull/116462#issuecomment-2517853226
+int ternary_in_assume(int a, int b) {
+  // FIXME notes
+  // Currently, if this test is run without the core.builtin.Builtin checker, the above function with the __builtin_assume behaves identically to the following test
+  // i.e. calls to `clang_analyzer_dump` result in "extraneous"  prints of the SVal(s) `reg_$2<int > b ...` 
+  // as opposed to 4 or 10
+  // which likely implies the Program State(s) did not get narrowed.
+  // A new checker is likely needed to be implemented to properly handle the expressions within `[[assume]]` to eliminate the states where `b` is not narrowed.
+
+  [[assume(a > 10 ? b == 4 : b == 10)]];
+  clang_analyzer_value(a); // we should have 2 dumps here.  expected-warning{{[-2147483648, 10]}} expected-warning{{[11, 2147483647]}}
+  clang_analyzer_dump(b); // This should be either 4 or 10. expected-warning{{4}} expected-warning{{10}} FIXME: expected-warning{{reg_$2<int b>}}
+  if (a > 20) {
+    clang_analyzer_dump(b + 100); // expecting 104 expected-warning{{104}} FIXME: expected-warning{{(reg_$2<int b>) + 100}}
+    return 2;
+  }
+  if (a > 10) {
+    clang_analyzer_dump(b + 200); // expecting 204 expected-warning{{204}} FIXME: expected-warning{{(reg_$2<int b>) + 200}}
+    return 1;
+  }
+  clang_analyzer_dump(b + 300); // expecting 310 expected-warning{{310}} FIXME: expected-warning{{(reg_$2<int b>) + 300}}
+  return 0;
+}

From c3d1c1a3c84521aefb9e2b0dfd2a7feabe45a62e Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh@users.noreply.github.com>
Date: Tue, 17 Dec 2024 16:35:04 +0530
Subject: [PATCH 11/14] clean

---
 clang/lib/Analysis/CFG.cpp                      |  2 +-
 clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp   | 16 ----------------
 clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp |  9 +--------
 3 files changed, 2 insertions(+), 25 deletions(-)

diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index fb411043a8b1f..569e063e30168 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -461,7 +461,7 @@ reverse_children::reverse_children(Stmt *S, ASTContext *AstC) {
       assert(AstC &&
              "Attributes need the ast context to determine side-effects");
       AttributedStmt *AS = cast<AttributedStmt>(S);
-      assert(attrStmt);
+      assert(AS);
 
       // for an attributed stmt, the "children()" returns only the NullStmt
       // (;) but semantically the "children" are supposed to be the
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index 046d33164070b..8848a8f1e1689 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -819,22 +819,6 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex,
   bool hasValue = false;
   SVal V;
 
-  llvm::errs() << "\nSrcBlock"; SrcBlock->dump();
-  llvm::errs() << "\nempty=" << SrcBlock->empty();
-  llvm::errs() << "\nsize=" << SrcBlock->size();
-  CFGBlock::const_iterator  bb = SrcBlock->begin();
-  llvm::errs() << "\nbegin=|" ;   bb->dump(); llvm::errs() << "|";
-  CFGElement const&  xx = *bb;
-  llvm::errs() << "\nbegin-deref=|" ;   xx.dump();llvm::errs() << "|";
-  // manual iteration
-  for(auto it = SrcBlock->begin(); it != SrcBlock->end() ; ++it) {
-    llvm::errs() << "\nit.kind=|" << it->getKind() << "|";
-  }
-
-  llvm::errs() << "\nend=" ; (SrcBlock->end()-1)->dump();
-  llvm::errs() << "\nrbegin=" << SrcBlock->rbegin();
-  llvm::errs() << "\nrend=" << SrcBlock->rend();
-
   for (CFGElement CE : llvm::reverse(*SrcBlock)) {
     if (std::optional<CFGStmt> CS = CE.getAs<CFGStmt>()) {
       const Expr *ValEx = cast<Expr>(CS->getStmt());
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 88a96f422f846..786820c61c41c 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -1223,14 +1223,7 @@ void ExprEngine::VisitAttributedStmt(const AttributedStmt *A,
     }
     Expr *AssumeExpr = AssumeAttr->getAssumption();
     for (ExplodedNode *N : CheckerPreStmt) {
-      // Assume State to narrow down here?
-      // for now
-      // ?????? VINAY HERE, need to narrow it down here
-      ExplodedNode CloneWithAssume = *N;
-      auto stateWithAssume = CloneWithAssume.getState();
-      Bldr.generateNode(AssumeExpr, N, stateWithAssume);
-
-      Visit(AssumeExpr, &CloneWithAssume, EvalSet);
+      Visit(AssumeExpr, N, EvalSet);
     }
   }
 

From 37368545ff5f28f33dbfdfcc9c0771dd8c2e02dc Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs@gmail.com>
Date: Thu, 19 Dec 2024 09:40:01 +0100
Subject: [PATCH 12/14] Cleanup

---
 clang/include/clang/AST/AttrIterator.h        | 12 +++
 clang/lib/Analysis/CFG.cpp                    | 84 ++++++++-----------
 clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp | 10 +--
 .../lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 17 +---
 .../test/Analysis/cxx23-assume-attribute.cpp  |  2 +-
 5 files changed, 55 insertions(+), 70 deletions(-)

diff --git a/clang/include/clang/AST/AttrIterator.h b/clang/include/clang/AST/AttrIterator.h
index 7e2bb0381d4c8..2f39c144dc160 100644
--- a/clang/include/clang/AST/AttrIterator.h
+++ b/clang/include/clang/AST/AttrIterator.h
@@ -16,6 +16,7 @@
 #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>
@@ -124,6 +125,17 @@ 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
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 569e063e30168..65f915ef087af 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -433,7 +433,7 @@ class reverse_children {
   ArrayRef<Stmt *> children;
 
 public:
-  reverse_children(Stmt *S, ASTContext *astContext = nullptr);
+  reverse_children(Stmt *S, ASTContext &Ctx);
 
   using iterator = ArrayRef<Stmt *>::reverse_iterator;
 
@@ -443,61 +443,47 @@ class reverse_children {
 
 } // namespace
 
-reverse_children::reverse_children(Stmt *S, ASTContext *AstC) {
-  if (CallExpr *CE = dyn_cast<CallExpr>(S)) {
-    children = CE->getRawSubExprs();
+reverse_children::reverse_children(Stmt *S, ASTContext &Ctx) {
+  switch (S->getStmtClass()) {
+  case Stmt::CallExprClass: {
+    children = cast<CallExpr>(S)->getRawSubExprs();
     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;
-    }
-    case Stmt::AttributedStmtClass: {
-      assert(S->getStmtClass() == Stmt::AttributedStmtClass);
-      assert(AstC &&
-             "Attributes need the ast context to determine side-effects");
-      AttributedStmt *AS = cast<AttributedStmt>(S);
-      assert(AS);
-
-      // 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"
-      for (const Attr *Attr : AS->getAttrs()) {
-        // Only handles [[ assume(<assumption>) ]] right now
-        CXXAssumeAttr const *AssumeAttr = llvm::dyn_cast<CXXAssumeAttr>(Attr);
-        if (!AssumeAttr) {
-          continue;
-        }
-        Expr *AssumeExpr = AssumeAttr->getAssumption();
-        // If we skip adding the assumption expression to CFG,
-        // it doesn't get "branch"-ed by symbol analysis engine
-        // presumably because it's literally not in the CFG
 
-        if (AssumeExpr->HasSideEffects(*AstC)) {
-          continue;
+  // 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;
+  }
+  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"
+
+    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);
         }
-        childrenBuf.push_back(AssumeExpr);
       }
-      // children() for an CXXAssumeAttr is NullStmt(;)
-      // for others, it will have existing behavior
+      // Visit the actual children AST nodes.
+      // For CXXAssumeAttrs, this is always a NullStmt.
       llvm::append_range(childrenBuf, AS->children());
       children = childrenBuf;
-      return;
     }
-    default:
-      break;
+    return;
+  }
+  default:
+    // Default case for all other statements.
+    llvm::append_range(childrenBuf, S->children());
+    children = childrenBuf;
   }
-
-  // Default case for all other statements.
-  llvm::append_range(childrenBuf, S->children());
-
-  // This needs to be done *after* childrenBuf has been populated.
-  children = childrenBuf;
 }
 
 namespace {
@@ -2464,7 +2450,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, *Context);
   for (Stmt *Child : RChildren) {
     if (Child)
       if (CFGBlock *R = Visit(Child))
@@ -2480,7 +2466,7 @@ CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) {
   }
   CFGBlock *B = Block;
 
-  reverse_children RChildren(ILE);
+  reverse_children RChildren(ILE, *Context);
   for (Stmt *Child : RChildren) {
     if (!Child)
       continue;
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index 8848a8f1e1689..1315bd10496f5 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -785,8 +785,7 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex,
                                   const Expr *R,
                                   ExplodedNode *Pred,
                                   ExplodedNodeSet &Dst) {
-  assert(L && "L does not exist!");
-  assert(R && "R does not exist!");
+  assert(L && R);
 
   StmtNodeBuilder B(Pred, Dst, *currBldrCtx);
   ProgramStateRef state = Pred->getState();
@@ -797,8 +796,8 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex,
   ProgramStateRef SrcState = state;
 
   for (const ExplodedNode *N = Pred ; N ; N = *N->pred_begin()) {
-    ProgramPoint PP = N->getLocation();
-    if (PP.getAs<PreStmtPurgeDeadSymbols>() || PP.getAs<BlockEntrance>() || PP.getAs<StmtPoint>()) {
+    auto Edge = N->getLocationAs<BlockEdge>();
+    if (!Edge.has_value()) {
       // 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
@@ -806,8 +805,7 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex,
       // FIXME: a more robust solution which does not walk up the tree.
       continue;
     }
-    assert(PP.getAs<BlockEdge>());
-    SrcBlock = PP.castAs<BlockEdge>().getSrc();
+    SrcBlock = Edge->getSrc();
     SrcState = N->getState();
     break;
   }
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 786820c61c41c..5f9dbcb55e811 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -10,23 +10,17 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/AST/ASTContext.h"
+#include "clang/AST/AttrIterator.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/ParentMap.h"
 #include "clang/AST/StmtCXX.h"
-#include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Analysis/ConstructionContext.h"
-#include "clang/Analysis/ProgramPoint.h"
 #include "clang/Basic/PrettyStackTrace.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Sequence.h"
 #include <optional>
@@ -1216,14 +1210,9 @@ void ExprEngine::VisitAttributedStmt(const AttributedStmt *A,
   ExplodedNodeSet EvalSet;
   StmtNodeBuilder Bldr(CheckerPreStmt, EvalSet, *currBldrCtx);
 
-  for (const auto *attr : A->getAttrs()) {
-    CXXAssumeAttr const *AssumeAttr = llvm::dyn_cast<CXXAssumeAttr>(attr);
-    if (!AssumeAttr) {
-      continue;
-    }
-    Expr *AssumeExpr = AssumeAttr->getAssumption();
+  for (const auto *Attr : getSpecificAttrs<CXXAssumeAttr>(A->getAttrs())) {
     for (ExplodedNode *N : CheckerPreStmt) {
-      Visit(AssumeExpr, N, EvalSet);
+      Visit(Attr->getAssumption(), N, EvalSet);
     }
   }
 
diff --git a/clang/test/Analysis/cxx23-assume-attribute.cpp b/clang/test/Analysis/cxx23-assume-attribute.cpp
index c68a77aef42d8..6e7b1f64cf792 100644
--- a/clang/test/Analysis/cxx23-assume-attribute.cpp
+++ b/clang/test/Analysis/cxx23-assume-attribute.cpp
@@ -28,7 +28,7 @@ int ternary_in_builtin_assume(int a, int b) {
 int ternary_in_assume(int a, int b) {
   // FIXME notes
   // Currently, if this test is run without the core.builtin.Builtin checker, the above function with the __builtin_assume behaves identically to the following test
-  // i.e. calls to `clang_analyzer_dump` result in "extraneous"  prints of the SVal(s) `reg_$2<int > b ...` 
+  // i.e. calls to `clang_analyzer_dump` result in "extraneous"  prints of the SVal(s) `reg_$2<int > b ...`
   // as opposed to 4 or 10
   // which likely implies the Program State(s) did not get narrowed.
   // A new checker is likely needed to be implemented to properly handle the expressions within `[[assume]]` to eliminate the states where `b` is not narrowed.

From 342ef040d8924bda87b98539c814ce63a1b81b2c Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs@gmail.com>
Date: Thu, 19 Dec 2024 10:48:39 +0100
Subject: [PATCH 13/14] Update clang/test/Analysis/cxx23-assume-attribute.cpp

---
 clang/test/Analysis/cxx23-assume-attribute.cpp | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/clang/test/Analysis/cxx23-assume-attribute.cpp b/clang/test/Analysis/cxx23-assume-attribute.cpp
index 6e7b1f64cf792..78b4b6ad43df4 100644
--- a/clang/test/Analysis/cxx23-assume-attribute.cpp
+++ b/clang/test/Analysis/cxx23-assume-attribute.cpp
@@ -1,7 +1,4 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -verify %s \
-// RUN:   -analyzer-checker=unix,core,alpha.security.ArrayBoundV2,debug.ExprInspection
-
-// TODO: fix the checker arguments
+// RUN: %clang_analyze_cc1 -std=c++23 -analyzer-checker=core,debug.ExprInspection -verify %s
 
 template <typename T> void clang_analyzer_dump(T);
 template <typename T> void clang_analyzer_value(T);

From 670b3584725c49ded9a5036a9eec996a1ec9e36a Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs@gmail.com>
Date: Thu, 19 Dec 2024 12:14:43 +0100
Subject: [PATCH 14/14] Fix test

---
 .../test/Analysis/cxx23-assume-attribute.cpp  | 38 ++++++++++++-------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/clang/test/Analysis/cxx23-assume-attribute.cpp b/clang/test/Analysis/cxx23-assume-attribute.cpp
index 78b4b6ad43df4..defcdeec282f6 100644
--- a/clang/test/Analysis/cxx23-assume-attribute.cpp
+++ b/clang/test/Analysis/cxx23-assume-attribute.cpp
@@ -1,23 +1,27 @@
-// RUN: %clang_analyze_cc1 -std=c++23 -analyzer-checker=core,debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -std=c++23 -triple x86_64-pc-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection -verify %s
 
 template <typename T> void clang_analyzer_dump(T);
 template <typename T> void clang_analyzer_value(T);
 
 int ternary_in_builtin_assume(int a, int b) {
-
   __builtin_assume(a > 10 ? b == 4 : b == 10);
 
-  clang_analyzer_value(a); // we should have 2 dumps here.  expected-warning{{[-2147483648, 10]}} expected-warning{{[11, 2147483647]}}
-  clang_analyzer_dump(b); // This should be either 4 or 10. expected-warning{{4}} expected-warning{{10}}
+  clang_analyzer_value(a);
+  // expected-warning@-1 {{[-2147483648, 10]}}
+  // expected-warning@-2 {{[11, 2147483647]}}
+
+  clang_analyzer_dump(b); // expected-warning{{4}} expected-warning{{10}}
+
   if (a > 20) {
-    clang_analyzer_dump(b + 100); // expecting 104 expected-warning{{104}}
+    clang_analyzer_dump(b + 100); // expected-warning {{104}}
     return 2;
   }
   if (a > 10) {
-    clang_analyzer_dump(b + 200); // expecting 204 expected-warning{{204}}
+    clang_analyzer_dump(b + 200); // expected-warning {{204}}
     return 1;
   }
-  clang_analyzer_dump(b + 300); // expecting 310 expected-warning{{310}}
+  clang_analyzer_dump(b + 300); // expected-warning {{310}}
   return 0;
 }
 
@@ -25,22 +29,30 @@ int ternary_in_builtin_assume(int a, int b) {
 int ternary_in_assume(int a, int b) {
   // FIXME notes
   // Currently, if this test is run without the core.builtin.Builtin checker, the above function with the __builtin_assume behaves identically to the following test
-  // i.e. calls to `clang_analyzer_dump` result in "extraneous"  prints of the SVal(s) `reg_$2<int > b ...`
+  // i.e. calls to `clang_analyzer_dump` result in "extraneous"  prints of the SVal(s) `reg<int b> ...`
   // as opposed to 4 or 10
   // which likely implies the Program State(s) did not get narrowed.
   // A new checker is likely needed to be implemented to properly handle the expressions within `[[assume]]` to eliminate the states where `b` is not narrowed.
 
   [[assume(a > 10 ? b == 4 : b == 10)]];
-  clang_analyzer_value(a); // we should have 2 dumps here.  expected-warning{{[-2147483648, 10]}} expected-warning{{[11, 2147483647]}}
-  clang_analyzer_dump(b); // This should be either 4 or 10. expected-warning{{4}} expected-warning{{10}} FIXME: expected-warning{{reg_$2<int b>}}
+  clang_analyzer_value(a);
+  // expected-warning@-1 {{[-2147483648, 10]}}
+  // expected-warning@-2 {{[11, 2147483647]}}
+
+  clang_analyzer_dump(b); // expected-warning {{4}} expected-warning {{10}}
+  // expected-warning-re@-1 {{reg_${{[0-9]+}}<int b>}} FIXME: We shouldn't have this dump.
+
   if (a > 20) {
-    clang_analyzer_dump(b + 100); // expecting 104 expected-warning{{104}} FIXME: expected-warning{{(reg_$2<int b>) + 100}}
+    clang_analyzer_dump(b + 100); // expected-warning {{104}}
+    // expected-warning-re@-1 {{(reg_${{[0-9]+}}<int b>) + 100}} FIXME: We shouldn't have this dump.
     return 2;
   }
   if (a > 10) {
-    clang_analyzer_dump(b + 200); // expecting 204 expected-warning{{204}} FIXME: expected-warning{{(reg_$2<int b>) + 200}}
+    clang_analyzer_dump(b + 200); // expected-warning {{204}}
+    // expected-warning-re@-1 {{(reg_${{[0-9]+}}<int b>) + 200}} FIXME: We shouldn't have this dump.
     return 1;
   }
-  clang_analyzer_dump(b + 300); // expecting 310 expected-warning{{310}} FIXME: expected-warning{{(reg_$2<int b>) + 300}}
+  clang_analyzer_dump(b + 300); // expected-warning {{310}}
+  // expected-warning-re@-1 {{(reg_${{[0-9]+}}<int b>) + 300}} FIXME: We shouldn't have this dump.
   return 0;
 }