martong updated this revision to Diff 245651.
martong marked 8 inline comments as done.
martong added a comment.
- Add new Checker that does the report
- Refactor with negated RangeValues
- Add overload to findFunctionSummary
- Add tests for symbolic values
- Add test file for bug path
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73898/new/
https://reviews.llvm.org/D73898
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/test/Analysis/analyzer-enabled-checkers.c
clang/test/Analysis/std-c-library-functions-arg-constraints-bug-path.c
clang/test/Analysis/std-c-library-functions-arg-constraints.c
clang/test/Analysis/std-c-library-functions.c
Index: clang/test/Analysis/std-c-library-functions.c
===================================================================
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -1,8 +1,34 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux -analyzer-checker=apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -triple armv7-a15-linux -analyzer-checker=apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -triple thumbv7-a15-linux -analyzer-checker=apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN: -analyzer-checker=debug.ExprInspection \
+// RUN: -analyzer-config eagerly-assume=false \
+// RUN: -triple i686-unknown-linux \
+// RUN: -verify
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN: -analyzer-checker=debug.ExprInspection \
+// RUN: -analyzer-config eagerly-assume=false \
+// RUN: -triple x86_64-unknown-linux \
+// RUN: -verify
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN: -analyzer-checker=debug.ExprInspection \
+// RUN: -analyzer-config eagerly-assume=false \
+// RUN: -triple armv7-a15-linux \
+// RUN: -verify
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN: -analyzer-checker=debug.ExprInspection \
+// RUN: -analyzer-config eagerly-assume=false \
+// RUN: -triple thumbv7-a15-linux \
+// RUN: -verify
void clang_analyzer_eval(int);
Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctionArgs \
+// RUN: -analyzer-checker=debug.ExprInspection \
+// RUN: -triple x86_64-unknown-linux-gnu \
+// RUN: -verify
+
+void clang_analyzer_eval(int);
+
+int glob;
+
+#define EOF -1
+
+int isalnum(int);
+
+void test_alnum_concrete(int v) {
+ int ret = isalnum(256); // expected-warning{{Function argument constraint is not satisfied}}
+ (void)ret;
+}
+
+void test_alnum_symbolic(int x) {
+ int ret = isalnum(x);
+ (void)ret;
+ clang_analyzer_eval(EOF <= x && x <= 255); // expected-warning{{TRUE}}
+}
+
+void test_alnum_symbolic2(int x) {
+ if (x > 255) {
+ int ret = isalnum(x); // expected-warning{{Function argument constraint is not satisfied}}
+ (void)ret;
+ }
+}
+
+void test_alnum_infeasible_path(int x, int y) {
+ int ret = isalnum(x);
+ y = 0;
+ clang_analyzer_eval(EOF <= x && x <= 255); // expected-warning{{TRUE}}
+
+ if (x > 255) { // This path is no longer feasible.
+ ret = isalnum(x);
+ ret = x / y; // No warning here
+ }
+
+ ret = x / y; // expected-warning{{Division by zero}}
+}
Index: clang/test/Analysis/std-c-library-functions-arg-constraints-bug-path.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-arg-constraints-bug-path.c
@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctionArgs \
+// RUN: -analyzer-checker=debug.ExprInspection \
+// RUN: -triple x86_64-unknown-linux-gnu \
+// RUN: -analyzer-output=text \
+// RUN: -verify
+
+void clang_analyzer_eval(int);
+
+int glob;
+
+#define EOF -1
+
+int isalnum(int);
+
+void test_alnum_symbolic(int x) {
+ int ret = isalnum(x);
+ (void)ret;
+ clang_analyzer_eval(EOF <= x && x <= 255); // expected-warning{{TRUE}} expected-note{{TRUE}} expected-note{{Left side of '&&' is true}} expected-note{{'x' is <= 255}}
+}
+
+void test_alnum_symbolic2(int x) {
+ if (x > 255) { // expected-note{{Assuming 'x' is > 255}} expected-note{{Taking true branch}}
+ int ret = isalnum(x); // expected-warning{{Function argument constraint is not satisfied}} expected-note{{Function argument constraint is not satisfied}}
+ (void)ret;
+ }
+}
Index: clang/test/Analysis/analyzer-enabled-checkers.c
===================================================================
--- clang/test/Analysis/analyzer-enabled-checkers.c
+++ clang/test/Analysis/analyzer-enabled-checkers.c
@@ -7,6 +7,7 @@
// CHECK: OVERVIEW: Clang Static Analyzer Enabled Checkers List
// CHECK-EMPTY:
// CHECK-NEXT: apiModeling.StdCLibraryFunctions
+// CHECK-NEXT: apiModeling.StdCLibraryFunctionArgs
// CHECK-NEXT: apiModeling.TrustNonnull
// CHECK-NEXT: apiModeling.llvm.CastValue
// CHECK-NEXT: apiModeling.llvm.ReturnValue
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -51,6 +51,7 @@
//===----------------------------------------------------------------------===//
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
@@ -61,7 +62,8 @@
using namespace clang::ento;
namespace {
-class StdLibraryFunctionsChecker : public Checker<check::PostCall, eval::Call> {
+class StdLibraryFunctionsChecker
+ : public Checker<check::PreCall, check::PostCall, eval::Call> {
/// Below is a series of typedefs necessary to define function specs.
/// We avoid nesting types here because each additional qualifier
/// would need to be repeated in every function spec.
@@ -155,6 +157,24 @@
}
llvm_unreachable("Unknown ValueRange kind!");
}
+
+ ValueRange negate() const {
+ ValueRange tmp(*this);
+ switch (Kind) {
+ case OutOfRange:
+ tmp.Kind = WithinRange;
+ break;
+ case WithinRange:
+ tmp.Kind = OutOfRange;
+ break;
+ case ComparesToArgument:
+ // TODO implement negation to each binary operation kind.
+ llvm_unreachable("Not implemented");
+ default:
+ llvm_unreachable("Unknown ValueRange kind!");
+ }
+ return tmp;
+ }
};
/// The complete list of ranges that defines a single branch.
@@ -163,10 +183,15 @@
using ArgTypes = std::vector<QualType>;
using Ranges = std::vector<ValueRangeSet>;
- /// Includes information about function prototype (which is necessary to
- /// ensure we're modeling the right function and casting values properly),
- /// approach to invalidation, and a list of branches - essentially, a list
- /// of list of ranges - essentially, a list of lists of lists of segments.
+ /// Includes information about
+ /// * function prototype (which is necessary to
+ /// ensure we're modeling the right function and casting values properly),
+ /// * approach to invalidation,
+ /// * a list of branches - a list of list of ranges -
+ /// i.e. a list of lists of lists of segments,
+ /// * a list of argument constraints, that must be true on every branch.
+ /// If these constraints are not satisfied that means a fatal error
+ /// usually resulting in undefined behaviour.
struct Summary {
const ArgTypes ArgTys;
const QualType RetTy;
@@ -181,6 +206,10 @@
Cases.push_back(VRS);
return *this;
}
+ Summary &ArgConstraint(ValueRange VR) {
+ ArgConstraints.push_back(VR);
+ return *this;
+ }
private:
static void assertTypeSuitableForSummary(QualType T) {
@@ -216,6 +245,8 @@
// lazily, and it doesn't change after initialization.
mutable llvm::StringMap<Summaries> FunctionSummaryMap;
+ BugType BT{this, "Unsatisfied argument constraints", categories::LogicError};
+
// Auxiliary functions to support ArgNo within all structures
// in a unified manner.
static QualType getArgType(const Summary &Summary, ArgNo ArgN) {
@@ -234,13 +265,20 @@
}
public:
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
+ enum CheckKind { CK_StdCLibraryFunctionArgsChecker, CK_NumCheckKinds };
+ DefaultBool ChecksEnabled[CK_NumCheckKinds];
+ CheckerNameRef CheckNames[CK_NumCheckKinds];
+
private:
Optional<Summary> findFunctionSummary(const FunctionDecl *FD,
const CallExpr *CE,
CheckerContext &C) const;
+ Optional<Summary> findFunctionSummary(const CallEvent &Call,
+ CheckerContext &C) const;
void initFunctionSummaries(CheckerContext &C) const;
};
@@ -353,17 +391,48 @@
return State;
}
-void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
- CheckerContext &C) const {
- const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
- if (!FD)
+void StdLibraryFunctionsChecker::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ Optional<Summary> FoundSummary = findFunctionSummary(Call, C);
+ if (!FoundSummary)
return;
- const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
- return;
+ const Summary &Summary = *FoundSummary;
+ ProgramStateRef State = C.getState();
+
+ auto Report = [&](ExplodedNode *N) {
+ if (!ChecksEnabled[CK_StdCLibraryFunctionArgsChecker])
+ return;
+ // FIXME Add detailed diagnostic.
+ std::string Msg = "Function argument constraint is not satisfied";
+ auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N);
+ bugreporter::trackExpressionValue(N, Call.getArgExpr(0), *R);
+ C.emitReport(std::move(R));
+ };
- Optional<Summary> FoundSummary = findFunctionSummary(FD, CE, C);
+ for (const ValueRange &VR : Summary.ArgConstraints) {
+ ProgramStateRef SuccessSt = VR.apply(State, Call, Summary);
+ ProgramStateRef FailureSt = VR.negate().apply(State, Call, Summary);
+ // The argument constraint is not satisfied.
+ if (FailureSt && !SuccessSt) {
+ C.addTransition(FailureSt);
+ if (ExplodedNode *N = C.generateErrorNode(FailureSt))
+ Report(N);
+ break;
+ } else {
+ // Apply the constraint even if we cannot reason about the argument. This
+ // means both SuccessSt and FailureSt can be true. If we weren't applying
+ // the constraint that would mean that symbolic execution continues on a
+ // code whose behaviour is undefined.
+ assert(SuccessSt);
+ C.addTransition(SuccessSt);
+ }
+ }
+}
+
+void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ Optional<Summary> FoundSummary = findFunctionSummary(Call, C);
if (!FoundSummary)
return;
@@ -387,15 +456,7 @@
bool StdLibraryFunctionsChecker::evalCall(const CallEvent &Call,
CheckerContext &C) const {
- const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
- if (!FD)
- return false;
-
- const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
- return false;
-
- Optional<Summary> FoundSummary = findFunctionSummary(FD, CE, C);
+ Optional<Summary> FoundSummary = findFunctionSummary(Call, C);
if (!FoundSummary)
return false;
@@ -404,6 +465,7 @@
case EvalCallAsPure: {
ProgramStateRef State = C.getState();
const LocationContext *LC = C.getLocationContext();
+ const auto *CE = cast_or_null<CallExpr>(Call.getOriginExpr());
SVal V = C.getSValBuilder().conjureSymbolVal(
CE, LC, CE->getType().getCanonicalType(), C.blockCount());
State = State->BindExpr(CE, LC, V);
@@ -483,6 +545,18 @@
return None;
}
+Optional<StdLibraryFunctionsChecker::Summary>
+StdLibraryFunctionsChecker::findFunctionSummary(const CallEvent &Call,
+ CheckerContext &C) const {
+ const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
+ if (!FD)
+ return None;
+ const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return None;
+ return findFunctionSummary(FD, CE, C);
+}
+
void StdLibraryFunctionsChecker::initFunctionSummaries(
CheckerContext &C) const {
if (!FunctionSummaryMap.empty())
@@ -569,7 +643,6 @@
auto IsLessThan = [](ArgNo ArgN) { return IntRangeVector{{BO_LE, ArgN}}; };
using RetType = QualType;
-
// Templates for summaries that are reused by many functions.
auto Getc = [&]() {
return Summary(ArgTypes{Irrelevant}, RetType{IntTy}, NoEvalCall)
@@ -597,6 +670,9 @@
FunctionSummaryMap = {
// The isascii() family of functions.
+ // The behavior is undefined if the value of the argument is not
+ // representable as unsigned char or is not equal to EOF. See e.g. C99
+ // 7.4.1.2 The isalpha function (p: 181-182).
{
"isalnum",
Summaries{
@@ -615,7 +691,9 @@
{'A', 'Z'},
{'a', 'z'},
{128, UCharMax}}),
- ReturnValueCondition(WithinRange, SingleValue(0))})},
+ ReturnValueCondition(WithinRange, SingleValue(0))})
+ .ArgConstraint(ArgumentCondition(
+ 0U, WithinRange, {{EOFv, EOFv}, {0, UCharMax}}))},
},
{
"isalpha",
@@ -788,13 +866,22 @@
}
void ento::registerStdCLibraryFunctionsChecker(CheckerManager &mgr) {
- // If this checker grows large enough to support C++, Objective-C, or other
- // standard libraries, we could use multiple register...Checker() functions,
- // which would register various checkers with the help of the same Checker
- // class, turning on different function summaries.
mgr.registerChecker<StdLibraryFunctionsChecker>();
}
bool ento::shouldRegisterStdCLibraryFunctionsChecker(const LangOptions &LO) {
return true;
}
+
+#define REGISTER_CHECKER(name) \
+ void ento::register##name(CheckerManager &mgr) { \
+ StdLibraryFunctionsChecker *checker = \
+ mgr.getChecker<StdLibraryFunctionsChecker>(); \
+ checker->ChecksEnabled[StdLibraryFunctionsChecker::CK_##name] = true; \
+ checker->CheckNames[StdLibraryFunctionsChecker::CK_##name] = \
+ mgr.getCurrentCheckerName(); \
+ } \
+ \
+ bool ento::shouldRegister##name(const LangOptions &LO) { return true; }
+
+REGISTER_CHECKER(StdCLibraryFunctionArgsChecker)
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -292,6 +292,11 @@
HelpText<"Improve modeling of the C standard library functions">,
Documentation<NotDocumented>;
+def StdCLibraryFunctionArgsChecker : Checker<"StdCLibraryFunctionArgs">,
+ HelpText<"Check constraints of arguments of C standard library functions">,
+ Dependencies<[StdCLibraryFunctionsChecker]>,
+ Documentation<NotDocumented>;
+
def TrustNonnullChecker : Checker<"TrustNonnull">,
HelpText<"Trust that returns from framework methods annotated with _Nonnull "
"are not null">,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
