martong updated this revision to Diff 342342.
martong marked an inline comment as done.
martong added a comment.
- Add the description to the note tag only if the SVal is interesting
- Use 'Assuming the nth arg is in ...' form for the descriptions
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101526/new/
https://reviews.llvm.org/D101526
Files:
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
clang/test/Analysis/std-c-library-functions-arg-constraints.c
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
@@ -238,6 +238,7 @@
// 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}} \
@@ -251,7 +252,6 @@
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}} \
Index: clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
@@ -0,0 +1,19 @@
+// 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
+
+int __single_val_0(int); // [0, 0]
+
+int test_note(int x, int y) {
+ __single_val_0(x); // expected-note{{Assuming the 1st arg is within the range [0, 0]}}
+ return y / x; // expected-warning{{Division by zero}} \
+ // expected-note{{Division by zero}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -142,9 +142,18 @@
virtual StringRef getName() const = 0;
+ // Represents that in which context do we require a description of the
+ // constraint.
+ enum class DescritptionKind {
+ // 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(DescritptionKind 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
@@ -182,7 +191,7 @@
RangeConstraint(ArgNo ArgN, RangeKind Kind, const IntRangeVector &Ranges)
: ValueConstraint(ArgN), Kind(Kind), Ranges(Ranges) {}
- std::string describe(ProgramStateRef State,
+ std::string describe(DescritptionKind DK, ProgramStateRef State,
const Summary &Summary) const override;
const IntRangeVector &getRanges() const { return Ranges; }
@@ -252,7 +261,7 @@
bool CannotBeNull = true;
public:
- std::string describe(ProgramStateRef State,
+ std::string describe(DescritptionKind DK, ProgramStateRef State,
const Summary &Summary) const override;
StringRef getName() const override { return "NonNull"; }
ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
@@ -324,7 +333,7 @@
return Result;
}
- std::string describe(ProgramStateRef State,
+ std::string describe(DescritptionKind DK, ProgramStateRef State,
const Summary &Summary) const override;
ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
@@ -598,9 +607,12 @@
// 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::DescritptionKind::Violation, C.getState(), Summary);
+ // Capitalize the firs 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));
}
@@ -618,24 +630,26 @@
}
std::string StdLibraryFunctionsChecker::NotNullConstraint::describe(
- ProgramStateRef State, const Summary &Summary) const {
+ DescritptionKind DK, ProgramStateRef State, const Summary &Summary) const {
SmallString<48> Result;
- Result += "The ";
+ const auto Violation = ValueConstraint::DescritptionKind::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 {
+ DescritptionKind 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::DescritptionKind::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";
@@ -672,11 +686,13 @@
}
std::string StdLibraryFunctionsChecker::BufferSizeConstraint::describe(
- ProgramStateRef State, const Summary &Summary) const {
+ DescritptionKind DK, ProgramStateRef State, const Summary &Summary) const {
SmallString<96> Result;
- Result += "The size of the ";
+ const auto Violation = ValueConstraint::DescritptionKind::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 less than the value of ";
if (ConcreteSize) {
ConcreteSize->toString(Result);
} else if (SizeArgN) {
@@ -810,6 +826,7 @@
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 =
@@ -819,17 +836,28 @@
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::DescritptionKind::Assumption,
+ NewState, Summary);
+ const auto ArgSVal = Call.getArgSVal(Constraint->getArgNo());
+ NewNode = C.addTransition(
+ NewState, NewNode,
+ C.getNoteTag([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,
@@ -2570,6 +2598,10 @@
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)
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
