From: Jordan Rose Date: Fri, 31 Aug 2012 17:06:49 +0000 (+0000) Subject: [analyzer] Though C++ inlining is enabled, don't inline ctors and dtors. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=de5277fc555551857602bd7a7e5e616274e2d4a6;p=clang [analyzer] Though C++ inlining is enabled, don't inline ctors and dtors. More generally, this adds a new configuration option 'c++-inlining', which controls which C++ member functions can be considered for inlining. This uses the new -analyzer-config table, so the cc1 arguments will look like this: ... -analyzer-config c++-inlining=[none|methods|constructors|destructors] Note that each mode implies that all the previous member function kinds will be inlined as well; it doesn't make sense to inline destructors without inlining constructors, for example. The default mode is 'methods'. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@163004 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/docs/analyzer/IPA.txt b/docs/analyzer/IPA.txt index 90c92270bf..5a6039de89 100644 --- a/docs/analyzer/IPA.txt +++ b/docs/analyzer/IPA.txt @@ -1,6 +1,9 @@ Inlining ======== +There are several options that control which calls the analyzer will consider for +inlining. The major one is -analyzer-ipa: + -analyzer-ipa=none - All inlining is disabled. This is the only mode available in LLVM 3.1 and earlier and in Xcode 4.3 and earlier. @@ -25,6 +28,23 @@ Inlining Currently, -analyzer-ipa=dynamic-bifurcate is the default mode. + +A second setting, c++-inlining, controls which C++ member functions may be +inlined. This option uses the analyzer's configuration table, so it is specified +as shown here: + + -analyzer-config c++-inlining=[none | methods | constructors | destructors] + +Each of these modes implies that all the previous member function kinds will be +inlined as well; it doesn't make sense to inline destructors without inlining +constructors, for example. + +The default c++-inlining mode is 'methods', meaning only regular member +functions and overloaded operators will be inlined. Note that no C++ member +functions will be inlined under -analyzer-ipa=none or +-analyzer-ipa=basic-inlining. + + Basics of Implementation ----------------------- diff --git a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h index d126453c31..1ed5658876 100644 --- a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h +++ b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h @@ -76,6 +76,31 @@ enum AnalysisInliningMode { NumInliningModes }; +/// \brief Describes the different kinds of C++ member functions which can be +/// considered for inlining by the analyzer. +/// +/// These options are cumulative; enabling one kind of member function will +/// enable all kinds with lower enum values. +enum CXXInlineableMemberKind { + // Uninitialized = 0, + + /// A dummy mode in which no C++ inlining is enabled. + CIMK_None = 1, + + /// Refers to regular member function and operator calls. + CIMK_MemberFunctions, + + /// Refers to constructors (implicit or explicit). + /// + /// Note that a constructor will not be inlined if the corresponding + /// destructor is non-trivial. + CIMK_Constructors, + + /// Refers to destructors (implicit or explicit). + CIMK_Destructors +}; + + class AnalyzerOptions : public llvm::RefCountedBase { public: typedef llvm::StringMap ConfigTable; @@ -139,8 +164,19 @@ public: /// \brief The mode of function selection used during inlining. AnalysisInliningMode InliningMode; +private: + /// Controls which C++ member functions will be considered for inlining. + CXXInlineableMemberKind CXXMemberInliningMode; + +public: + /// Returns the option controlling which C++ member functions will be + /// considered for inlining. + /// + /// \sa CXXMemberInliningMode + bool mayInlineCXXMemberFunction(CXXInlineableMemberKind K) const; + public: - AnalyzerOptions() { + AnalyzerOptions() : CXXMemberInliningMode() { AnalysisStoreOpt = RegionStoreModel; AnalysisConstraintsOpt = RangeConstraintsModel; AnalysisDiagOpt = PD_HTML; diff --git a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp new file mode 100644 index 0000000000..5574a2f226 --- /dev/null +++ b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp @@ -0,0 +1,48 @@ +//===-- AnalyzerOptions.cpp - Analysis Engine Options -----------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This file contains special accessors for analyzer configuration options +// with string representations. +// +//===----------------------------------------------------------------------===// + +#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h" +#include "llvm/ADT/StringSwitch.h" + +using namespace clang; + +bool +AnalyzerOptions::mayInlineCXXMemberFunction(CXXInlineableMemberKind K) const { + if (IPAMode < Inlining) + return false; + + if (!CXXMemberInliningMode) { + static const char *ModeKey = "c++-inlining"; + std::string ModeStr = Config.lookup(ModeKey); + + CXXInlineableMemberKind &MutableMode = + const_cast(CXXMemberInliningMode); + + MutableMode = llvm::StringSwitch(ModeStr) + .Case("", CIMK_MemberFunctions) + .Case("constructors", CIMK_Constructors) + .Case("destructors", CIMK_Destructors) + .Case("none", CIMK_None) + .Case("methods", CIMK_MemberFunctions) + .Default(CXXInlineableMemberKind()); + + if (!MutableMode) { + // FIXME: We should emit a warning here about an unknown inlining kind, + // but the AnalyzerOptions doesn't have access to a diagnostic engine. + MutableMode = CIMK_None; + } + } + + return CXXMemberInliningMode >= K; +} diff --git a/lib/StaticAnalyzer/Core/CMakeLists.txt b/lib/StaticAnalyzer/Core/CMakeLists.txt index 359a34ff7d..43d42b3e0d 100644 --- a/lib/StaticAnalyzer/Core/CMakeLists.txt +++ b/lib/StaticAnalyzer/Core/CMakeLists.txt @@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangStaticAnalyzerCore APSIntType.cpp AnalysisManager.cpp + AnalyzerOptions.cpp BasicValueFactory.cpp BlockCounter.cpp BugReporter.cpp diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 4313e727c5..ad30596b92 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -316,21 +316,6 @@ template<> struct ProgramStateTrait }} -static bool shouldInlineCXX(AnalysisManager &AMgr) { - switch (AMgr.options.IPAMode) { - case None: - case BasicInlining: - return false; - case Inlining: - case DynamicDispatch: - case DynamicDispatchBifurcate: - return true; - case NumIPAModes: - llvm_unreachable("not actually a valid option"); - } - llvm_unreachable("bogus IPAMode"); -} - bool ExprEngine::inlineCall(const CallEvent &Call, const Decl *D, NodeBuilder &Bldr, ExplodedNode *Pred, ProgramStateRef State) { @@ -340,17 +325,19 @@ bool ExprEngine::inlineCall(const CallEvent &Call, const Decl *D, const StackFrameContext *CallerSFC = CurLC->getCurrentStackFrame(); const LocationContext *ParentOfCallee = 0; + const AnalyzerOptions &Opts = getAnalysisManager().options; + // FIXME: Refactor this check into a hypothetical CallEvent::canInline. switch (Call.getKind()) { case CE_Function: break; case CE_CXXMember: case CE_CXXMemberOperator: - if (!shouldInlineCXX(getAnalysisManager())) + if (!Opts.mayInlineCXXMemberFunction(CIMK_MemberFunctions)) return false; break; case CE_CXXConstructor: { - if (!shouldInlineCXX(getAnalysisManager())) + if (!Opts.mayInlineCXXMemberFunction(CIMK_Constructors)) return false; const CXXConstructorCall &Ctor = cast(Call); @@ -369,15 +356,17 @@ bool ExprEngine::inlineCall(const CallEvent &Call, const Decl *D, if (isa(Parent)) return false; + // Inlining constructors requires including initializers in the CFG. + const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext(); + assert(ADC->getCFGBuildOptions().AddInitializers && "No CFG initializers"); + // If the destructor is trivial, it's always safe to inline the constructor. if (Ctor.getDecl()->getParent()->hasTrivialDestructor()) break; - // For other types, only inline constructors if we built the CFGs for the - // destructor properly. - const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext(); - assert(ADC->getCFGBuildOptions().AddInitializers && "No CFG initializers"); - if (!ADC->getCFGBuildOptions().AddImplicitDtors) + // For other types, only inline constructors if destructor inlining is + // also enabled. + if (!Opts.mayInlineCXXMemberFunction(CIMK_Destructors)) return false; // FIXME: This is a hack. We don't handle temporary destructors @@ -389,14 +378,12 @@ bool ExprEngine::inlineCall(const CallEvent &Call, const Decl *D, break; } case CE_CXXDestructor: { - if (!shouldInlineCXX(getAnalysisManager())) + if (!Opts.mayInlineCXXMemberFunction(CIMK_Destructors)) return false; - // Only inline constructors and destructors if we built the CFGs for them - // properly. + // Inlining destructors requires building the CFG correctly. const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext(); - if (!ADC->getCFGBuildOptions().AddImplicitDtors) - return false; + assert(ADC->getCFGBuildOptions().AddImplicitDtors && "No CFG destructors"); const CXXDestructorCall &Dtor = cast(Call); @@ -408,9 +395,6 @@ bool ExprEngine::inlineCall(const CallEvent &Call, const Decl *D, break; } case CE_CXXAllocator: - if (!shouldInlineCXX(getAnalysisManager())) - return false; - // Do not inline allocators until we model deallocators. // This is unfortunate, but basically necessary for smart pointers and such. return false; diff --git a/test/Analysis/base-init.cpp b/test/Analysis/base-init.cpp index b8d0689713..34e01aa5d7 100644 --- a/test/Analysis/base-init.cpp +++ b/test/Analysis/base-init.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-store region -analyzer-ipa=inlining -verify %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-ipa=inlining -analyzer-config c++-inlining=constructors -verify %s void clang_analyzer_eval(bool); diff --git a/test/Analysis/ctor-inlining.mm b/test/Analysis/ctor-inlining.mm index da44a795c1..918de0a456 100644 --- a/test/Analysis/ctor-inlining.mm +++ b/test/Analysis/ctor-inlining.mm @@ -1,6 +1,7 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -fobjc-arc -analyzer-ipa=inlining -cfg-add-implicit-dtors -Wno-null-dereference -verify %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -fobjc-arc -analyzer-ipa=inlining -analyzer-config c++-inlining=constructors -Wno-null-dereference -verify %s void clang_analyzer_eval(bool); +void clang_analyzer_checkInlined(bool); struct Wrapper { __strong id obj; @@ -86,4 +87,19 @@ namespace ConstructorVirtualCalls { } } +namespace TemporaryConstructor { + class BoolWrapper { + public: + BoolWrapper() { + clang_analyzer_checkInlined(true); // expected-warning{{TRUE}} + value = true; + } + bool value; + }; + void test() { + // PR13717 - Don't crash when a CXXTemporaryObjectExpr is inlined. + if (BoolWrapper().value) + return; + } +} diff --git a/test/Analysis/dtor.cpp b/test/Analysis/dtor.cpp index d6f66c3845..a5d3d3f015 100644 --- a/test/Analysis/dtor.cpp +++ b/test/Analysis/dtor.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store region -analyzer-ipa=inlining -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 -cfg-add-implicit-dtors -Wno-null-dereference -verify %s void clang_analyzer_eval(bool); void clang_analyzer_checkInlined(bool); diff --git a/test/Analysis/initializer.cpp b/test/Analysis/initializer.cpp index 8785a002c6..92d581b82a 100644 --- a/test/Analysis/initializer.cpp +++ b/test/Analysis/initializer.cpp @@ -1,6 +1,4 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-ipa=inlining -cfg-add-implicit-dtors -std=c++11 -verify %s - -// We don't inline constructors unless we have destructors turned on. +// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-ipa=inlining -analyzer-config c++-inlining=constructors -std=c++11 -verify %s void clang_analyzer_eval(bool); @@ -65,13 +63,13 @@ struct RefWrapper { void testReferenceMember() { int *p = 0; - RefWrapper X(p); // expected-warning@61 {{Dereference of null pointer}} + RefWrapper X(p); // expected-warning@-7 {{Dereference of null pointer}} } void testReferenceMember2() { int *p = 0; // FIXME: We should warn here, since we're creating the reference here. - RefWrapper X(*p); // expected-warning@62 {{Dereference of null pointer}} + RefWrapper X(*p); // expected-warning@-12 {{Dereference of null pointer}} } diff --git a/test/Analysis/inline.cpp b/test/Analysis/inline.cpp index 26b1035c06..573b1647a3 100644 --- a/test/Analysis/inline.cpp +++ b/test/Analysis/inline.cpp @@ -267,20 +267,3 @@ namespace OperatorNew { clang_analyzer_eval(obj->value == 42); // expected-warning{{UNKNOWN}} } } - -namespace TemporaryConstructor { - class BoolWrapper { - public: - BoolWrapper() { - clang_analyzer_checkInlined(true); // expected-warning{{TRUE}} - value = true; - } - bool value; - }; - - void test() { - // PR13717 - Don't crash when a CXXTemporaryObjectExpr is inlined. - if (BoolWrapper().value) - return; - } -} diff --git a/test/Analysis/method-call.cpp b/test/Analysis/method-call.cpp index 65bd5155dd..cfb6cd55ad 100644 --- a/test/Analysis/method-call.cpp +++ b/test/Analysis/method-call.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-ipa=inlining -analyzer-store region -verify %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-ipa=inlining -analyzer-config c++-inlining=constructors -verify %s void clang_analyzer_eval(bool);