Author: Thurston Dang Date: 2024-12-19T17:02:16Z New Revision: 2b9abf0db2d106c7208b4372e662ef5df869e6f1
URL: https://github.com/llvm/llvm-project/commit/2b9abf0db2d106c7208b4372e662ef5df869e6f1 DIFF: https://github.com/llvm/llvm-project/commit/2b9abf0db2d106c7208b4372e662ef5df869e6f1.diff LOG: Revert "[analyzer] Handle [[assume(cond)]] as __builtin_assume(cond) (#116462)" This reverts commit 89da344e5879e5347b5057520d5230e40ae24831. Reason: buildbot breakages e.g., https://lab.llvm.org/buildbot/#/builders/55/builds/4556 (for which the reverted patch is the only code change) Added: Modified: clang/include/clang/AST/AttrIterator.h clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h clang/lib/Analysis/CFG.cpp clang/lib/StaticAnalyzer/Core/ExprEngine.cpp clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp clang/test/Analysis/out-of-bounds-new.cpp Removed: clang/test/Analysis/cxx23-assume-attribute.cpp ################################################################################ diff --git a/clang/include/clang/AST/AttrIterator.h b/clang/include/clang/AST/AttrIterator.h index 2f39c144dc1608a..7e2bb0381d4c8f0 100644 --- a/clang/include/clang/AST/AttrIterator.h +++ b/clang/include/clang/AST/AttrIterator.h @@ -16,7 +16,6 @@ #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> @@ -125,17 +124,6 @@ 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/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index 078a1d840d0516d..8c7493e27fcaa63 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -498,10 +498,6 @@ 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 65f915ef087afab..304bbb2b422c61d 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 &Ctx); + reverse_children(Stmt *S); using iterator = ArrayRef<Stmt *>::reverse_iterator; @@ -443,47 +443,28 @@ class reverse_children { } // namespace -reverse_children::reverse_children(Stmt *S, ASTContext &Ctx) { - switch (S->getStmtClass()) { - case Stmt::CallExprClass: { - children = cast<CallExpr>(S)->getRawSubExprs(); +reverse_children::reverse_children(Stmt *S) { + if (CallExpr *CE = dyn_cast<CallExpr>(S)) { + children = CE->getRawSubExprs(); return; } - - // 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; + 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; } - 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" + // Default case for all other statements. + llvm::append_range(childrenBuf, S->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); - } - } - // 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; - } + // This needs to be done *after* childrenBuf has been populated. + children = childrenBuf; } namespace { @@ -2450,7 +2431,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); for (Stmt *Child : RChildren) { if (Child) if (CFGBlock *R = Visit(Child)) @@ -2466,7 +2447,7 @@ CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) { } CFGBlock *B = Block; - reverse_children RChildren(ILE, *Context); + reverse_children RChildren(ILE); for (Stmt *Child : RChildren) { if (!Child) continue; @@ -2501,14 +2482,6 @@ 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, @@ -2524,11 +2497,6 @@ 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 44c9e54bde5e37d..0a74a80a6a62f9d 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1941,6 +1941,7 @@ 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: @@ -1971,13 +1972,6 @@ 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/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp index 1315bd10496f5c7..7a900780384a91d 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -794,10 +794,9 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex, // Find the predecessor block. ProgramStateRef SrcState = state; - for (const ExplodedNode *N = Pred ; N ; N = *N->pred_begin()) { - auto Edge = N->getLocationAs<BlockEdge>(); - if (!Edge.has_value()) { + ProgramPoint PP = N->getLocation(); + if (PP.getAs<PreStmtPurgeDeadSymbols>() || PP.getAs<BlockEntrance>()) { // 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 @@ -805,7 +804,7 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex, // FIXME: a more robust solution which does not walk up the tree. continue; } - SrcBlock = Edge->getSrc(); + SrcBlock = PP.castAs<BlockEdge>().getSrc(); SrcState = N->getState(); break; } diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 5f9dbcb55e811ca..f7020da2e6da200 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -10,7 +10,6 @@ // //===----------------------------------------------------------------------===// -#include "clang/AST/AttrIterator.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/ParentMap.h" #include "clang/AST/StmtCXX.h" @@ -1201,20 +1200,3 @@ 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); -} diff --git a/clang/test/Analysis/cxx23-assume-attribute.cpp b/clang/test/Analysis/cxx23-assume-attribute.cpp deleted file mode 100644 index defcdeec282f61e..000000000000000 --- a/clang/test/Analysis/cxx23-assume-attribute.cpp +++ /dev/null @@ -1,58 +0,0 @@ -// 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; -} diff --git a/clang/test/Analysis/out-of-bounds-new.cpp b/clang/test/Analysis/out-of-bounds-new.cpp index 39a40eb10bea7d5..f541bdf810d79ce 100644 --- a/clang/test/Analysis/out-of-bounds-new.cpp +++ b/clang/test/Analysis/out-of-bounds-new.cpp @@ -1,11 +1,4 @@ -// 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); +// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -analyzer-checker=unix,core,alpha.security.ArrayBoundV2 -verify %s // Tests doing an out-of-bounds access after the end of an array using: // - constant integer index @@ -187,58 +180,3 @@ 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; -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
