[clang] 59a960b - [analyzer] Skip analysis of inherited ctor as top-level function

[Gabor Marton via cfe-commits]

Author: Gabor Marton
Date: 2020-03-09T12:05:11+01:00
New Revision: 59a960b83c2d1559f31e1dd75728dd24fae2f68c

URL: https://github.com/llvm/llvm-project/commit/59a960b83c2d1559f31e1dd75728dd24fae2f68c
DIFF: https://github.com/llvm/llvm-project/commit/59a960b83c2d1559f31e1dd75728dd24fae2f68c.diff

LOG: [analyzer] Skip analysis of inherited ctor as top-level function

Summary:
This fixes a regression introduced in https://reviews.llvm.org/D74735

Reviewers: NoQ, Szelethus

Tags: #clang

Differential Revision: https://reviews.llvm.org/D75678

Added: 
    clang/test/Analysis/cxx-inherited-ctor-is-skipped-as-top-level.cpp

Modified: 
    clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
    clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
    clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index 60705dd27d6b..402acf667d53 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -896,6 +896,23 @@ class CXXConstructorCall : public AnyCXXConstructorCall {
 /// Represents a call to a C++ inherited constructor.
 ///
 /// Example: \c class T : public S { using S::S; }; T(1);
+///
+// Note, it is 
diff icult to model the parameters. This is one of the reasons
+// why we skip analysis of inheriting constructors as top-level functions.
+// CXXInheritedCtorInitExpr doesn't take arguments and doesn't model parameter
+// initialization because there is none: the arguments in the outer
+// CXXConstructExpr directly initialize the parameters of the base class
+// constructor, and no copies are made. (Making a copy of the parameter is
+// incorrect, at least if it's done in an observable way.) The derived class
+// constructor doesn't even exist in the formal model.
+/// E.g., in:
+///
+/// struct X { X *p = this; ~X() {} };
+/// struct A { A(X x) : b(x.p == &x) {} bool b; };
+/// struct B : A { using A::A; };
+/// B b = X{};
+///
+/// ... b.b is initialized to true.
 class CXXInheritedConstructorCall : public AnyCXXConstructorCall {
   friend class CallEventManager;
 

diff  --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index 417eb7ec3d31..a908aede68bb 100644
--- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -519,6 +519,13 @@ static bool shouldSkipFunction(const Decl *D,
   if (VisitedAsTopLevel.count(D))
     return true;
 
+  // Skip analysis of inheriting constructors as top-level functions. These
+  // constructors don't even have a body written down in the code, so even if
+  // we find a bug, we won't be able to display it.
+  if (const auto *CD = dyn_cast<CXXConstructorDecl>(D))
+    if (CD->isInheritingConstructor())
+      return true;
+
   // We want to re-analyse the functions as top level in the following cases:
   // - The 'init' methods should be reanalyzed because
   //   ObjCNonNilReturnValueChecker assumes that '[super init]' never returns

diff  --git a/clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp b/clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp
index ff82c129bc70..8370ebfbde09 100644
--- a/clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp
+++ b/clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp
@@ -57,3 +57,19 @@ void test_B() {
   clang_analyzer_eval(b.z == 3); // expected-warning{{TRUE}}
 }
 } // namespace arguments_with_constructors
+
+namespace inherited_constructor_crash {
+class a {
+public:
+  a(int);
+};
+struct b : a {
+  using a::a; // Ihnerited ctor.
+};
+void c() {
+  int d;
+  // This construct expr utilizes the inherited ctor.
+  // Note that d must be uninitialized to cause the crash.
+  (b(d)); // expected-warning{{1st function call argument is an uninitialized value}}
+}
+} // namespace inherited_constructor_crash

diff  --git a/clang/test/Analysis/cxx-inherited-ctor-is-skipped-as-top-level.cpp b/clang/test/Analysis/cxx-inherited-ctor-is-skipped-as-top-level.cpp
new file mode 100644
index 000000000000..be7982e64114
--- /dev/null
+++ b/clang/test/Analysis/cxx-inherited-ctor-is-skipped-as-top-level.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-display-progress %s 2>&1 | FileCheck %s
+
+// Test that inheriting constructors are not analyzed as top-level functions.
+
+// CHECK: ANALYZE (Path,  Inline_Regular): {{.*}} c()
+// CHECK: ANALYZE (Path,  Inline_Regular): {{.*}} a::a(int)
+// CHECK-NOT: ANALYZE (Path,  Inline_Regular): {{.*}} b::a(int)
+
+class a {
+public:
+  a(int) {}
+};
+struct b : a {
+  using a::a; // Ihnerited ctor.
+};
+void c() {
+  int d;
+  (b(d));
+  (a(d));
+}