Author: Donát Nagy Date: 2026-06-01T11:10:43Z New Revision: 73ded458839c0a105b19476929f6206356f92991
URL: https://github.com/llvm/llvm-project/commit/73ded458839c0a105b19476929f6206356f92991 DIFF: https://github.com/llvm/llvm-project/commit/73ded458839c0a105b19476929f6206356f92991.diff LOG: [Liveness][analyzer] Fix handling of [[assume]] attributes (#198618) Before this commit, if the analyzer encountered code like ``` int f(int a, int b) { [[assume(a == 2), assume(b == 3)]]; return a + b; } ``` it performed the following steps: 1. It visited the expression `a == 2` with `ExprEngine::Visit` (after visiting its sub-expressions, within the regular visitation that visits each statement of the `CFGBlock`). This triggered the `EagerlyAssume` logic and separated two execution paths. 2. It discarded the result bound to `a == 2` from the `Environment` because `a == 2` is not a direct child of the `AttributedStmt`. 3. Analogously, it visited an evaluated `b == 3`. 4. Analogously, it discarded the result bound to `b == 3`. 5. On each execution path `VisitAttributedStmt` was reached, it ran the `PreStmt<AttributedStmt>` checkers and stored their results in `CheckerPreStmt`. As there are AFAIK no such checkers, after this step the only element of `CheckerPreStmt` was the node `Pred`. 6. It constructed a `NodeBuilder`, which was unused, but its constructor inserted the nodes in `CheckerPreStmt` into `EvalSet`. This was essentially a shortcut that skipped the handling of the assumptions -- but ultimately its effects were absorbed and negated by the tricky side effects of `ExprEngine::Visit`. 7. For each node `N` in `CheckerPreStmt` (i.e. for `N == Pred`) and for each assume attribute, it created a child node of `N` where the assumption of **that one attribute** was visited again with `ExprEngine::Visit`. (I.e. it separated execution paths and implicitly placed "OR" relationships between the assumptions.) 8. It **completely ignored** the value produced by the assumption expression: did not update the state and did not discard execution paths where the [[assume]] assumption contradicted earlier knowledge. This commit fixes all four logic errors: 1. The liveness analysis is updated to ensure that an `AttributedStmt` keeps alive the assumption expressions of its [[assume]] attributes. This ensured that their results remain in the `Environment`. 2. Now there is no `NodeBuilder` that skips the handling the assumptions. 3. The invocation of `ExprEngine::Visit` is removed from `VisitAttributedStmt`: there is no reason to revisit those statements (now that we don't lose the evaluation results). 4. The state is updated to record that the values of the assumption expressions are ALL assumed to be true. If the assumption leads to a contradiction, the execution path is discarded (by not generating a node). This commit also adds a few new testcases and resolves some FIXME comments in previously existing tests. Added: Modified: clang/lib/Analysis/LiveVariables.cpp clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp clang/test/Analysis/cxx23-assume-attribute.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/LiveVariables.cpp b/clang/lib/Analysis/LiveVariables.cpp index ba382e766440f..af3055c3d5f8d 100644 --- a/clang/lib/Analysis/LiveVariables.cpp +++ b/clang/lib/Analysis/LiveVariables.cpp @@ -285,6 +285,15 @@ void TransferFunctions::Visit(Stmt *S) { } break; } + case Stmt::AttributedStmtClass: { + // In an attributed statement, include the assumptions of the + // [[assume(...)]] attributes as being live. + AttributedStmt *AS = cast<AttributedStmt>(S); + for (const auto *Attr : getSpecificAttrs<CXXAssumeAttr>(AS->getAttrs())) { + AddLiveExpr(val.liveExprs, LV.ESetFact, Attr->getAssumption()); + } + break; + } case Stmt::PseudoObjectExprClass: { // A pseudo-object operation only directly consumes its result // expression. diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 07bc5f6920293..291533d0c3289 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -1215,16 +1215,28 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE, ExplodedNode *Pred, void ExprEngine::VisitAttributedStmt(const AttributedStmt *A, ExplodedNode *Pred, ExplodedNodeSet &Dst) { + const StackFrame *SF = Pred->getStackFrame(); ExplodedNodeSet CheckerPreStmt; getCheckerManager().runCheckersForPreStmt(CheckerPreStmt, Pred, A, *this); ExplodedNodeSet EvalSet; - NodeBuilder Bldr(CheckerPreStmt, EvalSet, *currBldrCtx); - for (const auto *Attr : getSpecificAttrs<CXXAssumeAttr>(A->getAttrs())) { - for (ExplodedNode *N : CheckerPreStmt) { - Visit(Attr->getAssumption()->IgnoreParens(), N, EvalSet); + for (ExplodedNode *N : CheckerPreStmt) { + ProgramStateRef State = N->getState(); + for (const auto *Attr : getSpecificAttrs<CXXAssumeAttr>(A->getAttrs())) { + SVal AssumedVal = State->getSVal(Attr->getAssumption(), SF); + // This code ignores assumptions that evaluate to UndefinedVal. + // Perhaps there should be a checker that reports this situation. + if (auto ValidAssumedVal = AssumedVal.getAs<DefinedOrUnknownSVal>()) { + State = State->assume(*ValidAssumedVal, true); + } + + if (!State) + break; } + + if (State) + EvalSet.insert(Engine.makePostStmtNode(A, State, N)); } getCheckerManager().runCheckersForPostStmt(Dst, EvalSet, A, *this); diff --git a/clang/test/Analysis/cxx23-assume-attribute.cpp b/clang/test/Analysis/cxx23-assume-attribute.cpp index 4cc16446572dc..ce9ad29e55c36 100644 --- a/clang/test/Analysis/cxx23-assume-attribute.cpp +++ b/clang/test/Analysis/cxx23-assume-attribute.cpp @@ -50,24 +50,23 @@ int ternary_in_assume(int a, int b) { int assume_and_fallthrough_at_the_same_attrstmt(int a, int b) { [[assume(a == 2)]]; clang_analyzer_dump(a); // expected-warning {{2 S32b}} - // expected-warning-re@-1 {{reg_${{[0-9]+}}<int a>}} FIXME: We shouldn't have this dump. switch (a) { case 2: [[fallthrough, assume(b == 30)]]; case 4: { clang_analyzer_dump(b); // expected-warning {{30 S32b}} - // expected-warning-re@-1 {{reg_${{[0-9]+}}<int b>}} FIXME: We shouldn't have this dump. return b; } } // This code should be unreachable. - clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + clang_analyzer_warnIfReached(); // no-warning + return 0; +} +void assume_false() { [[assume(false)]]; // This should definitely make it so. clang_analyzer_warnIfReached(); // no-warning - - return 0; } void assume_opaque_gh151854_no_crash() { @@ -75,3 +74,26 @@ void assume_opaque_gh151854_no_crash() { [[assume(opaque())]]; // no-crash // expected-warning@-1 {{assumption is ignored because it contains (potential) side-effects}} } + +int multiple_assumptions(int a, int b) { + [[assume(a == 2), assume(b == 3)]]; + clang_analyzer_dump(a); // expected-warning {{2 S32b}} + clang_analyzer_dump(b); // expected-warning {{3 S32b}} + clang_analyzer_dump(a+b); // expected-warning {{5 S32b}} + return a + b; +} + +int trivial_assumption(int a) { + [[assume(a == 2)]]; + clang_analyzer_dump(a); // expected-warning {{2 S32b}} + return a; +} + +int undefined_assumption() { + // Theoretically the analyzer should report that the assumption expression of + // the [[assume]] attribute has an undefined value; currently these + // attributes are ignored by the analyzer. + int a; + [[assume(a)]]; + return a; // expected-warning {{Undefined or garbage value returned to caller}} + } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
