[clang] a7cb951 - [Analyzer][StdLibraryFunctionsChecker] Describe arg constraints
Gabor Marton via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 23 08:28:04 PDT 2021
Author: Gabor Marton
Date: 2021-04-23T17:27:54+02:00
New Revision: a7cb951fa40df14d98c51059194ae42855b96a08
URL: https://github.com/llvm/llvm-project/commit/a7cb951fa40df14d98c51059194ae42855b96a08
DIFF: https://github.com/llvm/llvm-project/commit/a7cb951fa40df14d98c51059194ae42855b96a08.diff
LOG: [Analyzer][StdLibraryFunctionsChecker] Describe arg constraints
In this patch, I provide a detailed explanation for each argument
constraint. This explanation is added in an extra 'note' tag, which is
displayed alongside the warning.
Since these new notes describe clearly the constraint, there is no need
to provide the number of the argument (e.g. 'Arg3') within the warning.
However, I decided to keep the name of the constraint in the warning (but
this could be a subject of discussion) in order to be able to identify
the different kind of constraint violations easily in a bug database
(e.g. CodeChecker).
Differential Revision: https://reviews.llvm.org/D101060
Added:
clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp
Modified:
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/test/Analysis/std-c-library-functions-arg-constraints.c
clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index cbb45347a6afe..a2409b028fb0c 100644
--- 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/DynamicExtent.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
+
+#include <string>
using namespace clang;
using namespace clang::ento;
@@ -87,6 +91,10 @@ class StdLibraryFunctionsChecker
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 @@ class StdLibraryFunctionsChecker
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 @@ class StdLibraryFunctionsChecker
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 @@ class StdLibraryFunctionsChecker
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 @@ class StdLibraryFunctionsChecker
: 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 @@ class StdLibraryFunctionsChecker
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 @@ class StdLibraryFunctionsChecker
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 @@ class StdLibraryFunctionsChecker
// 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 @@ const StdLibraryFunctionsChecker::ArgNo StdLibraryFunctionsChecker::Ret =
} // 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 + 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 @@ void StdLibraryFunctionsChecker::checkPreCall(const CallEvent &Call,
// 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 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
// 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
index 0000000000000..baa7ad5dc0fe7
--- /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
index afc2ce28efc62..4d3749e4685ff 100644
--- 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 @@ int isalnum(int);
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 @@ void test_alnum_symbolic2(int x) {
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 @@ int toupper(int);
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 @@ void test_toupper_symbolic2(int x) {
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 @@ int tolower(int);
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 @@ void test_tolower_symbolic2(int x) {
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 @@ int toascii(int);
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 @@ void test_toascii_symbolic2(int x) {
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 @@ size_t fread(void *restrict, size_t, size_t, FILE *restrict);
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 @@ void test_notnull_symbolic2(FILE *fp, int *buf) {
// 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 @@ void ARR38_C_F(FILE *file) {
// 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 @@ int __variadic(void *stream, const char *format, ...);
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 @@ void test_buf_size_concrete() {
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 @@ void test_buf_size_concrete_with_multiplication() {
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 @@ void test_min_buf_size() {
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
index ae26834402e69..2e646dc2574a8 100644
--- 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 @@ int __defaultparam(void *, int y = 3);
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{{}}
}
More information about the cfe-commits
mailing list