From f47666e2ffb701c546caaeaf035add60b5d915e6 Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Sun, 9 Mar 2014 08:13:49 +0000 Subject: [PATCH] [-Wunreachable-code] Handle Objective-C bool literals in 'isConfigurationValue'. This includes special casing 'YES' and 'NO', which are constants defined as macros. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@203380 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../clang/Analysis/Analyses/ReachableCode.h | 4 +- lib/Analysis/ReachableCode.cpp | 87 ++++++++++++++----- lib/Sema/AnalysisBasedWarnings.cpp | 2 +- test/SemaObjC/warn-unreachable.m | 25 ++++++ 4 files changed, 92 insertions(+), 26 deletions(-) diff --git a/include/clang/Analysis/Analyses/ReachableCode.h b/include/clang/Analysis/Analyses/ReachableCode.h index 30c5b2d7a5..14967ba298 100644 --- a/include/clang/Analysis/Analyses/ReachableCode.h +++ b/include/clang/Analysis/Analyses/ReachableCode.h @@ -27,6 +27,7 @@ namespace llvm { namespace clang { class AnalysisDeclContext; class CFGBlock; + class Preprocessor; } //===----------------------------------------------------------------------===// @@ -49,7 +50,8 @@ public: unsigned ScanReachableFromBlock(const CFGBlock *Start, llvm::BitVector &Reachable); -void FindUnreachableCode(AnalysisDeclContext &AC, Callback &CB); +void FindUnreachableCode(AnalysisDeclContext &AC, Preprocessor &PP, + Callback &CB); }} // end namespace clang::reachable_code diff --git a/lib/Analysis/ReachableCode.cpp b/lib/Analysis/ReachableCode.cpp index 1210fcdd3a..994f9d2a12 100644 --- a/lib/Analysis/ReachableCode.cpp +++ b/lib/Analysis/ReachableCode.cpp @@ -13,6 +13,7 @@ //===----------------------------------------------------------------------===// #include "clang/Analysis/Analyses/ReachableCode.h" +#include "clang/Lex/Preprocessor.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/ExprObjC.h" @@ -172,14 +173,39 @@ static bool isTrivialReturnOrDoWhile(const CFGBlock *B, const Stmt *S) { return false; } +static SourceLocation getTopMostMacro(SourceLocation Loc, SourceManager &SM) { + assert(Loc.isMacroID()); + SourceLocation Last; + while (Loc.isMacroID()) { + Last = Loc; + Loc = SM.getImmediateMacroCallerLoc(Loc); + } + return Last; +} + /// Returns true if the statement is expanded from a configuration macro. -static bool isExpandedFromConfigurationMacro(const Stmt *S) { +static bool isExpandedFromConfigurationMacro(const Stmt *S, + Preprocessor &PP, + bool IgnoreYES_NO = false) { // FIXME: This is not very precise. Here we just check to see if the // value comes from a macro, but we can do much better. This is likely // to be over conservative. This logic is factored into a separate function // so that we can refine it later. SourceLocation L = S->getLocStart(); - return L.isMacroID(); + if (L.isMacroID()) { + if (IgnoreYES_NO) { + // The Objective-C constant 'YES' and 'NO' + // are defined as macros. Do not treat them + // as configuration values. + SourceManager &SM = PP.getSourceManager(); + SourceLocation TopL = getTopMostMacro(L, SM); + StringRef MacroName = PP.getImmediateMacroName(TopL); + if (MacroName == "YES" || MacroName == "NO") + return false; + } + return true; + } + return false; } /// Returns true if the statement represents a configuration value. @@ -190,6 +216,7 @@ static bool isExpandedFromConfigurationMacro(const Stmt *S) { /// to report as unreachable, and may mask truly unreachable code within /// those blocks. static bool isConfigurationValue(const Stmt *S, + Preprocessor &PP, bool IncludeIntegers = true) { if (!S) return false; @@ -202,7 +229,7 @@ static bool isConfigurationValue(const Stmt *S, const DeclRefExpr *DR = cast(S); const ValueDecl *D = DR->getDecl(); if (const EnumConstantDecl *ED = dyn_cast(D)) - return isConfigurationValue(ED->getInitExpr()); + return isConfigurationValue(ED->getInitExpr(), PP); if (const VarDecl *VD = dyn_cast(D)) { // As a heuristic, treat globals as configuration values. Note // that we only will get here if Sema evaluated this @@ -216,8 +243,11 @@ static bool isConfigurationValue(const Stmt *S, return false; } case Stmt::IntegerLiteralClass: - return IncludeIntegers ? isExpandedFromConfigurationMacro(S) + return IncludeIntegers ? isExpandedFromConfigurationMacro(S, PP) : false; + case Stmt::ObjCBoolLiteralExprClass: + return isExpandedFromConfigurationMacro(S, PP, /* IgnoreYES_NO */ true); + case Stmt::UnaryExprOrTypeTraitExprClass: return true; case Stmt::BinaryOperatorClass: { @@ -226,13 +256,13 @@ static bool isConfigurationValue(const Stmt *S, // values if they are used in a logical or comparison operator // (not arithmetic). IncludeIntegers &= (B->isLogicalOp() || B->isComparisonOp()); - return isConfigurationValue(B->getLHS(), IncludeIntegers) || - isConfigurationValue(B->getRHS(), IncludeIntegers); + return isConfigurationValue(B->getLHS(), PP, IncludeIntegers) || + isConfigurationValue(B->getRHS(), PP, IncludeIntegers); } case Stmt::UnaryOperatorClass: { const UnaryOperator *UO = cast(S); return UO->getOpcode() == UO_LNot && - isConfigurationValue(UO->getSubExpr()); + isConfigurationValue(UO->getSubExpr(), PP); } default: return false; @@ -240,20 +270,22 @@ static bool isConfigurationValue(const Stmt *S, } /// Returns true if we should always explore all successors of a block. -static bool shouldTreatSuccessorsAsReachable(const CFGBlock *B) { +static bool shouldTreatSuccessorsAsReachable(const CFGBlock *B, + Preprocessor &PP) { if (const Stmt *Term = B->getTerminator()) { if (isa(Term)) return true; // Specially handle '||' and '&&'. if (isa(Term)) - return isConfigurationValue(Term); + return isConfigurationValue(Term, PP); } - return isConfigurationValue(B->getTerminatorCondition()); + return isConfigurationValue(B->getTerminatorCondition(), PP); } static unsigned scanFromBlock(const CFGBlock *Start, llvm::BitVector &Reachable, + Preprocessor *PP, bool IncludeSometimesUnreachableEdges) { unsigned count = 0; @@ -291,9 +323,11 @@ static unsigned scanFromBlock(const CFGBlock *Start, if (!UB) break; - if (!TreatAllSuccessorsAsReachable.hasValue()) + if (!TreatAllSuccessorsAsReachable.hasValue()) { + assert(PP); TreatAllSuccessorsAsReachable = - shouldTreatSuccessorsAsReachable(item); + shouldTreatSuccessorsAsReachable(item, *PP); + } if (TreatAllSuccessorsAsReachable.getValue()) { B = UB; @@ -316,8 +350,9 @@ static unsigned scanFromBlock(const CFGBlock *Start, } static unsigned scanMaybeReachableFromBlock(const CFGBlock *Start, + Preprocessor &PP, llvm::BitVector &Reachable) { - return scanFromBlock(Start, Reachable, true); + return scanFromBlock(Start, Reachable, &PP, true); } //===----------------------------------------------------------------------===// @@ -329,6 +364,7 @@ namespace { llvm::BitVector Visited; llvm::BitVector &Reachable; SmallVector WorkList; + Preprocessor &PP; typedef SmallVector, 12> DeferredLocsTy; @@ -336,9 +372,10 @@ namespace { DeferredLocsTy DeferredLocs; public: - DeadCodeScan(llvm::BitVector &reachable) + DeadCodeScan(llvm::BitVector &reachable, Preprocessor &PP) : Visited(reachable.size()), - Reachable(reachable) {} + Reachable(reachable), + PP(PP) {} void enqueue(const CFGBlock *block); unsigned scanBackwards(const CFGBlock *Start, @@ -450,13 +487,13 @@ unsigned DeadCodeScan::scanBackwards(const clang::CFGBlock *Start, // Specially handle macro-expanded code. if (S->getLocStart().isMacroID()) { - count += scanMaybeReachableFromBlock(Block, Reachable); + count += scanMaybeReachableFromBlock(Block, PP, Reachable); continue; } if (isDeadCodeRoot(Block)) { reportDeadCode(Block, S, CB); - count += scanMaybeReachableFromBlock(Block, Reachable); + count += scanMaybeReachableFromBlock(Block, PP, Reachable); } else { // Record this statement as the possibly best location in a @@ -476,7 +513,7 @@ unsigned DeadCodeScan::scanBackwards(const clang::CFGBlock *Start, if (Reachable[Block->getBlockID()]) continue; reportDeadCode(Block, I->second, CB); - count += scanMaybeReachableFromBlock(Block, Reachable); + count += scanMaybeReachableFromBlock(Block, PP, Reachable); } } @@ -576,19 +613,21 @@ void Callback::anchor() { } unsigned ScanReachableFromBlock(const CFGBlock *Start, llvm::BitVector &Reachable) { - return scanFromBlock(Start, Reachable, false); + return scanFromBlock(Start, Reachable, /* SourceManager* */ 0, false); } -void FindUnreachableCode(AnalysisDeclContext &AC, Callback &CB) { +void FindUnreachableCode(AnalysisDeclContext &AC, Preprocessor &PP, + Callback &CB) { + CFG *cfg = AC.getCFG(); if (!cfg) return; - // Scan for reachable blocks from the entrance of the CFG. + // Scan for reachable blocks from the entrance of the CFG. // If there are no unreachable blocks, we're done. llvm::BitVector reachable(cfg->getNumBlockIDs()); unsigned numReachable = - scanMaybeReachableFromBlock(&cfg->getEntry(), reachable); + scanMaybeReachableFromBlock(&cfg->getEntry(), PP, reachable); if (numReachable == cfg->getNumBlockIDs()) return; @@ -597,7 +636,7 @@ void FindUnreachableCode(AnalysisDeclContext &AC, Callback &CB) { if (!AC.getCFGBuildOptions().AddEHEdges) { for (CFG::try_block_iterator I = cfg->try_blocks_begin(), E = cfg->try_blocks_end() ; I != E; ++I) { - numReachable += scanMaybeReachableFromBlock(*I, reachable); + numReachable += scanMaybeReachableFromBlock(*I, PP, reachable); } if (numReachable == cfg->getNumBlockIDs()) return; @@ -611,7 +650,7 @@ void FindUnreachableCode(AnalysisDeclContext &AC, Callback &CB) { if (reachable[block->getBlockID()]) continue; - DeadCodeScan DS(reachable); + DeadCodeScan DS(reachable, PP); numReachable += DS.scanBackwards(block, CB); if (numReachable == cfg->getNumBlockIDs()) diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp index 967f521b20..46243f402d 100644 --- a/lib/Sema/AnalysisBasedWarnings.cpp +++ b/lib/Sema/AnalysisBasedWarnings.cpp @@ -84,7 +84,7 @@ static void CheckUnreachable(Sema &S, AnalysisDeclContext &AC) { return; UnreachableCodeHandler UC(S); - reachable_code::FindUnreachableCode(AC, UC); + reachable_code::FindUnreachableCode(AC, S.getPreprocessor(), UC); } //===----------------------------------------------------------------------===// diff --git a/test/SemaObjC/warn-unreachable.m b/test/SemaObjC/warn-unreachable.m index 832cbd23d2..e2ce8a7110 100644 --- a/test/SemaObjC/warn-unreachable.m +++ b/test/SemaObjC/warn-unreachable.m @@ -15,3 +15,28 @@ void test_unreachable() { return; // expected-warning {{will never be executed}} } +#define NO __objc_no +#define YES __objc_yes +#define CONFIG NO + +// Test that 'NO' and 'YES' are not treated as configuration macros. +int test_NO() { + if (NO) + return 1; // expected-warning {{will never be executed}} + else + return 0; +} + +int test_YES() { + if (YES) + return 1; + else + return 0; // expected-warning {{will never be executed}} +} + +int test_CONFIG() { + if (CONFIG) + return 1; + else + return 0; +} -- 2.40.0