maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

Allow to specify macro names for android-comparison-in-temp-failure-retry 

Some projects do not use the TEMP_FAILURE_RETRY macro but define their
own one, as not to depend on glibc / Bionic details. By allowing the
user to override the list of macros, these projects can also benefit
from this check.

Differential Revision: https://reviews.llvm.org/D83144
This commit is contained in:
Florian Mayer 2020-10-01 10:08:33 -07:00 committed by George Burgess IV
parent cdfb95ad58
commit 9d40fb808f
4 changed files with 104 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
clang-tools-extra/clang-tidy/android/ComparisonInTempFailureRetryCheck.cpp
View File

@ -18,32 +18,17 @@ namespace clang {
namespace tidy {
namespace android {
namespace {
AST_MATCHER(BinaryOperator, isRHSATempFailureRetryArg) {
if (!Node.getBeginLoc().isMacroID())
return false;
const SourceManager &SM = Finder->getASTContext().getSourceManager();
if (!SM.isMacroArgExpansion(Node.getRHS()->IgnoreParenCasts()->getBeginLoc()))
return false;
const LangOptions &Opts = Finder->getASTContext().getLangOpts();
SourceLocation LocStart = Node.getBeginLoc();
while (LocStart.isMacroID()) {
SourceLocation Invocation = SM.getImmediateMacroCallerLoc(LocStart);
Token Tok;
if (!Lexer::getRawToken(SM.getSpellingLoc(Invocation), Tok, SM, Opts,
/*IgnoreWhiteSpace=*/true)) {
if (Tok.getKind() == tok::raw_identifier &&
Tok.getRawIdentifier() == "TEMP_FAILURE_RETRY")
return true;
}
LocStart = Invocation;
}
return false;
ComparisonInTempFailureRetryCheck::ComparisonInTempFailureRetryCheck(
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
RawRetryList(Options.get("RetryMacros", "TEMP_FAILURE_RETRY")) {
StringRef(RawRetryList).split(RetryMacros, ",", -1, false);
}
void ComparisonInTempFailureRetryCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "RetryMacros", RawRetryList);
}
} // namespace
void ComparisonInTempFailureRetryCheck::registerMatchers(MatchFinder *Finder) {
// Both glibc's and Bionic's TEMP_FAILURE_RETRY macros structurally look like:
@ -63,15 +48,43 @@ void ComparisonInTempFailureRetryCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
binaryOperator(hasOperatorName("="),
hasRHS(ignoringParenCasts(
binaryOperator(isComparisonOperator()).bind("binop"))),
isRHSATempFailureRetryArg()),
binaryOperator(isComparisonOperator()).bind("inner"))))
.bind("outer"),
this);
}
void ComparisonInTempFailureRetryCheck::check(
const MatchFinder::MatchResult &Result) {
const auto &BinOp = *Result.Nodes.getNodeAs<BinaryOperator>("binop");
diag(BinOp.getOperatorLoc(), "top-level comparison in TEMP_FAILURE_RETRY");
StringRef RetryMacroName;
const auto &Node = *Result.Nodes.getNodeAs<BinaryOperator>("outer");
if (!Node.getBeginLoc().isMacroID())
return;
const SourceManager &SM = *Result.SourceManager;
if (!SM.isMacroArgExpansion(Node.getRHS()->IgnoreParenCasts()->getBeginLoc()))
return;
const LangOptions &Opts = Result.Context->getLangOpts();
SourceLocation LocStart = Node.getBeginLoc();
while (LocStart.isMacroID()) {
SourceLocation Invocation = SM.getImmediateMacroCallerLoc(LocStart);
Token Tok;
if (!Lexer::getRawToken(SM.getSpellingLoc(Invocation), Tok, SM, Opts,
/*IgnoreWhiteSpace=*/true)) {
if (Tok.getKind() == tok::raw_identifier &&
llvm::is_contained(RetryMacros, Tok.getRawIdentifier())) {
RetryMacroName = Tok.getRawIdentifier();
break;
}
}
LocStart = Invocation;
}
if (RetryMacroName.empty())
return;
const auto &Inner = *Result.Nodes.getNodeAs<BinaryOperator>("inner");
diag(Inner.getOperatorLoc(), "top-level comparison in %0") << RetryMacroName;
// FIXME: FixIts would be nice, but potentially nontrivial when nested macros
// happen, e.g. `TEMP_FAILURE_RETRY(IS_ZERO(foo()))`

11
clang-tools-extra/clang-tidy/android/ComparisonInTempFailureRetryCheck.h
View File

@ -10,6 +10,9 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_COMPARISONINTEMPFAILURERETRYCHECK_H
#include "../ClangTidyCheck.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include <string>
namespace clang {
namespace tidy {
@ -22,10 +25,14 @@ namespace android {
/// TEMP_FAILURE_RETRY is a macro provided by both glibc and Bionic.
class ComparisonInTempFailureRetryCheck : public ClangTidyCheck {
public:
ComparisonInTempFailureRetryCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
ComparisonInTempFailureRetryCheck(StringRef Name, ClangTidyContext *Context);
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
private:
const std::string RawRetryList;
SmallVector<StringRef, 5> RetryMacros;
};
} // namespace android

7
clang-tools-extra/docs/clang-tidy/checks/android-comparison-in-temp-failure-retry.rst
View File

@ -34,3 +34,10 @@ If you encounter this, the fix is simple: lift the comparison out of the
while (TEMP_FAILURE_RETRY(read(STDIN_FILENO, cs, sizeof(cs))) != 0) {
// Do something with cs.
}
Options
-------
.. option:: RetryMacros
A comma-separated list of the names of retry macros to be checked.

46
clang-tools-extra/test/clang-tidy/checkers/android-comparison-in-temp-failure-retry-custom-macro.c Normal file
View File

@ -0,0 +1,46 @@
// RUN: %check_clang_tidy %s android-comparison-in-temp-failure-retry %t -- -config="{CheckOptions: [{key: android-comparison-in-temp-failure-retry.RetryMacros, value: 'MY_TEMP_FAILURE_RETRY,MY_OTHER_TEMP_FAILURE_RETRY'}]}"
#define MY_TEMP_FAILURE_RETRY(x) \
({ \
typeof(x) __z; \
do \
__z = (x); \
while (__z == -1); \
__z; \
})
#define MY_OTHER_TEMP_FAILURE_RETRY(x) \
({ \
typeof(x) __z; \
do \
__z = (x); \
while (__z == -1); \
__z; \
})
int foo();
int bar(int a);
void with_custom_macro() {
MY_TEMP_FAILURE_RETRY(foo());
MY_TEMP_FAILURE_RETRY(foo() == 1);
// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: top-level comparison in MY_TEMP_FAILURE_RETRY
MY_TEMP_FAILURE_RETRY((foo()));
MY_TEMP_FAILURE_RETRY((int)(foo() == 1));
// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: top-level comparison in MY_TEMP_FAILURE_RETRY
MY_TEMP_FAILURE_RETRY((bar(foo() == 1)));
MY_TEMP_FAILURE_RETRY((int)((bar(foo() == 1)) == 1));
// CHECK-MESSAGES: :[[@LINE-1]]:49: warning: top-level comparison in MY_TEMP_FAILURE_RETRY
}
void with_other_custom_macro() {
MY_OTHER_TEMP_FAILURE_RETRY(foo());
MY_OTHER_TEMP_FAILURE_RETRY(foo() == 1);
// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: top-level comparison in MY_OTHER_TEMP_FAILURE_RETRY
MY_OTHER_TEMP_FAILURE_RETRY((foo()));
MY_OTHER_TEMP_FAILURE_RETRY((int)(foo() == 1));
// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: top-level comparison in MY_OTHER_TEMP_FAILURE_RETRY
MY_OTHER_TEMP_FAILURE_RETRY((bar(foo() == 1)));
MY_OTHER_TEMP_FAILURE_RETRY((int)((bar(foo() == 1)) == 1));
// CHECK-MESSAGES: :[[@LINE-1]]:55: warning: top-level comparison in MY_OTHER_TEMP_FAILURE_RETRY
}