<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 5 December 2017 at 09:08, Artem Dergachev via Phabricator via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">NoQ created this revision.<br>
Herald added subscribers: cfe-commits, rnkovacs.<br>
<br>
Yet another situation where we cannot easily guess, by looking at the CFG, the target region for C++ constructor call.<br>
<br>
Consider the following brace initialization:<br>
<br>
  struct A {<br>
    A();<br>
   };<br>
<br>
  struct B : public A {<br>
    int x;<br>
  };<br>
<br>
void foo() {<br>
<br>
  B b = {};<br>
<br>
}<br>
<br>
AST in C++14:<br>
<br>
  `-VarDecl 0x7ff07884eab8 <col:3, col:10> col:5 b 'struct B' cinit<br>
    `-CXXConstructExpr 0x7ff07887a940 <col:9, col:10> 'struct B' 'void (void) noexcept(false)' list zeroing<br>
<br>
AST in C++17:<br>
<br>
  `-VarDecl 0x7fb1cf012b18 <col:3, col:10> col:5 b 'struct B' cinit<br>
    `-InitListExpr 0x7fb1cf012f38 <col:9, col:10> 'struct B'<br>
      |-CXXConstructExpr 0x7fb1cf012fd0 <col:10> 'struct A' 'void (void)' list<br>
      `-ImplicitValueInitExpr 0x7fb1cf013000 <<invalid sloc>> 'int'<br>
<br>
CFG in C++14:<br>
<br>
  [B1]<br>
    1: {} (CXXConstructExpr, struct B)<br>
    2: B b = {};<br>
<br>
CFG in C++17:<br>
<br>
  [B1]<br>
    1: {} (CXXConstructExpr, struct A)<br>
    2: /*implicit*/(int)0<br>
    3: {}<br>
    4: B b = {};<br>
<br>
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.<br>
<br>
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.<br>
<br>
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.<br>
<br>
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).<br>
<br>
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.</blockquote><div><br></div><div>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:</div><div><br></div><div>```</div><div><div>struct A {</div><div>  A();</div><div>};</div><div><br></div><div>struct B {</div><div>  A a;</div><div>  int x;</div><div>};</div><div><br></div><div>void foo() {</div><div>  B b = {};</div><div>}                                                        </div></div><div>```</div><div><br></div><div>The analyzer should presumably treat the new example the exact same way it treats that case.</div></div></div></div>