martong updated this revision to Diff 339934.
martong marked 3 inline comments as done.
martong added a comment.
- Assert on SizeArgN
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101060/new/
https://reviews.llvm.org/D101060
Files:
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp
clang/test/Analysis/std-c-library-functions-arg-constraints.c
clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
Index: clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
===================================================================
--- clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
+++ 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{{}}
}
Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===================================================================
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ 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}}
}
Index: clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp
===================================================================
--- /dev/null
+++ 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 0th 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 0th 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 0th arg should be equal to or less than the value of the 1st arg}} \
+ // expected-warning{{}}
+ break;
+ }
+ case 3: {
+ char buf[3];
+ __buf_size_arg_constraint_mul(buf, 4, 2); // \
+ // expected-note{{The size of the 0th arg should be equal to or less than the value of the 1st arg times the 2nd 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 0th 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;
+ }
+}
+
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ 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 <string>
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<BugType>(
@@ -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<RangeInt, RangeInt> &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);
+ Result += llvm::getOrdinalSuffix(ArgN);
+ 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}),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
