Revert "[analyzer] Add checker modeling potential C++ self-assignment"

This reverts commit r275820. It is failing on the bots.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@275880 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/StaticAnalyzer/Checkers/Checkers.td

@@ -247,10 +247,6 @@ def NewDeleteLeaksChecker : Checker<"NewDeleteLeaks">,
   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 {

include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h

@@ -331,22 +331,6 @@ public:
                                  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

lib/StaticAnalyzer/Checkers/CMakeLists.txt

@@ -22,7 +22,6 @@ add_clang_library(clangStaticAnalyzerCheckers
   CheckerDocumentation.cpp
   ChrootChecker.cpp
   ClangCheckers.cpp
-  CXXSelfAssignmentChecker.cpp
   DeadStoresChecker.cpp
   DebugCheckers.cpp
   DereferenceChecker.cpp

lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp

@@ -1,62 +0,0 @@
-//=== 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>();
-}

lib/StaticAnalyzer/Core/BugReporter.cpp

@@ -3104,7 +3104,6 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD,
     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;

lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

@@ -1693,53 +1693,3 @@ UndefOrNullArgVisitor::VisitNode(const ExplodedNode *N,
   }
   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;
-
-  const auto Msg = "Assuming " + Met->getParamDecl(0)->getName() +
-                   ((Param == This) ? " == " : " != ") + "*this";
-
-  auto *Piece = new PathDiagnosticEventPiece(L, Msg.str());
-  Piece->addRange(Met->getSourceRange());
-
-  return Piece;
-}

lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

@@ -431,13 +431,6 @@ static bool shouldSkipFunction(const Decl *D,
   //   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);
@@ -449,7 +442,9 @@ AnalysisConsumer::getInliningModeForFunction(const Decl *D,
   // 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) && isa<ObjCMethodDecl>(D)) {
+  if (Visited.count(D)) {
+    assert(isa<ObjCMethodDecl>(D) &&
+           "We are only reanalyzing ObjCMethods.");
     const ObjCMethodDecl *ObjCM = cast<ObjCMethodDecl>(D);
     if (ObjCM->getMethodFamily() != OMF_init)
       return ExprEngine::Inline_Minimal;

test/Analysis/self-assign.cpp

@@ -1,89 +0,0 @@
-// 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;
-}