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 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 + ]>, Documentation; } // end DeadCode 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; const CFGBlock *currentBlock; std::unique_ptr> 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 &escaped) + llvm::SmallPtrSet &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,16 @@ 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 +482,8 @@ namespace { class DeadStoresChecker : public Checker { public: + bool WarnForDeadNestedAssignments = false; + void checkASTCodeBody(const Decl *D, AnalysisManager& mgr, BugReporter &BR) const { @@ -491,15 +501,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(); +void ento::registerDeadStoresChecker(CheckerManager &Mgr) { + auto Chk = Mgr.registerChecker(); + + const AnalyzerOptions &AnOpts = Mgr.getAnalyzerOptions(); + Chk->WarnForDeadNestedAssignments = + AnOpts.getCheckerBooleanOption(Chk, "WarnForDeadNestedAssignments"); } bool ento::shouldRegisterDeadStoresChecker(const LangOptions &LO) { 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/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/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; } //===----------------------------------------------------------------------===//