[PATCH] D40841: [analyzer] Fix a crash on C++17 AST for non-trivial construction into a trivial brace initializer.

[Artem Dergachev via Phabricator via cfe-commits]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40841.125544.patch
Type: text/x-patch
Size: 4131 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171205/8867e3c2/attachment.bin>