[clang] 1a27d63 - [Analyzer] Only add container note tags to the operations of the affected container
Adam Balogh via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 26 01:45:01 PDT 2020
Author: Adam Balogh
Date: 2020-03-26T09:44:16+01:00
New Revision: 1a27d63a8891076ad9176f1a70f372003bc55c2f
URL: https://github.com/llvm/llvm-project/commit/1a27d63a8891076ad9176f1a70f372003bc55c2f
DIFF: https://github.com/llvm/llvm-project/commit/1a27d63a8891076ad9176f1a70f372003bc55c2f.diff
LOG: [Analyzer] Only add container note tags to the operations of the affected container
If an error happens which is related to a container the Container
Modeling checker adds note tags to all the container operations along
the bug path. This may be disturbing if there are other containers
beside the one which is affected by the bug. This patch restricts the
note tags to only the affected container and adjust the debug checkers
to be able to test this change.
Differential Revision: https://reviews.llvm.org/D75514
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
clang/test/Analysis/container-modeling.cpp
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
index b225a61db439..8126fe8260c8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
@@ -721,6 +721,9 @@ const NoteTag *ContainerModeling::getChangeTag(CheckerContext &C,
return C.getNoteTag(
[Text, Name, ContReg](PathSensitiveBugReport &BR) -> std::string {
+ if (!BR.isInteresting(ContReg))
+ return "";
+
SmallString<256> Msg;
llvm::raw_svector_ostream Out(Msg);
Out << "Container " << (!Name.empty() ? ("'" + Name.str() + "' ") : "" )
diff --git a/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
index 8d0572723991..ce8dccb22333 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
@@ -92,7 +92,19 @@ void DebugContainerModeling::analyzerContainerDataField(const CallExpr *CE,
if (Field) {
State = State->BindExpr(CE, C.getLocationContext(),
nonloc::SymbolVal(Field));
- C.addTransition(State);
+
+ // Progpagate interestingness from the container's data (marked
+ // interesting by an `ExprInspection` debug call to the container
+ // itself.
+ const NoteTag *InterestingTag =
+ C.getNoteTag(
+ [Cont, Field](PathSensitiveBugReport &BR) -> std::string {
+ if (BR.isInteresting(Field)) {
+ BR.markInteresting(Cont);
+ }
+ return "";
+ });
+ C.addTransition(State, InterestingTag);
return;
}
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
index 10b27831d89f..97e287e7a221 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -52,9 +52,12 @@ class ExprInspectionChecker : public Checker<eval::Call, check::DeadSymbols,
typedef void (ExprInspectionChecker::*FnCheck)(const CallExpr *,
CheckerContext &C) const;
- ExplodedNode *reportBug(llvm::StringRef Msg, CheckerContext &C) const;
+ // Optional parameter `ExprVal` for expression value to be marked interesting.
+ ExplodedNode *reportBug(llvm::StringRef Msg, CheckerContext &C,
+ Optional<SVal> ExprVal = None) const;
ExplodedNode *reportBug(llvm::StringRef Msg, BugReporter &BR,
- ExplodedNode *N) const;
+ ExplodedNode *N,
+ Optional<SVal> ExprVal = None) const;
public:
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
@@ -144,22 +147,28 @@ static const char *getArgumentValueString(const CallExpr *CE,
}
ExplodedNode *ExprInspectionChecker::reportBug(llvm::StringRef Msg,
- CheckerContext &C) const {
+ CheckerContext &C,
+ Optional<SVal> ExprVal) const {
ExplodedNode *N = C.generateNonFatalErrorNode();
- reportBug(Msg, C.getBugReporter(), N);
+ reportBug(Msg, C.getBugReporter(), N, ExprVal);
return N;
}
ExplodedNode *ExprInspectionChecker::reportBug(llvm::StringRef Msg,
BugReporter &BR,
- ExplodedNode *N) const {
+ ExplodedNode *N,
+ Optional<SVal> ExprVal) const {
if (!N)
return nullptr;
if (!BT)
BT.reset(new BugType(this, "Checking analyzer assumptions", "debug"));
- BR.emitReport(std::make_unique<PathSensitiveBugReport>(*BT, Msg, N));
+ auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N);
+ if (ExprVal) {
+ R->markInteresting(*ExprVal);
+ }
+ BR.emitReport(std::move(R));
return N;
}
@@ -406,7 +415,8 @@ void ExprInspectionChecker::analyzerExpress(const CallExpr *CE,
return;
}
- SymbolRef Sym = C.getSVal(CE->getArg(0)).getAsSymbol();
+ SVal ArgVal = C.getSVal(CE->getArg(0));
+ SymbolRef Sym = ArgVal.getAsSymbol();
if (!Sym) {
reportBug("Not a symbol", C);
return;
@@ -419,7 +429,7 @@ void ExprInspectionChecker::analyzerExpress(const CallExpr *CE,
return;
}
- reportBug(*Str, C);
+ reportBug(*Str, C, ArgVal);
}
void ExprInspectionChecker::analyzerIsTainted(const CallExpr *CE,
diff --git a/clang/test/Analysis/container-modeling.cpp b/clang/test/Analysis/container-modeling.cpp
index 9d63cc2fce69..bf4a12a0e0fe 100644
--- a/clang/test/Analysis/container-modeling.cpp
+++ b/clang/test/Analysis/container-modeling.cpp
@@ -76,8 +76,7 @@ void push_back(std::vector<int> &V, int n) {
clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
- V.push_back(n); // expected-note{{Container 'V' extended to the back by 1 position}}
- // expected-note at -1{{Container 'V' extended to the back by 1 position}}
+ V.push_back(n); // expected-note 2{{Container 'V' extended to the back by 1 position}}
clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}}
// expected-note at -1{{$V.begin()}}
@@ -99,7 +98,6 @@ void emplace_back(std::vector<int> &V, int n) {
V.emplace_back(n); // expected-note 2{{Container 'V' extended to the back by 1 position}}
-
clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}}
// expected-note at -1{{$V.begin()}}
clang_analyzer_express(clang_analyzer_container_end(V)); // expected-warning{{$V.end() + 1}}
@@ -217,10 +215,10 @@ void push_back1(std::vector<int> &V1, std::vector<int> &V2, int n) {
clang_analyzer_denote(clang_analyzer_container_begin(V1), "$V1.begin()");
- V2.push_back(n); // expected-note{{Container 'V2' extended to the back by 1 position}} FIXME: This note should not appear since `V2` is not affected in the "bug"
+ V2.push_back(n); // no-note
clang_analyzer_express(clang_analyzer_container_begin(V1)); // expected-warning{{$V1.begin()}}
- // expected-note at -1{{$V1.begin()}}
+ // expected-note at -1{{$V1.begin()}}
}
void push_back2(std::vector<int> &V1, std::vector<int> &V2, int n) {
@@ -232,15 +230,14 @@ void push_back2(std::vector<int> &V1, std::vector<int> &V2, int n) {
clang_analyzer_denote(clang_analyzer_container_begin(V1), "$V1.begin()");
clang_analyzer_denote(clang_analyzer_container_begin(V2), "$V2.begin()");
- V1.push_back(n); // expected-note 2{{Container 'V1' extended to the back by 1 position}}
- // FIXME: This should appear only once since there is only
- // one "bug" where `V1` is affected
+ V1.push_back(n); // expected-note{{Container 'V1' extended to the back by 1 position}}
+ // Only once!
clang_analyzer_express(clang_analyzer_container_begin(V1)); // expected-warning{{$V1.begin()}}
- // expected-note at -1{{$V1.begin()}}
+ // expected-note at -1{{$V1.begin()}}
clang_analyzer_express(clang_analyzer_container_begin(V2)); // expected-warning{{$V2.begin()}}
- // expected-note at -1{{$V2.begin()}}
+ // expected-note at -1{{$V2.begin()}}
}
/// Print Container Data as Part of the Program State
More information about the cfe-commits
mailing list