mirrors/llvm-project
0
0
Fork 0
mirror of https://github.com/llvm/llvm-project.git synced 2025-04-29 05:56:06 +00:00
Code Issues Projects Releases Wiki Activity

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

Browse Source 
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)
This commit is contained in:
Thurston Dang 2024-12-19 17:02:16 +00:00
parent 527595f927
commit 2b9abf0db2
8 changed files with 27 additions and 220 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
clang/include/clang/AST/AttrIterator.h
View File

@ -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

4
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
View File

@ -498,10 +498,6 @@ public:
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);

76
clang/lib/Analysis/CFG.cpp
View File

@ -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 @@ public:
} // namespace
reverse_children::reverse_children(Stmt *S, ASTContext &Ctx) {
reverse_children::reverse_children(Stmt *S) {
if (CallExpr *CE = dyn_cast<CallExpr>(S)) {
children = CE->getRawSubExprs();
return;
}
switch (S->getStmtClass()) {
case Stmt::CallExprClass: {
children = cast<CallExpr>(S)->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;
}
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);
}
}
// Visit the actual children AST nodes.
// For CXXAssumeAttrs, this is always a NullStmt.
llvm::append_range(childrenBuf, AS->children());
children = childrenBuf;
// 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;
}
return;
}
default:
// Default case for all other statements.
llvm::append_range(childrenBuf, S->children());
children = childrenBuf;
default:
break;
}
// 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 {
@ -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);
}

8
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
View File

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

7
clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
View File

@ -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;
}

18
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
View File

@ -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);
}

58
clang/test/Analysis/cxx23-assume-attribute.cpp
View File

@ -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;
}

64
clang/test/Analysis/out-of-bounds-new.cpp
View File

@ -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;
}