jcai19 created this revision.
Herald added subscribers: cfe-commits, mgorny, srhines.
Herald added a project: clang.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D63623
Files:
clang-tools-extra/clang-tidy/android/AndroidTidyModule.cpp
clang-tools-extra/clang-tidy/android/CMakeLists.txt
clang-tools-extra/clang-tidy/android/PosixReturnCheck.cpp
clang-tools-extra/clang-tidy/android/PosixReturnCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/android-posix-return.rst
clang-tools-extra/test/clang-tidy/android-posix-return.cpp
Index: clang-tools-extra/test/clang-tidy/android-posix-return.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/android-posix-return.cpp
@@ -0,0 +1,89 @@
+// RUN: %check_clang_tidy %s android-posix-return %t
+
+#define NULL nullptr
+#define ZERO 0
+#define NEGATIVE_ONE -1
+
+typedef long off_t;
+typedef int size_t;
+typedef int pid_t;
+typedef struct __posix_spawn_file_actions* posix_spawn_file_actions_t;
+typedef struct __posix_spawnattr* posix_spawnattr_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);
+extern "C" int posix_madvise(void *addr, size_t len, int advice);
+extern "C" int posix_memalign(void **memptr, size_t alignment, size_t size);
+extern "C" int posix_openpt(int flags);
+extern "C" int posix_spawn(pid_t *pid, const char *path,
+ const posix_spawn_file_actions_t *file_actions,
+ const posix_spawnattr_t *attrp,
+ char *const argv[], char *const envp[]);
+extern "C" int posix_spawnp(pid_t *pid, const char *file,
+ const posix_spawn_file_actions_t *file_actions,
+ const posix_spawnattr_t *attrp,
+ char *const argv[], char *const envp[]);
+
+void warningLtZero() {
+ if (posix_fadvise(0, 0, 0, 0) < 0) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+ if (posix_fallocate(0, 0, 0) < 0) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:32: warning:
+ if (posix_madvise(NULL, 0, 0) < 0) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+ if (posix_memalign(NULL, 0, 0) < 0) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning:
+ if (posix_spawn(NULL, NULL, NULL, NULL, {NULL}, {NULL}) < 0) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:59: warning:
+ if (posix_spawnp(NULL, NULL, NULL, NULL, {NULL}, {NULL}) < 0) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:60: warning:
+}
+
+void warningEqualsNegative() {
+ if (posix_fadvise(0, 0, 0, 0) == -1) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+ if (posix_fallocate(0, 0, 0) == -1) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:32: warning:
+ if (posix_madvise(NULL, 0, 0) == -1) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+ if (posix_memalign(NULL, 0, 0) == -1) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning:
+ if (posix_spawn(NULL, NULL, NULL, NULL, {NULL}, {NULL}) == -1) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:59: warning:
+ if (posix_spawnp(NULL, NULL, NULL, NULL, {NULL}, {NULL}) == -1) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:60: warning:
+}
+
+void WarningWithMacro () {
+ if (posix_fadvise(0, 0, 0, 0) < ZERO) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+ if (posix_fadvise(0, 0, 0, 0) == NEGATIVE_ONE) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+}
+
+void noWarning() {
+ if (posix_openpt(0) < 0) {}
+ if (posix_openpt(0) == -1) {}
+ if (posix_fadvise(0, 0, 0, 0) >= 0) {}
+ if (posix_fadvise(0, 0, 0, 0) == 1) {}
+}
+
+namespace i {
+int posix_fadvise(int fd, off_t offset, off_t len, int advice);
+
+void noWarning() {
+ if (posix_fadvise(0, 0, 0, 0) < 0) {}
+ if (posix_fadvise(0, 0, 0, 0) == -1) {}
+}
+
+} // namespace i
+
+class G {
+ public:
+ int posix_fadvise(int fd, off_t offset, off_t len, int advice);
+
+ void noWarning() {
+ if (posix_fadvise(0, 0, 0, 0) < 0) {}
+ if (posix_fadvise(0, 0, 0, 0) == -1) {}
+ }
+};
Index: clang-tools-extra/docs/clang-tidy/checks/android-posix-return.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/android-posix-return.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - android-posix-return
+
+android-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.
+
+Example buggy usage looks like:
+
+.. code-block:: c
+
+ if (posix_fadvise(...) < 0) {
+
+This will never happen as the return value is always non-negative. A simple fix could be
+
+.. code-block:: c
+
+ int ret = posix_fadvise(...);
+ if (ret != 0) ...
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -196,6 +196,12 @@
`WarnOnLargeObject` and `MaxSize` options to warn on any large trivial
object caught by value.
+- New :doc:`android-posix-return
+ <clang-tidy/checks/android-posix-return>` check.
+
+ Checks if any alls to posix functions (except posix_openpt) expect negative
+ return values
+
- Added `UseAssignment` option to :doc:`cppcoreguidelines-pro-type-member-init`
If set to true, the check will provide fix-its with literal initializers
Index: clang-tools-extra/clang-tidy/android/PosixReturnCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/android/PosixReturnCheck.h
@@ -0,0 +1,30 @@
+//===--- 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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_POSIX_RETURN_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_POSIX_RETURN_CHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace android {
+
+class PosixReturnCheck: public ClangTidyCheck {
+public:
+ PosixReturnCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace android
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_POSIX_RETURN_CHECK_H
Index: clang-tools-extra/clang-tidy/android/PosixReturnCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/android/PosixReturnCheck.cpp
@@ -0,0 +1,44 @@
+//===--- PosixReturnCheck.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 "../utils/Matchers.h"
+#include "PosixReturnCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace android {
+
+void PosixReturnCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ binaryOperator(
+ hasOperatorName("<"),
+ hasLHS(callExpr(callee(functionDecl(matchesName("^::posix_"), unless(hasName("posix_openpt")))))),
+ hasRHS(integerLiteral(equals(0)))).bind("binop"),
+ this);
+ Finder->addMatcher(
+ binaryOperator(
+ hasOperatorName("=="),
+ hasLHS(callExpr(callee(functionDecl(matchesName("^::posix_"), unless(hasName("posix_openpt")))))),
+ hasRHS(unaryOperator(hasOperatorName("-"), hasUnaryOperand(integerLiteral())))).bind("binop"),
+ this);
+}
+
+
+
+void PosixReturnCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto &BinOp = *Result.Nodes.getNodeAs<BinaryOperator>("binop");
+ diag(BinOp.getOperatorLoc(), "posix functions (except posix_openpt) never return negative values");
+}
+
+} // namespace android
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/android/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/android/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/android/CMakeLists.txt
@@ -18,6 +18,7 @@
CloexecPipe2Check.cpp
CloexecSocketCheck.cpp
ComparisonInTempFailureRetryCheck.cpp
+ PosixReturnCheck.cpp
LINK_LIBS
clangAST
Index: clang-tools-extra/clang-tidy/android/AndroidTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/android/AndroidTidyModule.cpp
+++ clang-tools-extra/clang-tidy/android/AndroidTidyModule.cpp
@@ -24,6 +24,7 @@
#include "CloexecPipe2Check.h"
#include "CloexecSocketCheck.h"
#include "ComparisonInTempFailureRetryCheck.h"
+#include "PosixReturnCheck.h"
using namespace clang::ast_matchers;
@@ -56,6 +57,8 @@
CheckFactories.registerCheck<CloexecSocketCheck>("android-cloexec-socket");
CheckFactories.registerCheck<ComparisonInTempFailureRetryCheck>(
"android-comparison-in-temp-failure-retry");
+ CheckFactories.registerCheck<PosixReturnCheck>(
+ "android-posix-return");
}
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
