From: Ted Kremenek Date: Tue, 23 Mar 2010 00:13:23 +0000 (+0000) Subject: Only perform CFG-based warnings on 'static inline' functions that X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d064fdc4b7b64ca55b40b70490c79d6f569df78e;p=clang Only perform CFG-based warnings on 'static inline' functions that are called (transitively) by regular functions/blocks within a translation untion. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@99233 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Analysis/AnalysisContext.h b/include/clang/Analysis/AnalysisContext.h index bde4417412..9ebd93b75b 100644 --- a/include/clang/Analysis/AnalysisContext.h +++ b/include/clang/Analysis/AnalysisContext.h @@ -33,7 +33,7 @@ class ParentMap; class ImplicitParamDecl; class LocationContextManager; class StackFrameContext; - + /// AnalysisContext contains the context data for the function or method under /// analysis. class AnalysisContext { @@ -41,6 +41,7 @@ class AnalysisContext { // AnalysisContext owns the following data. CFG *cfg; + bool builtCFG; LiveVariables *liveness; ParentMap *PM; llvm::DenseMap *ReferencedBlockVars; @@ -48,8 +49,8 @@ class AnalysisContext { bool AddEHEdges; public: AnalysisContext(const Decl *d, bool addehedges = false) - : D(d), cfg(0), liveness(0), PM(0), ReferencedBlockVars(0), - AddEHEdges(addehedges) {} + : D(d), cfg(0), builtCFG(false), liveness(0), PM(0), + ReferencedBlockVars(0), AddEHEdges(addehedges) {} ~AnalysisContext(); @@ -69,7 +70,7 @@ public: std::pair getReferencedBlockVars(const BlockDecl *BD); - + /// Return the ImplicitParamDecl* associated with 'self' if this /// AnalysisContext wraps an ObjCMethodDecl. Returns NULL otherwise. const ImplicitParamDecl *getSelfDecl() const; @@ -82,7 +83,7 @@ public: ~AnalysisContextManager(); AnalysisContext *getContext(const Decl *D); - + // Discard all previously created AnalysisContexts. void clear(); }; @@ -103,7 +104,7 @@ protected: public: virtual ~LocationContext(); - + ContextKind getKind() const { return Kind; } AnalysisContext *getAnalysisContext() const { return Ctx; } @@ -120,14 +121,14 @@ public: return getAnalysisContext()->getLiveVariables(); } - ParentMap &getParentMap() const { + ParentMap &getParentMap() const { return getAnalysisContext()->getParentMap(); } const ImplicitParamDecl *getSelfDecl() const { return Ctx->getSelfDecl(); } - + const StackFrameContext *getCurrentStackFrame() const; const StackFrameContext * getStackFrameForDeclContext(const DeclContext *DC) const; @@ -157,7 +158,7 @@ class StackFrameContext : public LocationContext { friend class LocationContextManager; StackFrameContext(AnalysisContext *ctx, const LocationContext *parent, const Stmt *s, const CFGBlock *blk, unsigned idx) - : LocationContext(StackFrame, ctx, parent), CallSite(s), Block(blk), + : LocationContext(StackFrame, ctx, parent), CallSite(s), Block(blk), Index(idx) {} public: @@ -170,9 +171,9 @@ public: unsigned getIndex() const { return Index; } void Profile(llvm::FoldingSetNodeID &ID); - + static void Profile(llvm::FoldingSetNodeID &ID, AnalysisContext *ctx, - const LocationContext *parent, const Stmt *s, + const LocationContext *parent, const Stmt *s, const CFGBlock *blk, unsigned idx) { ProfileCommon(ID, StackFrame, ctx, parent, s); ID.AddPointer(blk); @@ -186,7 +187,7 @@ public: class ScopeContext : public LocationContext { const Stmt *Enter; - + friend class LocationContextManager; ScopeContext(AnalysisContext *ctx, const LocationContext *parent, const Stmt *s) @@ -229,7 +230,7 @@ public: const LocationContext *parent, const BlockDecl *bd) { ProfileCommon(ID, Block, ctx, parent, bd); } - + static bool classof(const LocationContext* Ctx) { return Ctx->getKind() == Block; } @@ -239,7 +240,7 @@ class LocationContextManager { llvm::FoldingSet Contexts; public: ~LocationContextManager(); - + const StackFrameContext *getStackFrame(AnalysisContext *ctx, const LocationContext *parent, const Stmt *s, const CFGBlock *blk, @@ -248,7 +249,7 @@ public: const ScopeContext *getScope(AnalysisContext *ctx, const LocationContext *parent, const Stmt *s); - + /// Discard all previously created LocationContext objects. void clear(); private: diff --git a/lib/Analysis/AnalysisContext.cpp b/lib/Analysis/AnalysisContext.cpp index 5640c4a461..06d8aec391 100644 --- a/lib/Analysis/AnalysisContext.cpp +++ b/lib/Analysis/AnalysisContext.cpp @@ -54,8 +54,12 @@ const ImplicitParamDecl *AnalysisContext::getSelfDecl() const { } CFG *AnalysisContext::getCFG() { - if (!cfg) + if (!builtCFG) { cfg = CFG::buildCFG(D, getBody(), &D->getASTContext(), AddEHEdges); + // Even when the cfg is not successfully built, we don't + // want to try building it again. + builtCFG = true; + } return cfg; } @@ -126,9 +130,9 @@ LocationContextManager::getLocationContext(AnalysisContext *ctx, llvm::FoldingSetNodeID ID; LOC::Profile(ID, ctx, parent, d); void *InsertPos; - + LOC *L = cast_or_null(Contexts.FindNodeOrInsertPos(ID, InsertPos)); - + if (!L) { L = new LOC(ctx, parent, d); Contexts.InsertNode(L, InsertPos); @@ -144,7 +148,7 @@ LocationContextManager::getStackFrame(AnalysisContext *ctx, llvm::FoldingSetNodeID ID; StackFrameContext::Profile(ID, ctx, parent, s, blk, idx); void *InsertPos; - StackFrameContext *L = + StackFrameContext *L = cast_or_null(Contexts.FindNodeOrInsertPos(ID, InsertPos)); if (!L) { L = new StackFrameContext(ctx, parent, s, blk, idx); @@ -253,7 +257,7 @@ public: IgnoredContexts.insert(BR->getBlockDecl()); Visit(BR->getBlockDecl()->getBody()); } -}; +}; } // end anonymous namespace typedef BumpVector DeclVec; @@ -263,16 +267,16 @@ static DeclVec* LazyInitializeReferencedDecls(const BlockDecl *BD, llvm::BumpPtrAllocator &A) { if (Vec) return (DeclVec*) Vec; - + BumpVectorContext BC(A); DeclVec *BV = (DeclVec*) A.Allocate(); new (BV) DeclVec(BC, 10); - + // Find the referenced variables. FindBlockDeclRefExprsVals F(*BV, BC); F.Visit(BD->getBody()); - - Vec = BV; + + Vec = BV; return BV; } @@ -281,7 +285,7 @@ std::pair(); - + DeclVec *V = LazyInitializeReferencedDecls(BD, (*ReferencedBlockVars)[BD], A); return std::make_pair(V->begin(), V->end()); } @@ -310,12 +314,12 @@ LocationContextManager::~LocationContextManager() { void LocationContextManager::clear() { for (llvm::FoldingSet::iterator I = Contexts.begin(), - E = Contexts.end(); I != E; ) { + E = Contexts.end(); I != E; ) { LocationContext *LC = &*I; ++I; delete LC; } - + Contexts.clear(); } diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp index c4ceec0f81..a044576f5d 100644 --- a/lib/Sema/AnalysisBasedWarnings.cpp +++ b/lib/Sema/AnalysisBasedWarnings.cpp @@ -189,7 +189,7 @@ struct CheckFallThroughDiagnostics { unsigned diag_AlwaysFallThrough_ReturnsNonVoid; unsigned diag_NeverFallThroughOrReturn; bool funMode; - + static CheckFallThroughDiagnostics MakeForFunction() { CheckFallThroughDiagnostics D; D.diag_MaybeFallThrough_HasNoReturn = @@ -205,7 +205,7 @@ struct CheckFallThroughDiagnostics { D.funMode = true; return D; } - + static CheckFallThroughDiagnostics MakeForBlock() { CheckFallThroughDiagnostics D; D.diag_MaybeFallThrough_HasNoReturn = @@ -221,7 +221,7 @@ struct CheckFallThroughDiagnostics { D.funMode = false; return D; } - + bool checkDiagnostics(Diagnostic &D, bool ReturnsVoid, bool HasNoReturn) const { if (funMode) { @@ -232,7 +232,7 @@ struct CheckFallThroughDiagnostics { && (D.getDiagnosticLevel(diag::warn_suggest_noreturn_block) == Diagnostic::Ignored || !ReturnsVoid); } - + // For blocks. return ReturnsVoid && !HasNoReturn && (D.getDiagnosticLevel(diag::warn_suggest_noreturn_block) @@ -262,7 +262,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, HasNoReturn = MD->hasAttr(); } else if (isa(D)) { - if (const FunctionType *FT = + if (const FunctionType *FT = BlockTy->getPointeeType()->getAs()) { if (FT->getResultType()->isVoidType()) ReturnsVoid = true; @@ -276,7 +276,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, // Short circuit for compilation speed. if (CD.checkDiagnostics(Diags, ReturnsVoid, HasNoReturn)) return; - + // FIXME: Function try block if (const CompoundStmt *Compound = dyn_cast(Body)) { switch (CheckFallThrough(AC)) { @@ -312,25 +312,23 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, // warnings on a function, method, or block. //===----------------------------------------------------------------------===// -clang::sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s) : S(s) { - Diagnostic &D = S.getDiagnostics(); - +clang::sema::AnalysisBasedWarnings::Policy::Policy() { enableCheckFallThrough = 1; + enableCheckUnreachable = 0; +} - enableCheckUnreachable = (unsigned) +clang::sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s) : S(s) { + Diagnostic &D = S.getDiagnostics(); + DefaultPolicy.enableCheckUnreachable = (unsigned) (D.getDiagnosticLevel(diag::warn_unreachable) != Diagnostic::Ignored); } -void clang::sema::AnalysisBasedWarnings::IssueWarnings(const Decl *D, - QualType BlockTy) { - +void clang::sema:: +AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P, + const Decl *D, QualType BlockTy, + const bool analyzeStaticInline) { + assert(BlockTy.isNull() || isa(D)); - - // Do not do any analysis for declarations in system headers if we are - // going to just ignore them. - if (S.getDiagnostics().getSuppressSystemWarnings() && - S.SourceMgr.isInSystemHeader(D->getLocation())) - return; // We avoid doing analysis-based warnings when there are errors for // two reasons: @@ -339,13 +337,24 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(const Decl *D, // (2) The code already has problems; running the analysis just takes more // time. if (S.getDiagnostics().hasErrorOccurred()) - return; - + return; + + // Do not do any analysis for declarations in system headers if we are + // going to just ignore them. + if (S.getDiagnostics().getSuppressSystemWarnings() && + S.SourceMgr.isInSystemHeader(D->getLocation())) + return; + if (const FunctionDecl *FD = dyn_cast(D)) { // For function templates, class templates and member function templates // we'll do the analysis at instantiation time. if (FD->isDependentContext()) return; + + // Only analyze 'static inline' functions when explicitly asked. + if (!analyzeStaticInline && FD->isInlineSpecified() && + FD->getStorageClass() == FunctionDecl::Static) + return; } const Stmt *Body = D->getBody(); @@ -354,16 +363,53 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(const Decl *D, // Don't generate EH edges for CallExprs as we'd like to avoid the n^2 // explosion for destrutors that can result and the compile time hit. AnalysisContext AC(D, false); + bool performedCheck = false; // Warning: check missing 'return' - if (enableCheckFallThrough) { + if (P.enableCheckFallThrough) { const CheckFallThroughDiagnostics &CD = (isa(D) ? CheckFallThroughDiagnostics::MakeForBlock() : CheckFallThroughDiagnostics::MakeForFunction()); CheckFallThroughForBody(S, D, Body, BlockTy, CD, AC); + performedCheck = true; } // Warning: check for unreachable code - if (enableCheckUnreachable) + if (P.enableCheckUnreachable) { CheckUnreachable(S, AC); + performedCheck = true; + } + + // If this block or function calls a 'static inline' function, + // we should analyze those functions as well. + if (performedCheck) { + // The CFG should already be constructed, so this should not + // incur any extra cost. We might not have a CFG, however, for + // invalid code. + if (const CFG *cfg = AC.getCFG()) { + // All CallExprs are block-level expressions in the CFG. This means + // that walking the basic blocks in the CFG is more efficient + // than walking the entire AST to find all calls. + for (CFG::const_iterator I=cfg->begin(), E=cfg->end(); I!=E; ++I) { + const CFGBlock *B = *I; + for (CFGBlock::const_iterator BI=B->begin(), BE=B->end(); BI!=BE; ++BI) + if (const CallExpr *CE = dyn_cast(*BI)) + if (const DeclRefExpr *DR = + dyn_cast(CE->getCallee()->IgnoreParenCasts())) + if (const FunctionDecl *calleeD = + dyn_cast(DR->getDecl())) + if (calleeD->isInlineSpecified() && + calleeD->getStorageClass() == FunctionDecl::Static) { + // Have we analyzed this static inline function before? + unsigned &visited = VisitedFD[calleeD]; + if (!visited) { + // Mark the callee visited prior to analyzing it + // so we terminate in case of recursion. + visited = 1; + IssueWarnings(DefaultPolicy, calleeD, QualType(), true); + } + } + } + } + } } diff --git a/lib/Sema/AnalysisBasedWarnings.h b/lib/Sema/AnalysisBasedWarnings.h index 39da1b14d4..26e973a3f4 100644 --- a/lib/Sema/AnalysisBasedWarnings.h +++ b/lib/Sema/AnalysisBasedWarnings.h @@ -14,22 +14,43 @@ #ifndef LLVM_CLANG_SEMA_ANALYSIS_WARNINGS_H #define LLVM_CLANG_SEMA_ANALYSIS_WARNINGS_H -namespace clang { namespace sema { +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/DenseMap.h" + +namespace clang { + +class Sema; + +namespace sema { class AnalysisBasedWarnings { - Sema &S; - // The warnings to run. - unsigned enableCheckFallThrough : 1; - unsigned enableCheckUnreachable : 1; - public: + class Policy { + friend class AnalysisBasedWarnings; + // The warnings to run. + unsigned enableCheckFallThrough : 1; + unsigned enableCheckUnreachable : 1; + public: + Policy(); + void disableCheckFallThrough() { enableCheckFallThrough = 0; } + }; + +private: + Sema &S; + Policy DefaultPolicy; + llvm::DenseMap VisitedFD; + +public: AnalysisBasedWarnings(Sema &s); - void IssueWarnings(const Decl *D, QualType BlockTy = QualType()); - - void disableCheckFallThrough() { enableCheckFallThrough = 0; } + + Policy getDefaultPolicy() { return DefaultPolicy; } + + void IssueWarnings(Policy P, const Decl *D, QualType BlockTy = QualType(), + const bool analyzeStaticInline = false); + }; - + }} // end namespace clang::sema #endif diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp index 7112687605..7190cf0f1f 100644 --- a/lib/Sema/Sema.cpp +++ b/lib/Sema/Sema.cpp @@ -132,7 +132,8 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer, GlobalNewDeleteDeclared(false), CompleteTranslationUnit(CompleteTranslationUnit), NumSFINAEErrors(0), NonInstantiationEntries(0), - CurrentInstantiationScope(0), TyposCorrected(0) + CurrentInstantiationScope(0), TyposCorrected(0), + AnalysisWarnings(*this) { TUScope = 0; if (getLangOptions().CPlusPlus) diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h index 25277e92c3..23c74a854f 100644 --- a/lib/Sema/Sema.h +++ b/lib/Sema/Sema.h @@ -19,6 +19,7 @@ #include "CXXFieldCollector.h" #include "SemaOverload.h" #include "SemaTemplate.h" +#include "AnalysisBasedWarnings.h" #include "clang/AST/Attr.h" #include "clang/AST/DeclBase.h" #include "clang/AST/Decl.h" @@ -298,7 +299,7 @@ public: /// \brief The set of static functions seen so far that have not been used. std::vector UnusedStaticFuncs; - + class AccessedEntity { public: /// A member declaration found through lookup. The target is the @@ -3542,6 +3543,9 @@ public: /// \brief The number of typos corrected by CorrectTypo. unsigned TyposCorrected; + /// \brief Worker object for performing CFG-based warnings. + sema::AnalysisBasedWarnings AnalysisWarnings; + /// \brief An entity for which implicit template instantiation is required. /// /// The source location associated with the declaration is the first place in diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 2b1ad0af49..aaf39ef1f9 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -14,7 +14,6 @@ #include "Sema.h" #include "SemaInit.h" #include "Lookup.h" -#include "AnalysisBasedWarnings.h" #include "clang/AST/APValue.h" #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" @@ -4276,7 +4275,7 @@ Sema::DeclPtrTy Sema::ActOnFinishFunctionBody(DeclPtrTy D, StmtArg BodyArg, else FD = dyn_cast_or_null(dcl); - sema::AnalysisBasedWarnings W(*this); + sema::AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getDefaultPolicy(); if (FD) { FD->setBody(Body); @@ -4284,7 +4283,7 @@ Sema::DeclPtrTy Sema::ActOnFinishFunctionBody(DeclPtrTy D, StmtArg BodyArg, // C and C++ allow for main to automagically return 0. // Implements C++ [basic.start.main]p5 and C99 5.1.2.2.3. FD->setHasImplicitReturnZero(true); - W.disableCheckFallThrough(); + WP.disableCheckFallThrough(); } if (!FD->isInvalidDecl()) @@ -4381,7 +4380,7 @@ Sema::DeclPtrTy Sema::ActOnFinishFunctionBody(DeclPtrTy D, StmtArg BodyArg, ObjCMethodDecl *MD = cast(dcl); ResultType = MD->getResultType(); } - W.IssueWarnings(dcl); + AnalysisWarnings.IssueWarnings(WP, dcl); } assert(ExprTemporaries.empty() && "Leftover temporaries in function"); diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 007d625b41..97f520d86f 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -7012,8 +7012,9 @@ Sema::OwningExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc, } // Issue any analysis-based warnings. - sema::AnalysisBasedWarnings W(*this); - W.IssueWarnings(BSI->TheDecl, BlockTy); + const sema::AnalysisBasedWarnings::Policy &WP = + AnalysisWarnings.getDefaultPolicy(); + AnalysisWarnings.IssueWarnings(WP, BSI->TheDecl, BlockTy); Expr *Result = new (Context) BlockExpr(BSI->TheDecl, BlockTy, BSI->hasBlockDeclRefExprs); diff --git a/test/Sema/return.c b/test/Sema/return.c index 3ab23f4f8f..fad81ad09a 100644 --- a/test/Sema/return.c +++ b/test/Sema/return.c @@ -222,3 +222,17 @@ void test32() { void test33() { if (j) while (1) { } } + +// Test that 'static inline' functions are only analyzed for CFG-based warnings +// when they are used. +static inline int si_has_missing_return() {} // no-warning +static inline int si_has_missing_return_2() {}; // expected-warning{{control reaches end of non-void function}} +static inline int si_has_missing_return_3(int x) { + if (x) + return si_has_missing_return_3(x+1); +} // expected-warning{{control may reach end of non-void function}} + +int test_static_inline(int x) { + return x ? si_has_missing_return_2() : si_has_missing_return_3(x); +} +