jcai19 updated this revision to Diff 216768.
jcai19 added a comment.
Fix typos.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66627/new/
https://reviews.llvm.org/D66627
Files:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/PthreadReturnCheck.cpp
clang-tools-extra/clang-tidy/bugprone/PthreadReturnCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone-pthread-return.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/bugprone-pthread-return.cpp
Index: clang-tools-extra/test/clang-tidy/bugprone-pthread-return.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/bugprone-pthread-return.cpp
@@ -0,0 +1,187 @@
+// RUN: %check_clang_tidy %s bugprone-pthread-return %t
+
+#define NULL nullptr
+#define ZERO 0
+#define NEGATIVE_ONE -1
+
+typedef decltype(sizeof(int)) size_t;
+# define __CPU_SETSIZE 1024
+# define __NCPUBITS (8 * sizeof (__cpu_mask))
+typedef unsigned long int __cpu_mask;
+typedef struct
+{
+ __cpu_mask __bits[__CPU_SETSIZE / __NCPUBITS];
+} cpu_set_t;
+typedef int clockid_t;
+# define _SIGSET_NWORDS (1024 / (8 * sizeof (unsigned long int)))
+typedef struct
+{
+ unsigned long int __val[_SIGSET_NWORDS];
+} sigset_t;
+typedef struct _opaque_pthread_t *__darwin_pthread_t;
+typedef __darwin_pthread_t pthread_t;
+typedef struct pthread_attr_t_ *pthread_attr_t;
+typedef struct pthread_mutex_t_ * pthread_mutex_t;
+typedef struct pthread_mutexattr_t_ * pthread_mutexattr_t;
+typedef struct pthread_rwlockattr_t_ * pthread_rwlockattr_t;
+typedef struct pthread_spinlock_t_ * pthread_spinlock_t;
+
+
+extern "C" void pthread_cleanup_push(void (*routine)(void *), void *arg);
+extern "C" void pthread_cleanup_pop(int execute);
+extern "C" void pthread_cleanup_push_defer_np(void (*routine)(void *), void *arg);
+extern "C" void pthread_cleanup_pop_restore_np(int execute);
+extern "C" void pthread_exit(void *retval);
+extern "C" void pthread_kill_other_threads_np(void);
+extern "C" void pthread_testcancel(void);
+
+extern "C" pthread_t pthread_self(void);
+
+extern "C" int pthread_attr_init(pthread_attr_t *attr);
+extern "C" int pthread_attr_destroy(pthread_attr_t *attr);
+extern "C" int pthread_attr_setaffinity_np(pthread_attr_t *attr, size_t cpusetsize, const cpu_set_t *cpuset);
+extern "C" int pthread_attr_getaffinity_np(const pthread_attr_t *attr, size_t cpusetsize, cpu_set_t *cpuset);
+extern "C" int pthread_attr_setdetachstate(pthread_attr_t *attr, int detachstate);
+extern "C" int pthread_attr_getdetachstate(const pthread_attr_t *attr, int *detachstate);
+extern "C" int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize);
+extern "C" int pthread_attr_getguardsize(const pthread_attr_t *attr, size_t *guardsize);
+extern "C" int pthread_attr_setinheritsched(pthread_attr_t *attr, int inheritsched);
+extern "C" int pthread_attr_getinheritsched(const pthread_attr_t *attr, int *inheritsched);
+extern "C" int pthread_attr_setschedparam(pthread_attr_t *attr, const struct sched_param *param);
+extern "C" int pthread_attr_getschedparam(const pthread_attr_t *attr, struct sched_param *param);
+extern "C" int pthread_attr_setschedpolicy(pthread_attr_t *attr, int policy);
+extern "C" int pthread_attr_getschedpolicy(const pthread_attr_t *attr, int *policy);
+extern "C" int pthread_attr_setscope(pthread_attr_t *attr, int scope);
+extern "C" int pthread_attr_getscope(const pthread_attr_t *attr, int *scope);
+extern "C" int pthread_attr_setstack(pthread_attr_t *attr, void *stackaddr, size_t stacksize);
+extern "C" int pthread_attr_getstack(const pthread_attr_t *attr, void **stackaddr, size_t *stacksize);
+extern "C" int pthread_attr_setstackaddr(pthread_attr_t *attr, void *stackaddr);
+extern "C" int pthread_attr_getstackaddr(const pthread_attr_t *attr, void **stackaddr);
+extern "C" int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize);
+extern "C" int pthread_attr_getstacksize(const pthread_attr_t *attr, size_t *stacksize);
+extern "C" int pthread_cancel(pthread_t thread);
+extern "C" int pthread_create(pthread_t *thread, const pthread_attr_t *attr, void *(*start_routine)(void *), void *arg);
+extern "C" int pthread_detach(pthread_t thread);
+extern "C" int pthread_equal(pthread_t t1, pthread_t t2);
+extern "C" int pthread_setaffinity_np(pthread_t thread, size_t cpusetsize, const cpu_set_t *cpuset);
+extern "C" int pthread_getaffinity_np(pthread_t thread, size_t cpusetsize, cpu_set_t *cpuset);
+extern "C" int pthread_getattr_default_np(pthread_attr_t *attr);
+extern "C" int pthread_setattr_default_np(pthread_attr_t *attr);
+extern "C" int pthread_getattr_np(pthread_t thread, pthread_attr_t *attr);
+extern "C" int pthread_setconcurrency(int new_level);
+extern "C" int pthread_getconcurrency(void);
+extern "C" int pthread_getcpuclockid(pthread_t thread, clockid_t *clock_id);
+extern "C" int pthread_setname_np(pthread_t thread, const char *name);
+extern "C" int pthread_getname_np(pthread_t thread, char *name, size_t len);
+extern "C" int pthread_setschedparam(pthread_t thread, int policy, const struct sched_param *param);
+extern "C" int pthread_getschedparam(pthread_t thread, int *policy, struct sched_param *param);
+extern "C" int pthread_join(pthread_t thread, void **retval);
+extern "C" int pthread_kill(pthread_t thread, int sig);
+extern "C" int pthread_mutexattr_getpshared(const pthread_mutexattr_t *attr, int *pshared);
+extern "C" int pthread_mutexattr_setpshared(pthread_mutexattr_t *attr, int pshared);
+extern "C" int pthread_mutexattr_getrobust(const pthread_mutexattr_t *attr, int *robustness);
+extern "C" int pthread_mutexattr_setrobust(const pthread_mutexattr_t *attr, int robustness);
+extern "C" int pthread_mutexattr_getrobust_np(const pthread_mutexattr_t *attr, int *robustness);
+extern "C" int pthread_mutexattr_setrobust_np(const pthread_mutexattr_t *attr, int robustness);
+extern "C" int pthread_mutex_consistent(pthread_mutex_t *mutex);
+extern "C" int pthread_rwlockattr_setkind_np(pthread_rwlockattr_t *attr, int pref);
+extern "C" int pthread_rwlockattr_getkind_np(const pthread_rwlockattr_t *attr, int *pref);
+extern "C" int pthread_setcancelstate(int state, int *oldstate);
+extern "C" int pthread_setcanceltype(int type, int *oldtype);
+extern "C" int pthread_setschedprio(pthread_t thread, int prio);
+extern "C" int pthread_sigmask(int how, const sigset_t *set, sigset_t *oldset);
+extern "C" int pthread_sigqueue(pthread_t thread, int sig, const union sigval value);
+extern "C" int pthread_spin_init(pthread_spinlock_t *lock, int pshared);
+extern "C" int pthread_spin_destroy(pthread_spinlock_t *lock);
+extern "C" int pthread_spin_lock(pthread_spinlock_t *lock);
+extern "C" int pthread_spin_trylock(pthread_spinlock_t *lock);
+extern "C" int pthread_spin_unlock(pthread_spinlock_t *lock);
+extern "C" int pthread_tryjoin_np(pthread_t thread, void **retval);
+extern "C" int pthread_timedjoin_np(pthread_t thread, void **retval, const struct timespec *abstime);
+extern "C" int pthread_yield(void);
+
+void warningLessThanZero() {
+ if (pthread_create(NULL, NULL, NULL, NULL) < 0) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:46: warning: the comparison always evaluates to false because pthread_create always returns non-negative values
+ // CHECK-FIXES: pthread_create(NULL, NULL, NULL, NULL) > 0
+ if (pthread_attr_setaffinity_np(NULL, 0, NULL) < 0) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:50: warning:
+ // CHECK-FIXES: pthread_attr_setaffinity_np(NULL, 0, NULL) > 0
+ if (pthread_attr_setschedpolicy(NULL, 0) < 0) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:44: warning:
+ // CHECK-FIXES: pthread_attr_setschedpolicy(NULL, 0) > 0)
+ if (pthread_attr_init(NULL) < 0) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning:
+ // CHECK-FIXES: pthread_attr_init(NULL) > 0
+ if (pthread_yield() < 0) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning:
+ // CHECK-FIXES: pthread_yield() > 0
+}
+
+void warningAlwaysTrue() {
+ if (pthread_create(NULL, NULL, NULL, NULL) >= 0) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:46: warning: the comparison always evaluates to true because pthread_create always returns non-negative values
+ if (pthread_attr_setaffinity_np(NULL, 0, NULL) >= 0) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:50: warning:
+ if (pthread_attr_setschedpolicy(NULL, 0) >= 0) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:44: warning:
+ if (pthread_attr_init(NULL) >= 0) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning:
+ if (pthread_yield() >= 0) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning:
+}
+
+void warningEqualsNegative() {
+ if (pthread_create(NULL, NULL, NULL, NULL) == -1) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:46: warning: pthread_create
+ if (pthread_create(NULL, NULL, NULL, NULL) != -1) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
+ if (pthread_create(NULL, NULL, NULL, NULL) <= -1) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
+ if (pthread_create(NULL, NULL, NULL, NULL) < -1) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
+}
+
+void WarningWithMacro() {
+ if (pthread_create(NULL, NULL, NULL, NULL) < ZERO) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
+ // CHECK-FIXES: pthread_create(NULL, NULL, NULL, NULL) > ZERO
+ if (pthread_create(NULL, NULL, NULL, NULL) >= ZERO) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
+ if (pthread_create(NULL, NULL, NULL, NULL) == NEGATIVE_ONE) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
+ if (pthread_create(NULL, NULL, NULL, NULL) != NEGATIVE_ONE) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
+ if (pthread_create(NULL, NULL, NULL, NULL) <= NEGATIVE_ONE) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
+ if (pthread_create(NULL, NULL, NULL, NULL) < NEGATIVE_ONE) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
+}
+
+namespace i {
+int pthread_create(pthread_t *thread, const pthread_attr_t *attr, void *(*start_routine)(void *), void *arg);
+
+void noWarning() {
+ if (pthread_create(NULL, NULL, NULL, NULL) < 0) {}
+ if (pthread_create(NULL, NULL, NULL, NULL) >= 0) {}
+ if (pthread_create(NULL, NULL, NULL, NULL) == -1) {}
+ if (pthread_create(NULL, NULL, NULL, NULL) != -1) {}
+ if (pthread_create(NULL, NULL, NULL, NULL) <= -1) {}
+ if (pthread_create(NULL, NULL, NULL, NULL) < -1) {}
+}
+
+} // namespace i
+
+class G {
+public:
+ int pthread_create(pthread_t *thread, const pthread_attr_t *attr, void *(*start_routine)(void *), void *arg);
+
+ void noWarning() {
+ if (pthread_create(NULL, NULL, NULL, NULL) < 0) {}
+ if (pthread_create(NULL, NULL, NULL, NULL) >= 0) {}
+ if (pthread_create(NULL, NULL, NULL, NULL) == -1) {}
+ if (pthread_create(NULL, NULL, NULL, NULL) != -1) {}
+ if (pthread_create(NULL, NULL, NULL, NULL) <= -1) {}
+ if (pthread_create(NULL, NULL, NULL, NULL) < -1) {}
+ }
+};
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
@@ -60,6 +60,7 @@
bugprone-multiple-statement-macro
bugprone-parent-virtual-call
bugprone-posix-return
+ bugprone-pthread-return
bugprone-sizeof-container
bugprone-sizeof-expression
bugprone-string-constructor
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-pthread-return.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-pthread-return.rst
@@ -0,0 +1,20 @@
+.. title:: clang-tidy - bugprone-pthread-return
+
+bugprone-posix-return
+=====================
+
+Checks if any calls to POSIX thread functions expect negative return values.
+These functions return either ``0`` on success or an ``errno`` on failure,
+which is positive only.
+
+Example buggy usage looks like:
+
+.. code-block:: c
+
+ if (posix_create(...) < 0) {
+
+This will never happen as the return value is always non-negative. A simple fix could be:
+
+.. code-block:: c
+
+ if (posix_create(...) > 0) {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,6 +67,11 @@
Improvements to clang-tidy
--------------------------
+- New :doc:`bugprone-pthread-return
+ <clang-tidy/checks/bugprone-pthread-return>` check.
+
+ Checks if any calls to POSIX thread functions expect negative return values.
+
- New :doc:`linuxkernel-must-use-errs
<clang-tidy/checks/linuxkernel-must-use-errs>` check.
@@ -79,7 +84,6 @@
Finds uses of deprecated Googletest APIs with names containing ``case`` and
replaces them with equivalent APIs with ``suite``.
-
Improvements to include-fixer
-----------------------------
Index: clang-tools-extra/clang-tidy/bugprone/PthreadReturnCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/PthreadReturnCheck.h
@@ -0,0 +1,30 @@
+//===--- PthreadReturnCheck.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_BUGPRONE_PTHREAD_RETURN_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PTHREAD_RETURN_CHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+class PthreadReturnCheck: public ClangTidyCheck {
+public:
+ PthreadReturnCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PTHREAD_RETURN_CHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/PthreadReturnCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/PthreadReturnCheck.cpp
@@ -0,0 +1,81 @@
+//===--- PthreadReturnCheck.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 "PthreadReturnCheck.h"
+#include "../utils/Matchers.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+static StringRef getFunctionSpelling(const MatchFinder::MatchResult &Result,
+ const char *BindingStr) {
+ const auto *MatchedCall = cast<CallExpr>(
+ (Result.Nodes.getNodeAs<BinaryOperator>(BindingStr))->getLHS());
+ const SourceManager &SM = *Result.SourceManager;
+ return Lexer::getSourceText(CharSourceRange::getTokenRange(
+ MatchedCall->getCallee()->getSourceRange()),
+ SM, Result.Context->getLangOpts());
+}
+
+void PthreadReturnCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ binaryOperator(
+ hasOperatorName("<"),
+ hasLHS(callExpr(callee(functionDecl(matchesName("^::pthread_"))))),
+ hasRHS(integerLiteral(equals(0))))
+ .bind("ltzop"),
+ this);
+ Finder->addMatcher(
+ binaryOperator(
+ hasOperatorName(">="),
+ hasLHS(callExpr(callee(functionDecl(matchesName("^::pthread_"))))),
+ hasRHS(integerLiteral(equals(0))))
+ .bind("atop"),
+ this);
+ Finder->addMatcher(
+ binaryOperator(
+ anyOf(hasOperatorName("=="), hasOperatorName("!="),
+ hasOperatorName("<="), hasOperatorName("<")),
+ hasLHS(callExpr(callee(functionDecl(matchesName("^::pthread_"))))),
+ hasRHS(unaryOperator(hasOperatorName("-"),
+ hasUnaryOperand(integerLiteral()))))
+ .bind("binop"),
+ this);
+}
+
+void PthreadReturnCheck::check(const MatchFinder::MatchResult &Result) {
+ if (const auto *LessThanZeroOp =
+ Result.Nodes.getNodeAs<BinaryOperator>("ltzop")) {
+ SourceLocation OperatorLoc = LessThanZeroOp->getOperatorLoc();
+ diag(OperatorLoc, "the comparison always evaluates to false because %0 "
+ "always returns non-negative values")
+ << getFunctionSpelling(Result, "ltzop")
+ << FixItHint::CreateReplacement(OperatorLoc, Twine(">").str());
+ return;
+ }
+ if (const auto *AlwaysTrueOp =
+ Result.Nodes.getNodeAs<BinaryOperator>("atop")) {
+ diag(AlwaysTrueOp->getOperatorLoc(),
+ "the comparison always evaluates to true because %0 always returns "
+ "non-negative values")
+ << getFunctionSpelling(Result, "atop");
+ return;
+ }
+ const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binop");
+ diag(BinOp->getOperatorLoc(), "%0 only returns non-negative values")
+ << getFunctionSpelling(Result, "binop");
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -24,6 +24,7 @@
MultipleStatementMacroCheck.cpp
ParentVirtualCallCheck.cpp
PosixReturnCheck.cpp
+ PthreadReturnCheck.cpp
SizeofContainerCheck.cpp
SizeofExpressionCheck.cpp
StringConstructorCheck.cpp
Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -32,6 +32,7 @@
#include "MultipleStatementMacroCheck.h"
#include "ParentVirtualCallCheck.h"
#include "PosixReturnCheck.h"
+#include "PthreadReturnCheck.h"
#include "SizeofContainerCheck.h"
#include "SizeofExpressionCheck.h"
#include "StringConstructorCheck.h"
@@ -107,6 +108,8 @@
"bugprone-parent-virtual-call");
CheckFactories.registerCheck<PosixReturnCheck>(
"bugprone-posix-return");
+ CheckFactories.registerCheck<PthreadReturnCheck>(
+ "bugprone-pthread-return");
CheckFactories.registerCheck<SizeofContainerCheck>(
"bugprone-sizeof-container");
CheckFactories.registerCheck<SizeofExpressionCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
