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 #include @@ -124,6 +125,17 @@ inline auto *getSpecificAttr(const Container &container) { return It != specific_attr_end(container) ? *It : nullptr; } +template +inline auto getSpecificAttrs(const Container &container) { + using ValueTy = llvm::detail::ValueOfRange; + using ValuePointeeTy = std::remove_pointer_t; + using IterTy = std::conditional_t, + const SpecificAttr, SpecificAttr>; + auto Begin = specific_attr_begin(container); + auto End = specific_attr_end(container); + return llvm::make_range(Begin, End); +} + } // namespace clang #endif // LLVM_CLANG_AST_ATTRITERATOR_H 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 304bbb2b422c6..65f915ef087af 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -433,7 +433,7 @@ class reverse_children { ArrayRef children; public: - reverse_children(Stmt *S); + reverse_children(Stmt *S, ASTContext &Ctx); using iterator = ArrayRef::reverse_iterator; @@ -443,28 +443,47 @@ class reverse_children { } // namespace -reverse_children::reverse_children(Stmt *S) { - if (CallExpr *CE = dyn_cast(S)) { - children = CE->getRawSubExprs(); +reverse_children::reverse_children(Stmt *S, ASTContext &Ctx) { + switch (S->getStmtClass()) { + case Stmt::CallExprClass: { + children = cast(S)->getRawSubExprs(); return; } - switch (S->getStmtClass()) { - // Note: Fill in this switch with more cases we want to optimize. - case Stmt::InitListExprClass: { - InitListExpr *IE = cast(S); - children = llvm::ArrayRef(reinterpret_cast(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(S); + children = llvm::ArrayRef(reinterpret_cast(IE->getInits()), + IE->getNumInits()); + return; } + case Stmt::AttributedStmtClass: { + auto *AS = cast(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(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 { @@ -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)) @@ -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; @@ -2482,6 +2501,14 @@ static bool isFallthroughStatement(const AttributedStmt *A) { return isFallthrough; } +static bool isCXXAssumeAttr(const AttributedStmt *A) { + bool hasAssumeAttr = hasSpecificAttr(A->getAttrs()); + + assert((!hasAssumeAttr || isa(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, @@ -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); } diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index 0a74a80a6a62f..44c9e54bde5e3 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -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: @@ -1972,6 +1971,13 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, break; } + case Stmt::AttributedStmtClass: { + Bldr.takeNodes(Pred); + VisitAttributedStmt(cast(S), Pred, Dst); + Bldr.addNodes(Dst); + break; + } + case Stmt::CXXDefaultArgExprClass: case Stmt::CXXDefaultInitExprClass: { Bldr.takeNodes(Pred); diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp index 7a900780384a9..1315bd10496f5 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -794,9 +794,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() || PP.getAs()) { + auto Edge = N->getLocationAs(); + 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 @@ -804,7 +805,7 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex, // FIXME: a more robust solution which does not walk up the tree. continue; } - SrcBlock = PP.castAs().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 f7020da2e6da2..5f9dbcb55e811 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +#include "clang/AST/AttrIterator.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/ParentMap.h" #include "clang/AST/StmtCXX.h" @@ -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(A->getAttrs())) { + for (ExplodedNode *N : CheckerPreStmt) { + Visit(Attr->getAssumption(), N, EvalSet); + } + } + + getCheckerManager().runCheckersForPostStmt(Dst, EvalSet, A, *this); +} diff --git a/clang/test/Analysis/cxx23-assume-attribute.cpp b/clang/test/Analysis/cxx23-assume-attribute.cpp new file mode 100644 index 0000000000000..defcdeec282f6 --- /dev/null +++ b/clang/test/Analysis/cxx23-assume-attribute.cpp @@ -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 void clang_analyzer_dump(T); +template 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 ...` + // 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]+}}}} 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]+}}) + 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]+}}) + 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]+}}) + 300}} FIXME: We shouldn't have this dump. + return 0; +} diff --git a/clang/test/Analysis/out-of-bounds-new.cpp b/clang/test/Analysis/out-of-bounds-new.cpp index f541bdf810d79..39a40eb10bea7 100644 --- a/clang/test/Analysis/out-of-bounds-new.cpp +++ b/clang/test/Analysis/out-of-bounds-new.cpp @@ -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 void clang_analyzer_dump(T); +template void clang_analyzer_value(T); +void clang_analyzer_eval(bool); +template +void clang_analyzer_explain(T); // Tests doing an out-of-bounds access after the end of an array using: // - constant integer index @@ -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; +}