From fa6ef180c0d3609124217387618fbb51bbdd2e48 Mon Sep 17 00:00:00 2001 From: Mike Stump Date: Wed, 13 Jan 2010 02:59:54 +0000 Subject: [PATCH] Add an unreachable code checker. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@93287 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 2 + lib/Analysis/AnalysisContext.cpp | 4 + lib/Sema/Sema.h | 10 ++- lib/Sema/SemaDecl.cpp | 90 +++++++++++++++------- lib/Sema/SemaExpr.cpp | 5 +- test/SemaCXX/unreachable-code.cpp | 41 ++++++++++ 6 files changed, 120 insertions(+), 32 deletions(-) create mode 100644 test/SemaCXX/unreachable-code.cpp diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 95747a875b..33f1b98182 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -160,6 +160,8 @@ def warn_suggest_noreturn_function : Warning< def warn_suggest_noreturn_block : Warning< "block could be attribute 'noreturn'">, InGroup>, DefaultIgnore; +def warn_unreachable : Warning<"will never be executed">, + InGroup>, DefaultIgnore; /// Built-in functions. def ext_implicit_lib_function_decl : ExtWarn< diff --git a/lib/Analysis/AnalysisContext.cpp b/lib/Analysis/AnalysisContext.cpp index 97e6d914d4..2093b5e23e 100644 --- a/lib/Analysis/AnalysisContext.cpp +++ b/lib/Analysis/AnalysisContext.cpp @@ -18,6 +18,7 @@ #include "clang/Analysis/CFG.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclObjC.h" +#include "clang/AST/DeclTemplate.h" #include "clang/AST/ParentMap.h" #include "clang/AST/StmtVisitor.h" #include "clang/Analysis/Support/BumpVector.h" @@ -38,6 +39,9 @@ Stmt *AnalysisContext::getBody() { return MD->getBody(); else if (const BlockDecl *BD = dyn_cast(D)) return BD->getBody(); + else if (const FunctionTemplateDecl *FunTmpl + = dyn_cast_or_null(D)) + return FunTmpl->getTemplatedDecl()->getBody(); llvm_unreachable("unknown code decl"); } diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h index bc139c19a8..38657638b7 100644 --- a/lib/Sema/Sema.h +++ b/lib/Sema/Sema.h @@ -42,6 +42,7 @@ namespace llvm { } namespace clang { + class AnalysisContext; class ASTContext; class ASTConsumer; class CodeCompleteConsumer; @@ -1097,6 +1098,9 @@ public: OwningExprResult BuildOverloadedArrowExpr(Scope *S, ExprArg Base, SourceLocation OpLoc); + /// CheckUnreachable - Check for unreachable code. + void CheckUnreachable(AnalysisContext &); + /// CheckCallReturnType - Checks that a call expression's return type is /// complete. Returns true on failure. The location passed in is the location /// that best represents the call. @@ -1104,14 +1108,14 @@ public: CallExpr *CE, FunctionDecl *FD); /// Helpers for dealing with blocks and functions. - void CheckFallThroughForFunctionDef(Decl *D, Stmt *Body); - void CheckFallThroughForBlock(QualType BlockTy, Stmt *Body); + void CheckFallThroughForFunctionDef(Decl *D, Stmt *Body, AnalysisContext &); + void CheckFallThroughForBlock(QualType BlockTy, Stmt *, AnalysisContext &); bool CheckParmsForFunctionDef(FunctionDecl *FD); void CheckCXXDefaultArguments(FunctionDecl *FD); void CheckExtraCXXDefaultArguments(Declarator &D); enum ControlFlowKind { NeverFallThrough = 0, MaybeFallThrough = 1, AlwaysFallThrough = 2, NeverFallThroughOrReturn = 3 }; - ControlFlowKind CheckFallThrough(Stmt *); + ControlFlowKind CheckFallThrough(AnalysisContext &); Scope *getNonFieldDeclScope(Scope *S); diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index d2f254c718..59cb447ecd 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -14,6 +14,7 @@ #include "Sema.h" #include "SemaInit.h" #include "Lookup.h" +#include "clang/Analysis/PathSensitive/AnalysisContext.h" #include "clang/AST/APValue.h" #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" @@ -1305,30 +1306,10 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) { New->setPreviousDeclaration(Old); } -/// CheckFallThrough - Check that we don't fall off the end of a -/// Statement that should return a value. -/// -/// \returns AlwaysFallThrough iff we always fall off the end of the statement, -/// MaybeFallThrough iff we might or might not fall off the end, -/// NeverFallThroughOrReturn iff we never fall off the end of the statement or -/// return. We assume NeverFallThrough iff we never fall off the end of the -/// statement but we may return. We assume that functions not marked noreturn -/// will return. -Sema::ControlFlowKind Sema::CheckFallThrough(Stmt *Root) { - // FIXME: Eventually share this CFG object when we have other warnings based - // of the CFG. This can be done using AnalysisContext. - llvm::OwningPtr cfg (CFG::buildCFG(Root, &Context)); - - // FIXME: They should never return 0, fix that, delete this code. - if (cfg == 0) - // FIXME: This should be NeverFallThrough - return NeverFallThroughOrReturn; - // The CFG leaves in dead things, and we don't want to dead code paths to - // confuse us, so we mark all live things first. +static void MarkLive(CFGBlock *e, llvm::BitVector &live) { std::queue workq; - llvm::BitVector live(cfg->getNumBlockIDs()); // Prep work queue - workq.push(&cfg->getEntry()); + workq.push(e); // Solve while (!workq.empty()) { CFGBlock *item = workq.front(); @@ -1344,6 +1325,54 @@ Sema::ControlFlowKind Sema::CheckFallThrough(Stmt *Root) { } } } +} + +/// CheckUnreachable - Check for unreachable code. +void Sema::CheckUnreachable(AnalysisContext &AC) { + if (Diags.getDiagnosticLevel(diag::warn_unreachable) == Diagnostic::Ignored) + return; + + CFG *cfg = AC.getCFG(); + // FIXME: They should never return 0, fix that, delete this code. + if (cfg == 0) + return; + + llvm::BitVector live(cfg->getNumBlockIDs()); + // Mark all live things first. + MarkLive(&cfg->getEntry(), live); + + for (unsigned i = 0; i < cfg->getNumBlockIDs(); ++i) { + if (!live[i]) { + CFGBlock &b = *(cfg->begin()[i]); + if (!b.empty()) + Diag(b[0].getStmt()->getLocStart(), diag::warn_unreachable); + // Avoid excessive errors by marking everything reachable from here + MarkLive(&b, live); + } + } +} + +/// CheckFallThrough - Check that we don't fall off the end of a +/// Statement that should return a value. +/// +/// \returns AlwaysFallThrough iff we always fall off the end of the statement, +/// MaybeFallThrough iff we might or might not fall off the end, +/// NeverFallThroughOrReturn iff we never fall off the end of the statement or +/// return. We assume NeverFallThrough iff we never fall off the end of the +/// statement but we may return. We assume that functions not marked noreturn +/// will return. +Sema::ControlFlowKind Sema::CheckFallThrough(AnalysisContext &AC) { + CFG *cfg = AC.getCFG(); + // FIXME: They should never return 0, fix that, delete this code. + if (cfg == 0) + // FIXME: This should be NeverFallThrough + return NeverFallThroughOrReturn; + + // The CFG leaves in dead things, and we don't want to dead code paths to + // confuse us, so we mark all live things first. + std::queue workq; + llvm::BitVector live(cfg->getNumBlockIDs()); + MarkLive(&cfg->getEntry(), live); // Now we know what is live, we check the live precessors of the exit block // and look for fall through paths, being careful to ignore normal returns, @@ -1419,7 +1448,8 @@ Sema::ControlFlowKind Sema::CheckFallThrough(Stmt *Root) { /// function that should return a value. Check that we don't fall off the end /// of a noreturn function. We assume that functions and blocks not marked /// noreturn will return. -void Sema::CheckFallThroughForFunctionDef(Decl *D, Stmt *Body) { +void Sema::CheckFallThroughForFunctionDef(Decl *D, Stmt *Body, + AnalysisContext &AC) { // FIXME: Would be nice if we had a better way to control cascading errors, // but for now, avoid them. The problem is that when Parse sees: // int foo() { return a; } @@ -1457,7 +1487,7 @@ void Sema::CheckFallThroughForFunctionDef(Decl *D, Stmt *Body) { return; // FIXME: Function try block if (CompoundStmt *Compound = dyn_cast(Body)) { - switch (CheckFallThrough(Body)) { + switch (CheckFallThrough(AC)) { case MaybeFallThrough: if (HasNoReturn) Diag(Compound->getRBracLoc(), diag::warn_falloff_noreturn_function); @@ -1484,7 +1514,8 @@ void Sema::CheckFallThroughForFunctionDef(Decl *D, Stmt *Body) { /// that should return a value. Check that we don't fall off the end of a /// noreturn block. We assume that functions and blocks not marked noreturn /// will return. -void Sema::CheckFallThroughForBlock(QualType BlockTy, Stmt *Body) { +void Sema::CheckFallThroughForBlock(QualType BlockTy, Stmt *Body, + AnalysisContext &AC) { // FIXME: Would be nice if we had a better way to control cascading errors, // but for now, avoid them. The problem is that when Parse sees: // int foo() { return a; } @@ -1510,7 +1541,7 @@ void Sema::CheckFallThroughForBlock(QualType BlockTy, Stmt *Body) { return; // FIXME: Funtion try block if (CompoundStmt *Compound = dyn_cast(Body)) { - switch (CheckFallThrough(Body)) { + switch (CheckFallThrough(AC)) { case MaybeFallThrough: if (HasNoReturn) Diag(Compound->getRBracLoc(), diag::err_noreturn_block_has_return_expr); @@ -4330,6 +4361,7 @@ Sema::DeclPtrTy Sema::ActOnFinishFunctionBody(DeclPtrTy D, StmtArg BodyArg, Decl *dcl = D.getAs(); Stmt *Body = BodyArg.takeAs(); + AnalysisContext AC(dcl); FunctionDecl *FD = 0; FunctionTemplateDecl *FunTmpl = dyn_cast_or_null(dcl); if (FunTmpl) @@ -4344,7 +4376,7 @@ Sema::DeclPtrTy Sema::ActOnFinishFunctionBody(DeclPtrTy D, StmtArg BodyArg, // Implements C++ [basic.start.main]p5 and C99 5.1.2.2.3. FD->setHasImplicitReturnZero(true); else - CheckFallThroughForFunctionDef(FD, Body); + CheckFallThroughForFunctionDef(FD, Body, AC); if (!FD->isInvalidDecl()) DiagnoseUnusedParameters(FD->param_begin(), FD->param_end()); @@ -4356,7 +4388,7 @@ Sema::DeclPtrTy Sema::ActOnFinishFunctionBody(DeclPtrTy D, StmtArg BodyArg, } else if (ObjCMethodDecl *MD = dyn_cast_or_null(dcl)) { assert(MD == getCurMethodDecl() && "Method parsing confused"); MD->setBody(Body); - CheckFallThroughForFunctionDef(MD, Body); + CheckFallThroughForFunctionDef(MD, Body, AC); MD->setEndLoc(Body->getLocEnd()); if (!MD->isInvalidDecl()) @@ -4414,6 +4446,8 @@ Sema::DeclPtrTy Sema::ActOnFinishFunctionBody(DeclPtrTy D, StmtArg BodyArg, if (!Body) return D; + CheckUnreachable(AC); + // Verify that that gotos and switch cases don't jump into scopes illegally. if (CurFunctionNeedsScopeChecking) DiagnoseInvalidJumps(Body); diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index f3093c1bad..e02dbd6beb 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -14,6 +14,7 @@ #include "Sema.h" #include "SemaInit.h" #include "Lookup.h" +#include "clang/Analysis/PathSensitive/AnalysisContext.h" #include "clang/AST/ASTContext.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/DeclTemplate.h" @@ -6859,7 +6860,9 @@ Sema::OwningExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc, CurFunctionNeedsScopeChecking = BSI->SavedFunctionNeedsScopeChecking; BSI->TheDecl->setBody(body.takeAs()); - CheckFallThroughForBlock(BlockTy, BSI->TheDecl->getBody()); + AnalysisContext AC(BSI->TheDecl); + CheckFallThroughForBlock(BlockTy, BSI->TheDecl->getBody(), AC); + CheckUnreachable(AC); return Owned(new (Context) BlockExpr(BSI->TheDecl, BlockTy, BSI->hasBlockDeclRefExprs)); } diff --git a/test/SemaCXX/unreachable-code.cpp b/test/SemaCXX/unreachable-code.cpp new file mode 100644 index 0000000000..528bba7d5e --- /dev/null +++ b/test/SemaCXX/unreachable-code.cpp @@ -0,0 +1,41 @@ +// RUN: %clang_cc1 -fsyntax-only -Wunreachable-code -fblocks -verify %s + +int j; +void bar() { } +int test1() { + for (int i = 0; + i != 10; + ++i) { // expected-warning {{will never be executed}} + if (j == 23) // missing {}'s + bar(); + return 1; + } + return 0; + return 1; // expected-warning {{will never be executed}} +} + +void test2(int i) { + switch (i) { + case 0: + break; + bar(); // expected-warning {{will never be executed}} + case 2: + switch (i) { + default: + a: goto a; + } + bar(); // expected-warning {{will never be executed}} + } + b: goto b; + bar(); // expected-warning {{will never be executed}} +} + +void test3() { + ^{ return; + bar(); // expected-warning {{will never be executed}} + }(); + while (++j) { + continue; + bar(); // expected-warning {{will never be executed}} + } +} -- 2.40.0