martong created this revision.
martong added reviewers: steakhal, NoQ, vsavchenko.
Herald added subscribers: ASDenysPetrov, Charusso, gamesh411, dkrupp,
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet,
baloghadamsoftware, xazax.hun, whisperity.
Herald added a reviewer: Szelethus.
martong requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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).
Repository:
rG LLVM Github Monorepo
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
@@ -1,4 +1,3 @@
-//=== StdLibraryFunctionsChecker.cpp - Model standard functions -*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -57,6 +56,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 +90,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 +135,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 +175,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 +245,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 +308,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,7 +322,8 @@
const SVal SizeV = [this, &State, &Call, &Summary, &SvalBuilder]() {
if (ConcreteSize) {
return SVal(SvalBuilder.makeIntVal(*ConcreteSize));
- } else if (SizeArgN) {
+ }
+ if (SizeArgN) {
// The size argument.
SVal SizeV = getArgSVal(Call, *SizeArgN);
// Multiply with another argument if given.
@@ -307,10 +333,9 @@
Summary.getArgType(*SizeArgN));
}
return SizeV;
- } else {
- llvm_unreachable("The constraint must be either a concrete value or "
- "encoded in an arguement.");
}
+ llvm_unreachable("The constraint must be either a concrete value or "
+ "encoded in an argument.");
}();
// The dynamic size of the buffer argument, got from the analyzer engine.
@@ -539,13 +564,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 +582,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 +595,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 +801,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 +2549,36 @@
// Functions for testing.
if (ChecksEnabled[CK_StdCLibraryFunctionsTesterChecker]) {
+ // Test range values.
+ 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
