From 6d671cc34a221c62a011bb4717526388fc7aa4cb Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Wed, 5 Sep 2012 22:55:23 +0000 Subject: [PATCH] [analyzer] Always include destructors in the analysis CFG. While destructors will continue to not be inlined (unless the analyzer config option 'c++-inlining' is set to 'destructors'), leaving them out of the CFG is an incomplete model of the behavior of an object, and can cause false positive warnings (like PR13751, now working). Destructors for temporaries are still not on by default, since (a) we haven't actually checked this code to be sure it's fully correct (in particular, we probably need to be very careful with regard to lifetime-extension when a temporary is bound to a reference, C++11 [class.temporary]p5), and (b) ExprEngine doesn't actually do anything when it sees a temporary destructor in the CFG -- not even invalidate the object region. To enable temporary destructors, set the 'cfg-temporary-dtors' analyzer config option to '1'. The old -cfg-add-implicit-dtors cc1 option, which controlled all implicit destructors, has been removed. llvm-svn: 163264 --- .../include/clang/Analysis/AnalysisContext.h | 5 ++-- clang/include/clang/Analysis/CFG.h | 4 ++- clang/include/clang/Driver/CC1Options.td | 2 -- .../StaticAnalyzer/Core/AnalyzerOptions.h | 11 ++++++-- clang/lib/Analysis/AnalysisDeclContext.cpp | 6 +++-- clang/lib/Analysis/CFG.cpp | 8 +++--- clang/lib/Frontend/CompilerInvocation.cpp | 1 - .../StaticAnalyzer/Core/AnalysisManager.cpp | 5 ++-- .../StaticAnalyzer/Core/AnalyzerOptions.cpp | 4 +++ .../Analysis/auto-obj-dtors-cfg-output.cpp | 2 +- clang/test/Analysis/dtor.cpp | 2 +- .../Analysis/dtors-in-dtor-cfg-output.cpp | 2 +- clang/test/Analysis/malloc.cpp | 25 +++++++++++++++++++ .../Analysis/temp-obj-dtors-cfg-output.cpp | 2 +- 14 files changed, 60 insertions(+), 19 deletions(-) diff --git a/clang/include/clang/Analysis/AnalysisContext.h b/clang/include/clang/Analysis/AnalysisContext.h index 46b4e93bb7d4..41b57b3104bb 100644 --- a/clang/include/clang/Analysis/AnalysisContext.h +++ b/clang/include/clang/Analysis/AnalysisContext.h @@ -382,8 +382,9 @@ class AnalysisDeclContextManager { public: AnalysisDeclContextManager(bool useUnoptimizedCFG = false, - bool addImplicitDtors = false, - bool addInitializers = false); + bool addImplicitDtors = false, + bool addInitializers = false, + bool addTemporaryDtors = false); ~AnalysisDeclContextManager(); diff --git a/clang/include/clang/Analysis/CFG.h b/clang/include/clang/Analysis/CFG.h index 4d087e749830..30a1db2f6222 100644 --- a/clang/include/clang/Analysis/CFG.h +++ b/clang/include/clang/Analysis/CFG.h @@ -568,6 +568,7 @@ public: bool AddEHEdges; bool AddInitializers; bool AddImplicitDtors; + bool AddTemporaryDtors; bool alwaysAdd(const Stmt *stmt) const { return alwaysAddMask[stmt->getStmtClass()]; @@ -587,7 +588,8 @@ public: : forcedBlkExprs(0), PruneTriviallyFalseEdges(true) ,AddEHEdges(false) ,AddInitializers(false) - ,AddImplicitDtors(false) {} + ,AddImplicitDtors(false) + ,AddTemporaryDtors(false) {} }; /// \brief Provides a custom implementation of the iterator class to have the diff --git a/clang/include/clang/Driver/CC1Options.td b/clang/include/clang/Driver/CC1Options.td index 8089a0a7327f..d65bf2099151 100644 --- a/clang/include/clang/Driver/CC1Options.td +++ b/clang/include/clang/Driver/CC1Options.td @@ -37,8 +37,6 @@ def triple_EQ : Joined<"-triple=">, Alias; def analysis_UnoptimizedCFG : Flag<"-unoptimized-cfg">, HelpText<"Generate unoptimized CFGs for all analyses">; -def analysis_CFGAddImplicitDtors : Flag<"-cfg-add-implicit-dtors">, - HelpText<"Add C++ implicit destructors to CFGs for all analyses">; def analyzer_store : Separate<"-analyzer-store">, HelpText<"Source Code Analysis - Abstract Memory Store Models">; diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h index 1ed5658876ef..5dba158f1238 100644 --- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h +++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h @@ -147,7 +147,6 @@ public: unsigned visualizeExplodedGraphWithGraphViz : 1; unsigned visualizeExplodedGraphWithUbiGraph : 1; unsigned UnoptimizedCFG : 1; - unsigned CFGAddImplicitDtors : 1; unsigned eagerlyTrimExplodedGraph : 1; unsigned PrintStats : 1; @@ -172,9 +171,18 @@ public: /// Returns the option controlling which C++ member functions will be /// considered for inlining. /// + /// This is controlled by the 'c++-inlining' config option. + /// /// \sa CXXMemberInliningMode bool mayInlineCXXMemberFunction(CXXInlineableMemberKind K) const; + /// Returns whether or not the destructors for C++ temporary objects should + /// be included in the CFG. + /// + /// This is controlled by the 'cfg-temporary-dtors' config option. Any + /// non-empty value is considered to be 'true'. + bool includeTemporaryDtorsInCFG() const; + public: AnalyzerOptions() : CXXMemberInliningMode() { AnalysisStoreOpt = RegionStoreModel; @@ -191,7 +199,6 @@ public: visualizeExplodedGraphWithGraphViz = 0; visualizeExplodedGraphWithUbiGraph = 0; UnoptimizedCFG = 0; - CFGAddImplicitDtors = 0; eagerlyTrimExplodedGraph = 0; PrintStats = 0; NoRetryExhausted = 0; diff --git a/clang/lib/Analysis/AnalysisDeclContext.cpp b/clang/lib/Analysis/AnalysisDeclContext.cpp index 7de7f395e8a7..f2ef0defd772 100644 --- a/clang/lib/Analysis/AnalysisDeclContext.cpp +++ b/clang/lib/Analysis/AnalysisDeclContext.cpp @@ -62,11 +62,13 @@ AnalysisDeclContext::AnalysisDeclContext(AnalysisDeclContextManager *Mgr, } AnalysisDeclContextManager::AnalysisDeclContextManager(bool useUnoptimizedCFG, - bool addImplicitDtors, - bool addInitializers) { + bool addImplicitDtors, + bool addInitializers, + bool addTemporaryDtors) { cfgBuildOptions.PruneTriviallyFalseEdges = !useUnoptimizedCFG; cfgBuildOptions.AddImplicitDtors = addImplicitDtors; cfgBuildOptions.AddInitializers = addInitializers; + cfgBuildOptions.AddTemporaryDtors = addTemporaryDtors; } void AnalysisDeclContextManager::clear() { diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index f811fa3f0f86..a179b6dffe2d 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -706,7 +706,7 @@ CFGBlock *CFGBuilder::addInitializer(CXXCtorInitializer *I) { IsReference = FD->getType()->isReferenceType(); HasTemporaries = isa(Init); - if (BuildOpts.AddImplicitDtors && HasTemporaries) { + if (BuildOpts.AddTemporaryDtors && HasTemporaries) { // Generate destructors for temporaries in initialization expression. VisitForTemporaryDtors(cast(Init)->getSubExpr(), IsReference); @@ -1617,7 +1617,7 @@ CFGBlock *CFGBuilder::VisitDeclSubExpr(DeclStmt *DS) { IsReference = VD->getType()->isReferenceType(); HasTemporaries = isa(Init); - if (BuildOpts.AddImplicitDtors && HasTemporaries) { + if (BuildOpts.AddTemporaryDtors && HasTemporaries) { // Generate destructors for temporaries in initialization expression. VisitForTemporaryDtors(cast(Init)->getSubExpr(), IsReference); @@ -2972,7 +2972,7 @@ CFGBlock *CFGBuilder::VisitCXXForRangeStmt(CXXForRangeStmt *S) { CFGBlock *CFGBuilder::VisitExprWithCleanups(ExprWithCleanups *E, AddStmtChoice asc) { - if (BuildOpts.AddImplicitDtors) { + if (BuildOpts.AddTemporaryDtors) { // If adding implicit destructors visit the full expression for adding // destructors of temporaries. VisitForTemporaryDtors(E->getSubExpr()); @@ -3052,6 +3052,8 @@ CFGBlock *CFGBuilder::VisitIndirectGotoStmt(IndirectGotoStmt *I) { } CFGBlock *CFGBuilder::VisitForTemporaryDtors(Stmt *E, bool BindToTemporary) { + assert(BuildOpts.AddImplicitDtors && BuildOpts.AddTemporaryDtors); + tryAgain: if (!E) { badCFG = true; diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index bf1a80c94c7d..6d6dbfcc2865 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -1130,7 +1130,6 @@ static bool ParseAnalyzerArgs(AnalyzerOptions &Opts, ArgList &Args, Opts.eagerlyAssumeBinOpBifurcation = Args.hasArg(OPT_analyzer_eagerly_assume); Opts.AnalyzeSpecificFunction = Args.getLastArgValue(OPT_analyze_function); Opts.UnoptimizedCFG = Args.hasArg(OPT_analysis_UnoptimizedCFG); - Opts.CFGAddImplicitDtors = Args.hasArg(OPT_analysis_CFGAddImplicitDtors); Opts.TrimGraph = Args.hasArg(OPT_trim_egraph); Opts.MaxNodes = Args.getLastArgIntValue(OPT_analyzer_max_nodes, 150000,Diags); Opts.maxBlockVisitOnPath = Args.getLastArgIntValue(OPT_analyzer_max_loop, 4, Diags); diff --git a/clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp b/clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp index 6a01edf7ce11..ebd2336080a5 100644 --- a/clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp +++ b/clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp @@ -22,8 +22,9 @@ AnalysisManager::AnalysisManager(ASTContext &ctx, DiagnosticsEngine &diags, CheckerManager *checkerMgr, const AnalyzerOptions &Options) : AnaCtxMgr(Options.UnoptimizedCFG, - Options.CFGAddImplicitDtors, - /*addInitializers=*/true), + /*AddImplicitDtors=*/true, + /*AddInitializers=*/true, + Options.includeTemporaryDtorsInCFG()), Ctx(ctx), Diags(diags), LangOpts(lang), diff --git a/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp index 5574a2f22690..9e45a115283a 100644 --- a/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp +++ b/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp @@ -46,3 +46,7 @@ AnalyzerOptions::mayInlineCXXMemberFunction(CXXInlineableMemberKind K) const { return CXXMemberInliningMode >= K; } + +bool AnalyzerOptions::includeTemporaryDtorsInCFG() const { + return !Config.lookup("cfg-temporary-dtors").empty(); +} diff --git a/clang/test/Analysis/auto-obj-dtors-cfg-output.cpp b/clang/test/Analysis/auto-obj-dtors-cfg-output.cpp index 566e6caed60d..e4b49dc10f1b 100644 --- a/clang/test/Analysis/auto-obj-dtors-cfg-output.cpp +++ b/clang/test/Analysis/auto-obj-dtors-cfg-output.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -cfg-add-implicit-dtors %s > %t 2>&1 +// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG %s > %t 2>&1 // RUN: FileCheck --input-file=%t %s // XPASS: * diff --git a/clang/test/Analysis/dtor.cpp b/clang/test/Analysis/dtor.cpp index a5d3d3f015f0..a762ebed1223 100644 --- a/clang/test/Analysis/dtor.cpp +++ b/clang/test/Analysis/dtor.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-ipa=inlining -analyzer-config c++-inlining=destructors -cfg-add-implicit-dtors -Wno-null-dereference -verify %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-ipa=inlining -analyzer-config c++-inlining=destructors -Wno-null-dereference -verify %s void clang_analyzer_eval(bool); void clang_analyzer_checkInlined(bool); diff --git a/clang/test/Analysis/dtors-in-dtor-cfg-output.cpp b/clang/test/Analysis/dtors-in-dtor-cfg-output.cpp index 68ba37ebf399..f0546fc8bffa 100644 --- a/clang/test/Analysis/dtors-in-dtor-cfg-output.cpp +++ b/clang/test/Analysis/dtors-in-dtor-cfg-output.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=debug.DumpCFG -cfg-add-implicit-dtors %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -analyze -analyzer-checker=debug.DumpCFG %s 2>&1 | FileCheck %s // XPASS: * class A { diff --git a/clang/test/Analysis/malloc.cpp b/clang/test/Analysis/malloc.cpp index 864477b587bc..220d74625bc0 100644 --- a/clang/test/Analysis/malloc.cpp +++ b/clang/test/Analysis/malloc.cpp @@ -6,6 +6,11 @@ void free(void *); void *realloc(void *ptr, size_t size); void *calloc(size_t nmemb, size_t size); + +void checkThatMallocCheckerIsRunning() { + malloc(4); // expected-warning{{leak}} +} + // Test for radar://11110132. struct Foo { mutable void* m_data; @@ -35,3 +40,23 @@ void r11160612_3(CanFreeMemory* p) { const_ptr_and_callback_def_param(0, x, 12, p->myFree); } + +namespace PR13751 { + class OwningVector { + void **storage; + size_t length; + public: + OwningVector(); + ~OwningVector(); + void push_back(void *Item) { + storage[length++] = Item; + } + }; + + void testDestructors() { + OwningVector v; + v.push_back(malloc(4)); + // no leak warning; freed in destructor + } +} + diff --git a/clang/test/Analysis/temp-obj-dtors-cfg-output.cpp b/clang/test/Analysis/temp-obj-dtors-cfg-output.cpp index 6dbbc821bbbb..d13083ddf011 100644 --- a/clang/test/Analysis/temp-obj-dtors-cfg-output.cpp +++ b/clang/test/Analysis/temp-obj-dtors-cfg-output.cpp @@ -1,5 +1,5 @@ // RUN: rm -f %t -// RUN: %clang_cc1 -analyze -analyzer-checker=debug.DumpCFG -cfg-add-implicit-dtors %s > %t 2>&1 +// RUN: %clang_cc1 -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-temporary-dtors=1 %s > %t 2>&1 // RUN: FileCheck --input-file=%t %s // XPASS: *