[PATCH] D99262: [analyzer] Fix dead store checker false positive

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko created this revision.
vsavchenko added reviewers: NoQ, steakhal, xazax.hun, ASDenysPetrov.
Herald added subscribers: martong, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

It is common to zero-initialize not only scalar variables,
but also structs.  This is also defensive programming and
we shouldn't complain about that.

rdar://34122265


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99262

Files:
  clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  clang/test/Analysis/dead-stores.c


Index: clang/test/Analysis/dead-stores.c
===================================================================
--- clang/test/Analysis/dead-stores.c
+++ clang/test/Analysis/dead-stores.c
@@ -635,3 +635,21 @@
   volatile int v;
   v = 0; // no warning
 }
+
+struct Foo {
+  int x;
+  int y;
+};
+
+struct Foo rdar34122265_getFoo(void);
+
+int rdar34122265_test_struct(int input) {
+  // This is allowed for defensive programming.
+  struct Foo foo = {0, 0};
+  if (input > 0) {
+    foo = rdar34122265_getFoo();
+  } else {
+    return 0;
+  }
+  return foo.x + foo.y;
+}
Index: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -11,17 +11,18 @@
 //
 
//===----------------------------------------------------------------------===//
 
-#include "clang/Lex/Lexer.h"
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/ParentMap.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "llvm/ADT/BitVector.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/SaveAndRestore.h"
 
@@ -415,6 +416,14 @@
               if (E->isEvaluatable(Ctx))
                 return;
 
+              // We should also allow defensive initialization of structs.
+              if (const auto *ILE =
+                      dyn_cast<InitListExpr>(E->IgnoreParenCasts()))
+                if (llvm::all_of(ILE->inits(), [this](const Expr *Init) {
+                      return Init->isEvaluatable(Ctx);
+                    }))
+                  return;
+
               if (const DeclRefExpr *DRE =
                   dyn_cast<DeclRefExpr>(E->IgnoreParenCasts()))
                 if (const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) {



Index: clang/test/Analysis/dead-stores.c
===================================================================
--- clang/test/Analysis/dead-stores.c
+++ clang/test/Analysis/dead-stores.c
@@ -635,3 +635,21 @@
   volatile int v;
   v = 0; // no warning
 }
+
+struct Foo {
+  int x;
+  int y;
+};
+
+struct Foo rdar34122265_getFoo(void);
+
+int rdar34122265_test_struct(int input) {
+  // This is allowed for defensive programming.
+  struct Foo foo = {0, 0};
+  if (input > 0) {
+    foo = rdar34122265_getFoo();
+  } else {
+    return 0;
+  }
+  return foo.x + foo.y;
+}
Index: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -11,17 +11,18 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/Lex/Lexer.h"
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/ParentMap.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "llvm/ADT/BitVector.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/SaveAndRestore.h"
 
@@ -415,6 +416,14 @@
               if (E->isEvaluatable(Ctx))
                 return;
 
+              // We should also allow defensive initialization of structs.
+              if (const auto *ILE =
+                      dyn_cast<InitListExpr>(E->IgnoreParenCasts()))
+                if (llvm::all_of(ILE->inits(), [this](const Expr *Init) {
+                      return Init->isEvaluatable(Ctx);
+                    }))
+                  return;
+
               if (const DeclRefExpr *DRE =
                   dyn_cast<DeclRefExpr>(E->IgnoreParenCasts()))
                 if (const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) {

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits