[clang] 82a5081 - [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg
Gabor Marton via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 26 07:34:38 PDT 2022
Author: Gabor Marton
Date: 2022-10-26T16:33:25+02:00
New Revision: 82a50812f7e539b69fb8031bee9c823e5290d1c9
URL: https://github.com/llvm/llvm-project/commit/82a50812f7e539b69fb8031bee9c823e5290d1c9
DIFF: https://github.com/llvm/llvm-project/commit/82a50812f7e539b69fb8031bee9c823e5290d1c9.diff
LOG: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg
constraints
In this patch I add a new NoteTag for each applied argument constraint.
This way, any other checker that reports a bug - where the applied
constraint is relevant - will display the corresponding note. With this
change we provide more information for the users to understand some
bug reports easier.
Differential Revision: https://reviews.llvm.org/D101526
Reviewed By: NoQ
Added:
clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
Modified:
clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
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
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
index 195a5fa977438..f9efcc5ccccd9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -115,7 +115,8 @@ bool ExprInspectionChecker::evalCall(const CallEvent &Call,
.Case("clang_analyzer_hashDump",
&ExprInspectionChecker::analyzerHashDump)
.Case("clang_analyzer_denote", &ExprInspectionChecker::analyzerDenote)
- .Case("clang_analyzer_express",
+ .Case("clang_analyzer_express", // This also marks the argument as
+ // interesting.
&ExprInspectionChecker::analyzerExpress)
.StartsWith("clang_analyzer_isTainted",
&ExprInspectionChecker::analyzerIsTainted)
@@ -530,14 +531,14 @@ void ExprInspectionChecker::analyzerExpress(const CallExpr *CE,
SVal ArgVal = C.getSVal(CE->getArg(0));
SymbolRef Sym = ArgVal.getAsSymbol();
if (!Sym) {
- reportBug("Not a symbol", C);
+ reportBug("Not a symbol", C, ArgVal);
return;
}
SymbolExpressor V(C.getState());
auto Str = V.Visit(Sym);
if (!Str) {
- reportBug("Unable to express", C);
+ reportBug("Unable to express", C, ArgVal);
return;
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 25e80545154ee..c5fa43ac23d1e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -134,9 +134,18 @@ class StdLibraryFunctionsChecker
virtual StringRef getName() const = 0;
+ // Represents that in which context do we require a description of the
+ // constraint.
+ enum class DescriptionKind {
+ // The constraint is violated.
+ Violation,
+ // We assume that the constraint is satisfied.
+ Assumption
+ };
+
// Give a description that explains the constraint to the user. Used when
// the bug is reported.
- virtual std::string describe(ProgramStateRef State,
+ virtual std::string describe(DescriptionKind DK, 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
@@ -174,7 +183,7 @@ class StdLibraryFunctionsChecker
RangeConstraint(ArgNo ArgN, RangeKind Kind, const IntRangeVector &Ranges)
: ValueConstraint(ArgN), Kind(Kind), Ranges(Ranges) {}
- std::string describe(ProgramStateRef State,
+ std::string describe(DescriptionKind DK, ProgramStateRef State,
const Summary &Summary) const override;
const IntRangeVector &getRanges() const { return Ranges; }
@@ -244,7 +253,7 @@ class StdLibraryFunctionsChecker
bool CannotBeNull = true;
public:
- std::string describe(ProgramStateRef State,
+ std::string describe(DescriptionKind DK, ProgramStateRef State,
const Summary &Summary) const override;
StringRef getName() const override { return "NonNull"; }
ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
@@ -316,7 +325,7 @@ class StdLibraryFunctionsChecker
return Result;
}
- std::string describe(ProgramStateRef State,
+ std::string describe(DescriptionKind DK, ProgramStateRef State,
const Summary &Summary) const override;
ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
@@ -704,9 +713,12 @@ 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()));
+ // Describe the argument constraint violation in a note.
+ std::string Descr = VC->describe(
+ ValueConstraint::DescriptionKind::Violation, C.getState(), Summary);
+ // Capitalize the first letter b/c we want a full sentence.
+ Descr[0] = toupper(Descr[0]);
+ R->addNote(Descr, R->getLocation(), Call.getArgSourceRange(VC->getArgNo()));
C.emitReport(std::move(R));
}
@@ -735,24 +747,26 @@ static BasicValueFactory &getBVF(ProgramStateRef State) {
}
std::string StdLibraryFunctionsChecker::NotNullConstraint::describe(
- ProgramStateRef State, const Summary &Summary) const {
+ DescriptionKind DK, ProgramStateRef State, const Summary &Summary) const {
SmallString<48> Result;
- Result += "The ";
+ const auto Violation = ValueConstraint::DescriptionKind::Violation;
+ Result += "the ";
Result += getArgDesc(ArgN);
- Result += " should not be NULL";
+ Result += DK == Violation ? " should not be NULL" : " is not NULL";
return Result.c_str();
}
std::string StdLibraryFunctionsChecker::RangeConstraint::describe(
- ProgramStateRef State, const Summary &Summary) const {
+ DescriptionKind DK, ProgramStateRef State, const Summary &Summary) const {
BasicValueFactory &BVF = getBVF(State);
QualType T = Summary.getArgType(getArgNo());
SmallString<48> Result;
- Result += "The ";
+ const auto Violation = ValueConstraint::DescriptionKind::Violation;
+ Result += "the ";
Result += getArgDesc(ArgN);
- Result += " should be ";
+ Result += DK == Violation ? " should be " : " is ";
// Range kind as a string.
Kind == OutOfRange ? Result += "out of" : Result += "within";
@@ -784,16 +798,18 @@ StdLibraryFunctionsChecker::getArgDesc(StdLibraryFunctionsChecker::ArgNo ArgN) {
SmallString<8> Result;
Result += std::to_string(ArgN + 1);
Result += llvm::getOrdinalSuffix(ArgN + 1);
- Result += " arg";
+ Result += " argument";
return Result;
}
std::string StdLibraryFunctionsChecker::BufferSizeConstraint::describe(
- ProgramStateRef State, const Summary &Summary) const {
+ DescriptionKind DK, ProgramStateRef State, const Summary &Summary) const {
SmallString<96> Result;
- Result += "The size of the ";
+ const auto Violation = ValueConstraint::DescriptionKind::Violation;
+ Result += "the size of the ";
Result += getArgDesc(ArgN);
- Result += " should be equal to or less than the value of ";
+ Result += DK == Violation ? " should be " : " is ";
+ Result += "equal to or greater than the value of ";
if (ConcreteSize) {
ConcreteSize->toString(Result);
} else if (SizeArgN) {
@@ -927,6 +943,7 @@ void StdLibraryFunctionsChecker::checkPreCall(const CallEvent &Call,
ProgramStateRef State = C.getState();
ProgramStateRef NewState = State;
+ ExplodedNode *NewNode = C.getPredecessor();
for (const ValueConstraintPtr &Constraint : Summary.getArgConstraints()) {
ProgramStateRef SuccessSt = Constraint->apply(NewState, Call, Summary, C);
ProgramStateRef FailureSt =
@@ -936,17 +953,28 @@ void StdLibraryFunctionsChecker::checkPreCall(const CallEvent &Call,
if (ExplodedNode *N = C.generateErrorNode(NewState))
reportBug(Call, N, Constraint.get(), Summary, C);
break;
- } else {
- // We will 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);
- NewState = SuccessSt;
+ }
+ // We will 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);
+ NewState = SuccessSt;
+ if (NewState != State) {
+ SmallString<64> Msg;
+ Msg += "Assuming ";
+ Msg += Constraint->describe(ValueConstraint::DescriptionKind::Assumption,
+ NewState, Summary);
+ const auto ArgSVal = Call.getArgSVal(Constraint->getArgNo());
+ NewNode = C.addTransition(
+ NewState, NewNode,
+ C.getNoteTag([Msg = std::move(Msg), ArgSVal](
+ PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
+ if (BR.isInteresting(ArgSVal))
+ OS << Msg;
+ }));
}
}
- if (NewState && NewState != State)
- C.addTransition(NewState);
}
void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
@@ -2882,6 +2910,10 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
Summary(EvalCallAsPure).ArgConstraint(NotNull(ArgNo(0))));
// Test range values.
+ addToFunctionSummaryMap(
+ "__single_val_0", Signature(ArgTypes{IntTy}, RetType{IntTy}),
+ Summary(EvalCallAsPure)
+ .ArgConstraint(ArgumentCondition(0U, WithinRange, SingleValue(0))));
addToFunctionSummaryMap(
"__single_val_1", Signature(ArgTypes{IntTy}, RetType{IntTy}),
Summary(EvalCallAsPure)
diff --git a/clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp b/clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
new file mode 100644
index 0000000000000..1f36b2b03fac9
--- /dev/null
+++ b/clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
@@ -0,0 +1,51 @@
+// 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: -analyzer-output=text \
+// RUN: -verify
+
+template <typename T>
+void clang_analyzer_express(T x);
+void clang_analyzer_eval(bool);
+int clang_analyzer_getExtent(void *);
+
+
+// Check NotNullConstraint assumption notes.
+int __not_null(int *);
+int test_not_null_note(int *x, int y) {
+ __not_null(x); // expected-note{{Assuming the 1st argument is not NULL}}
+ if (x) // expected-note{{'x' is non-null}} \
+ // expected-note{{Taking true branch}}
+ if (!y) // expected-note{{Assuming 'y' is 0}} \
+ // expected-note{{Taking true branch}}
+ return 1 / y; // expected-warning{{Division by zero}} \
+ // expected-note{{Division by zero}}
+
+ return 0;
+}
+
+// Check the RangeConstraint assumption notes.
+int __single_val_0(int); // [0, 0]
+int test_range_constraint_note(int x, int y) {
+ __single_val_0(x); // expected-note{{Assuming the 1st argument is within the range [0, 0]}}
+ return y / x; // expected-warning{{Division by zero}} \
+ // expected-note{{Division by zero}}
+}
+
+// Check the BufferSizeConstraint assumption notes.
+int __buf_size_arg_constraint_concrete(const void *buf); // size of buf must be >= 10
+void test_buffer_size_note(char *buf, int y) {
+ __buf_size_arg_constraint_concrete(buf); // expected-note {{Assuming the size of the 1st argument is equal to or greater than the value of 10}}
+ clang_analyzer_eval(clang_analyzer_getExtent(buf) >= 10); // expected-warning{{TRUE}} \
+ // expected-note{{TRUE}}
+
+ // clang_analyzer_express marks the argument as interesting.
+ clang_analyzer_express(buf); // expected-warning {{}} // the message does not really matter \
+ // expected-note {{}}
+}
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
index baa7ad5dc0fe7..7481a015e5580 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp
+++ b/clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp
@@ -15,7 +15,7 @@
int __not_null(int *);
void test_not_null(int *x) {
__not_null(nullptr); // \
- // expected-note{{The 1st arg should not be NULL}} \
+ // expected-note{{The 1st argument should not be NULL}} \
// expected-warning{{}}
}
@@ -29,21 +29,21 @@ void test_buffer_size(int 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-note{{The size of the 1st argument should be equal to or greater 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-note{{The size of the 1st argument should be equal to or greater 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-note{{The size of the 1st argument should be equal to or greater than the value of the 2nd argument times the 3rd argument}} \
// expected-warning{{}}
break;
}
@@ -56,7 +56,7 @@ 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-note{{The 1st argument should be within the range [1, 1]}} \
// expected-warning{{}}
}
// Do more specific check against the range strings.
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 32511c619addc..b5f1df8877943 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -239,6 +239,7 @@ void test_constraints_on_multiple_args(int x, int y) {
// State split should not happen here. I.e. x == 1 should not be evaluated
// FALSE.
__two_constrained_args(x, y);
+ //NOTE! Because of the second `clang_analyzer_eval` call we have two bug
clang_analyzer_eval(x == 1); // \
// report-warning{{TRUE}} \
// bugpath-warning{{TRUE}} \
@@ -252,7 +253,6 @@ void test_constraints_on_multiple_args(int x, int y) {
int __arg_constrained_twice(int);
void test_multiple_constraints_on_same_arg(int x) {
__arg_constrained_twice(x);
- // Check that both constraints are applied and only one branch is there.
clang_analyzer_eval(x < 1 || x > 2); // \
// report-warning{{TRUE}} \
// bugpath-warning{{TRUE}} \
More information about the cfe-commits
mailing list