NoQ created this revision. Herald added subscribers: cfe-commits, rnkovacs.
Yet another situation where we cannot easily guess, by looking at the CFG, the target region for C++ constructor call. Consider the following brace initialization: struct A { A(); }; struct B : public A { int x; }; void foo() { B b = {}; } AST in C++14: `-VarDecl 0x7ff07884eab8 <col:3, col:10> col:5 b 'struct B' cinit `-CXXConstructExpr 0x7ff07887a940 <col:9, col:10> 'struct B' 'void (void) noexcept(false)' list zeroing AST in C++17: `-VarDecl 0x7fb1cf012b18 <col:3, col:10> col:5 b 'struct B' cinit `-InitListExpr 0x7fb1cf012f38 <col:9, col:10> 'struct B' |-CXXConstructExpr 0x7fb1cf012fd0 <col:10> 'struct A' 'void (void)' list `-ImplicitValueInitExpr 0x7fb1cf013000 <<invalid sloc>> 'int' CFG in C++14: [B1] 1: {} (CXXConstructExpr, struct B) 2: B b = {}; CFG in C++17: [B1] 1: {} (CXXConstructExpr, struct A) 2: /*implicit*/(int)0 3: {} 4: B b = {}; So, essentially, in C++17 we don't have the trivial constructor call for `B` present anywhere at all, neither in AST, nor in CFG. This causes a crash in the analyzer because he tries to find the target region for `CK_NonVirtualBase` constructor of `A` by looking at the parent stack frame, expecting to find the derived class constructor (for `B`) here, and then taking a base region within its this-object. This fix is a quick-and-dirty suppression for the crash. It follows the tradition of "when unsure, make up a temporary and construct into that, then discard the result" - which is wrong, but at least it gets some invalidation done. In our example it would be correct to construct into `base{A, b}`. There can also be situations when such initializer-list is instead lifetime-extended by a `const&` or `&&` reference (see the tests), so in this case we'd need to construct into a respective sub-region of the temporary (but not into the temporary itself, of course). Finally, from the perspective of checker development, i believe it would be useful to actually revive the constructor call, so that PreCall/PostCall event occured. Repository: rC Clang https://reviews.llvm.org/D40841 Files: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp lib/StaticAnalyzer/Core/ExprEngineCXX.cpp test/Analysis/initializer.cpp
Index: test/Analysis/initializer.cpp =================================================================== --- test/Analysis/initializer.cpp +++ test/Analysis/initializer.cpp @@ -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); @@ -220,3 +221,40 @@ C c{x}; // no-warning } } + +namespace CXX17_default_brace_syntax { +struct A { + A(); +}; + +struct B: public A { +}; + +struct C: public B { +}; + +struct D: public virtual A { +}; + +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 = {}; + +#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 +} +} Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -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" @@ -277,6 +278,19 @@ } // FALLTHROUGH case CXXConstructExpr::CK_NonVirtualBase: + // FIXME: support C++17 construction into initializer lists. Instead of the + // incorrect temporary region we use as a placeholder here, we should figure + // out what region is being initialized by the surrounding InitListExpr and + // construct into the relevant part of it. We might as well model the + // trivial constructor call that's missing in the AST and CFG in this case + // (which is why we cannot simply get the target region by sub-regioning the + // this-region of the parent stack frame). + 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, Index: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp +++ lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp @@ -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,14 @@ 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: Support C++17 constructors into initializer lists. + if (dyn_cast_or_null<InitListExpr>( + LCtx->getParentMap().getParent(Ctor->getOriginExpr()))) + return; + + recordFixedType(Target, cast<CXXConstructorDecl>(LCtx->getDecl()), C); } return; }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits