steakhal updated this revision to Diff 217162.
steakhal added a comment.
Reformatted using `clang-format-diff.py`.
Minor fixes which were requested by @Szelethus.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66733/new/
https://reviews.llvm.org/D66733
Files:
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/dead-stores.c
clang/test/Analysis/dead-stores.cpp
Index: clang/test/Analysis/dead-stores.cpp
===================================================================
--- clang/test/Analysis/dead-stores.cpp
+++ clang/test/Analysis/dead-stores.cpp
@@ -1,11 +1,13 @@
// RUN: %clang_analyze_cc1 -fcxx-exceptions -fexceptions -fblocks -std=c++11 -analyzer-checker=deadcode.DeadStores -verify -Wno-unreachable-code %s
// RUN: %clang_analyze_cc1 -fcxx-exceptions -fexceptions -fblocks -std=c++11 -analyzer-store=region -analyzer-checker=deadcode.DeadStores -verify -Wno-unreachable-code %s
+// RUN: %clang_analyze_cc1 -fcxx-exceptions -fexceptions -fblocks -std=c++11 -analyzer-checker=deadcode.DeadStores -analyzer-config deadcode.DeadStores:WarnForDeadNestedAssignments=true -verify -Wno-unreachable-code -DWARN_FOR_DEAD_NESTED %s
//===----------------------------------------------------------------------===//
// Basic dead store checking (but in C++ mode).
//===----------------------------------------------------------------------===//
int j;
+int make_int();
void test1() {
int x = 4;
@@ -17,6 +19,15 @@
(void)x;
break;
}
+
+ int y;
+ (void)y;
+#ifdef WARN_FOR_DEAD_NESTED
+ if ((y = make_int())) // expected-warning{{Although the value stored}}
+#else
+ if ((y = make_int())) // no-warning
+#endif
+ return;
}
//===----------------------------------------------------------------------===//
Index: clang/test/Analysis/dead-stores.c
===================================================================
--- clang/test/Analysis/dead-stores.c
+++ clang/test/Analysis/dead-stores.c
@@ -1,5 +1,6 @@
// RUN: %clang_analyze_cc1 -Wunused-variable -analyzer-checker=core,deadcode.DeadStores -fblocks -verify -Wno-unreachable-code -analyzer-opt-analyze-nested-blocks %s
// RUN: %clang_analyze_cc1 -Wunused-variable -analyzer-checker=core,deadcode.DeadStores -analyzer-store=region -fblocks -verify -Wno-unreachable-code -analyzer-opt-analyze-nested-blocks %s
+// RUN: %clang_analyze_cc1 -Wunused-variable -analyzer-checker=core,deadcode.DeadStores -analyzer-config deadcode.DeadStores:WarnForDeadNestedAssignments=true -fblocks -verify -Wno-unreachable-code -analyzer-opt-analyze-nested-blocks -DWARN_FOR_DEAD_NESTED %s
void f1() {
int k, y; // expected-warning{{unused variable 'k'}} expected-warning{{unused variable 'y'}}
@@ -77,7 +78,11 @@
// to see a real bug in this scenario.
int f8(int *p) {
extern int *baz();
+#ifdef WARN_FOR_DEAD_NESTED
+ if ((p = baz())) // expected-warning{{Although the value stored}}
+#else
if ((p = baz())) // no-warning
+#endif
return 1;
return 0;
}
@@ -152,8 +157,11 @@
// to see a real bug in this scenario.
int f16(int x) {
x = x * 2;
- x = sizeof(int [x = (x || x + 1) * 2])
- ? 5 : 8;
+#ifdef WARN_FOR_DEAD_NESTED
+ x = sizeof(int[x = (x || x + 1) * 2]) ? 5 : 8; // expected-warning{{Although the value stored}}
+#else
+ x = sizeof(int[x = (x || x + 1) * 2]) ? 5 : 8; // no-warning
+#endif
return x;
}
@@ -182,7 +190,11 @@
int f18_a() {
int x = 0; // no-warning
+#ifdef WARN_FOR_DEAD_NESTED
+ return (x = 10); // expected-warning{{Although the value stored}}
+#else
return (x = 10); // no-warning
+#endif
}
void f18_b() {
@@ -535,7 +547,11 @@
int rdar11185138_bar() {
int y;
+#ifdef WARN_FOR_DEAD_NESTED
+ int x = y = 0; // expected-warning{{Although the value stored}}
+#else
int x = y = 0; // no-warning
+#endif
x = 2;
y = 2;
return x + y;
@@ -557,8 +573,14 @@
x3 = (getInt(), getInt(), 0); // expected-warning{{Value stored to 'x3' is never read}}
int x4 = (getInt(), (getInt(), 0)); // expected-warning{{unused variable 'x4'}}
int y;
+
+#ifdef WARN_FOR_DEAD_NESTED
+ int x5 = (getInt(), (y = 0)); // expected-warning{{unused variable 'x5'}} // expected-warning{{Although the value stored}}
+ int x6 = (getInt(), (y = getInt())); //expected-warning {{Value stored to 'x6' during its initialization is never read}} // expected-warning{{unused variable 'x6'}} // expected-warning{{Although the value stored}}
+#else
int x5 = (getInt(), (y = 0)); // expected-warning{{unused variable 'x5'}}
int x6 = (getInt(), (y = getInt())); //expected-warning {{Value stored to 'x6' during its initialization is never read}} // expected-warning{{unused variable 'x6'}}
+#endif
int x7 = 0, x8 = getInt(); //expected-warning {{Value stored to 'x8' during its initialization is never read}} // expected-warning{{unused variable 'x8'}} // expected-warning{{unused variable 'x7'}}
int x9 = getInt(), x10 = 0; //expected-warning {{Value stored to 'x9' during its initialization is never read}} // expected-warning{{unused variable 'x9'}} // expected-warning{{unused variable 'x10'}}
int m = getInt(), mm, mmm; //expected-warning {{Value stored to 'm' during its initialization is never read}} // expected-warning{{unused variable 'm'}} // expected-warning{{unused variable 'mm'}} // expected-warning{{unused variable 'mmm'}}
Index: clang/test/Analysis/analyzer-config.c
===================================================================
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -30,6 +30,7 @@
// CHECK-NEXT: ctu-dir = ""
// CHECK-NEXT: ctu-import-threshold = 100
// CHECK-NEXT: ctu-index-name = externalDefMap.txt
+// CHECK-NEXT: deadcode.DeadStores:WarnForDeadNestedAssignments = false
// CHECK-NEXT: debug.AnalysisOrder:* = false
// CHECK-NEXT: debug.AnalysisOrder:Bind = false
// CHECK-NEXT: debug.AnalysisOrder:EndFunction = false
@@ -92,4 +93,4 @@
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: widen-loops = false
// CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 89
+// CHECK-NEXT: num-entries = 90
Index: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -130,6 +130,7 @@
std::unique_ptr<ReachableCode> reachableCode;
const CFGBlock *currentBlock;
std::unique_ptr<llvm::DenseSet<const VarDecl *>> InEH;
+ const bool WarnForDeadNestedAssignments;
enum DeadStoreKind { Standard, Enclosing, DeadIncrement, DeadInit };
@@ -137,9 +138,11 @@
DeadStoreObs(const CFG &cfg, ASTContext &ctx, BugReporter &br,
const CheckerBase *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) {}
+ Escaped(escaped), currentBlock(nullptr),
+ WarnForDeadNestedAssignments(warnForDeadNestedAssignments) {}
~DeadStoreObs() override {}
@@ -217,11 +220,17 @@
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 (!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(),
@@ -474,6 +483,8 @@
namespace {
class DeadStoresChecker : public Checker<check::ASTCodeBody> {
public:
+ bool WarnForDeadNestedAssignments = false;
+
void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
BugReporter &BR) const {
@@ -491,15 +502,20 @@
ParentMap &pmap = mgr.getParentMap(D);
FindEscaped FS;
cfg.VisitBlockStmts(FS);
- DeadStoreObs A(cfg, BR.getContext(), BR, this, AC, pmap, FS.Escaped);
+ 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");
}
bool ento::shouldRegisterDeadStoresChecker(const LangOptions &LO) {
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -636,6 +636,15 @@
def DeadStoresChecker : Checker<"DeadStores">,
HelpText<"Check for values stored to variables that are never read "
"afterwards">,
+ CheckerOptions<[
+ CmdLineOption<Boolean,
+ "WarnForDeadNestedAssignments",
+ "Warns for deadstores in nested assignments."
+ "E.g.: if ((P = f())) where P is unused."
+ "Enabling this may result in some false positive finds.",
+ "false",
+ Released>
+ ]>,
Documentation<HasDocumentation>;
} // end DeadCode
Index: clang/docs/analyzer/checkers.rst
===================================================================
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -294,6 +294,18 @@
x = 1; // warn
}
+**Options**
+
+This checker has several options which can be set from command line (e.g.
+``-analyzer-config deadcode.DeadStores:WarnForDeadNestedAssignments=true``):
+
+* ``WarnForDeadNestedAssignments`` (boolean). If set to false, the checker
+ won't emit warnings for nested dead assignments. When enabled likely reports
+ false-positives. *Defaults to false*.
+ Would warn for this e.g.:
+ if ((y = make_int())) {
+ }
+
.. _nullability-checkers:
nullability
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
