This revision was automatically updated to reflect the committed changes.
Closed by commit rL276365: [analyzer] Add checker modeling potential C++
self-assignment (authored by dcoughlin).
Changed prior to commit:
https://reviews.llvm.org/D19311?vs=64864&id=64996#toc
Repository:
rL LLVM
https://reviews.llvm.org/D19311
Files:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
cfe/trunk/test/Analysis/self-assign.cpp
Index: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -426,6 +426,13 @@
// Count naming convention errors more aggressively.
if (isa<ObjCMethodDecl>(D))
return false;
+ // We also want to reanalyze all C++ copy and move assignment operators to
+ // separately check the two cases where 'this' aliases with the parameter and
+ // where it may not. (cplusplus.SelfAssignmentChecker)
+ if (const auto *MD = dyn_cast<CXXMethodDecl>(D)) {
+ if (MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator())
+ return false;
+ }
// Otherwise, if we visited the function before, do not reanalyze it.
return Visited.count(D);
@@ -437,9 +444,7 @@
// We want to reanalyze all ObjC methods as top level to report Retain
// Count naming convention errors more aggressively. But we should tune down
// inlining when reanalyzing an already inlined function.
- if (Visited.count(D)) {
- assert(isa<ObjCMethodDecl>(D) &&
- "We are only reanalyzing ObjCMethods.");
+ if (Visited.count(D) && isa<ObjCMethodDecl>(D)) {
const ObjCMethodDecl *ObjCM = cast<ObjCMethodDecl>(D);
if (ObjCM->getMethodFamily() != OMF_init)
return ExprEngine::Inline_Minimal;
Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1693,3 +1693,56 @@
}
return nullptr;
}
+
+PathDiagnosticPiece *
+CXXSelfAssignmentBRVisitor::VisitNode(const ExplodedNode *Succ,
+ const ExplodedNode *Pred,
+ BugReporterContext &BRC, BugReport &BR) {
+ if (Satisfied)
+ return nullptr;
+
+ auto Edge = Succ->getLocation().getAs<BlockEdge>();
+ if (!Edge.hasValue())
+ return nullptr;
+
+ auto Tag = Edge->getTag();
+ if (!Tag)
+ return nullptr;
+
+ if (Tag->getTagDescription() != "cplusplus.SelfAssignment")
+ return nullptr;
+
+ Satisfied = true;
+
+ const auto *Met =
+ dyn_cast<CXXMethodDecl>(Succ->getCodeDecl().getAsFunction());
+ assert(Met && "Not a C++ method.");
+ assert((Met->isCopyAssignmentOperator() || Met->isMoveAssignmentOperator()) &&
+ "Not a copy/move assignment operator.");
+
+ const auto *LCtx = Edge->getLocationContext();
+
+ const auto &State = Succ->getState();
+ auto &SVB = State->getStateManager().getSValBuilder();
+
+ const auto Param =
+ State->getSVal(State->getRegion(Met->getParamDecl(0), LCtx));
+ const auto This =
+ State->getSVal(SVB.getCXXThis(Met, LCtx->getCurrentStackFrame()));
+
+ auto L = PathDiagnosticLocation::create(Met, BRC.getSourceManager());
+
+ if (!L.isValid() || !L.asLocation().isValid())
+ return nullptr;
+
+ SmallString<256> Buf;
+ llvm::raw_svector_ostream Out(Buf);
+
+ Out << "Assuming " << Met->getParamDecl(0)->getName() <<
+ ((Param == This) ? " == " : " != ") << "*this";
+
+ auto *Piece = new PathDiagnosticEventPiece(L, Out.str());
+ Piece->addRange(Met->getSourceRange());
+
+ return Piece;
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -3104,6 +3104,7 @@
R->addVisitor(llvm::make_unique<NilReceiverBRVisitor>());
R->addVisitor(llvm::make_unique<ConditionBRVisitor>());
R->addVisitor(llvm::make_unique<LikelyFalsePositiveSuppressionBRVisitor>());
+ R->addVisitor(llvm::make_unique<CXXSelfAssignmentBRVisitor>());
BugReport::VisitorList visitors;
unsigned origReportConfigToken, finalReportConfigToken;
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -22,6 +22,7 @@
CheckerDocumentation.cpp
ChrootChecker.cpp
ClangCheckers.cpp
+ CXXSelfAssignmentChecker.cpp
DeadStoresChecker.cpp
DebugCheckers.cpp
DereferenceChecker.cpp
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
@@ -0,0 +1,62 @@
+//=== CXXSelfAssignmentChecker.cpp -----------------------------*- C++ -*--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines CXXSelfAssignmentChecker, which tests all custom defined
+// copy and move assignment operators for the case of self assignment, thus
+// where the parameter refers to the same location where the this pointer
+// points to. The checker itself does not do any checks at all, but it
+// causes the analyzer to check every copy and move assignment operator twice:
+// once for when 'this' aliases with the parameter and once for when it may not.
+// It is the task of the other enabled checkers to find the bugs in these two
+// different cases.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class CXXSelfAssignmentChecker : public Checker<check::BeginFunction> {
+public:
+ CXXSelfAssignmentChecker();
+ void checkBeginFunction(CheckerContext &C) const;
+};
+}
+
+CXXSelfAssignmentChecker::CXXSelfAssignmentChecker() {}
+
+void CXXSelfAssignmentChecker::checkBeginFunction(CheckerContext &C) const {
+ if (!C.inTopFrame())
+ return;
+ const auto *LCtx = C.getLocationContext();
+ const auto *MD = dyn_cast<CXXMethodDecl>(LCtx->getDecl());
+ if (!MD)
+ return;
+ if (!MD->isCopyAssignmentOperator() && !MD->isMoveAssignmentOperator())
+ return;
+ auto &State = C.getState();
+ auto &SVB = C.getSValBuilder();
+ auto ThisVal =
+ State->getSVal(SVB.getCXXThis(MD, LCtx->getCurrentStackFrame()));
+ auto Param = SVB.makeLoc(State->getRegion(MD->getParamDecl(0), LCtx));
+ auto ParamVal = State->getSVal(Param);
+ ProgramStateRef SelfAssignState = State->bindLoc(Param, ThisVal);
+ C.addTransition(SelfAssignState);
+ ProgramStateRef NonSelfAssignState = State->bindLoc(Param, ParamVal);
+ C.addTransition(NonSelfAssignState);
+}
+
+void ento::registerCXXSelfAssignmentChecker(CheckerManager &Mgr) {
+ Mgr.registerChecker<CXXSelfAssignmentChecker>();
+}
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
===================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
@@ -331,6 +331,22 @@
BugReport &BR) override;
};
+class CXXSelfAssignmentBRVisitor final
+ : public BugReporterVisitorImpl<CXXSelfAssignmentBRVisitor> {
+
+ bool Satisfied;
+
+public:
+ CXXSelfAssignmentBRVisitor() : Satisfied(false) {}
+
+ void Profile(llvm::FoldingSetNodeID &ID) const override {}
+
+ PathDiagnosticPiece *VisitNode(const ExplodedNode *Succ,
+ const ExplodedNode *Pred,
+ BugReporterContext &BRC,
+ BugReport &BR) override;
+};
+
namespace bugreporter {
/// Attempts to add visitors to trace a null or undefined value back to its
Index: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -247,6 +247,10 @@
HelpText<"Check for memory leaks. Traces memory managed by new/delete.">,
DescFile<"MallocChecker.cpp">;
+def CXXSelfAssignmentChecker : Checker<"SelfAssignment">,
+ HelpText<"Checks C++ copy and move assignment operators for self assignment">,
+ DescFile<"CXXSelfAssignmentChecker.cpp">;
+
} // end: "cplusplus"
let ParentPackage = CplusplusAlpha in {
Index: cfe/trunk/test/Analysis/self-assign.cpp
===================================================================
--- cfe/trunk/test/Analysis/self-assign.cpp
+++ cfe/trunk/test/Analysis/self-assign.cpp
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -std=c++11 -analyze -analyzer-checker=core,cplusplus,unix.Malloc,debug.ExprInspection %s -verify -analyzer-output=text
+
+extern "C" char *strdup(const char* s);
+extern "C" void free(void* ptr);
+
+namespace std {
+template<class T> struct remove_reference { typedef T type; };
+template<class T> struct remove_reference<T&> { typedef T type; };
+template<class T> struct remove_reference<T&&> { typedef T type; };
+template<class T> typename remove_reference<T>::type&& move(T&& t);
+}
+
+void clang_analyzer_eval(int);
+
+class StringUsed {
+public:
+ StringUsed(const char *s = "") : str(strdup(s)) {}
+ StringUsed(const StringUsed &rhs) : str(strdup(rhs.str)) {}
+ ~StringUsed();
+ StringUsed& operator=(const StringUsed &rhs);
+ StringUsed& operator=(StringUsed &&rhs);
+ operator const char*() const;
+private:
+ char *str;
+};
+
+StringUsed::~StringUsed() {
+ free(str);
+}
+
+StringUsed& StringUsed::operator=(const StringUsed &rhs) { // expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}}
+ clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
+ free(str); // expected-note{{Memory is released}}
+ str = strdup(rhs.str); // expected-warning{{Use of memory after it is freed}} expected-note{{Use of memory after it is freed}}
+ return *this;
+}
+
+StringUsed& StringUsed::operator=(StringUsed &&rhs) { // expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}}
+ clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
+ str = rhs.str;
+ rhs.str = nullptr; // FIXME: An improved leak checker should warn here
+ return *this;
+}
+
+StringUsed::operator const char*() const {
+ return str;
+}
+
+class StringUnused {
+public:
+ StringUnused(const char *s = "") : str(strdup(s)) {}
+ StringUnused(const StringUnused &rhs) : str(strdup(rhs.str)) {}
+ ~StringUnused();
+ StringUnused& operator=(const StringUnused &rhs);
+ StringUnused& operator=(StringUnused &&rhs);
+ operator const char*() const;
+private:
+ char *str;
+};
+
+StringUnused::~StringUnused() {
+ free(str);
+}
+
+StringUnused& StringUnused::operator=(const StringUnused &rhs) { // expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}}
+ clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
+ free(str); // expected-note{{Memory is released}}
+ str = strdup(rhs.str); // expected-warning{{Use of memory after it is freed}} expected-note{{Use of memory after it is freed}}
+ return *this;
+}
+
+StringUnused& StringUnused::operator=(StringUnused &&rhs) { // expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}}
+ clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
+ str = rhs.str;
+ rhs.str = nullptr; // FIXME: An improved leak checker should warn here
+ return *this;
+}
+
+StringUnused::operator const char*() const {
+ return str;
+}
+
+
+int main() {
+ StringUsed s1 ("test"), s2;
+ s2 = s1;
+ s2 = std::move(s1);
+ return 0;
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
