Added some false positive checking to UnreachableCodeChecker

- Allowed reporting of dead macros
- Added path walking function to search for false positives in conditional statements
- Updated some affected tests
- Added some false positive test cases

llvm-svn: 109561
This commit is contained in:
Tom Care 2010-07-27 23:30:21 +00:00
parent 76bdd685c2
commit 29a6250bf0
6 changed files with 175 additions and 85 deletions

View File

@ -13,13 +13,14 @@
// A similar flow-sensitive only check exists in Analysis/ReachableCode.cpp
//===----------------------------------------------------------------------===//
#include "clang/AST/ParentMap.h"
#include "clang/Basic/Builtins.h"
#include "clang/Checker/PathSensitive/CheckerVisitor.h"
#include "clang/Checker/PathSensitive/ExplodedGraph.h"
#include "clang/Checker/PathSensitive/SVals.h"
#include "clang/Checker/PathSensitive/CheckerHelpers.h"
#include "clang/Checker/BugReporter/BugReporter.h"
#include "GRExprEngineExperimentalChecks.h"
#include "clang/AST/StmtCXX.h"
#include "clang/Basic/Builtins.h"
#include "llvm/ADT/SmallPtrSet.h"
// The number of CFGBlock pointers we want to reserve memory for. This is used
@ -35,9 +36,13 @@ public:
void VisitEndAnalysis(ExplodedGraph &G, BugReporter &B,
bool hasWorkRemaining);
private:
typedef bool (*ExplodedNodeHeuristic)(const ExplodedNode &EN);
static SourceLocation GetUnreachableLoc(const CFGBlock &b, SourceRange &R);
void FindUnreachableEntryPoints(const CFGBlock *CB);
void MarkSuccessorsReachable(const CFGBlock *CB);
static const Expr *getConditon(const Stmt *S);
static bool isInvalidPath(const CFGBlock *CB, const ParentMap &PM);
llvm::SmallPtrSet<const CFGBlock*, DEFAULT_CFGBLOCKS> reachable;
llvm::SmallPtrSet<const CFGBlock*, DEFAULT_CFGBLOCKS> visited;
@ -61,14 +66,18 @@ void UnreachableCodeChecker::VisitEndAnalysis(ExplodedGraph &G,
return;
CFG *C = 0;
ParentMap *PM = 0;
// Iterate over ExplodedGraph
for (ExplodedGraph::node_iterator I = G.nodes_begin(); I != G.nodes_end();
++I) {
const ProgramPoint &P = I->getLocation();
const LocationContext *LC = P.getLocationContext();
// Save the CFG if we don't have it already
if (!C)
C = P.getLocationContext()->getCFG();
C = LC->getCFG();
if (!PM)
PM = &LC->getParentMap();
if (const BlockEntrance *BE = dyn_cast<BlockEntrance>(&P)) {
const CFGBlock *CB = BE->getBlock();
@ -76,8 +85,8 @@ void UnreachableCodeChecker::VisitEndAnalysis(ExplodedGraph &G,
}
}
// Bail out if we didn't get the CFG
if (!C)
// Bail out if we didn't get the CFG or the ParentMap.
if (!C || !PM)
return;
ASTContext &Ctx = B.getContext();
@ -86,34 +95,39 @@ void UnreachableCodeChecker::VisitEndAnalysis(ExplodedGraph &G,
for (CFG::const_iterator I = C->begin(); I != C->end(); ++I) {
const CFGBlock *CB = *I;
// Check if the block is unreachable
if (!reachable.count(CB)) {
// Find the entry points for this block
FindUnreachableEntryPoints(CB);
// This block may have been pruned; check if we still want to report it
if (reachable.count(CB))
continue;
if (reachable.count(CB))
continue;
// We found a statement that wasn't covered
SourceRange S;
SourceLocation SL = GetUnreachableLoc(*CB, S);
if (S.getBegin().isMacroID() || S.getEnd().isMacroID() || S.isInvalid()
|| SL.isInvalid())
continue;
// Find the entry points for this block
FindUnreachableEntryPoints(CB);
// Special case for __builtin_unreachable.
// FIXME: This should be extended to include other unreachable markers,
// such as llvm_unreachable.
if (!CB->empty()) {
const Stmt *First = CB->front();
if (const CallExpr *CE = dyn_cast<CallExpr>(First)) {
if (CE->isBuiltinCall(Ctx) == Builtin::BI__builtin_unreachable)
continue;
}
// This block may have been pruned; check if we still want to report it
if (reachable.count(CB))
continue;
// Check for false positives
if (CB->size() > 0 && isInvalidPath(CB, *PM))
continue;
// We found a statement that wasn't covered
SourceRange S;
SourceLocation SL = GetUnreachableLoc(*CB, S);
if (S.isInvalid() || SL.isInvalid())
continue;
// Special case for __builtin_unreachable.
// FIXME: This should be extended to include other unreachable markers,
// such as llvm_unreachable.
if (!CB->empty()) {
const Stmt *First = CB->front();
if (const CallExpr *CE = dyn_cast<CallExpr>(First)) {
if (CE->isBuiltinCall(Ctx) == Builtin::BI__builtin_unreachable)
continue;
}
B.EmitBasicReport("Unreachable code", "This statement is never executed",
SL, S);
}
B.EmitBasicReport("Unreachable code", "This statement is never executed",
SL, S);
}
}
@ -156,3 +170,50 @@ SourceLocation UnreachableCodeChecker::GetUnreachableLoc(const CFGBlock &b,
R = S->getSourceRange();
return S->getLocStart();
}
// Returns the Expr* condition if this is a conditional statement, or 0
// otherwise
const Expr *UnreachableCodeChecker::getConditon(const Stmt *S) {
if (const IfStmt *IS = dyn_cast<IfStmt>(S)) {
return IS->getCond();
}
else if (const SwitchStmt *SS = dyn_cast<SwitchStmt>(S)) {
return SS->getCond();
}
else if (const WhileStmt *WS = dyn_cast<WhileStmt>(S)) {
return WS->getCond();
}
else if (const DoStmt *DS = dyn_cast<DoStmt>(S)) {
return DS->getCond();
}
else if (const ForStmt *FS = dyn_cast<ForStmt>(S)) {
return FS->getCond();
}
else if (const ConditionalOperator *CO = dyn_cast<ConditionalOperator>(S)) {
return CO->getCond();
}
return 0;
}
// Traverse the predecessor Stmt*s from this CFGBlock to find any signs of a
// path that is a false positive.
bool UnreachableCodeChecker::isInvalidPath(const CFGBlock *CB,
const ParentMap &PM) {
// Start at the end of the block and work up the path.
const Stmt *S = CB->back().getStmt();
while (S) {
// Find any false positives in the conditions on this path.
if (const Expr *cond = getConditon(S)) {
if (containsMacro(cond) || containsEnum(cond)
|| containsStaticLocal(cond) || containsBuiltinOffsetOf(cond)
|| containsStmt<SizeOfAlignOfExpr>(cond))
return true;
}
// Get the previous statement.
S = PM.getParent(S);
}
return false;
}

View File

@ -61,7 +61,7 @@ void eq_ne (unsigned a) {
if (a+1 != 0)
return; // no-warning
if (a-1 != UINT_MAX-1)
return; // expected-warning{{never executed}}
return; // no-warning
free(b);
}
@ -72,7 +72,7 @@ void ne_eq (unsigned a) {
if (a+1 == 0)
return; // no-warning
if (a-1 == UINT_MAX-1)
return; // expected-warning{{never executed}}
return; // no-warning
free(b);
}
@ -177,7 +177,7 @@ void adjustedLE (unsigned a) {
void tautologyGT (unsigned a) {
char* b = malloc(1);
if (a > UINT_MAX)
return; // expected-warning{{never executed}}
return; // no-warning
free(b);
}

View File

@ -53,7 +53,7 @@ void memcpy0 () {
memcpy(dst, src, 4); // no-warning
if (memcpy(dst, src, 4) != dst) {
(void)*(char*)0; // expected-warning{{never executed}}
(void)*(char*)0; // no-warning
}
}
@ -155,7 +155,7 @@ void memmove0 () {
memmove(dst, src, 4); // no-warning
if (memmove(dst, src, 4) != dst) {
(void)*(char*)0; // expected-warning{{never executed}}
(void)*(char*)0; // no-warning
}
}
@ -217,7 +217,7 @@ void memcmp3 () {
char a[] = {1, 2, 3, 4};
if (memcmp(a, a, 4))
(void)*(char*)0; // expected-warning{{never executed}}
(void)*(char*)0; // no-warning
}
void memcmp4 (char *input) {
@ -231,11 +231,11 @@ void memcmp5 (char *input) {
char a[] = {1, 2, 3, 4};
if (memcmp(a, 0, 0)) // no-warning
(void)*(char*)0; // expected-warning{{never executed}}
(void)*(char*)0; // no-warning
if (memcmp(0, a, 0)) // no-warning
(void)*(char*)0; // expected-warning{{never executed}}
(void)*(char*)0; // no-warning
if (memcmp(a, input, 0)) // no-warning
(void)*(char*)0; // expected-warning{{never executed}}
(void)*(char*)0; // no-warning
}
void memcmp6 (char *a, char *b, size_t n) {

View File

@ -9,51 +9,51 @@ void testComparisons (int a) {
// Sema can already catch the simple comparison a==a,
// since that's usually a logic error (and not path-dependent).
int b = a;
if (!(b==a)) WARN;
if (!(b>=a)) WARN;
if (!(b<=a)) WARN;
if (b!=a) WARN;
if (b>a) WARN;
if (b<a) WARN;
if (!(b==a)) WARN; // expected-warning{{never executed}}
if (!(b>=a)) WARN; // expected-warning{{never executed}}
if (!(b<=a)) WARN; // expected-warning{{never executed}}
if (b!=a) WARN; // expected-warning{{never executed}}
if (b>a) WARN; // expected-warning{{never executed}}
if (b<a) WARN; // expected-warning{{never executed}}
}
void testSelfOperations (int a) {
if ((a|a) != a) WARN; // expected-warning{{Both operands to '|' always have the same value}}
if ((a&a) != a) WARN; // expected-warning{{Both operands to '&' always have the same value}}
if ((a^a) != 0) WARN; // expected-warning{{Both operands to '^' always have the same value}}
if ((a-a) != 0) WARN; // expected-warning{{Both operands to '-' always have the same value}}
if ((a|a) != a) WARN; // expected-warning{{Both operands to '|' always have the same value}} expected-warning{{never executed}}
if ((a&a) != a) WARN; // expected-warning{{Both operands to '&' always have the same value}} expected-warning{{never executed}}
if ((a^a) != 0) WARN; // expected-warning{{Both operands to '^' always have the same value}} expected-warning{{never executed}}
if ((a-a) != 0) WARN; // expected-warning{{Both operands to '-' always have the same value}} expected-warning{{never executed}}
}
void testIdempotent (int a) {
if ((a*1) != a) WARN;
if ((a/1) != a) WARN;
if ((a+0) != a) WARN;
if ((a-0) != a) WARN;
if ((a<<0) != a) WARN;
if ((a>>0) != a) WARN;
if ((a^0) != a) WARN;
if ((a&(~0)) != a) WARN;
if ((a|0) != a) WARN;
if ((a*1) != a) WARN; // expected-warning{{never executed}}
if ((a/1) != a) WARN; // expected-warning{{never executed}}
if ((a+0) != a) WARN; // expected-warning{{never executed}}
if ((a-0) != a) WARN; // expected-warning{{never executed}}
if ((a<<0) != a) WARN; // expected-warning{{never executed}}
if ((a>>0) != a) WARN; // expected-warning{{never executed}}
if ((a^0) != a) WARN; // expected-warning{{never executed}}
if ((a&(~0)) != a) WARN; // expected-warning{{never executed}}
if ((a|0) != a) WARN; // expected-warning{{never executed}}
}
void testReductionToConstant (int a) {
if ((a*0) != 0) WARN;
if ((a&0) != 0) WARN;
if ((a|(~0)) != (~0)) WARN;
if ((a*0) != 0) WARN; // expected-warning{{never executed}}
if ((a&0) != 0) WARN; // expected-warning{{never executed}}
if ((a|(~0)) != (~0)) WARN; // expected-warning{{never executed}}
}
void testSymmetricIntSymOperations (int a) {
if ((2+a) != (a+2)) WARN;
if ((2*a) != (a*2)) WARN;
if ((2&a) != (a&2)) WARN;
if ((2^a) != (a^2)) WARN;
if ((2|a) != (a|2)) WARN;
if ((2+a) != (a+2)) WARN; // expected-warning{{never executed}}
if ((2*a) != (a*2)) WARN; // expected-warning{{never executed}}
if ((2&a) != (a&2)) WARN; // expected-warning{{never executed}}
if ((2^a) != (a^2)) WARN; // expected-warning{{never executed}}
if ((2|a) != (a|2)) WARN; // expected-warning{{never executed}}
}
void testAsymmetricIntSymOperations (int a) {
if (((~0) >> a) != (~0)) WARN;
if ((0 >> a) != 0) WARN;
if ((0 << a) != 0) WARN;
if (((~0) >> a) != (~0)) WARN; // expected-warning{{never executed}}
if ((0 >> a) != 0) WARN; // expected-warning{{never executed}}
if ((0 << a) != 0) WARN; // expected-warning{{never executed}}
// Unsigned right shift shifts in zeroes.
if ((((unsigned)(~0)) >> ((unsigned) a)) != ((unsigned)(~0)))
@ -62,11 +62,11 @@ void testAsymmetricIntSymOperations (int a) {
void testLocations (char *a) {
char *b = a;
if (!(b==a)) WARN;
if (!(b>=a)) WARN;
if (!(b<=a)) WARN;
if (b!=a) WARN;
if (b>a) WARN;
if (b<a) WARN;
if (b-a) WARN; // expected-warning{{Both operands to '-' always have the same value}}
if (!(b==a)) WARN; // expected-warning{{never executed}}
if (!(b>=a)) WARN; // expected-warning{{never executed}}
if (!(b<=a)) WARN; // expected-warning{{never executed}}
if (b!=a) WARN; // expected-warning{{never executed}}
if (b>a) WARN; // expected-warning{{never executed}}
if (b<a) WARN; // expected-warning{{never executed}}
if (b-a) WARN; // expected-warning{{Both operands to '-' always have the same value}} expected-warning{{never executed}}
}

View File

@ -35,13 +35,13 @@ size_t strlen(const char *s);
void strlen_constant0() {
if (strlen("123") != 3)
(void)*(char*)0; // expected-warning{{never executed}}
(void)*(char*)0;
}
void strlen_constant1() {
const char *a = "123";
if (strlen(a) != 3)
(void)*(char*)0; // expected-warning{{never executed}}
(void)*(char*)0;
}
void strlen_constant2(char x) {

View File

@ -2,6 +2,8 @@
extern void foo(int a);
// The first few tests are non-path specific - we should be able to find them
void test(unsigned a) {
switch (a) {
a += 5; // expected-warning{{never executed}}
@ -23,7 +25,16 @@ void test2(unsigned a) {
goto help;
}
void test3() {
void test3(unsigned a) {
while(1);
if (a > 5) { // expected-warning{{never executed}}
return;
}
}
// These next tests are path-sensitive
void test4() {
int a = 5;
while (a > 1)
@ -36,13 +47,6 @@ void test3() {
foo(a);
}
void test4(unsigned a) {
while(1);
if (a > 5) { // expected-warning{{never executed}}
return;
}
}
extern void bar(char c);
void test5(const char *c) {
@ -53,9 +57,34 @@ void test5(const char *c) {
}
}
// These next tests are false positives and should not generate warnings
void test6(const char *c) {
if (c) return;
if (!c) return;
__builtin_unreachable(); // no-warning
}
// Compile-time constant false positives
#define CONSTANT 0
enum test_enum { Off, On };
void test7() {
if (CONSTANT)
return; // no-warning
if (sizeof(int))
return; // no-warning
if (Off)
return; // no-warning
}
void test8() {
static unsigned a = 0;
if (a)
a = 123; // no-warning
a = 5;
}