Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
daddb9e
[analyzer] Handle `[[assume(cond)]]` as `__builtin_assume(cond)`
vinay-deshmukh Nov 15, 2024
7f1e341
NFC Simplify ExprEngine::VisitAttributedStmt
steakhal Nov 18, 2024
f4307e9
NFC Simplify AttributedStmt handling in clang CFG
steakhal Nov 18, 2024
c2b5e71
NFC Clarify test comment
steakhal Nov 18, 2024
3d4953f
NFC De-indent some tests
steakhal Nov 18, 2024
57751d1
NFC Mention the size of the array in its name
steakhal Nov 18, 2024
75e964a
Add broken test about sideeffects
steakhal Nov 18, 2024
d55851b
[clang] Generalize getSpecificAttr for const attributes
steakhal Nov 18, 2024
c288bdc
Ignore assumptions with side-effects
vinay-deshmukh Nov 29, 2024
f79ad87
Merge remote-tracking branch 'upstream/main' into vinay-issue-100762
vinay-deshmukh Nov 29, 2024
b4e278a
Merge branch 'main' into vinay-issue-100762
vinay-deshmukh Nov 29, 2024
8b877d1
Merge branch 'main' into vinay-issue-100762
vinay-deshmukh Dec 9, 2024
eabbef2
Add ternary test for assumption
vinay-deshmukh Dec 10, 2024
ae2038f
Merge remote-tracking branch 'upstream/main' into vinay-issue-100762
vinay-deshmukh Dec 17, 2024
c3d1c1a
clean
vinay-deshmukh Dec 17, 2024
e71023b
Merge branch 'main' into vinay-issue-100762
vinay-deshmukh Dec 18, 2024
02c009d
Merge branch 'main' into vinay-issue-100762
vinay-deshmukh Dec 18, 2024
3736854
Cleanup
steakhal Dec 19, 2024
d4c4e14
Merge remote-tracking branch 'llvm' into vinay-issue-100762
steakhal Dec 19, 2024
342ef04
Update clang/test/Analysis/cxx23-assume-attribute.cpp
steakhal Dec 19, 2024
670b358
Fix test
steakhal Dec 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions clang/include/clang/AST/AttrIterator.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
72 changes: 52 additions & 20 deletions clang/lib/Analysis/CFG.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ class reverse_children {
ArrayRef<Stmt *> children;

public:
reverse_children(Stmt *S);
reverse_children(Stmt *S, ASTContext &Ctx);

using iterator = ArrayRef<Stmt *>::reverse_iterator;

Expand All @@ -443,28 +443,47 @@ class reverse_children {

} // namespace

reverse_children::reverse_children(Stmt *S) {
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;
}
default:
break;

// 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);

// Default case for all other statements.
llvm::append_range(childrenBuf, S->children());
// 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"

// This needs to be done *after* childrenBuf has been populated.
children = childrenBuf;
for (const auto *Attr : AS->getAttrs()) {
if (const auto *AssumeAttr = dyn_cast<CXXAssumeAttr>(Attr)) {
Expr *AssumeExpr = AssumeAttr->getAssumption();
if (!AssumeExpr->HasSideEffects(Ctx)) {
childrenBuf.push_back(AssumeExpr);
}
}
// Visit the actual children AST nodes.
// For CXXAssumeAttrs, this is always a NullStmt.
llvm::append_range(childrenBuf, AS->children());
children = childrenBuf;
}
return;
}
default:
// Default case for all other statements.
llvm::append_range(childrenBuf, S->children());
children = childrenBuf;
}
}

namespace {
Expand Down Expand Up @@ -2431,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);
reverse_children RChildren(S, *Context);
for (Stmt *Child : RChildren) {
if (Child)
if (CFGBlock *R = Visit(Child))
Expand All @@ -2447,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;
Expand Down Expand Up @@ -2482,6 +2501,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,
Expand All @@ -2497,6 +2524,11 @@ CFGBlock *CFGBuilder::VisitAttributedStmt(AttributedStmt *A,
appendStmt(Block, A);
}

if (isCXXAssumeAttr(A) && asc.alwaysAdd(*this, A)) {
autoCreateBlock();
appendStmt(Block, A);
}

return VisitChildren(A);
}

Expand Down
8 changes: 7 additions & 1 deletion clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1941,7 +1941,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:
Expand Down Expand Up @@ -1972,6 +1971,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);
Expand Down
7 changes: 4 additions & 3 deletions clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -794,17 +794,18 @@ 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>()) {
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
// of observable behavior at N, and we can follow any of them.
// FIXME: a more robust solution which does not walk up the tree.
continue;
}
SrcBlock = PP.castAs<BlockEdge>().getSrc();
SrcBlock = Edge->getSrc();
SrcState = N->getState();
break;
}
Expand Down
18 changes: 18 additions & 0 deletions clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//

#include "clang/AST/AttrIterator.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/ParentMap.h"
#include "clang/AST/StmtCXX.h"
Expand Down Expand Up @@ -1200,3 +1201,20 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE, ExplodedNode *Pred,
// FIXME: Move all post/pre visits to ::Visit().
getCheckerManager().runCheckersForPostStmt(Dst, Tmp, LE, *this);
}

void ExprEngine::VisitAttributedStmt(const AttributedStmt *A,
ExplodedNode *Pred, ExplodedNodeSet &Dst) {
ExplodedNodeSet CheckerPreStmt;
getCheckerManager().runCheckersForPreStmt(CheckerPreStmt, Pred, A, *this);

ExplodedNodeSet EvalSet;
StmtNodeBuilder Bldr(CheckerPreStmt, EvalSet, *currBldrCtx);

for (const auto *Attr : getSpecificAttrs<CXXAssumeAttr>(A->getAttrs())) {
for (ExplodedNode *N : CheckerPreStmt) {
Visit(Attr->getAssumption(), N, EvalSet);
}
}

getCheckerManager().runCheckersForPostStmt(Dst, EvalSet, A, *this);
}
58 changes: 58 additions & 0 deletions clang/test/Analysis/cxx23-assume-attribute.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// 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);
// 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); // expected-warning {{104}}
return 2;
}
if (a > 10) {
clang_analyzer_dump(b + 200); // expected-warning {{204}}
return 1;
}
clang_analyzer_dump(b + 300); // 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<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);
// 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); // 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); // 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); // expected-warning {{310}}
// expected-warning-re@-1 {{(reg_${{[0-9]+}}<int b>) + 300}} FIXME: We shouldn't have this dump.
return 0;
}
64 changes: 63 additions & 1 deletion clang/test/Analysis/out-of-bounds-new.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
// 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);
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
Expand Down Expand Up @@ -180,3 +187,58 @@ int test_reference_that_might_be_after_the_end(int idx) {
return ref;
}

// From: https://github.com/llvm/llvm-project/issues/100762
extern int arrOf10[10];
void using_builtin(int x) {
__builtin_assume(x > 101); // CallExpr
arrOf10[x] = 404; // expected-warning {{Out of bound access to memory}}
}

void using_assume_attr(int ax) {
[[assume(ax > 100)]]; // NullStmt with an "assume" attribute.
arrOf10[ax] = 405; // expected-warning {{Out of bound access to memory}}
}

void using_many_assume_attr(int yx) {
[[assume(yx > 104), assume(yx > 200), assume(yx < 300)]]; // NullStmt with an attribute
arrOf10[yx] = 406; // expected-warning{{Out of bound access to memory}}
}


int using_builtin_assume_has_no_sideeffects(int y) {
// We should not apply sideeffects of the argument of [[assume(...)]].
// "y" should not get incremented;
__builtin_assume(++y == 43); // expected-warning {{assumption is ignored because it contains (potential) side-effects}}
clang_analyzer_eval(y == 42); // expected-warning {{FALSE}}
return y;
}



int using_assume_attr_has_no_sideeffects(int y) {

// We should not apply sideeffects of the argument of [[assume(...)]].
// "y" should not get incremented;
[[assume(++y == 43)]]; // expected-warning {{assumption is ignored because it contains (potential) side-effects}}

clang_analyzer_eval(y == 42); // expected-warning {{TRUE}} expected-warning {{FALSE}} FIXME: This should be only TRUE.

clang_analyzer_eval(y == 43); // expected-warning {{FALSE}} expected-warning {{TRUE}} FIXME: This should be only FALSE.

return y;
}


int using_builtinassume_has_no_sideeffects(int u) {
// We should not apply sideeffects of the argument of __builtin_assume(...)
// "u" should not get incremented;
__builtin_assume(++u == 43); // expected-warning {{assumption is ignored because it contains (potential) side-effects}}

// FIXME: evaluate this to true
clang_analyzer_eval(u == 42); // expected-warning {{FALSE}} current behavior

// FIXME: evaluate this to false
clang_analyzer_eval(u == 43); // expected-warning {{TRUE}} current behavior

return u;
}
Loading