This revision was automatically updated to reflect the committed changes. martong marked an inline comment as done. Closed by commit rG59a960b83c2d: [analyzer] Skip analysis of inherited ctor as top-level function (authored by martong).
Changed prior to commit: https://reviews.llvm.org/D75678?vs=248678&id=249058#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75678/new/ https://reviews.llvm.org/D75678 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp clang/test/Analysis/cxx-inherited-ctor-is-skipped-as-top-level.cpp Index: clang/test/Analysis/cxx-inherited-ctor-is-skipped-as-top-level.cpp =================================================================== --- /dev/null +++ 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)); +} Index: clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp =================================================================== --- clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp +++ clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp @@ -57,3 +57,19 @@ 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 Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp =================================================================== --- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp +++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp @@ -519,6 +519,13 @@ 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 Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h @@ -896,6 +896,23 @@ /// Represents a call to a C++ inherited constructor. /// /// Example: \c class T : public S { using S::S; }; T(1); +/// +// Note, it is difficult 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;
Index: clang/test/Analysis/cxx-inherited-ctor-is-skipped-as-top-level.cpp =================================================================== --- /dev/null +++ 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)); +} Index: clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp =================================================================== --- clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp +++ clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp @@ -57,3 +57,19 @@ 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 Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp =================================================================== --- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp +++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp @@ -519,6 +519,13 @@ 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 Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h @@ -896,6 +896,23 @@ /// Represents a call to a C++ inherited constructor. /// /// Example: \c class T : public S { using S::S; }; T(1); +/// +// Note, it is difficult 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;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits