[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
cfe-commits at lists.llvm.org
Fri May 10 20:14:43 PDT 2019
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>
More information about the cfe-commits
mailing list