1 files changed, 72 insertions, 34 deletions

diff --git a/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp b/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
index d5baa2bcba6fc..61441889fc649 100644
--- a/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -11,6 +11,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/Lex/Lexer.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
@@ -119,11 +120,20 @@ LookThroughTransitiveAssignmentsAndCommaOperators(const Expr *Ex) {
 }
 
 namespace {
+class DeadStoresChecker : public Checker<check::ASTCodeBody> {
+public:
+  bool ShowFixIts = false;
+  bool WarnForDeadNestedAssignments = true;
+
+  void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
+                        BugReporter &BR) const;
+};
+
 class DeadStoreObs : public LiveVariables::Observer {
   const CFG &cfg;
   ASTContext &Ctx;
   BugReporter& BR;
-  const CheckerBase *Checker;
+  const DeadStoresChecker *Checker;
   AnalysisDeclContext* AC;
   ParentMap& Parents;
   llvm::SmallPtrSet<const VarDecl*, 20> Escaped;
@@ -135,9 +145,10 @@ class DeadStoreObs : public LiveVariables::Observer {
 
 public:
   DeadStoreObs(const CFG &cfg, ASTContext &ctx, BugReporter &br,
-               const CheckerBase *checker, AnalysisDeclContext *ac,
+               const DeadStoresChecker *checker, AnalysisDeclContext *ac,
                ParentMap &parents,
-               llvm::SmallPtrSet<const VarDecl *, 20> &escaped)
+               llvm::SmallPtrSet<const VarDecl *, 20> &escaped,
+               bool warnForDeadNestedAssignments)
       : cfg(cfg), Ctx(ctx), BR(br), Checker(checker), AC(ac), Parents(parents),
         Escaped(escaped), currentBlock(nullptr) {}
 
@@ -202,12 +213,32 @@ public:
     llvm::raw_svector_ostream os(buf);
     const char *BugType = nullptr;
 
+    SmallVector<FixItHint, 1> Fixits;
+
     switch (dsk) {
-      case DeadInit:
+      case DeadInit: {
         BugType = "Dead initialization";
         os << "Value stored to '" << *V
            << "' during its initialization is never read";
+
+        ASTContext &ACtx = V->getASTContext();
+        if (Checker->ShowFixIts) {
+          if (V->getInit()->HasSideEffects(ACtx,
+                                           /*IncludePossibleEffects=*/true)) {
+            break;
+          }
+          SourceManager &SM = ACtx.getSourceManager();
+          const LangOptions &LO = ACtx.getLangOpts();
+          SourceLocation L1 =
+              Lexer::findNextToken(
+                  V->getTypeSourceInfo()->getTypeLoc().getEndLoc(),
+                  SM, LO)->getEndLoc();
+          SourceLocation L2 =
+              Lexer::getLocForEndOfToken(V->getInit()->getEndLoc(), 1, SM, LO);
+          Fixits.push_back(FixItHint::CreateRemoval({L1, L2}));
+        }
         break;
+      }
 
       case DeadIncrement:
         BugType = "Dead increment";
@@ -217,15 +248,20 @@ public:
         os << "Value stored to '" << *V << "' is never read";
         break;
 
+      // eg.: f((x = foo()))
       case Enclosing:
-        // Don't report issues in this case, e.g.: "if (x = foo())",
-        // where 'x' is unused later.  We have yet to see a case where
-        // this is a real bug.
-        return;
+        if (!Checker->WarnForDeadNestedAssignments)
+          return;
+        BugType = "Dead nested assignment";
+        os << "Although the value stored to '" << *V
+           << "' is used in the enclosing expression, the value is never "
+              "actually read from '"
+           << *V << "'";
+        break;
     }
 
     BR.EmitBasicReport(AC->getDecl(), Checker, BugType, "Dead store", os.str(),
-                       L, R);
+                       L, R, Fixits);
   }
 
   void CheckVarDecl(const VarDecl *VD, const Expr *Ex, const Expr *Val,
@@ -471,35 +507,37 @@ public:
 // DeadStoresChecker
 //===----------------------------------------------------------------------===//
 
-namespace {
-class DeadStoresChecker : public Checker<check::ASTCodeBody> {
-public:
-  void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
-                        BugReporter &BR) const {
-
-    // Don't do anything for template instantiations.
-    // Proving that code in a template instantiation is "dead"
-    // means proving that it is dead in all instantiations.
-    // This same problem exists with -Wunreachable-code.
-    if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
-      if (FD->isTemplateInstantiation())
-        return;
+void DeadStoresChecker::checkASTCodeBody(const Decl *D, AnalysisManager &mgr,
+                                         BugReporter &BR) const {
 
-    if (LiveVariables *L = mgr.getAnalysis<LiveVariables>(D)) {
-      CFG &cfg = *mgr.getCFG(D);
-      AnalysisDeclContext *AC = mgr.getAnalysisDeclContext(D);
-      ParentMap &pmap = mgr.getParentMap(D);
-      FindEscaped FS;
-      cfg.VisitBlockStmts(FS);
-      DeadStoreObs A(cfg, BR.getContext(), BR, this, AC, pmap, FS.Escaped);
-      L->runOnAllBlocks(A);
-    }
+  // Don't do anything for template instantiations.
+  // Proving that code in a template instantiation is "dead"
+  // means proving that it is dead in all instantiations.
+  // This same problem exists with -Wunreachable-code.
+  if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
+    if (FD->isTemplateInstantiation())
+      return;
+
+  if (LiveVariables *L = mgr.getAnalysis<LiveVariables>(D)) {
+    CFG &cfg = *mgr.getCFG(D);
+    AnalysisDeclContext *AC = mgr.getAnalysisDeclContext(D);
+    ParentMap &pmap = mgr.getParentMap(D);
+    FindEscaped FS;
+    cfg.VisitBlockStmts(FS);
+    DeadStoreObs A(cfg, BR.getContext(), BR, this, AC, pmap, FS.Escaped,
+                   WarnForDeadNestedAssignments);
+    L->runOnAllBlocks(A);
   }
-};
 }
 
-void ento::registerDeadStoresChecker(CheckerManager &mgr) {
-  mgr.registerChecker<DeadStoresChecker>();
+void ento::registerDeadStoresChecker(CheckerManager &Mgr) {
+  auto *Chk = Mgr.registerChecker<DeadStoresChecker>();
+
+  const AnalyzerOptions &AnOpts = Mgr.getAnalyzerOptions();
+  Chk->WarnForDeadNestedAssignments =
+      AnOpts.getCheckerBooleanOption(Chk, "WarnForDeadNestedAssignments");
+  Chk->ShowFixIts =
+      AnOpts.getCheckerBooleanOption(Chk, "ShowFixIts");
 }
 
 bool ento::shouldRegisterDeadStoresChecker(const LangOptions &LO) {