[clang-tools-extra] r372037 - [clang-tidy] add checks to bugprone-posix-return
Jian Cai via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 16 14:43:56 PDT 2019
Author: jcai19
Date: Mon Sep 16 14:43:56 2019
New Revision: 372037
URL: http://llvm.org/viewvc/llvm-project?rev=372037&view=rev
Log:
[clang-tidy] add checks to bugprone-posix-return
This check now also checks if any calls to pthread_* functions expect negative return values. These functions return either 0 on success or an errno on failure, which is positive only.
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/PosixReturnCheck.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/PosixReturnCheck.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-posix-return.rst
clang-tools-extra/trunk/test/clang-tidy/bugprone-posix-return.cpp
Modified: clang-tools-extra/trunk/clang-tidy/bugprone/PosixReturnCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/PosixReturnCheck.cpp?rev=372037&r1=372036&r2=372037&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/PosixReturnCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/PosixReturnCheck.cpp Mon Sep 16 14:43:56 2019
@@ -28,26 +28,31 @@ static StringRef getFunctionSpelling(con
}
void PosixReturnCheck::registerMatchers(MatchFinder *Finder) {
- Finder->addMatcher(binaryOperator(hasOperatorName("<"),
- hasLHS(callExpr(callee(functionDecl(
- matchesName("^::posix_"),
- unless(hasName("::posix_openpt")))))),
- hasRHS(integerLiteral(equals(0))))
- .bind("ltzop"),
- this);
- Finder->addMatcher(binaryOperator(hasOperatorName(">="),
- hasLHS(callExpr(callee(functionDecl(
- matchesName("^::posix_"),
- unless(hasName("::posix_openpt")))))),
- hasRHS(integerLiteral(equals(0))))
- .bind("atop"),
- this);
+ Finder->addMatcher(
+ binaryOperator(
+ hasOperatorName("<"),
+ hasLHS(callExpr(callee(functionDecl(
+ anyOf(matchesName("^::posix_"), matchesName("^::pthread_")),
+ unless(hasName("::posix_openpt")))))),
+ hasRHS(integerLiteral(equals(0))))
+ .bind("ltzop"),
+ this);
+ Finder->addMatcher(
+ binaryOperator(
+ hasOperatorName(">="),
+ hasLHS(callExpr(callee(functionDecl(
+ anyOf(matchesName("^::posix_"), matchesName("^::pthread_")),
+ unless(hasName("::posix_openpt")))))),
+ hasRHS(integerLiteral(equals(0))))
+ .bind("atop"),
+ this);
Finder->addMatcher(
binaryOperator(
anyOf(hasOperatorName("=="), hasOperatorName("!="),
hasOperatorName("<="), hasOperatorName("<")),
hasLHS(callExpr(callee(functionDecl(
- matchesName("^::posix_"), unless(hasName("::posix_openpt")))))),
+ anyOf(matchesName("^::posix_"), matchesName("^::pthread_")),
+ unless(hasName("::posix_openpt")))))),
hasRHS(unaryOperator(hasOperatorName("-"),
hasUnaryOperand(integerLiteral()))))
.bind("binop"),
Modified: clang-tools-extra/trunk/clang-tidy/bugprone/PosixReturnCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/PosixReturnCheck.h?rev=372037&r1=372036&r2=372037&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/PosixReturnCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/PosixReturnCheck.h Mon Sep 16 14:43:56 2019
@@ -1,4 +1,4 @@
-//===--- PosixReturnCheck.h - clang-tidy-------------------------*- C++ -*-===//
+//===--- PosixReturnCheck.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.
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=372037&r1=372036&r2=372037&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Mon Sep 16 14:43:56 2019
@@ -91,6 +91,11 @@ Improvements to clang-tidy
Finds historical use of ``unsigned`` to hold vregs and physregs and rewrites
them to use ``Register``
+- Improved :doc:`bugprone-posix-return
+ <clang-tidy/checks/bugprone-posix-return>` check.
+
+ Now also checks if any calls to ``pthread_*`` functions expect negative return
+ values.
Improvements to include-fixer
-----------------------------
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-posix-return.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-posix-return.rst?rev=372037&r1=372036&r2=372037&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-posix-return.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-posix-return.rst Mon Sep 16 14:43:56 2019
@@ -3,9 +3,9 @@
bugprone-posix-return
=====================
-Checks if any calls to POSIX functions (except ``posix_openpt``) expect negative
-return values. These functions return either ``0`` on success or an ``errno`` on failure,
-which is positive only.
+Checks if any calls to ``pthread_*`` or ``posix_*`` functions
+(except ``posix_openpt``) 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:
Modified: clang-tools-extra/trunk/test/clang-tidy/bugprone-posix-return.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-posix-return.cpp?rev=372037&r1=372036&r2=372037&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-posix-return.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-posix-return.cpp Mon Sep 16 14:43:56 2019
@@ -9,6 +9,16 @@ typedef long off_t;
typedef decltype(sizeof(int)) size_t;
typedef struct __posix_spawn_file_actions* posix_spawn_file_actions_t;
typedef struct __posix_spawnattr* posix_spawnattr_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 struct _opaque_pthread_t *__darwin_pthread_t;
+typedef __darwin_pthread_t pthread_t;
+typedef struct pthread_attr_t_ *pthread_attr_t;
extern "C" int posix_fadvise(int fd, off_t offset, off_t len, int advice);
extern "C" int posix_fallocate(int fd, off_t offset, off_t len);
@@ -23,6 +33,12 @@ extern "C" int posix_spawnp(pid_t *pid,
const posix_spawn_file_actions_t *file_actions,
const posix_spawnattr_t *attrp,
char *const argv[], char *const envp[]);
+extern "C" int pthread_create(pthread_t *thread, const pthread_attr_t *attr, void *(*start_routine)(void *), void *arg);
+extern "C" int pthread_attr_setaffinity_np(pthread_attr_t *attr, size_t cpusetsize, const cpu_set_t *cpuset);
+extern "C" int pthread_attr_setschedpolicy(pthread_attr_t *attr, int policy);
+extern "C" int pthread_attr_init(pthread_attr_t *attr);
+extern "C" int pthread_yield(void);
+
void warningLessThanZero() {
if (posix_fadvise(0, 0, 0, 0) < 0) {}
@@ -43,11 +59,38 @@ void warningLessThanZero() {
if (posix_spawnp(NULL, NULL, NULL, NULL, {NULL}, {NULL}) < 0) {}
// CHECK-MESSAGES: :[[@LINE-1]]:60: warning:
// CHECK-FIXES: posix_spawnp(NULL, NULL, NULL, NULL, {NULL}, {NULL}) > 0
+ 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 (posix_fadvise(0, 0, 0, 0) >= 0) {}
// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: the comparison always evaluates to true because posix_fadvise always returns non-negative values
+ 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() {
@@ -69,6 +112,15 @@ void warningEqualsNegative() {
// CHECK-MESSAGES: :[[@LINE-1]]:59: warning:
if (posix_spawnp(NULL, NULL, NULL, NULL, {NULL}, {NULL}) == -1) {}
// CHECK-MESSAGES: :[[@LINE-1]]:60: warning:
+ 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() {
@@ -85,6 +137,20 @@ void WarningWithMacro() {
// CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
if (posix_fadvise(0, 0, 0, 0) < NEGATIVE_ONE) {}
// CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+ 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:
+
}
void noWarning() {
@@ -100,6 +166,7 @@ void noWarning() {
namespace i {
int posix_fadvise(int fd, off_t offset, off_t len, int advice);
+int pthread_yield(void);
void noWarning() {
if (posix_fadvise(0, 0, 0, 0) < 0) {}
@@ -108,6 +175,12 @@ void noWarning() {
if (posix_fadvise(0, 0, 0, 0) != -1) {}
if (posix_fadvise(0, 0, 0, 0) <= -1) {}
if (posix_fadvise(0, 0, 0, 0) < -1) {}
+ if (pthread_yield() < 0) {}
+ if (pthread_yield() >= 0) {}
+ if (pthread_yield() == -1) {}
+ if (pthread_yield() != -1) {}
+ if (pthread_yield() <= -1) {}
+ if (pthread_yield() < -1) {}
}
} // namespace i
@@ -115,6 +188,7 @@ void noWarning() {
class G {
public:
int posix_fadvise(int fd, off_t offset, off_t len, int advice);
+ int pthread_yield(void);
void noWarning() {
if (posix_fadvise(0, 0, 0, 0) < 0) {}
@@ -123,5 +197,11 @@ class G {
if (posix_fadvise(0, 0, 0, 0) != -1) {}
if (posix_fadvise(0, 0, 0, 0) <= -1) {}
if (posix_fadvise(0, 0, 0, 0) < -1) {}
+ if (pthread_yield() < 0) {}
+ if (pthread_yield() >= 0) {}
+ if (pthread_yield() == -1) {}
+ if (pthread_yield() != -1) {}
+ if (pthread_yield() <= -1) {}
+ if (pthread_yield() < -1) {}
}
};
More information about the cfe-commits
mailing list