Author: dergachev Date: Tue Dec 19 16:40:38 2017 New Revision: 321128 URL: http://llvm.org/viewvc/llvm-project?rev=321128&view=rev Log: [analyzer] Fix a crash during C++17 aggregate construction of base objects.
Since C++17, classes that have base classes can potentially be initialized as aggregates. Trying to construct such objects through brace initialization was causing the analyzer to crash when the base class has a non-trivial constructor, while figuring target region for the base class constructor, because the parent stack frame didn't contain the constructor of the subclass, because there is no constructor for subclass, merely aggregate initialization. This patch avoids the crash, but doesn't provide the actually correct region for the constructor, which still remains to be fixed. Instead, construction goes into a fake temporary region which would be immediately discarded. Similar extremely conservative approach is used for other cases in which the logic for finding the target region is not yet implemented, including aggregate initialization with fields instead of base-regions (which is not C++17-specific but also never worked, just didn't crash). Differential revision: https://reviews.llvm.org/D40841 rdar://problem/35441058 Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp cfe/trunk/test/Analysis/initializer.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp?rev=321128&r1=321127&r2=321128&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp Tue Dec 19 16:40:38 2017 @@ -22,6 +22,7 @@ //===----------------------------------------------------------------------===// #include "ClangSACheckers.h" +#include "clang/AST/ParentMap.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/Basic/Builtins.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" @@ -262,8 +263,19 @@ void DynamicTypePropagation::checkPostCa if (const MemRegion *Target = Ctor->getCXXThisVal().getAsRegion()) { // We just finished a base constructor. Now we can use the subclass's // type when resolving virtual calls. - const Decl *D = C.getLocationContext()->getDecl(); - recordFixedType(Target, cast<CXXConstructorDecl>(D), C); + const LocationContext *LCtx = C.getLocationContext(); + + // FIXME: In C++17 classes with non-virtual bases may be treated as + // aggregates, and in such case no top-frame constructor will be called. + // Figure out if we need to do anything in this case. + // FIXME: Instead of relying on the ParentMap, we should have the + // trigger-statement (InitListExpr in this case) available in this + // callback, ideally as part of CallEvent. + if (dyn_cast_or_null<InitListExpr>( + LCtx->getParentMap().getParent(Ctor->getOriginExpr()))) + return; + + recordFixedType(Target, cast<CXXConstructorDecl>(LCtx->getDecl()), C); } return; } Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=321128&r1=321127&r2=321128&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Tue Dec 19 16:40:38 2017 @@ -14,6 +14,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/StmtCXX.h" +#include "clang/AST/ParentMap.h" #include "clang/Basic/PrettyStackTrace.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" @@ -267,6 +268,23 @@ void ExprEngine::VisitCXXConstructExpr(c } // FALLTHROUGH case CXXConstructExpr::CK_NonVirtualBase: + // In C++17, classes with non-virtual bases may be aggregates, so they would + // be initialized as aggregates without a constructor call, so we may have + // a base class constructed directly into an initializer list without + // having the derived-class constructor call on the previous stack frame. + // Initializer lists may be nested into more initializer lists that + // correspond to surrounding aggregate initializations. + // FIXME: For now this code essentially bails out. We need to find the + // correct target region and set it. + // FIXME: Instead of relying on the ParentMap, we should have the + // trigger-statement (InitListExpr in this case) passed down from CFG or + // otherwise always available during construction. + if (dyn_cast_or_null<InitListExpr>(LCtx->getParentMap().getParent(CE))) { + MemRegionManager &MRMgr = getSValBuilder().getRegionManager(); + Target = MRMgr.getCXXTempObjectRegion(CE, LCtx); + break; + } + // FALLTHROUGH case CXXConstructExpr::CK_Delegating: { const CXXMethodDecl *CurCtor = cast<CXXMethodDecl>(LCtx->getDecl()); Loc ThisPtr = getSValBuilder().getCXXThis(CurCtor, Modified: cfe/trunk/test/Analysis/initializer.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/initializer.cpp?rev=321128&r1=321127&r2=321128&view=diff ============================================================================== --- cfe/trunk/test/Analysis/initializer.cpp (original) +++ cfe/trunk/test/Analysis/initializer.cpp Tue Dec 19 16:40:38 2017 @@ -1,4 +1,5 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks,debug.ExprInspection -analyzer-config c++-inlining=constructors -std=c++11 -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks,debug.ExprInspection -analyzer-config c++-inlining=constructors -std=c++17 -DCPLUSPLUS17 -verify %s void clang_analyzer_eval(bool); @@ -224,3 +225,42 @@ void testPassListsWithExplicitConstructo (void)(std::initializer_list<int>){12}; // no-crash } } + +namespace CXX17_aggregate_construction { +struct A { + A(); +}; + +struct B: public A { +}; + +struct C: public B { +}; + +struct D: public virtual A { +}; + +// In C++17, classes B and C are aggregates, so they will be constructed +// without actually calling their trivial constructor. Used to crash. +void foo() { + B b = {}; // no-crash + const B &bl = {}; // no-crash + B &&br = {}; // no-crash + + C c = {}; // no-crash + const C &cl = {}; // no-crash + C &&cr = {}; // no-crash + + D d = {}; // no-crash + +#ifdef CPLUSPLUS17 + C cd = {{}}; // no-crash + const C &cdl = {{}}; // no-crash + C &&cdr = {{}}; // no-crash + + const B &bll = {{}}; // no-crash + const B &bcl = C({{}}); // no-crash + B &&bcr = C({{}}); // no-crash +#endif +} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
