Skip to content

Commit 9d2066a

Browse files
jcai19jcai19
committed
[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. llvm-svn: 372037
1 parent ec80f53 commit 9d2066a

File tree

5 files changed

+109
-19
lines changed

5 files changed

+109
-19
lines changed

‎clang-tools-extra/clang-tidy/bugprone/PosixReturnCheck.cpp

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,26 +28,31 @@ static StringRef getFunctionSpelling(const MatchFinder::MatchResult &Result,
2828
}
2929

3030
void PosixReturnCheck::registerMatchers(MatchFinder *Finder) {
31-
Finder->addMatcher(binaryOperator(hasOperatorName("<"),
32-
hasLHS(callExpr(callee(functionDecl(
33-
matchesName("^::posix_"),
34-
unless(hasName("::posix_openpt")))))),
35-
hasRHS(integerLiteral(equals(0))))
36-
.bind("ltzop"),
37-
this);
38-
Finder->addMatcher(binaryOperator(hasOperatorName(">="),
39-
hasLHS(callExpr(callee(functionDecl(
40-
matchesName("^::posix_"),
41-
unless(hasName("::posix_openpt")))))),
42-
hasRHS(integerLiteral(equals(0))))
43-
.bind("atop"),
44-
this);
31+
Finder->addMatcher(
32+
binaryOperator(
33+
hasOperatorName("<"),
34+
hasLHS(callExpr(callee(functionDecl(
35+
anyOf(matchesName("^::posix_"), matchesName("^::pthread_")),
36+
unless(hasName("::posix_openpt")))))),
37+
hasRHS(integerLiteral(equals(0))))
38+
.bind("ltzop"),
39+
this);
40+
Finder->addMatcher(
41+
binaryOperator(
42+
hasOperatorName(">="),
43+
hasLHS(callExpr(callee(functionDecl(
44+
anyOf(matchesName("^::posix_"), matchesName("^::pthread_")),
45+
unless(hasName("::posix_openpt")))))),
46+
hasRHS(integerLiteral(equals(0))))
47+
.bind("atop"),
48+
this);
4549
Finder->addMatcher(
4650
binaryOperator(
4751
anyOf(hasOperatorName("=="), hasOperatorName("!="),
4852
hasOperatorName("<="), hasOperatorName("<")),
4953
hasLHS(callExpr(callee(functionDecl(
50-
matchesName("^::posix_"), unless(hasName("::posix_openpt")))))),
54+
anyOf(matchesName("^::posix_"), matchesName("^::pthread_")),
55+
unless(hasName("::posix_openpt")))))),
5156
hasRHS(unaryOperator(hasOperatorName("-"),
5257
hasUnaryOperand(integerLiteral()))))
5358
.bind("binop"),

‎clang-tools-extra/clang-tidy/bugprone/PosixReturnCheck.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//===--- PosixReturnCheck.h - clang-tidy-------------------------*- C++ -*-===//
1+
//===--- PosixReturnCheck.h - clang-tidy ------------------------*- C++ -*-===//
22
//
33
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
44
// See https://llvm.org/LICENSE.txt for license information.

‎clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@ Improvements to clang-tidy
9191
Finds historical use of ``unsigned`` to hold vregs and physregs and rewrites
9292
them to use ``Register``
9393

94+
- Improved :doc:`bugprone-posix-return
95+
<clang-tidy/checks/bugprone-posix-return>` check.
96+
97+
Now also checks if any calls to ``pthread_*`` functions expect negative return
98+
values.
9499

95100
Improvements to include-fixer
96101
-----------------------------

‎clang-tools-extra/docs/clang-tidy/checks/bugprone-posix-return.rst

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
bugprone-posix-return
44
=====================
55

6-
Checks if any calls to POSIX functions (except ``posix_openpt``) expect negative
7-
return values. These functions return either ``0`` on success or an ``errno`` on failure,
8-
which is positive only.
6+
Checks if any calls to ``pthread_*`` or ``posix_*`` functions
7+
(except ``posix_openpt``) expect negative return values. These functions return
8+
either ``0`` on success or an ``errno`` on failure, which is positive only.
99

1010
Example buggy usage looks like:
1111

‎clang-tools-extra/test/clang-tidy/bugprone-posix-return.cpp

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,16 @@ typedef long off_t;
99
typedef decltype(sizeof(int)) size_t;
1010
typedef struct __posix_spawn_file_actions* posix_spawn_file_actions_t;
1111
typedef struct __posix_spawnattr* posix_spawnattr_t;
12+
# define __CPU_SETSIZE 1024
13+
# define __NCPUBITS (8 * sizeof (__cpu_mask))
14+
typedef unsigned long int __cpu_mask;
15+
typedef struct
16+
{
17+
__cpu_mask __bits[__CPU_SETSIZE / __NCPUBITS];
18+
} cpu_set_t;
19+
typedef struct _opaque_pthread_t *__darwin_pthread_t;
20+
typedef __darwin_pthread_t pthread_t;
21+
typedef struct pthread_attr_t_ *pthread_attr_t;
1222

1323
extern "C" int posix_fadvise(int fd, off_t offset, off_t len, int advice);
1424
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 char *file,
2333
const posix_spawn_file_actions_t *file_actions,
2434
const posix_spawnattr_t *attrp,
2535
char *const argv[], char *const envp[]);
36+
extern "C" int pthread_create(pthread_t *thread, const pthread_attr_t *attr, void *(*start_routine)(void *), void *arg);
37+
extern "C" int pthread_attr_setaffinity_np(pthread_attr_t *attr, size_t cpusetsize, const cpu_set_t *cpuset);
38+
extern "C" int pthread_attr_setschedpolicy(pthread_attr_t *attr, int policy);
39+
extern "C" int pthread_attr_init(pthread_attr_t *attr);
40+
extern "C" int pthread_yield(void);
41+
2642

2743
void warningLessThanZero() {
2844
if (posix_fadvise(0, 0, 0, 0) < 0) {}
@@ -43,11 +59,38 @@ void warningLessThanZero() {
4359
if (posix_spawnp(NULL, NULL, NULL, NULL, {NULL}, {NULL}) < 0) {}
4460
// CHECK-MESSAGES: :[[@LINE-1]]:60: warning:
4561
// CHECK-FIXES: posix_spawnp(NULL, NULL, NULL, NULL, {NULL}, {NULL}) > 0
62+
if (pthread_create(NULL, NULL, NULL, NULL) < 0) {}
63+
// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: the comparison always evaluates to false because pthread_create always returns non-negative values
64+
// CHECK-FIXES: pthread_create(NULL, NULL, NULL, NULL) > 0
65+
if (pthread_attr_setaffinity_np(NULL, 0, NULL) < 0) {}
66+
// CHECK-MESSAGES: :[[@LINE-1]]:50: warning:
67+
// CHECK-FIXES: pthread_attr_setaffinity_np(NULL, 0, NULL) > 0
68+
if (pthread_attr_setschedpolicy(NULL, 0) < 0) {}
69+
// CHECK-MESSAGES: :[[@LINE-1]]:44: warning:
70+
// CHECK-FIXES: pthread_attr_setschedpolicy(NULL, 0) > 0)
71+
if (pthread_attr_init(NULL) < 0) {}
72+
// CHECK-MESSAGES: :[[@LINE-1]]:31: warning:
73+
// CHECK-FIXES: pthread_attr_init(NULL) > 0
74+
if (pthread_yield() < 0) {}
75+
// CHECK-MESSAGES: :[[@LINE-1]]:23: warning:
76+
// CHECK-FIXES: pthread_yield() > 0
77+
4678
}
4779

4880
void warningAlwaysTrue() {
4981
if (posix_fadvise(0, 0, 0, 0) >= 0) {}
5082
// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: the comparison always evaluates to true because posix_fadvise always returns non-negative values
83+
if (pthread_create(NULL, NULL, NULL, NULL) >= 0) {}
84+
// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: the comparison always evaluates to true because pthread_create always returns non-negative values
85+
if (pthread_attr_setaffinity_np(NULL, 0, NULL) >= 0) {}
86+
// CHECK-MESSAGES: :[[@LINE-1]]:50: warning:
87+
if (pthread_attr_setschedpolicy(NULL, 0) >= 0) {}
88+
// CHECK-MESSAGES: :[[@LINE-1]]:44: warning:
89+
if (pthread_attr_init(NULL) >= 0) {}
90+
// CHECK-MESSAGES: :[[@LINE-1]]:31: warning:
91+
if (pthread_yield() >= 0) {}
92+
// CHECK-MESSAGES: :[[@LINE-1]]:23: warning:
93+
5194
}
5295

5396
void warningEqualsNegative() {
@@ -69,6 +112,15 @@ void warningEqualsNegative() {
69112
// CHECK-MESSAGES: :[[@LINE-1]]:59: warning:
70113
if (posix_spawnp(NULL, NULL, NULL, NULL, {NULL}, {NULL}) == -1) {}
71114
// CHECK-MESSAGES: :[[@LINE-1]]:60: warning:
115+
if (pthread_create(NULL, NULL, NULL, NULL) == -1) {}
116+
// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: pthread_create
117+
if (pthread_create(NULL, NULL, NULL, NULL) != -1) {}
118+
// CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
119+
if (pthread_create(NULL, NULL, NULL, NULL) <= -1) {}
120+
// CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
121+
if (pthread_create(NULL, NULL, NULL, NULL) < -1) {}
122+
// CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
123+
72124
}
73125

74126
void WarningWithMacro() {
@@ -85,6 +137,20 @@ void WarningWithMacro() {
85137
// CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
86138
if (posix_fadvise(0, 0, 0, 0) < NEGATIVE_ONE) {}
87139
// CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
140+
if (pthread_create(NULL, NULL, NULL, NULL) < ZERO) {}
141+
// CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
142+
// CHECK-FIXES: pthread_create(NULL, NULL, NULL, NULL) > ZERO
143+
if (pthread_create(NULL, NULL, NULL, NULL) >= ZERO) {}
144+
// CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
145+
if (pthread_create(NULL, NULL, NULL, NULL) == NEGATIVE_ONE) {}
146+
// CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
147+
if (pthread_create(NULL, NULL, NULL, NULL) != NEGATIVE_ONE) {}
148+
// CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
149+
if (pthread_create(NULL, NULL, NULL, NULL) <= NEGATIVE_ONE) {}
150+
// CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
151+
if (pthread_create(NULL, NULL, NULL, NULL) < NEGATIVE_ONE) {}
152+
// CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
153+
88154
}
89155

90156
void noWarning() {
@@ -100,6 +166,7 @@ void noWarning() {
100166

101167
namespace i {
102168
int posix_fadvise(int fd, off_t offset, off_t len, int advice);
169+
int pthread_yield(void);
103170

104171
void noWarning() {
105172
if (posix_fadvise(0, 0, 0, 0) < 0) {}
@@ -108,13 +175,20 @@ void noWarning() {
108175
if (posix_fadvise(0, 0, 0, 0) != -1) {}
109176
if (posix_fadvise(0, 0, 0, 0) <= -1) {}
110177
if (posix_fadvise(0, 0, 0, 0) < -1) {}
178+
if (pthread_yield() < 0) {}
179+
if (pthread_yield() >= 0) {}
180+
if (pthread_yield() == -1) {}
181+
if (pthread_yield() != -1) {}
182+
if (pthread_yield() <= -1) {}
183+
if (pthread_yield() < -1) {}
111184
}
112185

113186
} // namespace i
114187

115188
class G {
116189
public:
117190
int posix_fadvise(int fd, off_t offset, off_t len, int advice);
191+
int pthread_yield(void);
118192

119193
void noWarning() {
120194
if (posix_fadvise(0, 0, 0, 0) < 0) {}
@@ -123,5 +197,11 @@ class G {
123197
if (posix_fadvise(0, 0, 0, 0) != -1) {}
124198
if (posix_fadvise(0, 0, 0, 0) <= -1) {}
125199
if (posix_fadvise(0, 0, 0, 0) < -1) {}
200+
if (pthread_yield() < 0) {}
201+
if (pthread_yield() >= 0) {}
202+
if (pthread_yield() == -1) {}
203+
if (pthread_yield() != -1) {}
204+
if (pthread_yield() <= -1) {}
205+
if (pthread_yield() < -1) {}
126206
}
127207
};

0 commit comments

Comments
 (0)