baloghadamsoftware created this revision.
baloghadamsoftware added a reviewer: dcoughlin.
baloghadamsoftware added subscribers: cfe-commits, xazax.hun, o.gyorgy.
This checker checks copy and move assignment operators whether they are
protected against self-assignment. Since C++ core guidelines discourages
explicit checking for ``&rhs==this`` in general we take a different approach:
in top-frame analysis we branch the exploded graph for two cases, where
&rhs==this and &rhs!=this and let existing checkers (e.g. unix.Malloc) do the
rest of the work. It is important that we check all copy and move assignment
operator in top frame even if we checked them already since self-assignments
may happen undetected even in the same translation unit (e.g. using random
indices for an array what may or may not be the same).
http://reviews.llvm.org/D19311
Files:
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/SelfAssignmentChecker.cpp
lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
test/Analysis/self-assign-unused.cpp
test/Analysis/self-assign-used.cpp
Index: test/Analysis/self-assign-used.cpp
===================================================================
--- /dev/null
+++ test/Analysis/self-assign-used.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,cplusplus,unix.Malloc,debug.ExprInspection %s -verify
+
+extern "C" char *strdup(const char* s);
+extern "C" void free(void* ptr);
+
+class String {
+public:
+ String(const char *s = "") : str(strdup(s)) {}
+ String(const String &rhs) : str(strdup(rhs.str)) {}
+ ~String();
+ String& operator=(const String &rhs);
+ operator const char*() const;
+private:
+ char *str;
+};
+
+String::~String() {
+ free(str);
+}
+
+String& String::operator=(const String &rhs) {
+ free(str);
+ str = strdup(rhs.str); // expected-warning{{Use of memory after it is freed}}
+ return *this;
+}
+
+String::operator const char*() const {
+ return str;
+}
+
+int main() {
+ String s1 ("test"), s2;
+ s2 = s1;
+ return 0;
+}
Index: test/Analysis/self-assign-unused.cpp
===================================================================
--- /dev/null
+++ test/Analysis/self-assign-unused.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,cplusplus,unix.Malloc,debug.ExprInspection %s -verify
+
+extern "C" char *strdup(const char* s);
+extern "C" void free(void* ptr);
+
+class String {
+public:
+ String(const char *s = "") : str(strdup(s)) {}
+ String(const String &rhs) : str(strdup(rhs.str)) {}
+ ~String();
+ String& operator=(const String &rhs);
+ operator const char*() const;
+private:
+ char *str;
+};
+
+String::~String() {
+ free(str);
+}
+
+String& String::operator=(const String &rhs) {
+ free(str);
+ str = strdup(rhs.str); // expected-warning{{Use of memory after it is freed}}
+ return *this;
+}
+
+String::operator const char*() const {
+ return str;
+}
+
+int main() {
+ return 0;
+}
Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -432,6 +432,10 @@
// Count naming convention errors more aggressively.
if (isa<ObjCMethodDecl>(D))
return false;
+ 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);
@@ -443,9 +447,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: lib/StaticAnalyzer/Checkers/SelfAssignmentChecker.cpp
===================================================================
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/SelfAssignmentChecker.cpp
@@ -0,0 +1,42 @@
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class SelfAssignmentChecker : public Checker<check::BeginFunction> {
+public:
+ SelfAssignmentChecker();
+ void checkBeginFunction(CheckerContext &C) const;
+};
+}
+
+SelfAssignmentChecker::SelfAssignmentChecker() {}
+
+void SelfAssignmentChecker::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::registerSelfAssignmentChecker(CheckerManager &Mgr) {
+ Mgr.registerChecker<SelfAssignmentChecker>();
+}
Index: lib/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- lib/StaticAnalyzer/Checkers/Checkers.td
+++ lib/StaticAnalyzer/Checkers/Checkers.td
@@ -245,6 +245,10 @@
HelpText<"Check for memory leaks. Traces memory managed by new/delete.">,
DescFile<"MallocChecker.cpp">;
+def SelfAssignmentChecker : Checker<"SelfAssignment">,
+ HelpText<"Checks copy and move assignment operators for self assignment">,
+ DescFile<"SelfAssignmentChecker.cpp">;
+
} // end: "cplusplus"
let ParentPackage = CplusplusAlpha in {
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -65,6 +65,7 @@
RetainCountChecker.cpp
ReturnPointerRangeChecker.cpp
ReturnUndefChecker.cpp
+ SelfAssignmentChecker.cpp
SimpleStreamChecker.cpp
StackAddrEscapeChecker.cpp
StreamChecker.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
