[clang-tidy] Use CloexecCheck as base class.

Summary:
Simplify registerMatchers and check functions in CloexecCreatCheck,
CloexecSocketCheck, CloexecFopenCheck, and CloexecOpenCheck.

Differential Revision: https://reviews.llvm.org/D36761

llvm-svn: 311020
This commit is contained in:
Chih-Hung Hsieh 2017-08-16 16:59:26 +00:00
parent bf9751760a
commit fec506daaa
10 changed files with 60 additions and 158 deletions

View File

@ -20,10 +20,6 @@ namespace tidy {
namespace android {
namespace {
const char *const FuncDeclBindingStr = "funcDecl";
const char *const FuncBindingStr = "func";
// Helper function to form the correct string mode for Type3.
// Build the replace text. If it's string constant, add <Mode> directly in the
// end of the string. Else, add <Mode>.
@ -41,6 +37,10 @@ std::string buildFixMsgForStringFlag(const Expr *Arg, const SourceManager &SM,
}
} // namespace
constexpr char CloexecCheck::FuncDeclBindingStr[];
constexpr char CloexecCheck::FuncBindingStr[];
void CloexecCheck::registerMatchersImpl(
MatchFinder *Finder, internal::Matcher<FunctionDecl> Function) {
// We assume all the checked APIs are C functions.

View File

@ -90,6 +90,12 @@ protected:
/// Helper function to get the spelling of a particular argument.
StringRef getSpellingArg(const ast_matchers::MatchFinder::MatchResult &Result,
int N) const;
/// Binding name of the FuncDecl of a function call.
static constexpr char FuncDeclBindingStr[] = "funcDecl";
/// Binding name of the function call expression.
static constexpr char FuncBindingStr[] = "func";
};
} // namespace android

View File

@ -10,7 +10,6 @@
#include "CloexecCreatCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
using namespace clang::ast_matchers;
@ -21,37 +20,22 @@ namespace android {
void CloexecCreatCheck::registerMatchers(MatchFinder *Finder) {
auto CharPointerType = hasType(pointerType(pointee(isAnyCharacter())));
auto MODETType = hasType(namedDecl(hasName("mode_t")));
Finder->addMatcher(
callExpr(callee(functionDecl(isExternC(), returns(isInteger()),
hasName("creat"),
hasParameter(0, CharPointerType),
hasParameter(1, MODETType))
.bind("funcDecl")))
.bind("creatFn"),
this);
registerMatchersImpl(Finder,
functionDecl(isExternC(), returns(isInteger()),
hasName("creat"),
hasParameter(0, CharPointerType),
hasParameter(1, MODETType)));
}
void CloexecCreatCheck::check(const MatchFinder::MatchResult &Result) {
const auto *MatchedCall = Result.Nodes.getNodeAs<CallExpr>("creatFn");
const SourceManager &SM = *Result.SourceManager;
const std::string &ReplacementText =
(Twine("open (") +
Lexer::getSourceText(CharSourceRange::getTokenRange(
MatchedCall->getArg(0)->getSourceRange()),
SM, Result.Context->getLangOpts()) +
(Twine("open (") + getSpellingArg(Result, 0) +
", O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, " +
Lexer::getSourceText(CharSourceRange::getTokenRange(
MatchedCall->getArg(1)->getSourceRange()),
SM, Result.Context->getLangOpts()) +
")")
getSpellingArg(Result, 1) + ")")
.str();
diag(MatchedCall->getLocStart(),
"prefer open() to creat() because open() allows O_CLOEXEC")
<< FixItHint::CreateReplacement(MatchedCall->getSourceRange(),
ReplacementText);
replaceFunc(Result,
"prefer open() to creat() because open() allows O_CLOEXEC",
ReplacementText);
}
} // namespace android

View File

@ -10,7 +10,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_CREAT_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_CREAT_H
#include "../ClangTidy.h"
#include "CloexecCheck.h"
namespace clang {
namespace tidy {
@ -20,10 +20,10 @@ namespace android {
/// Find the usage of creat() and redirect user to use open().
/// http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-creat.html
class CloexecCreatCheck : public ClangTidyCheck {
class CloexecCreatCheck : public CloexecCheck {
public:
CloexecCreatCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
: CloexecCheck(Name, Context) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
};

View File

@ -18,55 +18,17 @@ namespace clang {
namespace tidy {
namespace android {
namespace {
static const char MODE = 'e';
// Build the replace text. If it's string constant, add 'e' directly in the end
// of the string. Else, add "e".
std::string BuildReplaceText(const Expr *Arg, const SourceManager &SM,
const LangOptions &LangOpts) {
if (Arg->getLocStart().isMacroID())
return (Lexer::getSourceText(
CharSourceRange::getTokenRange(Arg->getSourceRange()), SM,
LangOpts) +
" \"" + Twine(MODE) + "\"")
.str();
StringRef SR = cast<StringLiteral>(Arg->IgnoreParenCasts())->getString();
return ("\"" + SR + Twine(MODE) + "\"").str();
}
} // namespace
void CloexecFopenCheck::registerMatchers(MatchFinder *Finder) {
auto CharPointerType = hasType(pointerType(pointee(isAnyCharacter())));
Finder->addMatcher(
callExpr(callee(functionDecl(isExternC(), returns(asString("FILE *")),
hasName("fopen"),
hasParameter(0, CharPointerType),
hasParameter(1, CharPointerType))
.bind("funcDecl")))
.bind("fopenFn"),
this);
registerMatchersImpl(Finder,
functionDecl(isExternC(), returns(asString("FILE *")),
hasName("fopen"),
hasParameter(0, CharPointerType),
hasParameter(1, CharPointerType)));
}
void CloexecFopenCheck::check(const MatchFinder::MatchResult &Result) {
const auto *MatchedCall = Result.Nodes.getNodeAs<CallExpr>("fopenFn");
const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("funcDecl");
const Expr *ModeArg = MatchedCall->getArg(1);
// Check if the 'e' may be in the mode string.
const auto *ModeStr = dyn_cast<StringLiteral>(ModeArg->IgnoreParenCasts());
if (!ModeStr || (ModeStr->getString().find(MODE) != StringRef::npos))
return;
const std::string &ReplacementText = BuildReplaceText(
ModeArg, *Result.SourceManager, Result.Context->getLangOpts());
diag(ModeArg->getLocStart(), "use %0 mode 'e' to set O_CLOEXEC")
<< FD
<< FixItHint::CreateReplacement(ModeArg->getSourceRange(),
ReplacementText);
insertStringFlag(Result, /*Mode=*/'e', /*ArgPos=*/1);
}
} // namespace android

View File

@ -10,7 +10,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_FOPEN_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_FOPEN_H
#include "../ClangTidy.h"
#include "CloexecCheck.h"
namespace clang {
namespace tidy {
@ -23,10 +23,10 @@ namespace android {
/// constant propagation.
///
/// http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-fopen.html
class CloexecFopenCheck : public ClangTidyCheck {
class CloexecFopenCheck : public CloexecCheck {
public:
CloexecFopenCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
: CloexecCheck(Name, Context) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
};

View File

@ -8,10 +8,8 @@
//===----------------------------------------------------------------------===//
#include "CloexecOpenCheck.h"
#include "../utils/ASTUtils.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
using namespace clang::ast_matchers;
@ -19,54 +17,26 @@ namespace clang {
namespace tidy {
namespace android {
static constexpr const char *O_CLOEXEC = "O_CLOEXEC";
void CloexecOpenCheck::registerMatchers(MatchFinder *Finder) {
auto CharPointerType = hasType(pointerType(pointee(isAnyCharacter())));
Finder->addMatcher(
callExpr(callee(functionDecl(isExternC(), returns(isInteger()),
hasAnyName("open", "open64"),
hasParameter(0, CharPointerType),
hasParameter(1, hasType(isInteger())))
.bind("funcDecl")))
.bind("openFn"),
this);
Finder->addMatcher(
callExpr(callee(functionDecl(isExternC(), returns(isInteger()),
hasName("openat"),
hasParameter(0, hasType(isInteger())),
hasParameter(1, CharPointerType),
hasParameter(2, hasType(isInteger())))
.bind("funcDecl")))
.bind("openatFn"),
this);
registerMatchersImpl(Finder,
functionDecl(isExternC(), returns(isInteger()),
hasAnyName("open", "open64"),
hasParameter(0, CharPointerType),
hasParameter(1, hasType(isInteger()))));
registerMatchersImpl(Finder,
functionDecl(isExternC(), returns(isInteger()),
hasName("openat"),
hasParameter(0, hasType(isInteger())),
hasParameter(1, CharPointerType),
hasParameter(2, hasType(isInteger()))));
}
void CloexecOpenCheck::check(const MatchFinder::MatchResult &Result) {
const Expr *FlagArg = nullptr;
if (const auto *OpenFnCall = Result.Nodes.getNodeAs<CallExpr>("openFn"))
FlagArg = OpenFnCall->getArg(1);
else if (const auto *OpenFnCall =
Result.Nodes.getNodeAs<CallExpr>("openatFn"))
FlagArg = OpenFnCall->getArg(2);
assert(FlagArg);
const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("funcDecl");
// Check the required flag.
SourceManager &SM = *Result.SourceManager;
if (utils::exprHasBitFlagWithSpelling(FlagArg->IgnoreParenCasts(), SM,
Result.Context->getLangOpts(), O_CLOEXEC))
return;
SourceLocation EndLoc =
Lexer::getLocForEndOfToken(SM.getFileLoc(FlagArg->getLocEnd()), 0, SM,
Result.Context->getLangOpts());
diag(EndLoc, "%0 should use %1 where possible")
<< FD << O_CLOEXEC
<< FixItHint::CreateInsertion(EndLoc, (Twine(" | ") + O_CLOEXEC).str());
const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>(FuncDeclBindingStr);
assert(FD->param_size() > 1);
int ArgPos = (FD->param_size() > 2) ? 2 : 1;
insertMacroFlag(Result, /*MarcoFlag=*/"O_CLOEXEC", ArgPos);
}
} // namespace android

View File

@ -10,7 +10,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_OPEN_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_OPEN_H
#include "../ClangTidy.h"
#include "CloexecCheck.h"
namespace clang {
namespace tidy {
@ -25,10 +25,10 @@ namespace android {
///
/// Only the symbolic 'O_CLOEXEC' macro definition is checked, not the concrete
/// value.
class CloexecOpenCheck : public ClangTidyCheck {
class CloexecOpenCheck : public CloexecCheck {
public:
CloexecOpenCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
: CloexecCheck(Name, Context) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
};

View File

@ -8,7 +8,6 @@
//===----------------------------------------------------------------------===//
#include "CloexecSocketCheck.h"
#include "../utils/ASTUtils.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
@ -21,35 +20,16 @@ namespace android {
static constexpr const char *SOCK_CLOEXEC = "SOCK_CLOEXEC";
void CloexecSocketCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
callExpr(callee(functionDecl(isExternC(), returns(isInteger()),
hasName("socket"),
hasParameter(0, hasType(isInteger())),
hasParameter(1, hasType(isInteger())),
hasParameter(2, hasType(isInteger())))
.bind("funcDecl")))
.bind("socketFn"),
this);
registerMatchersImpl(Finder,
functionDecl(isExternC(), returns(isInteger()),
hasName("socket"),
hasParameter(0, hasType(isInteger())),
hasParameter(1, hasType(isInteger())),
hasParameter(2, hasType(isInteger()))));
}
void CloexecSocketCheck::check(const MatchFinder::MatchResult &Result) {
const auto *MatchedCall = Result.Nodes.getNodeAs<CallExpr>("socketFn");
const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("funcDecl");
const Expr *FlagArg = MatchedCall->getArg(1);
SourceManager &SM = *Result.SourceManager;
if (utils::exprHasBitFlagWithSpelling(FlagArg->IgnoreParenCasts(), SM,
Result.Context->getLangOpts(), SOCK_CLOEXEC))
return;
SourceLocation EndLoc =
Lexer::getLocForEndOfToken(SM.getFileLoc(FlagArg->getLocEnd()), 0, SM,
Result.Context->getLangOpts());
diag(EndLoc, "%0 should use %1 where possible")
<< FD << SOCK_CLOEXEC
<< FixItHint::CreateInsertion(EndLoc,
(Twine(" | ") + SOCK_CLOEXEC).str());
insertMacroFlag(Result, /*MarcoFlag=*/"SOCK_CLOEXEC", /*ArgPos=*/1);
}
} // namespace android

View File

@ -10,7 +10,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_SOCKET_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_SOCKET_H
#include "../ClangTidy.h"
#include "CloexecCheck.h"
namespace clang {
namespace tidy {
@ -20,10 +20,10 @@ namespace android {
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-socket.html
class CloexecSocketCheck : public ClangTidyCheck {
class CloexecSocketCheck : public CloexecCheck {
public:
CloexecSocketCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
: CloexecCheck(Name, Context) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
};