[clang-tidy] Move 'cert-msc51-cpp', 'cert-msc32-c' checks outside of 'cert' module and give a proper name (llvm#167143)

…'cert' module and give a proper name

Summary:
This change addresses sub-issue llvm#157296 of parent issue llvm#157287, which
aims to clean up the `cert` module by moving checks that don't directly
map to CERT security standards to more appropriate modules.

The checks `cert-msc51-cpp` and `cert-msc32-c` both implement the same
logic to detect random number generators that are not properly seeded.
This is a generic bug pattern rather than a CERT-specific security
constraint, making it more suitable for the `bugprone` module.

Closes: llvm#157296

---------

Co-authored-by: Baranov Victor <[email protected]>

Author: luobochuanqi

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

@@ -65,6 +65,7 @@
 #include "ParentVirtualCallCheck.h"
 #include "PointerArithmeticOnPolymorphicObjectCheck.h"
 #include "PosixReturnCheck.h"
+#include "RandomGeneratorSeedCheck.h"
 #include "RawMemoryCallOnNonTrivialTypeCheck.h"
 #include "RedundantBranchConditionCheck.h"
 #include "ReservedIdentifierCheck.h"
@@ -230,6 +231,8 @@ class BugproneModule : public ClangTidyModule {
     CheckFactories.registerCheck<ParentVirtualCallCheck>(
         "bugprone-parent-virtual-call");
     CheckFactories.registerCheck<PosixReturnCheck>("bugprone-posix-return");
+    CheckFactories.registerCheck<RandomGeneratorSeedCheck>(
+        "bugprone-random-generator-seed");
     CheckFactories.registerCheck<RawMemoryCallOnNonTrivialTypeCheck>(
         "bugprone-raw-memory-call-on-non-trivial-type");
     CheckFactories.registerCheck<ReservedIdentifierCheck>(

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

@@ -66,6 +66,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   ParentVirtualCallCheck.cpp
   PointerArithmeticOnPolymorphicObjectCheck.cpp
   PosixReturnCheck.cpp
+  RandomGeneratorSeedCheck.cpp
   RawMemoryCallOnNonTrivialTypeCheck.cpp
   RedundantBranchConditionCheck.cpp
   ReservedIdentifierCheck.cpp

clang-tools-extra/clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp renamed to clang-tools-extra/clang-tidy/bugprone/RandomGeneratorSeedCheck.cpp

@@ -6,29 +6,28 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "ProperlySeededRandomGeneratorCheck.h"
+#include "RandomGeneratorSeedCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "llvm/ADT/STLExtras.h"
 
 using namespace clang::ast_matchers;
 
-namespace clang::tidy::cert {
+namespace clang::tidy::bugprone {
 
-ProperlySeededRandomGeneratorCheck::ProperlySeededRandomGeneratorCheck(
-    StringRef Name, ClangTidyContext *Context)
+RandomGeneratorSeedCheck::RandomGeneratorSeedCheck(StringRef Name,
+                                                   ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       RawDisallowedSeedTypes(
           Options.get("DisallowedSeedTypes", "time_t,std::time_t")) {
   RawDisallowedSeedTypes.split(DisallowedSeedTypes, ',');
 }
 
-void ProperlySeededRandomGeneratorCheck::storeOptions(
-    ClangTidyOptions::OptionMap &Opts) {
+void RandomGeneratorSeedCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "DisallowedSeedTypes", RawDisallowedSeedTypes);
 }
 
-void ProperlySeededRandomGeneratorCheck::registerMatchers(MatchFinder *Finder) {
+void RandomGeneratorSeedCheck::registerMatchers(MatchFinder *Finder) {
   auto RandomGeneratorEngineDecl = cxxRecordDecl(hasAnyName(
       "::std::linear_congruential_engine", "::std::mersenne_twister_engine",
       "::std::subtract_with_carry_engine", "::std::discard_block_engine",
@@ -75,8 +74,7 @@ void ProperlySeededRandomGeneratorCheck::registerMatchers(MatchFinder *Finder) {
       this);
 }
 
-void ProperlySeededRandomGeneratorCheck::check(
-    const MatchFinder::MatchResult &Result) {
+void RandomGeneratorSeedCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructExpr>("ctor");
   if (Ctor)
     checkSeed(Result, Ctor);
@@ -91,8 +89,8 @@ void ProperlySeededRandomGeneratorCheck::check(
 }
 
 template <class T>
-void ProperlySeededRandomGeneratorCheck::checkSeed(
-    const MatchFinder::MatchResult &Result, const T *Func) {
+void RandomGeneratorSeedCheck::checkSeed(const MatchFinder::MatchResult &Result,
+                                         const T *Func) {
   if (Func->getNumArgs() == 0 || Func->getArg(0)->isDefaultArgument()) {
     diag(Func->getExprLoc(),
          "random number generator seeded with a default argument will generate "
@@ -118,4 +116,4 @@ void ProperlySeededRandomGeneratorCheck::checkSeed(
   }
 }
 
-} // namespace clang::tidy::cert
+} // namespace clang::tidy::bugprone

clang-tools-extra/clang-tidy/cert/ProperlySeededRandomGeneratorCheck.h renamed to clang-tools-extra/clang-tidy/bugprone/RandomGeneratorSeedCheck.h

@@ -6,24 +6,24 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_PROPERLYSEEDEDRANDOMGENERATORCHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_PROPERLYSEEDEDRANDOMGENERATORCHECK_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RANDOMGENERATORSEEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RANDOMGENERATORSEEDCHECK_H
 
 #include "../ClangTidyCheck.h"
 #include <string>
 
-namespace clang::tidy::cert {
+namespace clang::tidy::bugprone {
 
 /// Random number generator must be seeded properly.
 ///
 /// A random number generator initialized with default value or a
 /// constant expression is a security vulnerability.
 ///
 /// For the user-facing documentation see:
-/// https://clang.llvm.org/extra/clang-tidy/checks/cert/msc51-cpp.html
-class ProperlySeededRandomGeneratorCheck : public ClangTidyCheck {
+/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/random-generator-seed.html
+class RandomGeneratorSeedCheck : public ClangTidyCheck {
 public:
-  ProperlySeededRandomGeneratorCheck(StringRef Name, ClangTidyContext *Context);
+  RandomGeneratorSeedCheck(StringRef Name, ClangTidyContext *Context);
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
@@ -37,6 +37,6 @@ class ProperlySeededRandomGeneratorCheck : public ClangTidyCheck {
   SmallVector<StringRef, 5> DisallowedSeedTypes;
 };
 
-} // namespace clang::tidy::cert
+} // namespace clang::tidy::bugprone
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_PROPERLYSEEDEDRANDOMGENERATORCHECK_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RANDOMGENERATORSEEDCHECK_H

clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp

@@ -16,6 +16,7 @@
 #include "../bugprone/ExceptionCopyConstructorThrowsCheck.h"
 #include "../bugprone/FloatLoopCounterCheck.h"
 #include "../bugprone/PointerArithmeticOnPolymorphicObjectCheck.h"
+#include "../bugprone/RandomGeneratorSeedCheck.h"
 #include "../bugprone/RawMemoryCallOnNonTrivialTypeCheck.h"
 #include "../bugprone/ReservedIdentifierCheck.h"
 #include "../bugprone/SignalHandlerCheck.h"
@@ -41,7 +42,6 @@
 #include "../readability/EnumInitialValueCheck.h"
 #include "../readability/UppercaseLiteralSuffixCheck.h"
 #include "LimitedRandomnessCheck.h"
-#include "ProperlySeededRandomGeneratorCheck.h"
 
 namespace {
 
@@ -272,7 +272,7 @@ class CERTModule : public ClangTidyModule {
             "cert-mem57-cpp");
     // MSC
     CheckFactories.registerCheck<LimitedRandomnessCheck>("cert-msc50-cpp");
-    CheckFactories.registerCheck<ProperlySeededRandomGeneratorCheck>(
+    CheckFactories.registerCheck<bugprone::RandomGeneratorSeedCheck>(
         "cert-msc51-cpp");
     CheckFactories.registerCheck<bugprone::SignalHandlerCheck>(
         "cert-msc54-cpp");
@@ -325,7 +325,7 @@ class CERTModule : public ClangTidyModule {
     CheckFactories.registerCheck<bugprone::UnsafeFunctionsCheck>(
         "cert-msc24-c");
     CheckFactories.registerCheck<LimitedRandomnessCheck>("cert-msc30-c");
-    CheckFactories.registerCheck<ProperlySeededRandomGeneratorCheck>(
+    CheckFactories.registerCheck<bugprone::RandomGeneratorSeedCheck>(
         "cert-msc32-c");
     CheckFactories.registerCheck<bugprone::UnsafeFunctionsCheck>(
         "cert-msc33-c");

clang-tools-extra/clang-tidy/cert/CMakeLists.txt

@@ -6,7 +6,6 @@ set(LLVM_LINK_COMPONENTS
 add_clang_library(clangTidyCERTModule STATIC
   CERTTidyModule.cpp
   LimitedRandomnessCheck.cpp
-  ProperlySeededRandomGeneratorCheck.cpp
 
   LINK_LIBS
   clangTidy

clang-tools-extra/docs/ReleaseNotes.rst

@@ -283,6 +283,16 @@ New check aliases
   <clang-tidy/checks/bugprone/default-operator-new-on-overaligned-type>`
   keeping initial check as an alias to the new one.
 
+- Renamed :doc:`cert-msc32-c <clang-tidy/checks/cert/msc32-c>` to
+  :doc:`bugprone-random-generator-seed
+  <clang-tidy/checks/bugprone/random-generator-seed>`
+  keeping initial check as an alias to the new one.
+
+- Renamed :doc:`cert-msc51-cpp <clang-tidy/checks/cert/msc51-cpp>` to
+  :doc:`bugprone-random-generator-seed
+  <clang-tidy/checks/bugprone/random-generator-seed>`
+  keeping initial check as an alias to the new one.
+
 - Renamed :doc:`cert-oop57-cpp <clang-tidy/checks/cert/oop57-cpp>` to
   :doc:`bugprone-raw-memory-call-on-non-trivial-type
   <clang-tidy/checks/bugprone/raw-memory-call-on-non-trivial-type>`

clang-tools-extra/docs/clang-tidy/checks/bugprone/random-generator-seed.rst

@@ -0,0 +1,44 @@
+.. title:: clang-tidy - bugprone-random-generator-seed
+
+bugprone-random-generator-seed
+==============================
+
+Flags all pseudo-random number engines, engine adaptor
+instantiations and ``srand()`` when initialized or seeded with default argument,
+constant expression or any user-configurable type. Pseudo-random number
+engines seeded with a predictable value may cause vulnerabilities e.g. in
+security protocols.
+
+Examples:
+
+.. code-block:: c++
+
+  void foo() {
+    std::mt19937 engine1; // Diagnose, always generate the same sequence
+    std::mt19937 engine2(1); // Diagnose
+    engine1.seed(); // Diagnose
+    engine2.seed(1); // Diagnose
+
+    std::time_t t;
+    engine1.seed(std::time(&t)); // Diagnose, system time might be controlled by user
+
+    int x = atoi(argv[1]);
+    std::mt19937 engine3(x);  // Will not warn
+  }
+
+Options
+-------
+
+.. option:: DisallowedSeedTypes
+
+   A comma-separated list of the type names which are disallowed.
+   Default value is `time_t,std::time_t`.
+
+References
+----------
+
+This check corresponds to the CERT C++ Coding Standard rules
+`MSC51-CPP. Ensure your random number generator is properly seeded
+<https://wiki.sei.cmu.edu/confluence/display/cplusplus/MSC51-CPP.+Ensure+your+random+number+generator+is+properly+seeded>`_ and
+`MSC32-C. Properly seed pseudorandom number generators
+<https://wiki.sei.cmu.edu/confluence/display/c/MSC32-C.+Properly+seed+pseudorandom+number+generators>`_.

clang-tools-extra/docs/clang-tidy/checks/cert/msc32-c.rst

@@ -1,9 +1,14 @@
 .. title:: clang-tidy - cert-msc32-c
 .. meta::
-   :http-equiv=refresh: 5;URL=../cert/msc51-cpp.html
+   :http-equiv=refresh: 5;URL=../bugprone/random-generator-seed.html
 
 cert-msc32-c
 ============
 
 The `cert-msc32-c` check is an alias, please see
-:doc:`cert-msc51-cpp <../cert/msc51-cpp>` for more information.
+:doc:`bugprone-random-generator-seed <../bugprone/random-generator-seed>`
+for more information.
+
+This check corresponds to the CERT C Coding Standard rule
+`MSC32-C. Properly seed pseudorandom number generators
+<https://wiki.sei.cmu.edu/confluence/display/c/MSC32-C.+Properly+seed+pseudorandom+number+generators>`_.

clang-tools-extra/docs/clang-tidy/checks/cert/msc51-cpp.rst

@@ -1,40 +1,14 @@
 .. title:: clang-tidy - cert-msc51-cpp
+.. meta::
+   :http-equiv=refresh: 5;URL=../bugprone/random-generator-seed.html
 
 cert-msc51-cpp
 ==============
 
-This check flags all pseudo-random number engines, engine adaptor
-instantiations and ``srand()`` when initialized or seeded with default argument,
-constant expression or any user-configurable type. Pseudo-random number
-engines seeded with a predictable value may cause vulnerabilities e.g. in
-security protocols.
-This is a CERT security rule, see
-`MSC51-CPP. Ensure your random number generator is properly seeded
-<https://wiki.sei.cmu.edu/confluence/display/cplusplus/MSC51-CPP.+Ensure+your+random+number+generator+is+properly+seeded>`_ and
-`MSC32-C. Properly seed pseudorandom number generators
-<https://wiki.sei.cmu.edu/confluence/display/c/MSC32-C.+Properly+seed+pseudorandom+number+generators>`_.
-
-Examples:
-
-.. code-block:: c++
-
-  void foo() {
-    std::mt19937 engine1; // Diagnose, always generate the same sequence
-    std::mt19937 engine2(1); // Diagnose
-    engine1.seed(); // Diagnose
-    engine2.seed(1); // Diagnose
-
-    std::time_t t;
-    engine1.seed(std::time(&t)); // Diagnose, system time might be controlled by user
+The `cert-msc51-cpp` check is an alias, please see
+:doc:`bugprone-random-generator-seed <../bugprone/random-generator-seed>`
+for more information.
 
-    int x = atoi(argv[1]);
-    std::mt19937 engine3(x);  // Will not warn
-  }
-
-Options
--------
-
-.. option:: DisallowedSeedTypes
-
-   A comma-separated list of the type names which are disallowed.
-   Default value is `time_t,std::time_t`.
+This check corresponds to the CERT C++ Coding Standard rule
+`MSC51-CPP. Ensure your random number generator is properly seeded
+<https://wiki.sei.cmu.edu/confluence/display/cplusplus/MSC51-CPP.+Ensure+your+random+number+generator+is+properly+seeded>`_.