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 @@ -57,6 +57,10 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h" #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/ADT/StringExtras.h" + +#include using namespace clang; using namespace clang::ento; @@ -87,6 +91,10 @@ typedef uint32_t ArgNo; static const ArgNo Ret; + /// Returns the string representation of an argument index. + /// E.g.: (1) -> '1st arg', (2) - > '2nd arg' + static SmallString<8> getArgDesc(ArgNo); + class ValueConstraint; // Pointer to the ValueConstraint. We need a copyable, polymorphic and @@ -128,6 +136,16 @@ virtual StringRef getName() const = 0; + // Give a description that explains the constraint to the user. Used when + // the bug is reported. + virtual std::string describe(ProgramStateRef State, + const Summary &Summary) const { + // There are some descendant classes that are not used as argument + // constraints, e.g. ComparisonConstraint. In that case we can safely + // ignore the implementation of this function. + llvm_unreachable("Not implemented"); + } + protected: ArgNo ArgN; // Argument to which we apply the constraint. @@ -158,6 +176,9 @@ RangeConstraint(ArgNo ArgN, RangeKind Kind, const IntRangeVector &Ranges) : ValueConstraint(ArgN), Kind(Kind), Ranges(Ranges) {} + std::string describe(ProgramStateRef State, + const Summary &Summary) const override; + const IntRangeVector &getRanges() const { return Ranges; } private: @@ -225,6 +246,8 @@ bool CannotBeNull = true; public: + std::string describe(ProgramStateRef State, + const Summary &Summary) const override; StringRef getName() const override { return "NonNull"; } ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call, const Summary &Summary, @@ -286,6 +309,9 @@ : ValueConstraint(Buffer), SizeArgN(BufSize), SizeMultiplierArgN(BufSizeMultiplier) {} + std::string describe(ProgramStateRef State, + const Summary &Summary) const override; + ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call, const Summary &Summary, CheckerContext &C) const override { @@ -297,20 +323,18 @@ const SVal SizeV = [this, &State, &Call, &Summary, &SvalBuilder]() { if (ConcreteSize) { return SVal(SvalBuilder.makeIntVal(*ConcreteSize)); - } else if (SizeArgN) { - // The size argument. - SVal SizeV = getArgSVal(Call, *SizeArgN); - // Multiply with another argument if given. - if (SizeMultiplierArgN) { - SVal SizeMulV = getArgSVal(Call, *SizeMultiplierArgN); - SizeV = SvalBuilder.evalBinOp(State, BO_Mul, SizeV, SizeMulV, - Summary.getArgType(*SizeArgN)); - } - return SizeV; - } else { - llvm_unreachable("The constraint must be either a concrete value or " - "encoded in an arguement."); } + assert(SizeArgN && "The constraint must be either a concrete value or " + "encoded in an argument."); + // The size argument. + SVal SizeV = getArgSVal(Call, *SizeArgN); + // Multiply with another argument if given. + if (SizeMultiplierArgN) { + SVal SizeMulV = getArgSVal(Call, *SizeMultiplierArgN); + SizeV = SvalBuilder.evalBinOp(State, BO_Mul, SizeV, SizeMulV, + Summary.getArgType(*SizeArgN)); + } + return SizeV; }(); // The dynamic size of the buffer argument, got from the analyzer engine. @@ -539,13 +563,13 @@ void initFunctionSummaries(CheckerContext &C) const; void reportBug(const CallEvent &Call, ExplodedNode *N, - const ValueConstraint *VC, CheckerContext &C) const { + const ValueConstraint *VC, const Summary &Summary, + CheckerContext &C) const { if (!ChecksEnabled[CK_StdCLibraryFunctionArgsChecker]) return; - // TODO Add more detailed diagnostic. std::string Msg = (Twine("Function argument constraint is not satisfied, constraint: ") + - VC->getName().data() + ", ArgN: " + Twine(VC->getArgNo())) + VC->getName().data()) .str(); if (!BT_InvalidArg) BT_InvalidArg = std::make_unique( @@ -557,6 +581,10 @@ // Highlight the range of the argument that was violated. R->addRange(Call.getArgSourceRange(VC->getArgNo())); + // Describe the argument constraint in a note. + R->addNote(VC->describe(C.getState(), Summary), R->getLocation(), + Call.getArgSourceRange(VC->getArgNo())); + C.emitReport(std::move(R)); } }; @@ -566,6 +594,85 @@ } // end of anonymous namespace +static BasicValueFactory &getBVF(ProgramStateRef State) { + ProgramStateManager &Mgr = State->getStateManager(); + SValBuilder &SVB = Mgr.getSValBuilder(); + return SVB.getBasicValueFactory(); +} + +std::string StdLibraryFunctionsChecker::NotNullConstraint::describe( + ProgramStateRef State, const Summary &Summary) const { + SmallString<48> Result; + Result += "The "; + Result += getArgDesc(ArgN); + Result += " should not be NULL"; + return Result.c_str(); +} + +std::string StdLibraryFunctionsChecker::RangeConstraint::describe( + ProgramStateRef State, const Summary &Summary) const { + + BasicValueFactory &BVF = getBVF(State); + + QualType T = Summary.getArgType(getArgNo()); + SmallString<48> Result; + Result += "The "; + Result += getArgDesc(ArgN); + Result += " should be "; + + // Range kind as a string. + Kind == OutOfRange ? Result += "out of" : Result += "within"; + + // Get the range values as a string. + Result += " the range "; + if (Ranges.size() > 1) + Result += "["; + unsigned I = Ranges.size(); + for (const std::pair &R : Ranges) { + Result += "["; + const llvm::APSInt &Min = BVF.getValue(R.first, T); + const llvm::APSInt &Max = BVF.getValue(R.second, T); + Min.toString(Result); + Result += ", "; + Max.toString(Result); + Result += "]"; + if (--I > 0) + Result += ", "; + } + if (Ranges.size() > 1) + Result += "]"; + + return Result.c_str(); +} + +SmallString<8> +StdLibraryFunctionsChecker::getArgDesc(StdLibraryFunctionsChecker::ArgNo ArgN) { + SmallString<8> Result; + Result += std::to_string(ArgN + 1); + Result += llvm::getOrdinalSuffix(ArgN + 1); + Result += " arg"; + return Result; +} + +std::string StdLibraryFunctionsChecker::BufferSizeConstraint::describe( + ProgramStateRef State, const Summary &Summary) const { + SmallString<96> Result; + Result += "The size of the "; + Result += getArgDesc(ArgN); + Result += " should be equal to or less than the value of "; + if (ConcreteSize) { + ConcreteSize->toString(Result); + } else if (SizeArgN) { + Result += "the "; + Result += getArgDesc(*SizeArgN); + if (SizeMultiplierArgN) { + Result += " times the "; + Result += getArgDesc(*SizeMultiplierArgN); + } + } + return Result.c_str(); +} + ProgramStateRef StdLibraryFunctionsChecker::RangeConstraint::applyAsOutOfRange( ProgramStateRef State, const CallEvent &Call, const Summary &Summary) const { @@ -693,7 +800,7 @@ // The argument constraint is not satisfied. if (FailureSt && !SuccessSt) { if (ExplodedNode *N = C.generateErrorNode(NewState)) - reportBug(Call, N, Constraint.get(), C); + reportBug(Call, N, Constraint.get(), Summary, C); break; } else { // We will apply the constraint even if we cannot reason about the @@ -2441,6 +2548,35 @@ // Functions for testing. if (ChecksEnabled[CK_StdCLibraryFunctionsTesterChecker]) { + addToFunctionSummaryMap( + "__not_null", Signature(ArgTypes{IntPtrTy}, RetType{IntTy}), + Summary(EvalCallAsPure).ArgConstraint(NotNull(ArgNo(0)))); + + // Test range values. + addToFunctionSummaryMap( + "__single_val_1", Signature(ArgTypes{IntTy}, RetType{IntTy}), + Summary(EvalCallAsPure) + .ArgConstraint(ArgumentCondition(0U, WithinRange, SingleValue(1)))); + addToFunctionSummaryMap( + "__range_1_2", Signature(ArgTypes{IntTy}, RetType{IntTy}), + Summary(EvalCallAsPure) + .ArgConstraint(ArgumentCondition(0U, WithinRange, Range(1, 2)))); + addToFunctionSummaryMap("__range_1_2__4_5", + Signature(ArgTypes{IntTy}, RetType{IntTy}), + Summary(EvalCallAsPure) + .ArgConstraint(ArgumentCondition( + 0U, WithinRange, Range({1, 2}, {4, 5})))); + + // Test range kind. + addToFunctionSummaryMap( + "__within", Signature(ArgTypes{IntTy}, RetType{IntTy}), + Summary(EvalCallAsPure) + .ArgConstraint(ArgumentCondition(0U, WithinRange, SingleValue(1)))); + addToFunctionSummaryMap( + "__out_of", Signature(ArgTypes{IntTy}, RetType{IntTy}), + Summary(EvalCallAsPure) + .ArgConstraint(ArgumentCondition(0U, OutOfRange, SingleValue(1)))); + addToFunctionSummaryMap( "__two_constrained_args", Signature(ArgTypes{IntTy, IntTy}, RetType{IntTy}), diff --git a/clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp b/clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp @@ -0,0 +1,94 @@ +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \ +// RUN: -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \ +// RUN: -analyzer-checker=debug.StdCLibraryFunctionsTester \ +// RUN: -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -analyzer-config eagerly-assume=false \ +// RUN: -triple i686-unknown-linux \ +// RUN: -verify + +// In this test we verify that each argument constraints are described properly. + +// Check NotNullConstraint violation notes. +int __not_null(int *); +void test_not_null(int *x) { + __not_null(nullptr); // \ + // expected-note{{The 1st arg should not be NULL}} \ + // expected-warning{{}} +} + +// Check the BufferSizeConstraint violation notes. +using size_t = decltype(sizeof(int)); +int __buf_size_arg_constraint_concrete(const void *); // size <= 10 +int __buf_size_arg_constraint(const void *, size_t); // size <= Arg1 +int __buf_size_arg_constraint_mul(const void *, size_t, size_t); // size <= Arg1 * Arg2 +void test_buffer_size(int x) { + switch (x) { + case 1: { + char buf[9]; + __buf_size_arg_constraint_concrete(buf); // \ + // expected-note{{The size of the 1st arg should be equal to or less than the value of 10}} \ + // expected-warning{{}} + break; + } + case 2: { + char buf[3]; + __buf_size_arg_constraint(buf, 4); // \ + // expected-note{{The size of the 1st arg should be equal to or less than the value of the 2nd arg}} \ + // expected-warning{{}} + break; + } + case 3: { + char buf[3]; + __buf_size_arg_constraint_mul(buf, 4, 2); // \ + // expected-note{{The size of the 1st arg should be equal to or less than the value of the 2nd arg times the 3rd arg}} \ + // expected-warning{{}} + break; + } + } +} + +// Check the RangeConstraint violation notes. +int __single_val_1(int); // [1, 1] +int __range_1_2(int); // [1, 2] +int __range_1_2__4_5(int); // [1, 2], [4, 5] +void test_range(int x) { + __single_val_1(2); // \ + // expected-note{{The 1st arg should be within the range [1, 1]}} \ + // expected-warning{{}} +} +// Do more specific check against the range strings. +void test_range_values(int x) { + switch (x) { + case 1: + __single_val_1(2); // expected-note{{[1, 1]}} \ + // expected-warning{{}} + break; + case 2: + __range_1_2(3); // expected-note{{[1, 2]}} \ + // expected-warning{{}} + break; + case 3: + __range_1_2__4_5(3); // expected-note{{[[1, 2], [4, 5]]}} \ + // expected-warning{{}} + break; + } +} +// Do more specific check against the range kinds. +int __within(int); // [1, 1] +int __out_of(int); // [1, 1] +void test_range_kind(int x) { + switch (x) { + case 1: + __within(2); // expected-note{{within}} \ + // expected-warning{{}} + break; + case 2: + __out_of(1); // expected-note{{out of}} \ + // expected-warning{{}} + break; + } +} + diff --git a/clang/test/Analysis/std-c-library-functions-arg-constraints.c b/clang/test/Analysis/std-c-library-functions-arg-constraints.c --- a/clang/test/Analysis/std-c-library-functions-arg-constraints.c +++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c @@ -30,7 +30,9 @@ void test_alnum_concrete(int v) { int ret = isalnum(256); // \ // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ // bugpath-note{{Function argument constraint is not satisfied}} (void)ret; } @@ -54,7 +56,9 @@ int ret = isalnum(x); // \ // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ // bugpath-note{{Function argument constraint is not satisfied}} (void)ret; @@ -66,7 +70,9 @@ void test_toupper_concrete(int v) { int ret = toupper(256); // \ // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ // bugpath-note{{Function argument constraint is not satisfied}} (void)ret; } @@ -90,7 +96,9 @@ int ret = toupper(x); // \ // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ // bugpath-note{{Function argument constraint is not satisfied}} (void)ret; @@ -102,7 +110,9 @@ void test_tolower_concrete(int v) { int ret = tolower(256); // \ // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ // bugpath-note{{Function argument constraint is not satisfied}} (void)ret; } @@ -126,7 +136,9 @@ int ret = tolower(x); // \ // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ // bugpath-note{{Function argument constraint is not satisfied}} (void)ret; @@ -138,7 +150,9 @@ void test_toascii_concrete(int v) { int ret = toascii(256); // \ // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ // bugpath-note{{Function argument constraint is not satisfied}} (void)ret; } @@ -162,7 +176,9 @@ int ret = toascii(x); // \ // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ // bugpath-note{{Function argument constraint is not satisfied}} (void)ret; @@ -175,7 +191,9 @@ void test_notnull_concrete(FILE *fp) { fread(0, sizeof(int), 10, fp); // \ // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ // bugpath-note{{Function argument constraint is not satisfied}} } void test_notnull_symbolic(FILE *fp, int *buf) { @@ -191,7 +209,9 @@ // bugpath-note{{Taking true branch}} fread(buf, sizeof(int), 10, fp); // \ // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ // bugpath-note{{Function argument constraint is not satisfied}} } typedef __WCHAR_TYPE__ wchar_t; @@ -207,7 +227,9 @@ // the size in bytes. fread(wbuf, size, nitems, file); // \ // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ // bugpath-note{{Function argument constraint is not satisfied}} } @@ -242,7 +264,9 @@ void test_arg_constraint_on_variadic_fun() { __variadic(0, "%d%d", 1, 2); // \ // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ // bugpath-note{{Function argument constraint is not satisfied}} } @@ -251,7 +275,9 @@ char buf[3]; // bugpath-note{{'buf' initialized here}} __buf_size_arg_constraint(buf, 4); // \ // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ // bugpath-note{{Function argument constraint is not satisfied}} } void test_buf_size_symbolic(int s) { @@ -278,7 +304,9 @@ short buf[3]; // bugpath-note{{'buf' initialized here}} __buf_size_arg_constraint_mul(buf, 4, sizeof(short)); // \ // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ // bugpath-note{{Function argument constraint is not satisfied}} } void test_buf_size_symbolic_with_multiplication(size_t s) { @@ -304,6 +332,8 @@ char buf[9];// bugpath-note{{'buf' initialized here}} __buf_size_arg_constraint_concrete(buf); // \ // report-warning{{Function argument constraint is not satisfied}} \ + // report-note{{}} \ // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{}} \ // bugpath-note{{Function argument constraint is not satisfied}} } diff --git a/clang/test/Analysis/std-c-library-functions-arg-constraints.cpp b/clang/test/Analysis/std-c-library-functions-arg-constraints.cpp --- a/clang/test/Analysis/std-c-library-functions-arg-constraints.cpp +++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.cpp @@ -14,5 +14,6 @@ void test_arg_constraint_on_fun_with_default_param() { __defaultparam(nullptr); // \ - // expected-warning{{Function argument constraint is not satisfied}} + // expected-warning{{Function argument constraint is not satisfied}} \ + // expected-note{{}} }