diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -292,6 +292,11 @@ HelpText<"Improve modeling of the C standard library functions">, Documentation; +def StdCLibraryFunctionArgsChecker : Checker<"StdCLibraryFunctionArgs">, + HelpText<"Check constraints of arguments of C standard library functions">, + Dependencies<[StdCLibraryFunctionsChecker]>, + Documentation; + def TrustNonnullChecker : Checker<"TrustNonnull">, HelpText<"Trust that returns from framework methods annotated with _Nonnull " "are not null">, diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/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 { +class StdLibraryFunctionsChecker + : public Checker { /// 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; using Ranges = std::vector; - /// 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 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 findFunctionSummary(const FunctionDecl *FD, const CallExpr *CE, CheckerContext &C) const; + Optional 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(Call.getDecl()); - if (!FD) +void StdLibraryFunctionsChecker::checkPreCall(const CallEvent &Call, + CheckerContext &C) const { + Optional FoundSummary = findFunctionSummary(Call, C); + if (!FoundSummary) return; - const CallExpr *CE = dyn_cast_or_null(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(BT, Msg, N); + bugreporter::trackExpressionValue(N, Call.getArgExpr(0), *R); + C.emitReport(std::move(R)); + }; - Optional 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 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(Call.getDecl()); - if (!FD) - return false; - - const auto *CE = dyn_cast_or_null(Call.getOriginExpr()); - if (!CE) - return false; - - Optional FoundSummary = findFunctionSummary(FD, CE, C); + Optional 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(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::findFunctionSummary(const CallEvent &Call, + CheckerContext &C) const { + const FunctionDecl *FD = dyn_cast_or_null(Call.getDecl()); + if (!FD) + return None; + const CallExpr *CE = dyn_cast_or_null(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(); } bool ento::shouldRegisterStdCLibraryFunctionsChecker(const LangOptions &LO) { return true; } + +#define REGISTER_CHECKER(name) \ + void ento::register##name(CheckerManager &mgr) { \ + StdLibraryFunctionsChecker *checker = \ + mgr.getChecker(); \ + 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) diff --git a/clang/test/Analysis/analyzer-enabled-checkers.c b/clang/test/Analysis/analyzer-enabled-checkers.c --- a/clang/test/Analysis/analyzer-enabled-checkers.c +++ b/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 diff --git a/clang/test/Analysis/std-c-library-functions-arg-constraints-bug-path.c b/clang/test/Analysis/std-c-library-functions-arg-constraints-bug-path.c new file mode 100644 --- /dev/null +++ b/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; + } +} diff --git a/clang/test/Analysis/std-c-library-functions-arg-constraints.c b/clang/test/Analysis/std-c-library-functions-arg-constraints.c new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c @@ -0,0 +1,44 @@ +// 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 = x / y; // No warning here + + ret = x / y; // expected-warning{{Division by zero}} +} diff --git a/clang/test/Analysis/std-c-library-functions.c b/clang/test/Analysis/std-c-library-functions.c --- a/clang/test/Analysis/std-c-library-functions.c +++ b/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);