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
