[clang] f10a870 - [analyzer] Sink false [[assume]] execution paths (#130418)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 11 10:11:14 PDT 2025
Author: Balazs Benics
Date: 2025-03-11T18:11:09+01:00
New Revision: f10a8706a1443dec834929dadcce837082df64b7
URL: https://github.com/llvm/llvm-project/commit/f10a8706a1443dec834929dadcce837082df64b7
DIFF: https://github.com/llvm/llvm-project/commit/f10a8706a1443dec834929dadcce837082df64b7.diff
LOG: [analyzer] Sink false [[assume]] execution paths (#130418)
This PR splits the existing modeling of builtin assume from the
BuiltinFunctionChecker.
We just sink the execution path if we are about to leave the assume
expression with a false assumption.
Assumptions with side-effects are skipped, and ignored. Their values are
"UnknownVal" anyway.
Added:
clang/lib/StaticAnalyzer/Checkers/AssumeModeling.cpp
Modified:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/test/Analysis/analyzer-enabled-checkers.c
clang/test/Analysis/cxx23-assume-attribute.cpp
clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 21b585169cf40..7ee2ec1aaf09f 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -385,7 +385,7 @@ def TrustReturnsNonnullChecker : Checker<"TrustReturnsNonnull">,
} // end "apiModeling"
//===----------------------------------------------------------------------===//
-// Evaluate "builtin" functions.
+// Evaluate "builtin" functions and assumptions.
//===----------------------------------------------------------------------===//
let ParentPackage = CoreBuiltin in {
@@ -399,6 +399,10 @@ def BuiltinFunctionChecker : Checker<"BuiltinFunctions">,
HelpText<"Evaluate compiler builtin functions (e.g., alloca())">,
Documentation<NotDocumented>;
+def AssumeModeling : Checker<"AssumeModeling">,
+ HelpText<"Model compiler builtin assume functions and the assume attribute">,
+ Documentation<NotDocumented>;
+
} // end "core.builtin"
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/StaticAnalyzer/Checkers/AssumeModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/AssumeModeling.cpp
new file mode 100644
index 0000000000000..1e3adb4f266ca
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Checkers/AssumeModeling.cpp
@@ -0,0 +1,86 @@
+//=== AssumeModeling.cpp --------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This checker evaluates the builting assume functions.
+// This checker also sinks execution paths leaving [[assume]] attributes with
+// false assumptions.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/AST/AttrIterator.h"
+#include "clang/Basic/Builtins.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "llvm/ADT/STLExtras.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+class AssumeModelingChecker
+ : public Checker<eval::Call, check::PostStmt<AttributedStmt>> {
+public:
+ void checkPostStmt(const AttributedStmt *A, CheckerContext &C) const;
+ bool evalCall(const CallEvent &Call, CheckerContext &C) const;
+};
+} // namespace
+
+void AssumeModelingChecker::checkPostStmt(const AttributedStmt *A,
+ CheckerContext &C) const {
+ if (!hasSpecificAttr<CXXAssumeAttr>(A->getAttrs()))
+ return;
+
+ for (const auto *Attr : getSpecificAttrs<CXXAssumeAttr>(A->getAttrs())) {
+ SVal AssumptionVal = C.getSVal(Attr->getAssumption());
+
+ // The assumption is not evaluated at all if it had sideffects; skip them.
+ if (AssumptionVal.isUnknown())
+ continue;
+
+ const auto *Assumption = AssumptionVal.getAsInteger();
+ assert(Assumption && "We should know the exact outcome of an assume expr");
+ if (Assumption && Assumption->isZero()) {
+ C.addSink();
+ }
+ }
+}
+
+bool AssumeModelingChecker::evalCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
+ if (!FD)
+ return false;
+
+ if (!llvm::is_contained({Builtin::BI__builtin_assume, Builtin::BI__assume},
+ FD->getBuiltinID())) {
+ return false;
+ }
+
+ assert(Call.getNumArgs() > 0);
+ SVal Arg = Call.getArgSVal(0);
+ if (Arg.isUndef())
+ return true; // Return true to model purity.
+
+ State = State->assume(Arg.castAs<DefinedOrUnknownSVal>(), true);
+ if (!State) {
+ C.addSink();
+ return true;
+ }
+
+ C.addTransition(State);
+ return true;
+}
+
+void ento::registerAssumeModeling(CheckerManager &Mgr) {
+ Mgr.registerChecker<AssumeModelingChecker>();
+}
+
+bool ento::shouldRegisterAssumeModeling(const CheckerManager &) { return true; }
diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index cfdd3c9faa360..b1a11442dec53 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -23,7 +23,6 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
using namespace clang;
@@ -288,25 +287,6 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent &Call,
handleOverflowBuiltin(Call, C, BO_Add,
getOverflowBuiltinResultType(Call, C, BI));
return true;
- case Builtin::BI__builtin_assume:
- case Builtin::BI__assume: {
- assert (Call.getNumArgs() > 0);
- SVal Arg = Call.getArgSVal(0);
- if (Arg.isUndef())
- return true; // Return true to model purity.
-
- state = state->assume(Arg.castAs<DefinedOrUnknownSVal>(), true);
- // FIXME: do we want to warn here? Not right now. The most reports might
- // come from infeasible paths, thus being false positives.
- if (!state) {
- C.generateSink(C.getState(), C.getPredecessor());
- return true;
- }
-
- C.addTransition(state);
- return true;
- }
-
case Builtin::BI__builtin_unpredictable:
case Builtin::BI__builtin_expect:
case Builtin::BI__builtin_expect_with_probability:
diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 9aac200cd7370..878b48197cd58 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -8,6 +8,7 @@ add_clang_library(clangStaticAnalyzerCheckers
AnalysisOrderChecker.cpp
AnalyzerStatsChecker.cpp
ArrayBoundChecker.cpp
+ AssumeModeling.cpp
BasicObjCFoundationChecks.cpp
BitwiseShiftChecker.cpp
BlockInCriticalSectionChecker.cpp
diff --git a/clang/test/Analysis/analyzer-enabled-checkers.c b/clang/test/Analysis/analyzer-enabled-checkers.c
index c70aeb21ab045..e5d0acb84a039 100644
--- a/clang/test/Analysis/analyzer-enabled-checkers.c
+++ b/clang/test/Analysis/analyzer-enabled-checkers.c
@@ -24,6 +24,7 @@
// CHECK-NEXT: core.StackAddressEscape
// CHECK-NEXT: core.UndefinedBinaryOperatorResult
// CHECK-NEXT: core.VLASize
+// CHECK-NEXT: core.builtin.AssumeModeling
// CHECK-NEXT: core.builtin.BuiltinFunctions
// CHECK-NEXT: core.builtin.NoReturnFunctions
// CHECK-NEXT: core.uninitialized.ArraySubscript
diff --git a/clang/test/Analysis/cxx23-assume-attribute.cpp b/clang/test/Analysis/cxx23-assume-attribute.cpp
index ee049af9f13aa..86e7662cd2af9 100644
--- a/clang/test/Analysis/cxx23-assume-attribute.cpp
+++ b/clang/test/Analysis/cxx23-assume-attribute.cpp
@@ -1,6 +1,7 @@
// RUN: %clang_analyze_cc1 -std=c++23 -triple x86_64-pc-linux-gnu \
// RUN: -analyzer-checker=core,debug.ExprInspection -verify %s
+void clang_analyzer_warnIfReached();
template <typename T> void clang_analyzer_dump(T);
template <typename T> void clang_analyzer_value(T);
@@ -27,33 +28,22 @@ int ternary_in_builtin_assume(int a, int b) {
// 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 at -1 {{[-2147483648, 10]}}
// expected-warning at -2 {{[11, 2147483647]}}
clang_analyzer_dump(b); // expected-warning {{4}} expected-warning {{10}}
- // expected-warning-re at -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 at -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 at -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 at -1 {{(reg_${{[0-9]+}}<int b>) + 300}} FIXME: We shouldn't have this dump.
return 0;
}
@@ -70,8 +60,12 @@ int assume_and_fallthrough_at_the_same_attrstmt(int a, int b) {
return b;
}
}
+
// This code should be unreachable.
+ clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+
[[assume(false)]]; // This should definitely make it so.
- clang_analyzer_dump(33); // expected-warning {{33 S32b}}
+ clang_analyzer_warnIfReached(); // no-warning
+
return 0;
}
diff --git a/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c b/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
index faf0a8f19d919..d2900c6a42fff 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
@@ -32,6 +32,7 @@
// CHECK-NEXT: core.StackAddressEscape
// CHECK-NEXT: core.UndefinedBinaryOperatorResult
// CHECK-NEXT: core.VLASize
+// CHECK-NEXT: core.builtin.AssumeModeling
// CHECK-NEXT: core.builtin.BuiltinFunctions
// CHECK-NEXT: core.builtin.NoReturnFunctions
// CHECK-NEXT: core.uninitialized.ArraySubscript
More information about the cfe-commits
mailing list