https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/198618
From b73f83f2d2bfd7c89793b48f46c9ae375921fcdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Fri, 15 May 2026 14:13:32 +0200 Subject: [PATCH 1/7] [NFC] Add testcase 'multiple_assumptions' --- clang/test/Analysis/cxx23-assume-attribute.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/clang/test/Analysis/cxx23-assume-attribute.cpp b/clang/test/Analysis/cxx23-assume-attribute.cpp index 4cc16446572dc..f88248d954bce 100644 --- a/clang/test/Analysis/cxx23-assume-attribute.cpp +++ b/clang/test/Analysis/cxx23-assume-attribute.cpp @@ -75,3 +75,16 @@ 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}} + // expected-warning-re@-1 {{reg_${{[0-9]+}}<int a>}} FIXME: We shouldn't have this dump. + clang_analyzer_dump(b); // expected-warning {{3 S32b}} + // expected-warning-re@-1 {{reg_${{[0-9]+}}<int b>}} FIXME: We shouldn't have this dump. + clang_analyzer_dump(a+b); // expected-warning {{5 S32b}} + // expected-warning-re@-1 {{(reg_${{[0-9]+}}<int a>) + 3}} FIXME: We shouldn't have this dump. + // expected-warning-re@-2 {{(reg_${{[0-9]+}}<int b>) + 2}} FIXME: We shouldn't have this dump. + // expected-warning-re@-3 {{(reg_${{[0-9]+}}<int a>) + (reg_${{[0-9]+}}<int b>)}} FIXME: We shouldn't have this dump. + return a + b; +} From 74118f4d9f715f3f614fcbe434ae2cf32833ee5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Tue, 19 May 2026 18:00:24 +0200 Subject: [PATCH 2/7] [NFC] Add a trivial testcase --- clang/test/Analysis/cxx23-assume-attribute.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/clang/test/Analysis/cxx23-assume-attribute.cpp b/clang/test/Analysis/cxx23-assume-attribute.cpp index f88248d954bce..2128d8eb71a08 100644 --- a/clang/test/Analysis/cxx23-assume-attribute.cpp +++ b/clang/test/Analysis/cxx23-assume-attribute.cpp @@ -88,3 +88,10 @@ int multiple_assumptions(int a, int b) { // expected-warning-re@-3 {{(reg_${{[0-9]+}}<int a>) + (reg_${{[0-9]+}}<int b>)}} FIXME: We shouldn't have this dump. return a + b; } + +int trivial_assumption(int a) { + [[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. + return a; +} From f9a3a8e9f4a5344830ee82e3eb476328c91c1785 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Tue, 19 May 2026 18:44:16 +0200 Subject: [PATCH 3/7] Remove unwanted (but harmless) NodeBuilder This `NodeBuilder` was never used, so the only effect of its existence was that its constructor called `EvalSet.insert(CheckerPreStmt)`. There is no logical reason to offer this "shortcut" which avoids the actual checking of the `assume` attributes, so I'm removing this `NodeBuilder`. By the way, this change has no functional effect because for each node `N` in `CheckerPreStmt` the `NodeBuilder`s constructed at the beginning of the `Visit` call[1] inserts `N` into `EvalSet` (and then later parts of `Visit` almost always remove it from `EvalSet`). This change only _postpones_ the insertion of these nodes, and the contents of `EvalSet` are not examined during this period. [1] It is completely incorrect to call `Visit` at this point; see follow-up commits for details. --- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 5ac8f6fa669e2..214f4db62bb7d 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -1228,7 +1228,6 @@ void ExprEngine::VisitAttributedStmt(const AttributedStmt *A, 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) { From f64e22176d1b4a096240f22ec991a3533e461c8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Tue, 19 May 2026 19:15:00 +0200 Subject: [PATCH 4/7] Fix handling of [[assume(...)]] annotations 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. 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.) 7. 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 three 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. 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). 3. 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). The tests are also updated to remove the FIXME comments that are resolved by this commit. --- clang/lib/Analysis/LiveVariables.cpp | 10 ++++++++ .../lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 24 ++++++++++++++++--- .../test/Analysis/cxx23-assume-attribute.cpp | 15 ++++-------- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/clang/lib/Analysis/LiveVariables.cpp b/clang/lib/Analysis/LiveVariables.cpp index c529ca52abcb2..e10fcd867313d 100644 --- a/clang/lib/Analysis/LiveVariables.cpp +++ b/clang/lib/Analysis/LiveVariables.cpp @@ -279,6 +279,16 @@ 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()); + } + // FIXME: Also handle other attributes that have relevant substatements. + 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 214f4db62bb7d..ef44fad0be0a5 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -1224,15 +1224,33 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE, ExplodedNode *Pred, void ExprEngine::VisitAttributedStmt(const AttributedStmt *A, ExplodedNode *Pred, ExplodedNodeSet &Dst) { + const LocationContext *LCtx = Pred->getLocationContext(); ExplodedNodeSet CheckerPreStmt; getCheckerManager().runCheckersForPreStmt(CheckerPreStmt, Pred, A, *this); ExplodedNodeSet EvalSet; - 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(), LCtx); + if (auto ValidAssumedVal = AssumedVal.getAs<DefinedOrUnknownSVal>()) { + State = State->assume(*ValidAssumedVal, true); + } else { + // The assumption expression has evaluated to UndefinedVal. + // Theoretically this should be reported by a checker, but currently we + // just discard the execution path. + State = nullptr; + } + if (!State) + break; } + + // FIXME: If the analyzer needs to know about other attributes, insert + // handling for them here. + + 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 2128d8eb71a08..8f06591732347 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() { @@ -79,19 +78,13 @@ void assume_opaque_gh151854_no_crash() { int multiple_assumptions(int a, int b) { [[assume(a == 2), assume(b == 3)]]; clang_analyzer_dump(a); // expected-warning {{2 S32b}} - // expected-warning-re@-1 {{reg_${{[0-9]+}}<int a>}} FIXME: We shouldn't have this dump. clang_analyzer_dump(b); // expected-warning {{3 S32b}} - // expected-warning-re@-1 {{reg_${{[0-9]+}}<int b>}} FIXME: We shouldn't have this dump. clang_analyzer_dump(a+b); // expected-warning {{5 S32b}} - // expected-warning-re@-1 {{(reg_${{[0-9]+}}<int a>) + 3}} FIXME: We shouldn't have this dump. - // expected-warning-re@-2 {{(reg_${{[0-9]+}}<int b>) + 2}} FIXME: We shouldn't have this dump. - // expected-warning-re@-3 {{(reg_${{[0-9]+}}<int a>) + (reg_${{[0-9]+}}<int b>)}} FIXME: We shouldn't have this dump. return a + b; } int trivial_assumption(int a) { [[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. return a; } From 20617ab8f439c6463551c2e57c80b7543d38962b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Tue, 19 May 2026 20:47:56 +0200 Subject: [PATCH 5/7] [NFC] Add a test for undefined assumption --- clang/test/Analysis/cxx23-assume-attribute.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/clang/test/Analysis/cxx23-assume-attribute.cpp b/clang/test/Analysis/cxx23-assume-attribute.cpp index 8f06591732347..ea9d575f5c97d 100644 --- a/clang/test/Analysis/cxx23-assume-attribute.cpp +++ b/clang/test/Analysis/cxx23-assume-attribute.cpp @@ -88,3 +88,13 @@ int trivial_assumption(int a) { 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 is an undefined value; currently it just aborts + // the symbolic execution on that path. + int a; + [[assume(a)]]; + clang_analyzer_warnIfReached(); // no-warning + return a; +} From 0e3622db66807868854daa24a7b3fd467f143222 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Tue, 26 May 2026 13:49:37 +0200 Subject: [PATCH 6/7] [NFC] Remove FIXME notes about analogous attributes As I scrolled through the list of attributes supported by clang it seems that `assume` is the only attribute that evaluates an arbitrary subexpression (with somewhat unusual sematics). Therefore there is no need to provide placeholder comments for other similar attributes. (Of course, it is still possible to add handling if e.g. newer standards introduce another similar attribute.) --- clang/lib/Analysis/LiveVariables.cpp | 1 - clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 3 --- 2 files changed, 4 deletions(-) diff --git a/clang/lib/Analysis/LiveVariables.cpp b/clang/lib/Analysis/LiveVariables.cpp index e10fcd867313d..dcf4ce24c1a8a 100644 --- a/clang/lib/Analysis/LiveVariables.cpp +++ b/clang/lib/Analysis/LiveVariables.cpp @@ -286,7 +286,6 @@ void TransferFunctions::Visit(Stmt *S) { for (const auto *Attr : getSpecificAttrs<CXXAssumeAttr>(AS->getAttrs())) { AddLiveExpr(val.liveExprs, LV.ESetFact, Attr->getAssumption()); } - // FIXME: Also handle other attributes that have relevant substatements. break; } case Stmt::PseudoObjectExprClass: { diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index ef44fad0be0a5..e78f8a981780e 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -1246,9 +1246,6 @@ void ExprEngine::VisitAttributedStmt(const AttributedStmt *A, break; } - // FIXME: If the analyzer needs to know about other attributes, insert - // handling for them here. - if (State) EvalSet.insert(Engine.makePostStmtNode(A, State, N)); } From e189b20fd21b0c08de46899e3500f0a4aed15a10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Tue, 26 May 2026 14:24:41 +0200 Subject: [PATCH 7/7] Ignore assumptions that evaluate to undefined --- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 8 +++----- clang/test/Analysis/cxx23-assume-attribute.cpp | 9 ++++----- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index e78f8a981780e..b8e3f7a2a00db 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -1234,14 +1234,12 @@ void ExprEngine::VisitAttributedStmt(const AttributedStmt *A, ProgramStateRef State = N->getState(); for (const auto *Attr : getSpecificAttrs<CXXAssumeAttr>(A->getAttrs())) { SVal AssumedVal = State->getSVal(Attr->getAssumption(), LCtx); + // 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); - } else { - // The assumption expression has evaluated to UndefinedVal. - // Theoretically this should be reported by a checker, but currently we - // just discard the execution path. - State = nullptr; } + if (!State) break; } diff --git a/clang/test/Analysis/cxx23-assume-attribute.cpp b/clang/test/Analysis/cxx23-assume-attribute.cpp index ea9d575f5c97d..ce9ad29e55c36 100644 --- a/clang/test/Analysis/cxx23-assume-attribute.cpp +++ b/clang/test/Analysis/cxx23-assume-attribute.cpp @@ -91,10 +91,9 @@ int trivial_assumption(int a) { int undefined_assumption() { // Theoretically the analyzer should report that the assumption expression of - // the [[assume]] attribute is an undefined value; currently it just aborts - // the symbolic execution on that path. + // the [[assume]] attribute has an undefined value; currently these + // attributes are ignored by the analyzer. int a; [[assume(a)]]; - clang_analyzer_warnIfReached(); // no-warning - return 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
