abelkocsis created this revision.
abelkocsis added reviewers: aaron.ballman, alexfh, hokein, jfb.
abelkocsis added projects: clang-tools-extra, clang.
Herald added subscribers: cfe-commits, mgehre, dexonsmith, mgorny.
According to
https://wiki.sei.cmu.edu/confluence/display/cplusplus/CON54-CPP.+Wrap+functions+that+can+spuriously+wake+up+in+a+loop
and
https://wiki.sei.cmu.edu/confluence/display/c/CON36-C.+Wrap+functions+that+can+spuriously+wake+up+in+a+loop
misc-spuriously-wake-up-functions check is created. The check finds
``cnd_wait`` or `wait` function calls in an ``IfStmt`` and warns the user to
replace it with a ``WhileStmt`` or use it with a lambda parameter.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D70876
Files:
clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
clang-tools-extra/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/clang-tidy/misc/SpuriouslyWakeUpFunctionsCheck.cpp
clang-tools-extra/clang-tidy/misc/SpuriouslyWakeUpFunctionsCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cert-con36-c.rst
clang-tools-extra/docs/clang-tidy/checks/cert-con54-cpp.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/misc-spuriously-wake-up-functions.rst
clang-tools-extra/test/clang-tidy/misc-spuriously-wake-up-functions.cpp
Index: clang-tools-extra/test/clang-tidy/misc-spuriously-wake-up-functions.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/misc-spuriously-wake-up-functions.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy %s misc-spuriously-wake-up-functions %t -- -- -I %S/../../../libcxx/include/
+
+#include "chrono"
+#include "condition_variable"
+#include "mutex"
+
+struct Node1 {
+ void *Node1;
+ struct Node1 *next;
+};
+
+static Node1 list;
+static std::mutex m;
+static std::condition_variable condition;
+
+void consume_list_element(std::condition_variable &condition) {
+ std::unique_lock<std::mutex> lk(m);
+
+ if (list.next == nullptr) {
+ condition.wait(lk);
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'wait' should be placed inside a while statement or used with a condition parameter [misc-spuriously-wake-up-functions]
+ }
+
+ while (list.next == nullptr) {
+ condition.wait(lk);
+ }
+
+ if (list.next == nullptr) {
+ while (list.next == nullptr) {
+ condition.wait(lk);
+ }
+ }
+ using durtype = std::chrono::duration<int, std::milli>;
+ durtype dur = std::chrono::duration<int, std::milli>();
+ if (list.next == nullptr) {
+ condition.wait_for(lk, dur);
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'wait_for' should be placed inside a while statement or used with a condition parameter [misc-spuriously-wake-up-functions]
+ }
+ if (list.next == nullptr) {
+ condition.wait_for(lk, dur, [] { return 1; });
+ }
+ auto now = std::chrono::system_clock::now();
+ if (list.next == nullptr) {
+ condition.wait_until(lk, now + dur);
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'wait_until' should be placed inside a while statement or used with a condition parameter [misc-spuriously-wake-up-functions]
+ }
+ if (list.next == nullptr) {
+ condition.wait_until(lk, now + dur, [] { return 1; });
+ }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-spuriously-wake-up-functions.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-spuriously-wake-up-functions.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - misc-spuriously-wake-up-functions
+
+misc-spuriously-wake-up-functions
+=================================
+
+Finds ``cnd_wait`` or ``wait`` function calls in an ``IfStmt`` and tries to
+replace it with ``WhileStmt``.
+
+.. code-block: c++
+
+ if (condition_predicate) {
+ condition.wait(lk);
+ }
+
+.. code-block: c
+
+ if (condition_predicate) {
+ if (thrd_success != cnd_wait(&condition, &lock)) {
+ }
+ }
+
+This check corresponds to the CERT C++ Coding Standard rule
+`CON54-CPP. Wrap functions that can spuriously wake up in a loop
+<https://wiki.sei.cmu.edu/confluence/display/cplusplus/CON54-CPP.+Wrap+functions+that+can+spuriously+wake+up+in+a+loop>`_.
+and CERT C Coding Standard rule
+`CON36-C. Wrap functions that can spuriously wake up in a loop
+<https://wiki.sei.cmu.edu/confluence/display/c/CON36-C.+Wrap+functions+that+can+spuriously+wake+up+in+a+loop>`_.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -84,6 +84,8 @@
bugprone-unused-return-value
bugprone-use-after-move
bugprone-virtual-near-miss
+ cert-con36-c (redirects to misc-spuriously-wake-up-functions) <cert-con36-c>
+ cert-con54-cpp (redirects to misc-spuriously-wake-up-functions) <cert-con54-cpp>
cert-dcl03-c (redirects to misc-static-assert) <cert-dcl03-c>
cert-dcl16-c (redirects to readability-uppercase-literal-suffix) <cert-dcl16-c>
cert-dcl21-cpp
@@ -290,6 +292,7 @@
misc-non-copyable-objects
misc-non-private-member-variables-in-classes
misc-redundant-expression
+ misc-spuriously-wake-up-functions
misc-static-assert
misc-throw-by-value-catch-by-reference
misc-unconventional-assign-operator
Index: clang-tools-extra/docs/clang-tidy/checks/cert-con54-cpp.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cert-con54-cpp.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - cert-con54-cpp
+.. meta::
+:http-equiv=refresh: 5;URL=misc-spuriously-wake-up-functions.html
+
+cert-con54-cpp
+==============
+
+The cert-con54-cpp check is an alias, please see
+`misc-spuriously-wake-up-functions <misc-spuriously-wake-up-functions.html>`_
+for more information.
Index: clang-tools-extra/docs/clang-tidy/checks/cert-con36-c.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cert-con36-c.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - cert-con36-c
+.. meta::
+:http-equiv=refresh: 5;URL=misc-spuriously-wake-up-functions.html
+
+cert-con36-c
+============
+
+The cert-con36-c check is an alias, please see
+`misc-spuriously-wake-up-functions <misc-spuriously-wake-up-functions.html>`_
+for more information.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -94,6 +94,16 @@
Without the null terminator it can result in undefined behaviour when the
string is read.
+- New alias :doc:`cert-con36-c
+ <clang-tidy/checks/cert-con36-c>` to
+ :doc:`misc-spuriously-wake-up-functions
+ <clang-tidy/checks/misc-spuriously-wake-up-functions>` was added.
+
+- New alias :doc:`cert-con54-cpp
+ <clang-tidy/checks/cert-con54-cpp>` to
+ :doc:`misc-spuriously-wake-up-functions
+ <clang-tidy/checks/misc-spuriously-wake-up-functions>` was added.
+
- New :doc:`cert-mem57-cpp
<clang-tidy/checks/cert-mem57-cpp>` check.
@@ -179,6 +189,13 @@
Finds non-static member functions that can be made ``const``
because the functions don't use ``this`` in a non-const way.
+- New :doc:`misc-spuriously-wake-up-functions
+ <clang-tidy/checks/misc-spuriously-wake-up-functions>` check.
+
+ Finds ``cnd_wait`` or ``wait`` function calls when the function is not
+ invoked from a loop that checks whether a condition predicate holds or the
+ function has a condition parameter.
+
- Improved :doc:`modernize-use-override
<clang-tidy/checks/modernize-use-override>` check.
Index: clang-tools-extra/clang-tidy/misc/SpuriouslyWakeUpFunctionsCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/SpuriouslyWakeUpFunctionsCheck.h
@@ -0,0 +1,35 @@
+//===--- SpuriouslyWakeUpFunctionsCheck.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_MISC_SPURIOUSLYWAKEUPFUNCTIONSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SPURIOUSLYWAKEUPFUNCTIONSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds ``cnd_wait`` or `wait` function calls in an ``IfStmt`` and tries to
+/// replace it with ``WhileStm``.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-spuriously-wake-up-functions.html
+class SpuriouslyWakeUpFunctionsCheck : public ClangTidyCheck {
+public:
+ SpuriouslyWakeUpFunctionsCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SPURIOUSLYWAKEUPFUNCTIONSCHECK_H
Index: clang-tools-extra/clang-tidy/misc/SpuriouslyWakeUpFunctionsCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/SpuriouslyWakeUpFunctionsCheck.cpp
@@ -0,0 +1,87 @@
+//===--- SpuriouslyWakeUpFunctionsCheck.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 "SpuriouslyWakeUpFunctionsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void SpuriouslyWakeUpFunctionsCheck::registerMatchers(MatchFinder *Finder) {
+
+ if (getLangOpts().CPlusPlus) {
+ // Check for `CON54-CPP`
+ auto hasUniqueLock = hasDescendant(declRefExpr(hasDeclaration(
+ varDecl(hasType(asString("std::unique_lock<std::mutex>"))))));
+
+ auto hasWaitDescendantCPP = hasDescendant(
+ cxxMemberCallExpr(
+ anyOf(
+ allOf(hasDescendant(memberExpr(hasDeclaration(functionDecl(
+ allOf(hasName("std::condition_variable::wait"),
+ parameterCountIs(1)))))),
+ onImplicitObjectArgument(declRefExpr(to(varDecl(
+ hasType(asString("std::condition_variable &")))))),
+ hasUniqueLock),
+ allOf(hasDescendant(memberExpr(hasDeclaration(functionDecl(
+ allOf(hasName("std::condition_variable::wait_for"),
+ parameterCountIs(2)))))),
+ onImplicitObjectArgument(declRefExpr(to(varDecl(
+ hasType(asString("std::condition_variable &")))))),
+ hasUniqueLock),
+ allOf(hasDescendant(memberExpr(hasDeclaration(functionDecl(
+ allOf(hasName("std::condition_variable::wait_until"),
+ parameterCountIs(2)))))),
+ onImplicitObjectArgument(declRefExpr(to(varDecl(
+ hasType(asString("std::condition_variable &")))))),
+ hasUniqueLock)
+
+ ))
+ .bind("wait"));
+
+ Finder->addMatcher(
+ ifStmt(
+ allOf(hasWaitDescendantCPP,
+ unless(anyOf(hasDescendant(ifStmt(hasWaitDescendantCPP)),
+ hasDescendant(whileStmt(hasWaitDescendantCPP)),
+ hasDescendant(forStmt(hasWaitDescendantCPP)),
+ hasDescendant(doStmt(hasWaitDescendantCPP)))))),
+ this);
+ } else {
+ // Check for `CON36-C`
+ auto hasWaitDescendantC =
+ hasDescendant(callExpr(callee(functionDecl(allOf(hasName("cnd_wait"),
+ parameterCountIs(2)))))
+ .bind("wait"));
+ Finder->addMatcher(
+ ifStmt(allOf(hasWaitDescendantC,
+ unless(anyOf(hasDescendant(ifStmt(hasWaitDescendantC)),
+ hasDescendant(whileStmt(hasWaitDescendantC)),
+ hasDescendant(forStmt(hasWaitDescendantC)),
+ hasDescendant(doStmt(hasWaitDescendantC)))))),
+ this);
+ }
+}
+
+void SpuriouslyWakeUpFunctionsCheck::check(
+ const MatchFinder::MatchResult &Result) {
+
+ const auto *MatchedWait = Result.Nodes.getNodeAs<CallExpr>("wait");
+ diag(
+ MatchedWait->getExprLoc(),
+ "'%0' should be placed inside a while statement or used with a condition "
+ "parameter")
+ << MatchedWait->getDirectCallee()->getName();
+}
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -15,6 +15,7 @@
#include "NonCopyableObjects.h"
#include "NonPrivateMemberVariablesInClassesCheck.h"
#include "RedundantExpressionCheck.h"
+#include "SpuriouslyWakeUpFunctionsCheck.h"
#include "StaticAssertCheck.h"
#include "ThrowByValueCatchByReferenceCheck.h"
#include "UnconventionalAssignOperatorCheck.h"
@@ -41,6 +42,8 @@
"misc-non-private-member-variables-in-classes");
CheckFactories.registerCheck<RedundantExpressionCheck>(
"misc-redundant-expression");
+ CheckFactories.registerCheck<SpuriouslyWakeUpFunctionsCheck>(
+ "misc-spuriously-wake-up-functions");
CheckFactories.registerCheck<StaticAssertCheck>("misc-static-assert");
CheckFactories.registerCheck<ThrowByValueCatchByReferenceCheck>(
"misc-throw-by-value-catch-by-reference");
Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -8,6 +8,7 @@
NonCopyableObjects.cpp
NonPrivateMemberVariablesInClassesCheck.cpp
RedundantExpressionCheck.cpp
+ SpuriouslyWakeUpFunctionsCheck.cpp
StaticAssertCheck.cpp
ThrowByValueCatchByReferenceCheck.cpp
UnconventionalAssignOperatorCheck.cpp
Index: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -15,6 +15,7 @@
#include "../misc/NewDeleteOverloadsCheck.h"
#include "../misc/NonCopyableObjects.h"
#include "../misc/StaticAssertCheck.h"
+#include "../misc/SpuriouslyWakeUpFunctionsCheck.h"
#include "../misc/ThrowByValueCatchByReferenceCheck.h"
#include "../performance/MoveConstructorInitCheck.h"
#include "../readability/UppercaseLiteralSuffixCheck.h"
@@ -39,6 +40,9 @@
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
// C++ checkers
+ // CON
+ CheckFactories.registerCheck<misc::SpuriouslyWakeUpFunctionsCheck>(
+ "cert-con54-cpp");
// DCL
CheckFactories.registerCheck<PostfixOperatorCheck>(
"cert-dcl21-cpp");
@@ -71,6 +75,9 @@
"cert-oop54-cpp");
// C checkers
+ // CON
+ CheckFactories.registerCheck<misc::SpuriouslyWakeUpFunctionsCheck>(
+ "cert-con36-c");
// DCL
CheckFactories.registerCheck<misc::StaticAssertCheck>("cert-dcl03-c");
CheckFactories.registerCheck<readability::UppercaseLiteralSuffixCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
