On 5 December 2017 at 09:08, Artem Dergachev via Phabricator via
cfe-commits <cfe-commits@lists.llvm.org> wrote:
> 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.
Note that there is no constructor call here. This is aggregate
initialization. And there's not really any part of this that's new, except
that a class with base classes is now an a aggregate. You'll see the same
kind of AST formed in all C++ language modes with a slightly modified
example:
```
struct A {
A();
};
struct B {
A a;
int x;
};
void foo() {
B b = {};
}
```
The analyzer should presumably treat the new example the exact same way it
treats that case.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
