[clang] [analyzer] Handle [[assume(cond)]] as __builtin_assume(cond) (PR #116462)
Vinay Deshmukh via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 28 20:33:26 PST 2024
https://github.com/vinay-deshmukh 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 1/9] [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 2/9] 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 3/9] 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 4/9] 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 5/9] 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 6/9] 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 7/9] 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 8/9] [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 9/9] 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;
+}
More information about the cfe-commits
mailing list