From: Tom Care Date: Fri, 23 Jul 2010 23:04:53 +0000 (+0000) Subject: Added an path-sensitive unreachable code checker to the experimental analyzer checks. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c4b5bd89e1ef611c7a31b767763030acc45274c8;p=clang Added an path-sensitive unreachable code checker to the experimental analyzer checks. - Created a new class to do post-analysis - Updated several test cases with unreachable code to expect a warning - Added some general tests git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@109286 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Checker/CMakeLists.txt b/lib/Checker/CMakeLists.txt index c3dab22870..85117ac26d 100644 --- a/lib/Checker/CMakeLists.txt +++ b/lib/Checker/CMakeLists.txt @@ -74,6 +74,7 @@ add_clang_library(clangChecker UndefinedArraySubscriptChecker.cpp UndefinedAssignmentChecker.cpp UnixAPIChecker.cpp + UnreachableCodeChecker.cpp VLASizeChecker.cpp ValueManager.cpp ) diff --git a/lib/Checker/GRExprEngineExperimentalChecks.cpp b/lib/Checker/GRExprEngineExperimentalChecks.cpp index d138e81c46..84262b0043 100644 --- a/lib/Checker/GRExprEngineExperimentalChecks.cpp +++ b/lib/Checker/GRExprEngineExperimentalChecks.cpp @@ -18,13 +18,14 @@ using namespace clang; -void clang::RegisterExperimentalChecks(GRExprEngine &Eng) { +void clang::RegisterExperimentalChecks(GRExprEngine &Eng) { // These are checks that never belong as internal checks // within GRExprEngine. - RegisterPthreadLockChecker(Eng); + RegisterCStringChecker(Eng); RegisterMallocChecker(Eng); + RegisterPthreadLockChecker(Eng); RegisterStreamChecker(Eng); - RegisterCStringChecker(Eng); + RegisterUnreachableCodeChecker(Eng); } void clang::RegisterExperimentalInternalChecks(GRExprEngine &Eng) { @@ -34,11 +35,10 @@ void clang::RegisterExperimentalInternalChecks(GRExprEngine &Eng) { // Note that this must be registered after ReturnStackAddresEngsChecker. RegisterReturnPointerRangeChecker(Eng); + RegisterArrayBoundChecker(Eng); + RegisterCastSizeChecker(Eng); + RegisterCastToStructChecker(Eng); RegisterFixedAddressChecker(Eng); - RegisterPointerSubChecker(Eng); RegisterPointerArithChecker(Eng); - RegisterCastToStructChecker(Eng); - RegisterCastSizeChecker(Eng); - RegisterArrayBoundChecker(Eng); - + RegisterPointerSubChecker(Eng); } diff --git a/lib/Checker/GRExprEngineExperimentalChecks.h b/lib/Checker/GRExprEngineExperimentalChecks.h index 3c1c95ffef..ea8d3e3bf8 100644 --- a/lib/Checker/GRExprEngineExperimentalChecks.h +++ b/lib/Checker/GRExprEngineExperimentalChecks.h @@ -20,9 +20,10 @@ namespace clang { class GRExprEngine; void RegisterCStringChecker(GRExprEngine &Eng); -void RegisterPthreadLockChecker(GRExprEngine &Eng); void RegisterMallocChecker(GRExprEngine &Eng); +void RegisterPthreadLockChecker(GRExprEngine &Eng); void RegisterStreamChecker(GRExprEngine &Eng); +void RegisterUnreachableCodeChecker(GRExprEngine &Eng); } // end clang namespace #endif diff --git a/lib/Checker/UnreachableCodeChecker.cpp b/lib/Checker/UnreachableCodeChecker.cpp new file mode 100644 index 0000000000..0af49a33fb --- /dev/null +++ b/lib/Checker/UnreachableCodeChecker.cpp @@ -0,0 +1,143 @@ +//==- UnreachableCodeChecker.cpp - Generalized dead code checker -*- C++ -*-==// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// This file implements a generalized unreachable code checker using a +// path-sensitive analysis. We mark any path visited, and then walk the CFG as a +// post-analysis to determine what was never visited. +// +// A similar flow-sensitive only check exists in Analysis/UnreachableCode.cpp +//===----------------------------------------------------------------------===// + +#include "clang/Checker/PathSensitive/CheckerVisitor.h" +#include "clang/Checker/PathSensitive/ExplodedGraph.h" +#include "clang/Checker/PathSensitive/SVals.h" +#include "clang/Checker/BugReporter/BugReporter.h" +#include "GRExprEngineExperimentalChecks.h" +#include "clang/AST/StmtCXX.h" +#include "llvm/ADT/SmallPtrSet.h" + +// The number of CFGBlock pointers we want to reserve memory for. This is used +// once for each function we analyze. +#define DEFAULT_CFGBLOCKS 256 + +using namespace clang; + +namespace { +class UnreachableCodeChecker : public CheckerVisitor { +public: + static void *getTag(); + void VisitEndAnalysis(ExplodedGraph &G, BugReporter &B, + bool hasWorkRemaining); +private: + static SourceLocation GetUnreachableLoc(const CFGBlock &b, SourceRange &R); + void FindUnreachableEntryPoints(const CFGBlock *CB); + void MarkSuccessorsReachable(const CFGBlock *CB); + + llvm::SmallPtrSet reachable; + llvm::SmallPtrSet visited; +}; +} + +void *UnreachableCodeChecker::getTag() { + static int x = 0; + return &x; +} + +void clang::RegisterUnreachableCodeChecker(GRExprEngine &Eng) { + Eng.registerCheck(new UnreachableCodeChecker()); +} + +void UnreachableCodeChecker::VisitEndAnalysis(ExplodedGraph &G, + BugReporter &B, + bool hasWorkRemaining) { + // Bail out if we didn't cover all paths + if (hasWorkRemaining) + return; + + CFG *C = 0; + // Iterate over ExplodedGraph + for (ExplodedGraph::node_iterator I = G.nodes_begin(); I != G.nodes_end(); + ++I) { + const ProgramPoint &P = I->getLocation(); + + // Save the CFG if we don't have it already + if (!C) + C = P.getLocationContext()->getCFG(); + + if (const BlockEntrance *BE = dyn_cast(&P)) { + const CFGBlock *CB = BE->getBlock(); + reachable.insert(CB); + } + } + + // Bail out if we didn't get the CFG + if (!C) + return; + + // Find CFGBlocks that were not covered by any node + 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; + + // 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; + B.EmitBasicReport("Unreachable code", "This statement is never executed", + SL, S); + } + } +} + +// Recursively finds the entry point(s) for this dead CFGBlock. +void UnreachableCodeChecker::FindUnreachableEntryPoints(const CFGBlock *CB) { + bool allPredecessorsReachable = true; + + visited.insert(CB); + + for (CFGBlock::const_pred_iterator I = CB->pred_begin(); I != CB->pred_end(); + ++I) { + // Recurse over all unreachable blocks + if (!reachable.count(*I) && !visited.count(*I)) { + FindUnreachableEntryPoints(*I); + allPredecessorsReachable = false; + } + } + + // If at least one predecessor is unreachable, mark this block as reachable + // so we don't report this block. + if (!allPredecessorsReachable) { + reachable.insert(CB); + } +} + +// Find the SourceLocation and SourceRange to report this CFGBlock +SourceLocation UnreachableCodeChecker::GetUnreachableLoc(const CFGBlock &b, + SourceRange &R) { + const Stmt *S = 0; + unsigned sn = 0; + R = SourceRange(); + + if (sn < b.size()) + S = b[sn].getStmt(); + else if (b.getTerminator()) + S = b.getTerminator(); + else + return SourceLocation(); + + R = S->getSourceRange(); + return S->getLocStart(); +} diff --git a/test/Analysis/additive-folding.c b/test/Analysis/additive-folding.c index 15d758800a..6e81fcf8c2 100644 --- a/test/Analysis/additive-folding.c +++ b/test/Analysis/additive-folding.c @@ -18,7 +18,7 @@ void separateExpressions (int a) { char* buf = malloc(1); if (a != 0 && b == 0) - return; // no-warning + return; // expected-warning{{never executed}} free(buf); } @@ -29,7 +29,7 @@ void oneLongExpression (int a) { char* buf = malloc(1); if (a != 0 && b == 0) - return; // no-warning + return; // expected-warning{{never executed}} free(buf); } @@ -40,11 +40,11 @@ void mixedTypes (int a) { // This is part of PR7406. int b = a + 1LL; if (a != 0 && (b-1) == 0) // not crash - return; // no warning + return; // expected-warning{{never executed}} int c = a + 1U; if (a != 0 && (c-1) == 0) // not crash - return; // no warning + return; // expected-warning{{never executed}} free(buf); } @@ -61,7 +61,7 @@ void eq_ne (unsigned a) { if (a+1 != 0) return; // no-warning if (a-1 != UINT_MAX-1) - return; // no-warning + return; // expected-warning{{never executed}} 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; // no-warning + return; // expected-warning{{never executed}} free(b); } @@ -85,7 +85,7 @@ void mixed_eq_ne (int a) { if (a+1U != 2) return; // no-warning if (a-1U != 0) - return; // no-warning + return; // expected-warning{{never executed}} free(b); } @@ -96,7 +96,7 @@ void mixed_ne_eq (int a) { if (a+1U == 2) return; // no-warning if (a-1U == 0) - return; // no-warning + return; // expected-warning{{never executed}} free(b); } @@ -177,7 +177,7 @@ void adjustedLE (unsigned a) { void tautologyGT (unsigned a) { char* b = malloc(1); if (a > UINT_MAX) - return; // no-warning + return; // expected-warning{{never executed}} free(b); } @@ -191,7 +191,7 @@ void tautologyGE (unsigned a) { void tautologyLT (unsigned a) { char* b = malloc(1); if (a < 0) - return; // no-warning + return; // expected-warning{{never executed}} free(b); } diff --git a/test/Analysis/bstring.c b/test/Analysis/bstring.c index f4ddb0a3d0..3a5dfab33d 100644 --- a/test/Analysis/bstring.c +++ b/test/Analysis/bstring.c @@ -53,7 +53,7 @@ void memcpy0 () { memcpy(dst, src, 4); // no-warning if (memcpy(dst, src, 4) != dst) { - (void)*(char*)0; // no-warning -- should be unreachable + (void)*(char*)0; // expected-warning{{never executed}} } } @@ -155,7 +155,7 @@ void memmove0 () { memmove(dst, src, 4); // no-warning if (memmove(dst, src, 4) != dst) { - (void)*(char*)0; // no-warning -- should be unreachable + (void)*(char*)0; // expected-warning{{never executed}} } } @@ -217,7 +217,7 @@ void memcmp3 () { char a[] = {1, 2, 3, 4}; if (memcmp(a, a, 4)) - (void)*(char*)0; // no-warning + (void)*(char*)0; // expected-warning{{never executed}} } 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; // no-warning + (void)*(char*)0; // expected-warning{{never executed}} if (memcmp(0, a, 0)) // no-warning - (void)*(char*)0; // no-warning + (void)*(char*)0; // expected-warning{{never executed}} if (memcmp(a, input, 0)) // no-warning - (void)*(char*)0; // no-warning + (void)*(char*)0; // expected-warning{{never executed}} } void memcmp6 (char *a, char *b, size_t n) { diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c index b4c1314b34..91974f6725 100644 --- a/test/Analysis/malloc.c +++ b/test/Analysis/malloc.c @@ -117,7 +117,7 @@ char callocZeroesBad () { char *buf = calloc(2,2); char result = buf[3]; // no-warning if (buf[1] != 0) { - free(buf); + free(buf); // expected-warning{{never executed}} } return result; // expected-warning{{never released}} } diff --git a/test/Analysis/unreachable-code-path.c b/test/Analysis/unreachable-code-path.c new file mode 100644 index 0000000000..cfe0408c3f --- /dev/null +++ b/test/Analysis/unreachable-code-path.c @@ -0,0 +1,55 @@ +// RUN: %clang_cc1 -analyze -analyzer-experimental-checks -analyzer-check-objc-mem -analyzer-check-dead-stores -verify -analyzer-opt-analyze-nested-blocks %s + +extern void foo(int a); + +void test(unsigned a) { + switch (a) { + a += 5; // expected-warning{{never executed}} + case 2: + a *= 10; + case 3: + a %= 2; + } + foo(a); +} + +void test2(unsigned a) { + help: + if (a > 0) + return; + if (a == 0) + return; + foo(a); // expected-warning{{never executed}} + goto help; +} + +void test3() { + int a = 5; + + while (a > 1) + a -= 2; + + if (a > 1) { + a = a + 56; // expected-warning{{never executed}} + } + + 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) { + foo(c[0]); + + if (!c) { + bar(1); // expected-warning{{never executed}} + } +} +