Skip to content

Commit 9f0d8ba

Browse files
[analyzer] Fix dead store checker false positive
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 Differential Revision: https://reviews.llvm.org/D99262
1 parent b785e03 commit 9f0d8ba

File tree

2 files changed

+71
-4
lines changed

2 files changed

+71
-4
lines changed

clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,18 @@
1111
//
1212
//===----------------------------------------------------------------------===//
1313

14-
#include "clang/Lex/Lexer.h"
15-
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
1614
#include "clang/AST/ASTContext.h"
1715
#include "clang/AST/Attr.h"
1816
#include "clang/AST/ParentMap.h"
1917
#include "clang/AST/RecursiveASTVisitor.h"
2018
#include "clang/Analysis/Analyses/LiveVariables.h"
19+
#include "clang/Lex/Lexer.h"
20+
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
2121
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
2222
#include "clang/StaticAnalyzer/Core/Checker.h"
2323
#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
2424
#include "llvm/ADT/BitVector.h"
25+
#include "llvm/ADT/STLExtras.h"
2526
#include "llvm/ADT/SmallString.h"
2627
#include "llvm/Support/SaveAndRestore.h"
2728

@@ -408,15 +409,17 @@ class DeadStoreObs : public LiveVariables::Observer {
408409
// Special case: check for initializations with constants.
409410
//
410411
// e.g. : int x = 0;
412+
// struct A = {0, 1};
413+
// struct B = {{0}, {1, 2}};
411414
//
412415
// If x is EVER assigned a new value later, don't issue
413416
// a warning. This is because such initialization can be
414417
// due to defensive programming.
415-
if (E->isEvaluatable(Ctx))
418+
if (isConstant(E))
416419
return;
417420

418421
if (const DeclRefExpr *DRE =
419-
dyn_cast<DeclRefExpr>(E->IgnoreParenCasts()))
422+
dyn_cast<DeclRefExpr>(E->IgnoreParenCasts()))
420423
if (const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
421424
// Special case: check for initialization from constant
422425
// variables.
@@ -444,6 +447,29 @@ class DeadStoreObs : public LiveVariables::Observer {
444447
}
445448
}
446449
}
450+
451+
private:
452+
/// Return true if the given init list can be interpreted as constant
453+
bool isConstant(const InitListExpr *Candidate) const {
454+
// We consider init list to be constant if each member of the list can be
455+
// interpreted as constant.
456+
return llvm::all_of(Candidate->inits(),
457+
[this](const Expr *Init) { return isConstant(Init); });
458+
}
459+
460+
/// Return true if the given expression can be interpreted as constant
461+
bool isConstant(const Expr *E) const {
462+
// It looks like E itself is a constant
463+
if (E->isEvaluatable(Ctx))
464+
return true;
465+
466+
// We should also allow defensive initialization of structs, i.e. { 0 }
467+
if (const auto *ILE = dyn_cast<InitListExpr>(E->IgnoreParenCasts())) {
468+
return isConstant(ILE);
469+
}
470+
471+
return false;
472+
}
447473
};
448474

449475
} // end anonymous namespace

clang/test/Analysis/dead-stores.c

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,3 +635,44 @@ void testVolatile() {
635635
volatile int v;
636636
v = 0; // no warning
637637
}
638+
639+
struct Foo {
640+
int x;
641+
int y;
642+
};
643+
644+
struct Foo rdar34122265_getFoo(void);
645+
646+
int rdar34122265_test(int input) {
647+
// This is allowed for defensive programming.
648+
struct Foo foo = {0, 0};
649+
if (input > 0) {
650+
foo = rdar34122265_getFoo();
651+
} else {
652+
return 0;
653+
}
654+
return foo.x + foo.y;
655+
}
656+
657+
void rdar34122265_test_cast() {
658+
// This is allowed for defensive programming.
659+
struct Foo foo = {0, 0};
660+
(void)foo;
661+
}
662+
663+
struct Bar {
664+
struct Foo x, y;
665+
};
666+
667+
struct Bar rdar34122265_getBar(void);
668+
669+
int rdar34122265_test_nested(int input) {
670+
// This is allowed for defensive programming.
671+
struct Bar bar = {{0, 0}, {0, 0}};
672+
if (input > 0) {
673+
bar = rdar34122265_getBar();
674+
} else {
675+
return 0;
676+
}
677+
return bar.x.x + bar.y.y;
678+
}

0 commit comments

Comments
 (0)