baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, Szelethus.
baloghadamsoftware added a project: clang.
Herald added subscribers: Charusso, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, rnkovacs, szepet, xazax.hun.
Container operations such as `push_back()`, `pop_front()` etc. increment and
decrement the abstract begin and end symbols of containers. This patch
introduces note tags to `ContainerModeling` to track these changes. This helps
the user to better identify the source of errors related to containers and
iterators.
Repository:
rC Clang
https://reviews.llvm.org/D73720
Files:
clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
clang/test/Analysis/container-modeling.cpp
Index: clang/test/Analysis/container-modeling.cpp
===================================================================
--- clang/test/Analysis/container-modeling.cpp
+++ clang/test/Analysis/container-modeling.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,debug.DebugIteratorModeling,debug.ExprInspection -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,debug.DebugIteratorModeling,debug.ExprInspection -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false %s -analyzer-output=text -verify
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,debug.DebugIteratorModeling,debug.ExprInspection -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,debug.DebugIteratorModeling,debug.ExprInspection -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -analyzer-output=text -verify
// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorModeling,debug.ExprInspection -analyzer-config aggressive-binary-operation-simplification=true %s 2>&1 | FileCheck %s
@@ -20,14 +20,16 @@
V.begin();
clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
- clang_analyzer_express(clang_analyzer_container_begin(V)); //expected-warning{{$V.begin()}}
+ clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}}
+ // expected-note@-1{{$V.begin()}}
}
void end(const std::vector<int> &V) {
V.end();
clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
- clang_analyzer_express(clang_analyzer_container_end(V)); //expected-warning{{$V.end()}}
+ clang_analyzer_express(clang_analyzer_container_end(V)); // expected-warning{{$V.end()}}
+ // expected-note@-1{{$V.end()}}
}
////////////////////////////////////////////////////////////////////////////////
@@ -48,8 +50,10 @@
long B2 = clang_analyzer_container_begin(V2);
long E2 = clang_analyzer_container_end(V2);
V1 = std::move(V2);
- clang_analyzer_eval(clang_analyzer_container_begin(V1) == B2); //expected-warning{{TRUE}}
- clang_analyzer_eval(clang_analyzer_container_end(V2) == E2); //expected-warning{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_container_begin(V1) == B2); // expected-warning{{TRUE}}
+ // expected-note@-1{{TRUE}}
+ clang_analyzer_eval(clang_analyzer_container_end(V2) == E2); // expected-warning{{TRUE}}
+ // expected-note@-1{{TRUE}}
}
////////////////////////////////////////////////////////////////////////////////
@@ -70,10 +74,13 @@
clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
- V.push_back(n);
+ V.push_back(n); // expected-note{{Container 'V' extended to the right by 1 position}}
+ // expected-note@-1{{Container 'V' extended to the right by 1 position}}
clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}}
+ // expected-note@-1{{$V.begin()}}
clang_analyzer_express(clang_analyzer_container_end(V)); // expected-warning{{$V.end() + 1}}
+ // expected-note@-1{{$V.end() + 1}}
}
/// emplace_back()
@@ -88,10 +95,14 @@
clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
- V.emplace_back(n);
+ V.emplace_back(n); // expected-note{{Container 'V' extended to the right by 1 position}}
+ // expected-note@-1{{Container 'V' extended to the right by 1 position}}
+
clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}}
+ // expected-note@-1{{$V.begin()}}
clang_analyzer_express(clang_analyzer_container_end(V)); // expected-warning{{$V.end() + 1}}
+ // expected-note@-1{{$V.end() + 1}}
}
/// pop_back()
@@ -106,10 +117,14 @@
clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
- V.pop_back();
+ V.pop_back(); // expected-note{{Container 'V' shrinked from the right by 1 position}}
+ // expected-note@-1{{Container 'V' shrinked from the right by 1 position}}
+
clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}}
+ // expected-note@-1{{$V.begin()}}
clang_analyzer_express(clang_analyzer_container_end(V)); // expected-warning{{$V.end() - 1}}
+ // expected-note@-1{{$V.end() - 1}}
}
/// push_front()
@@ -117,17 +132,20 @@
/// Design decision: extends containers to the <-LEFT<- (i.e. the first
/// position of the container is decremented).
-void push_front(std::deque<int> &D, int n) {
- D.cbegin();
- D.cend();
+void push_front(std::list<int> &L, int n) {
+ L.cbegin();
+ L.cend();
- clang_analyzer_denote(clang_analyzer_container_begin(D), "$D.begin()");
- clang_analyzer_denote(clang_analyzer_container_end(D), "$D.end()");
+ clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
- D.push_front(n);
+ L.push_front(n); // expected-note{{Container 'L' extended to the left by 1 position}}
+ // expected-note@-1{{Container 'L' extended to the left by 1 position}}
- clang_analyzer_express(clang_analyzer_container_begin(D)); // expected-warning{{$D.begin()}} FIXME: Should be $D.begin() - 1 (to correctly track the container's size)
- clang_analyzer_express(clang_analyzer_container_end(D)); // expected-warning{{$D.end()}}
+ clang_analyzer_express(clang_analyzer_container_begin(L)); // expected-warning{{$L.begin() - 1}}
+ // expected-note@-1{{$L.begin() - 1}}
+ clang_analyzer_express(clang_analyzer_container_end(L)); // expected-warning{{$L.end()}}
+ // expected-note@-1{{$L.end()}}
}
/// emplace_front()
@@ -135,17 +153,20 @@
/// Design decision: extends containers to the <-LEFT<- (i.e. the first
/// position of the container is decremented).
-void deque_emplace_front(std::deque<int> &D, int n) {
- D.cbegin();
- D.cend();
+void emplace_front(std::list<int> &L, int n) {
+ L.cbegin();
+ L.cend();
- clang_analyzer_denote(clang_analyzer_container_begin(D), "$D.begin()");
- clang_analyzer_denote(clang_analyzer_container_end(D), "$D.end()");
+ clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
- D.emplace_front(n);
+ L.emplace_front(n); // expected-note{{Container 'L' extended to the left by 1 position}}
+ // expected-note@-1{{Container 'L' extended to the left by 1 position}}
- clang_analyzer_express(clang_analyzer_container_begin(D)); // expected-warning{{$D.begin()}} FIXME: Should be $D.begin - 1 (to correctly track the container's size)
- clang_analyzer_express(clang_analyzer_container_end(D)); // expected-warning{{$D.end()}}
+ clang_analyzer_express(clang_analyzer_container_begin(L)); // expected-warning{{$L.begin() - 1}}
+ // expected-note@-1{{$L.begin() - 1}}
+ clang_analyzer_express(clang_analyzer_container_end(L)); // expected-warning{{$L.end()}}
+ // expected-note@-1{{$L.end()}}
}
/// pop_front()
@@ -153,19 +174,47 @@
/// Design decision: shrinks containers to the ->RIGHT-> (i.e. the first
/// position of the container is incremented).
-void deque_pop_front(std::deque<int> &D, int n) {
- D.cbegin();
- D.cend();
+void pop_front(std::list<int> &L, int n) {
+ L.cbegin();
+ L.cend();
- clang_analyzer_denote(clang_analyzer_container_begin(D), "$D.begin()");
- clang_analyzer_denote(clang_analyzer_container_end(D), "$D.end()");
+ clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
- D.pop_front();
+ L.pop_front(); // expected-note{{Container 'L' shrinked from the left by 1 position}}
+ // expected-note@-1{{Container 'L' shrinked from the left by 1 position}}
- clang_analyzer_express(clang_analyzer_container_begin(D)); // expected-warning{{$D.begin() + 1}}
- clang_analyzer_express(clang_analyzer_container_end(D)); // expected-warning{{$D.end()}}
+ clang_analyzer_express(clang_analyzer_container_begin(L)); // expected-warning{{$L.begin() + 1}}
+ // expected-note@-1{{$L.begin() + 1}}
+ clang_analyzer_express(clang_analyzer_container_end(L)); // expected-warning{{$L.end()}}
+ // expected-note@-1{{$L.end()}}
}
+////////////////////////////////////////////////////////////////////////////////
+///
+/// O T H E R T E S T S
+///
+////////////////////////////////////////////////////////////////////////////////
+
+/// Track the right container
+
+void push_back(std::vector<int> &V1, std::vector<int> &V2, int n) {
+ V1.cbegin();
+ V1.cend();
+
+ clang_analyzer_denote(clang_analyzer_container_begin(V1), "$V1.begin()");
+ clang_analyzer_denote(clang_analyzer_container_end(V1), "$V1.end()");
+
+ V2.push_back(n); // no note expected
+
+ clang_analyzer_express(clang_analyzer_container_begin(V1)); // expected-warning{{$V1.begin()}}
+ // expected-note@-1{{$V1.begin()}}
+ clang_analyzer_express(clang_analyzer_container_end(V1)); // expected-warning{{$V1.end()}}
+ // expected-note@-1{{$V1.end()}}
+}
+
+/// Print Container Data as Part of the Program State
+
void clang_analyzer_printState();
void print_state(std::vector<int> &V) {
Index: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
@@ -38,12 +38,18 @@
void handleAssignment(CheckerContext &C, const SVal &Cont,
const Expr *CE = nullptr,
const SVal &OldCont = UndefinedVal()) const;
- void handleAssign(CheckerContext &C, const SVal &Cont) const;
- void handleClear(CheckerContext &C, const SVal &Cont) const;
- void handlePushBack(CheckerContext &C, const SVal &Cont) const;
- void handlePopBack(CheckerContext &C, const SVal &Cont) const;
- void handlePushFront(CheckerContext &C, const SVal &Cont) const;
- void handlePopFront(CheckerContext &C, const SVal &Cont) const;
+ void handleAssign(CheckerContext &C, const SVal &Cont,
+ const Expr *ContE) const;
+ void handleClear(CheckerContext &C, const SVal &Cont,
+ const Expr *ContE) const;
+ void handlePushBack(CheckerContext &C, const SVal &Cont,
+ const Expr *ContE) const;
+ void handlePopBack(CheckerContext &C, const SVal &Cont,
+ const Expr *ContE) const;
+ void handlePushFront(CheckerContext &C, const SVal &Cont,
+ const Expr *ContE) const;
+ void handlePopFront(CheckerContext &C, const SVal &Cont,
+ const Expr *ContE) const;
void handleInsert(CheckerContext &C, const SVal &Cont,
const SVal &Iter) const;
void handleErase(CheckerContext &C, const SVal &Cont, const SVal &Iter) const;
@@ -53,6 +59,8 @@
const SVal &Iter) const;
void handleEraseAfter(CheckerContext &C, const SVal &Cont, const SVal &Iter1,
const SVal &Iter2) const;
+ const NoteTag *getChangeTag(CheckerContext &C, StringRef Text,
+ const Expr *ContE) const;
void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
const char *Sep) const override;
@@ -64,7 +72,8 @@
void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
typedef void (ContainerModeling::*NoItParamFn)(CheckerContext &,
- const SVal &) const;
+ const SVal &,
+ const Expr *) const;
typedef void (ContainerModeling::*OneItParamFn)(CheckerContext &,
const SVal &,
const SVal &) const;
@@ -184,7 +193,8 @@
if (const auto *InstCall = dyn_cast<CXXInstanceCall>(&Call)) {
const NoItParamFn *Handler0 = NoIterParamFunctions.lookup(Call);
if (Handler0) {
- (this->**Handler0)(C, InstCall->getCXXThisVal());
+ (this->**Handler0)(C, InstCall->getCXXThisVal(),
+ InstCall->getCXXThisExpr());
return;
}
@@ -379,8 +389,8 @@
C.addTransition(State);
}
-void ContainerModeling::handleAssign(CheckerContext &C,
- const SVal &Cont) const {
+void ContainerModeling::handleAssign(CheckerContext &C, const SVal &Cont,
+ const Expr *ContE) const {
const auto *ContReg = Cont.getAsRegion();
if (!ContReg)
return;
@@ -393,7 +403,8 @@
C.addTransition(State);
}
-void ContainerModeling::handleClear(CheckerContext &C, const SVal &Cont) const {
+void ContainerModeling::handleClear(CheckerContext &C, const SVal &Cont,
+ const Expr *ContE) const {
const auto *ContReg = Cont.getAsRegion();
if (!ContReg)
return;
@@ -415,12 +426,14 @@
}
}
}
+ const NoteTag *ChangeTag =
+ getChangeTag(C, "became empty", ContE);
State = invalidateAllIteratorPositions(State, ContReg);
- C.addTransition(State);
+ C.addTransition(State, ChangeTag);
}
-void ContainerModeling::handlePushBack(CheckerContext &C,
- const SVal &Cont) const {
+void ContainerModeling::handlePushBack(CheckerContext &C, const SVal &Cont,
+ const Expr *ContE) const {
const auto *ContReg = Cont.getAsRegion();
if (!ContReg)
return;
@@ -452,13 +465,15 @@
nonloc::SymbolVal(EndSym),
nonloc::ConcreteInt(BVF.getValue(llvm::APSInt::get(1))),
SymMgr.getType(EndSym)).getAsSymbol();
+ const NoteTag *ChangeTag =
+ getChangeTag(C, "extended to the right by 1 position", ContE);
State = setContainerData(State, ContReg, CData->newEnd(newEndSym));
+ C.addTransition(State, ChangeTag);
}
- C.addTransition(State);
}
-void ContainerModeling::handlePopBack(CheckerContext &C,
- const SVal &Cont) const {
+void ContainerModeling::handlePopBack(CheckerContext &C, const SVal &Cont,
+ const Expr *ContE) const {
const auto *ContReg = Cont.getAsRegion();
if (!ContReg)
return;
@@ -479,6 +494,8 @@
nonloc::SymbolVal(EndSym),
nonloc::ConcreteInt(BVF.getValue(llvm::APSInt::get(1))),
SymMgr.getType(EndSym)).getAsSymbol();
+ const NoteTag *ChangeTag =
+ getChangeTag(C, "shrinked from the right by 1 position", ContE);
// For vector-like and deque-like containers invalidate the last and the
// past-end iterator positions. For list-like containers only invalidate
// the last position
@@ -491,12 +508,12 @@
}
auto newEndSym = BackSym;
State = setContainerData(State, ContReg, CData->newEnd(newEndSym));
- C.addTransition(State);
+ C.addTransition(State, ChangeTag);
}
}
-void ContainerModeling::handlePushFront(CheckerContext &C,
- const SVal &Cont) const {
+void ContainerModeling::handlePushFront(CheckerContext &C, const SVal &Cont,
+ const Expr *ContE) const {
const auto *ContReg = Cont.getAsRegion();
if (!ContReg)
return;
@@ -522,14 +539,16 @@
nonloc::SymbolVal(BeginSym),
nonloc::ConcreteInt(BVF.getValue(llvm::APSInt::get(1))),
SymMgr.getType(BeginSym)).getAsSymbol();
+ const NoteTag *ChangeTag =
+ getChangeTag(C, "extended to the left by 1 position", ContE);
State = setContainerData(State, ContReg, CData->newBegin(newBeginSym));
- C.addTransition(State);
+ C.addTransition(State, ChangeTag);
}
}
}
-void ContainerModeling::handlePopFront(CheckerContext &C,
- const SVal &Cont) const {
+void ContainerModeling::handlePopFront(CheckerContext &C, const SVal &Cont,
+ const Expr *ContE) const {
const auto *ContReg = Cont.getAsRegion();
if (!ContReg)
return;
@@ -557,8 +576,10 @@
nonloc::SymbolVal(BeginSym),
nonloc::ConcreteInt(BVF.getValue(llvm::APSInt::get(1))),
SymMgr.getType(BeginSym)).getAsSymbol();
+ const NoteTag *ChangeTag =
+ getChangeTag(C, "shrinked from the left by 1 position", ContE);
State = setContainerData(State, ContReg, CData->newBegin(newBeginSym));
- C.addTransition(State);
+ C.addTransition(State, ChangeTag);
}
}
@@ -700,6 +721,22 @@
C.addTransition(State);
}
+const NoteTag *ContainerModeling::getChangeTag(CheckerContext &C,
+ StringRef Text,
+ const Expr *ContE) const {
+ StringRef Name;
+ if (const auto *DRE = dyn_cast<DeclRefExpr>(ContE)) {
+ Name = DRE->getDecl()->getName();
+ }
+ return C.getNoteTag([Text, Name](BugReport &BR) -> std::string {
+ SmallString<256> Msg;
+ llvm::raw_svector_ostream Out(Msg);
+ Out << "Container " << (!Name.empty() ? ("'" + Name.str() + "' ") : "" )
+ << Text;
+ return std::string(Out.str());
+ });
+}
+
void ContainerModeling::printState(raw_ostream &Out, ProgramStateRef State,
const char *NL, const char *Sep) const {
auto ContMap = State->get<ContainerMap>();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
