[PATCH] D61816: [CFG] [analyzer] pr41300: Add a branch to skip virtual base initializers when they are handled by the superclass.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet.
Herald added a project: clang.

A copy-paste from https://bugs.llvm.org/show_bug.cgi?id=41300#c4:

> A cleaner repro:
> 
>   #include <iostream>
>   
>   struct S {
>     S() { std::cout << "  Calling S()" << std::endl; }
>   };
>   
>   struct A {
>     A() { std::cout << "  Calling A()" << std::endl; }
>     A(S s) { std::cout << "  Calling A(S)" << std::endl; }
>   };
>   
>   struct B : virtual A {
>     B() : A(S()) { std::cout << "  Calling B()" << std::endl; }
>   };
>   
>   struct C : B {
>     C() { std::cout << "  Calling C()" << std::endl; }
>   };
>   
>   int main() {
>     std::cout << "Constructing 'b':" << std::endl;
>     B b;
>   
>     std::cout << "Constructing 'c':" << std::endl;
>     C c;
>     return 0;
>   }
> 
> 
> The output of this program if you run it:
> 
> Constructing 'b':
> 
>   Calling S()
>   Calling A(S)
>   Calling B()
> 
> Constructing 'c':
> 
>   Calling A()
>   Calling B()
>   Calling C()
>    
> 
> So, like, when calling the constructor for class `B` from class `C`, we silently (even under -Weverything) ignore the initializer `A(S())`. That is, we don't even compute `S()`, we just ignore it the whole initializer. But when we invoke `B()` directly, we do compute `S()`.
> 
> This behavior implemented by compiling two versions of `B()`: one with the initializer for `A` and one without it, as can be seen in https://godbolt.org/z/8Sc-Re
> 
> However in the AST there's only one constructor for `B()`, so we should probably implement a runtime branch while modeling a `CXXCtorInitializer` in an inlined constructor call.

This patch adds the run-time CFG branch that would skip initialization of virtual base classes depending on whether the constructor is called from a superclass constructor or not. Previously the Static Analyzer was already skipping virtual base-class initializers in such constructors, but it wasn't skipping its arguments and their potential side effects, which was causing a crash (and was generally incorrect). The previous skipping behavior is replaced with a hard assertion that we're not even getting there due to how our CFG works.

The new CFG element is under a CFG build option so that not to break other consumers of the CFG by this change. Static Analyzer support for this change is implemented.

I've no idea how to write `CFGBuilder` code correctly because it's confusing because everything depends on the global mutable state of `Block` and `Succ` in the CFGBuilder. But i hope that `initializers-cfg-output.cpp`, together with the newly added test, provides some basic test coverage for my code.

Note that a similar functionality is necessary for destructors of virtual bases, but it remains to be done for now. We should be able to re-use the same terminator kind.


Repository:
  rC Clang

https://reviews.llvm.org/D61816

Files:
  clang/include/clang/Analysis/AnalysisDeclContext.h
  clang/include/clang/Analysis/CFG.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
  clang/lib/Analysis/AnalysisDeclContext.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
  clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/test/Analysis/initializer.cpp
  clang/test/Analysis/initializers-cfg-output.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61816.199119.patch
Type: text/x-patch
Size: 19772 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190511/f61b8e39/attachment-0001.bin>