[clang-tools-extra] [clang-tidy] Adding an initial version of the "Initialized Class Members" checker. (PR #65189)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 6 14:54:25 PDT 2023
https://github.com/adriannistor updated https://github.com/llvm/llvm-project/pull/65189:
>From c6d70f6f851f0553e3e83b762618d48744a4f706 Mon Sep 17 00:00:00 2001
From: Adrian Nistor <adriannistor at google.com>
Date: Fri, 1 Sep 2023 15:35:59 -0700
Subject: [PATCH 1/2] Adding an initial version of the "Initialized Class
Members" checker.
The goal of this checker is to eliminate UUM (Use of Uninitialized Memory) bugs caused by uninitialized class members.
This checker is different from ProTypeMemberInitCheck in that this checker attempts to eliminate UUMs as a bug class, at the expense of false positives.
This checker is WIP. We are incrementally adding features and increasing coverage until we get to a shape that is acceptable.
---
.../clang-tidy/google/CMakeLists.txt | 1 +
.../google/CppInitClassMembersCheck.cpp | 105 ++++++++++++++++++
.../google/CppInitClassMembersCheck.h | 51 +++++++++
.../clang-tidy/google/GoogleTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 7 ++
.../checks/google/cpp-init-class-members.rst | 32 ++++++
.../docs/clang-tidy/checks/list.rst | 1 +
.../google/cpp-init-class-members.cpp | 56 ++++++++++
8 files changed, 256 insertions(+)
create mode 100644 clang-tools-extra/clang-tidy/google/CppInitClassMembersCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/google/CppInitClassMembersCheck.h
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/google/cpp-init-class-members.rst
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/google/cpp-init-class-members.cpp
diff --git a/clang-tools-extra/clang-tidy/google/CMakeLists.txt b/clang-tools-extra/clang-tidy/google/CMakeLists.txt
index fcba2b1b214adce..527f682062bb358 100644
--- a/clang-tools-extra/clang-tidy/google/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/google/CMakeLists.txt
@@ -8,6 +8,7 @@ add_clang_library(clangTidyGoogleModule
AvoidNSObjectNewCheck.cpp
AvoidThrowingObjCExceptionCheck.cpp
AvoidUnderscoreInGoogletestNameCheck.cpp
+ CppInitClassMembersCheck.cpp
DefaultArgumentsCheck.cpp
ExplicitConstructorCheck.cpp
ExplicitMakePairCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/google/CppInitClassMembersCheck.cpp b/clang-tools-extra/clang-tidy/google/CppInitClassMembersCheck.cpp
new file mode 100644
index 000000000000000..fea8dcbd50ff031
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/google/CppInitClassMembersCheck.cpp
@@ -0,0 +1,105 @@
+//===--- CppInitClassMembersCheck.cpp - clang-tidy ------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include <string>
+
+#include "CppInitClassMembersCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Type.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::google {
+
+namespace {
+
+// Matches records that have a default constructor.
+AST_MATCHER(CXXRecordDecl, hasDefaultConstructor) {
+ return Node.hasDefaultConstructor();
+}
+
+// Returns the names of `Fields` in a comma separated string.
+std::string
+toCommaSeparatedString(const SmallVector<const FieldDecl *, 16> &Fields) {
+ std::string Buffer;
+ llvm::raw_string_ostream OS(Buffer);
+ llvm::interleave(
+ Fields, OS, [&OS](const FieldDecl *Decl) { OS << Decl->getName(); },
+ ", ");
+ return Buffer;
+}
+
+// Returns `true` for field types that should be reported (if additional
+// conditions are also met). For example, returns `true` for `int` because an
+// uninitialized `int` field can contain uninitialized values.
+bool shouldReportThisFieldType(QualType Ty) {
+ if (Ty.isNull())
+ return false;
+
+ // FIXME: For now, this checker focuses on several allowlisted types. We will
+ // expand coverage in future.
+ return Ty->isIntegerType() || Ty->isBooleanType();
+}
+
+} // anonymous namespace
+
+void CppInitClassMembersCheck::checkMissingMemberInitializer(
+ ASTContext &Context, const CXXRecordDecl &ClassDecl,
+ const CXXConstructorDecl *Ctor) {
+ SmallVector<const FieldDecl *, 16> FieldsToReport;
+
+ for (const FieldDecl *F : ClassDecl.fields()) {
+ if (shouldReportThisFieldType(F->getType()) && !F->hasInClassInitializer())
+ FieldsToReport.push_back(F);
+ }
+
+ if (FieldsToReport.empty())
+ return;
+
+ DiagnosticBuilder Diag =
+ diag(Ctor ? Ctor->getBeginLoc() : ClassDecl.getLocation(),
+ "%select{these fields should be initialized|constructor should "
+ "initialize these fields}0: %1")
+ << (Ctor != nullptr) << toCommaSeparatedString(FieldsToReport);
+
+ // FIXME: generate fixes.
+}
+
+void CppInitClassMembersCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(cxxConstructorDecl(isDefinition(), isDefaultConstructor(),
+ unless(isUserProvided()))
+ .bind("ctor"),
+ this);
+
+ Finder->addMatcher(cxxRecordDecl(isDefinition(), hasDefaultConstructor(),
+ unless(isInstantiated()),
+ unless(has(cxxConstructorDecl())))
+ .bind("record"),
+ this);
+}
+
+void CppInitClassMembersCheck::check(const MatchFinder::MatchResult &Result) {
+ if (const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor")) {
+ checkMissingMemberInitializer(*Result.Context, *Ctor->getParent(), Ctor);
+ } else if (const auto *Record =
+ Result.Nodes.getNodeAs<CXXRecordDecl>("record")) {
+ // For a record, perform the same action as for a constructor. However,
+ // emit the diagnostic for the record, not for the constructor.
+ checkMissingMemberInitializer(*Result.Context, *Record, nullptr);
+ }
+}
+
+} // namespace clang::tidy::google
\ No newline at end of file
diff --git a/clang-tools-extra/clang-tidy/google/CppInitClassMembersCheck.h b/clang-tools-extra/clang-tidy/google/CppInitClassMembersCheck.h
new file mode 100644
index 000000000000000..54fa3d233044470
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/google/CppInitClassMembersCheck.h
@@ -0,0 +1,51 @@
+//===--- CppInitClassMembersCheck.h - clang-tidy ----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_CPPINITCLASSMEMBERSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_CPPINITCLASSMEMBERSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::google {
+
+/// Checks that class members are initialized in constructors (implicitly or
+/// explicitly). Reports constructors or classes where class members are not
+/// initialized. The goal of this checker is to eliminate UUM (Use of
+/// Uninitialized Memory) bugs caused by uninitialized class members.
+///
+/// This checker is different from ProTypeMemberInitCheck in that this checker
+/// attempts to eliminate UUMs as a bug class, at the expense of false
+/// positives.
+///
+/// This checker is WIP. We are incrementally adding features and increasing
+/// coverage until we get to a shape that is acceptable.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/google/cpp-init-class-members.html
+class CppInitClassMembersCheck : public ClangTidyCheck {
+public:
+ CppInitClassMembersCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus && !LangOpts.ObjC;
+ }
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ // Issue a diagnostic for any constructor that does not initialize all member
+ // variables. If the record does not have a constructor (`Ctor` is `nullptr`),
+ // the diagnostic is for the record.
+ void checkMissingMemberInitializer(ASTContext &Context,
+ const CXXRecordDecl &ClassDecl,
+ const CXXConstructorDecl *Ctor);
+};
+
+} // namespace clang::tidy::google
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_CPPINITCLASSMEMBERSCHECK_H
\ No newline at end of file
diff --git a/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp b/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
index 830a37af1accf55..b861af9c7c84aa8 100644
--- a/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
@@ -16,6 +16,7 @@
#include "AvoidNSObjectNewCheck.h"
#include "AvoidThrowingObjCExceptionCheck.h"
#include "AvoidUnderscoreInGoogletestNameCheck.h"
+#include "CppInitClassMembersCheck.h"
#include "DefaultArgumentsCheck.h"
#include "ExplicitConstructorCheck.h"
#include "ExplicitMakePairCheck.h"
@@ -43,6 +44,8 @@ class GoogleModule : public ClangTidyModule {
"google-build-namespaces");
CheckFactories.registerCheck<build::UsingNamespaceDirectiveCheck>(
"google-build-using-namespace");
+ CheckFactories.registerCheck<CppInitClassMembersCheck>(
+ "google-cpp-init-class-members");
CheckFactories.registerCheck<DefaultArgumentsCheck>(
"google-default-arguments");
CheckFactories.registerCheck<ExplicitConstructorCheck>(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 95ee6daf7209e51..5a08cbcff12731b 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -148,6 +148,13 @@ New checks
Flags coroutines that suspend while a lock guard is in scope at the
suspension point.
+- New :doc:`google-cpp-init-class-members
+ <clang-tidy/checks/google/cpp-init-class-members>` check.
+
+ Checks that class members are initialized. This checker aims to completely
+ eliminate UUM (Use of Uninitialized Memory) bugs caused by uninitialized
+ class members.
+
- New :doc:`modernize-use-constraints
<clang-tidy/checks/modernize/use-constraints>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/google/cpp-init-class-members.rst b/clang-tools-extra/docs/clang-tidy/checks/google/cpp-init-class-members.rst
new file mode 100644
index 000000000000000..7940acbf05e25d9
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/google/cpp-init-class-members.rst
@@ -0,0 +1,32 @@
+.. title:: clang-tidy - google-cpp-init-class-members
+
+google-cpp-init-class-members
+=============================
+
+Checks that class members are initialized in constructors (implicitly or
+explicitly). Reports constructors or classes where class members are not
+initialized. The goal of this checker is to eliminate UUM (Use of
+Uninitialized Memory) bugs caused by uninitialized class members.
+
+This checker is different from ProTypeMemberInitCheck in that this checker
+attempts to eliminate UUMs as a bug class, at the expense of false
+positives.
+
+This checker is WIP. We are incrementally adding features and increasing
+coverage until we get to a shape that is acceptable.
+
+For now, this checker reports `X` in the following two patterns:
+
+.. code-block:: c++
+ class SomeClass {
+ public:
+ SomeClass() = default;
+
+ private:
+ int X;
+ };
+
+.. code-block:: c++
+ struct SomeStruct {
+ int X;
+ };
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 75a86431d264412..892f61322ebc05a 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -210,6 +210,7 @@ Clang-Tidy Checks
:doc:`google-build-explicit-make-pair <google/build-explicit-make-pair>`,
:doc:`google-build-namespaces <google/build-namespaces>`,
:doc:`google-build-using-namespace <google/build-using-namespace>`,
+ :doc:`google-cpp-init-class-members <google/cpp-init-class-members>`, "Yes"
:doc:`google-default-arguments <google/default-arguments>`,
:doc:`google-explicit-constructor <google/explicit-constructor>`, "Yes"
:doc:`google-global-names-in-headers <google/global-names-in-headers>`,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/cpp-init-class-members.cpp b/clang-tools-extra/test/clang-tidy/checkers/google/cpp-init-class-members.cpp
new file mode 100644
index 000000000000000..ca9601e2ebe6855
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/google/cpp-init-class-members.cpp
@@ -0,0 +1,56 @@
+// RUN: %check_clang_tidy %s google-cpp-init-class-members %t
+
+class PositiveDefaultedDefaultConstructor {
+public:
+ PositiveDefaultedDefaultConstructor() = default;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor should initialize these fields: X
+
+private:
+ int X;
+};
+
+class PositiveDefaultedDefaultConstructorWithInitializedField {
+public:
+ PositiveDefaultedDefaultConstructorWithInitializedField() = default;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor should initialize these fields: X
+
+private:
+ int X;
+ int Y = 4; // no-warning
+};
+
+class Helper {
+ public:
+ Helper(int x) : X(x) {}
+
+ private:
+ int X;
+};
+
+class PositiveDefaultedConstructorObjectAndPrimitive {
+ public:
+ PositiveDefaultedConstructorObjectAndPrimitive() = default;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor should initialize these fields: Y
+
+ Helper* GetHelper() { return &X; }
+
+ void SetY(bool enabled) { Y = enabled; }
+
+ bool IsY() { return Y; }
+
+ private:
+ Helper X;
+ bool Y;
+};
+
+struct PositiveStruct {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: these fields should be initialized: X, Y
+ int X;
+ int Y;
+};
+
+struct PositiveStructWithInitializedField {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: these fields should be initialized: Y
+ int X = 3; // no-warning
+ int Y;
+};
>From ef9c4b68c2ecaf6d092b3398f3bca9fb3e722d7d Mon Sep 17 00:00:00 2001
From: Adrian Nistor <adriannistor at google.com>
Date: Wed, 6 Sep 2023 14:09:02 -0700
Subject: [PATCH 2/2] Adding documentation to clarify that this is a checker
under active development.
---
.../clang-tidy/google/CppInitClassMembersCheck.h | 9 +++++----
clang-tools-extra/docs/ReleaseNotes.rst | 9 ++++++---
.../checks/google/cpp-init-class-members.rst | 10 ++++++----
3 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/google/CppInitClassMembersCheck.h b/clang-tools-extra/clang-tidy/google/CppInitClassMembersCheck.h
index 54fa3d233044470..63dac3640929ae8 100644
--- a/clang-tools-extra/clang-tidy/google/CppInitClassMembersCheck.h
+++ b/clang-tools-extra/clang-tidy/google/CppInitClassMembersCheck.h
@@ -18,13 +18,14 @@ namespace clang::tidy::google {
/// initialized. The goal of this checker is to eliminate UUM (Use of
/// Uninitialized Memory) bugs caused by uninitialized class members.
///
+/// This checker is under active development: the checker authors made a few
+/// commits and are actively working on more commits. Users who want a mature
+/// and stable checker should not use this checker yet.
+///
/// This checker is different from ProTypeMemberInitCheck in that this checker
/// attempts to eliminate UUMs as a bug class, at the expense of false
/// positives.
///
-/// This checker is WIP. We are incrementally adding features and increasing
-/// coverage until we get to a shape that is acceptable.
-///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/google/cpp-init-class-members.html
class CppInitClassMembersCheck : public ClangTidyCheck {
@@ -48,4 +49,4 @@ class CppInitClassMembersCheck : public ClangTidyCheck {
} // namespace clang::tidy::google
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_CPPINITCLASSMEMBERSCHECK_H
\ No newline at end of file
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_CPPINITCLASSMEMBERSCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 5a08cbcff12731b..adf934de25506b4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -151,9 +151,12 @@ New checks
- New :doc:`google-cpp-init-class-members
<clang-tidy/checks/google/cpp-init-class-members>` check.
- Checks that class members are initialized. This checker aims to completely
- eliminate UUM (Use of Uninitialized Memory) bugs caused by uninitialized
- class members.
+ Checks that class members are initialized in constructors (implicitly or
+ explicitly). This checker aims to completely eliminate UUM (Use of
+ Uninitialized Memory) bugs caused by uninitialized class members. This checker
+ is under active development: the checker authors made a few commits and are
+ actively working on more commits. Users who want a mature and stable checker
+ should not use this checker yet.
- New :doc:`modernize-use-constraints
<clang-tidy/checks/modernize/use-constraints>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/google/cpp-init-class-members.rst b/clang-tools-extra/docs/clang-tidy/checks/google/cpp-init-class-members.rst
index 7940acbf05e25d9..11eaf82e38eef19 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/google/cpp-init-class-members.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/google/cpp-init-class-members.rst
@@ -8,12 +8,14 @@ explicitly). Reports constructors or classes where class members are not
initialized. The goal of this checker is to eliminate UUM (Use of
Uninitialized Memory) bugs caused by uninitialized class members.
+This checker is under active development: the checker authors made a few commits
+and are actively working on more commits. Users who want a mature and stable
+checker should not use this checker yet.
+
This checker is different from ProTypeMemberInitCheck in that this checker
attempts to eliminate UUMs as a bug class, at the expense of false
-positives.
-
-This checker is WIP. We are incrementally adding features and increasing
-coverage until we get to a shape that is acceptable.
+positives. The authors of this checker will add more documentation about the
+differences with ProTypeMemberInitCheck as the checker evolves.
For now, this checker reports `X` in the following two patterns:
More information about the cfe-commits
mailing list