[clang] [analyzer] Handle [[assume(cond)]] as __builtin_assume(cond) (PR #116462)
Balazs Benics via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 19 01:48:18 PST 2024
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/116462
>From daddb9e13db6ca8373dc7298d17aa36a03014aeb Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Fri, 15 Nov 2024 07:37:17 -0500
Subject: [PATCH 01/12] [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 8c7493e27fcaa6..078a1d840d0516 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 f678ac6f2ff36a..fab10f51cf5cfc 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 22eab9f66418d4..cbc83f1dbda145 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 f7020da2e6da20..1a211d1adc44f7 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 f541bdf810d79c..4db351b10055b1 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 at gmail.com>
Date: Mon, 18 Nov 2024 11:33:42 +0100
Subject: [PATCH 02/12] 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 1a211d1adc44f7..86d57a8dee52ea 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 at gmail.com>
Date: Mon, 18 Nov 2024 11:34:25 +0100
Subject: [PATCH 03/12] 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 fab10f51cf5cfc..dbdb08506bb622 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 at gmail.com>
Date: Mon, 18 Nov 2024 11:35:32 +0100
Subject: [PATCH 04/12] 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 4db351b10055b1..c2b70580a90c33 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 at gmail.com>
Date: Mon, 18 Nov 2024 11:35:51 +0100
Subject: [PATCH 05/12] 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 c2b70580a90c33..4896893a6a231b 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 at gmail.com>
Date: Mon, 18 Nov 2024 11:36:40 +0100
Subject: [PATCH 06/12] 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 4896893a6a231b..c4abc70a2d953d 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 at gmail.com>
Date: Mon, 18 Nov 2024 12:25:06 +0100
Subject: [PATCH 07/12] 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 c4abc70a2d953d..200ee11d714421 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 at gmail.com>
Date: Mon, 18 Nov 2024 12:26:26 +0100
Subject: [PATCH 08/12] [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 66571e1cf0b8ec..7e2bb0381d4c8f 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 cdff7e50c4ee71..448571221ef81b 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 at users.noreply.github.com>
Date: Thu, 28 Nov 2024 22:51:46 -0500
Subject: [PATCH 09/12] 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 dbdb08506bb622..8d1db015907eba 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 86d57a8dee52ea..786820c61c41c6 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 200ee11d714421..39a40eb10bea7d 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 at users.noreply.github.com>
Date: Tue, 10 Dec 2024 16:45:20 +0530
Subject: [PATCH 10/12] 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 891c542aa55531..fb411043a8b1f5 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 7a900780384a91..046d33164070bc 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 786820c61c41c6..88a96f422f846f 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 00000000000000..c68a77aef42d84
--- /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 at users.noreply.github.com>
Date: Tue, 17 Dec 2024 16:35:04 +0530
Subject: [PATCH 11/12] 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 fb411043a8b1f5..569e063e301689 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 046d33164070bc..8848a8f1e16897 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 88a96f422f846f..786820c61c41c6 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 at gmail.com>
Date: Thu, 19 Dec 2024 09:40:01 +0100
Subject: [PATCH 12/12] 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 7e2bb0381d4c8f..2f39c144dc1608 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 569e063e301689..65f915ef087afa 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 8848a8f1e16897..1315bd10496f5c 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 786820c61c41c6..5f9dbcb55e811c 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 c68a77aef42d84..6e7b1f64cf792b 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.
More information about the cfe-commits
mailing list