Index: clang-tidy/zircon/CMakeLists.txt =================================================================== --- clang-tidy/zircon/CMakeLists.txt +++ clang-tidy/zircon/CMakeLists.txt @@ -1,6 +1,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyZirconModule + HumanReadableStatusCheck.cpp TemporaryObjectsCheck.cpp ZirconTidyModule.cpp Index: clang-tidy/zircon/HumanReadableStatusCheck.h =================================================================== --- /dev/null +++ clang-tidy/zircon/HumanReadableStatusCheck.h @@ -0,0 +1,45 @@ +//===--- HumanReadableStatusCheck.h - clang-tidy---------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_ZXHUMANREADABLESTATUSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_ZXHUMANREADABLESTATUSCHECK_H + +#include "../ClangTidy.h" +#include "../utils/OptionsUtils.h" + +namespace clang { +namespace tidy { +namespace zircon { + +/// Suggests fix for printf-family functions with a zx_status_t argument to +/// convert status argument to a human-readable string. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/zircon-human-readable-status.html +class HumanReadableStatusCheck : public ClangTidyCheck { +public: + HumanReadableStatusCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + FormatLikeFunctions(Options.get("FormatLikeFunctions", "")) {} + + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + std::string ReplaceFmtString(int Pos, StringRef FmtString); + + const std::string FormatLikeFunctions; +}; + +} // namespace zircon +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_ZXHUMANREADABLESTATUSCHECK_H Index: clang-tidy/zircon/HumanReadableStatusCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/zircon/HumanReadableStatusCheck.cpp @@ -0,0 +1,120 @@ +//===--- HumanReadableStatusCheck.cpp - clang-tidy-------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "HumanReadableStatusCheck.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "llvm/Support/raw_ostream.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace zircon { + +// Semicolon separated list of known format-like functions. The list must end +// with a semicolon. +static const char KnownFormatFunctions[] = "printf;" + "fprintf;" + "sprintf;" + "snprintf;" + "printf_s;" + "fprintf_s;" + "sprintf_s;" + "snprintf_s;" + "wprintf;" + "fwprintf;" + "swprintf;" + "wprintf_s;" + "fwprintf_s;" + "swprintf_s;" + "snwprintf_s;" + "vprintf;" + "vfprintf;" + "vsprintf;" + "vsnprintf;" + "vprintf_s;" + "vfprintf_s;" + "vsprintf_s;" + "vsnprintf_s;"; + +void HumanReadableStatusCheck::registerMatchers(MatchFinder *Finder) { + std::vector FunctionNames = utils::options::parseStringList( + (llvm::Twine(KnownFormatFunctions) + FormatLikeFunctions).str()); + Finder->addMatcher( + callExpr( + allOf(hasAnyArgument(ignoringParenImpCasts(declRefExpr( + hasType(typedefNameDecl(hasName("zx_status_t")))))), + callee(functionDecl(hasAnyName(llvm::SmallVector( + FunctionNames.begin(), FunctionNames.end())))))) + .bind("formatter"), + this); +} + +void HumanReadableStatusCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *D = Result.Nodes.getNodeAs("formatter")) { + auto Diag = diag(D->getLocStart(), "non-human-readable log message"); + // Check each argument (after the first string arg) for zx_status_t + int ArgCount = 0; + StringRef FmtString; + SourceRange FmtStringRange; + for (const auto *Arg : D->arguments()) { + if (const auto *ICE = dyn_cast(Arg)) { + // Parse the first argument. + if (ArgCount == 0) { + if (const auto *Str = dyn_cast(ICE->getSubExpr())) { + FmtString = Str->getString(); + FmtStringRange = Str->getSourceRange(); + ++ArgCount; + continue; + } + return; + } + + // Check subsequent arguments for zx_status_t args. + if (const auto *DRE = dyn_cast(ICE->getSubExpr())) { + if (DRE->getDecl()->getType().getAsString() == "zx_status_t") { + Diag << FixItHint::CreateReplacement( + FmtStringRange, + "\"" + ReplaceFmtString(ArgCount, FmtString) + "\""); + Diag << FixItHint::CreateReplacement( + DRE->getSourceRange(), + "zx_status_get_string(" + + DRE->getNameInfo().getName().getAsString() + ")"); + } + } + ++ArgCount; + } + } + } +} + +void HumanReadableStatusCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "FormatLikeFunctions", FormatLikeFunctions); +} + +std::string HumanReadableStatusCheck::ReplaceFmtString(int Pos, + StringRef FmtString) { + int Current = 1; + size_t Loc = 0; + while (Current <= Pos && Loc < FmtString.size() - 1) { + Loc = FmtString.find('%', Loc); + ++Current; + ++Loc; + } + std::string ReplacementString = FmtString; + if (Loc != StringRef::npos && ReplacementString[Loc] == 'd') + ReplacementString[Loc] = 's'; + return ReplacementString; +} + +} // namespace zircon +} // namespace tidy +} // namespace clang Index: clang-tidy/zircon/ZirconTidyModule.cpp =================================================================== --- clang-tidy/zircon/ZirconTidyModule.cpp +++ clang-tidy/zircon/ZirconTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "HumanReadableStatusCheck.h" #include "TemporaryObjectsCheck.h" using namespace clang::ast_matchers; @@ -22,6 +23,8 @@ class ZirconModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "zircon-human-readable-status"); CheckFactories.registerCheck( "zircon-temporary-objects"); } Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -196,6 +196,16 @@ - The 'google-runtime-member-string-references' check was removed. +- New `zircon-human-readable-status + `_ check + + FIXME: add release notes. + +- New `zircon-temporary-objects + `_ check + + Warns on construction of specific temporary objects in the Zircon kernel. + Improvements to include-fixer ----------------------------- Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -227,4 +227,5 @@ readability-static-definition-in-anonymous-namespace readability-string-compare readability-uniqueptr-delete-release + zircon-human-readable-status zircon-temporary-objects Index: docs/clang-tidy/checks/zircon-human-readable-status.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/zircon-human-readable-status.rst @@ -0,0 +1,30 @@ +.. title:: clang-tidy - zircon-human-readable-status + +zircon-human-readable-status +============================ + +Suggests fix for printf-family functions with a zx_status_t argument to convert +status argument to a human-readable string (zx_status_t is a Zircon kernel +status message, and zx_status_get_string() is a function that retrieves the +associated string value. + +For example: + +.. code-block:: c++ + + zx_status_t status; + printf("%d", status); // Suggests printf("%s", zx_status_get_string(status)) + + unsigned num; + printf("%d", num) // No warning + + +Options +------- + +.. option:: Names + + A semi-colon-separated list of fully-qualified names of functions that + are printf-like (i.e. are variadic, with the first argument being a + formatting string and subsequent argments being the printf replacements.). + Default is empty. \ No newline at end of file Index: test/clang-tidy/zircon-human-readable-status.cpp =================================================================== --- /dev/null +++ test/clang-tidy/zircon-human-readable-status.cpp @@ -0,0 +1,70 @@ +// RUN: %check_clang_tidy %s zircon-human-readable-status %t -- \ +// RUN: -config="{CheckOptions: [{key: zircon-human-readable-status.FormatLikeFunctions, value: 'my_printer'}]}" \ +// RUN: -header-filter=.* \ +// RUN: -- -std=c++11 + +#include + +#define PRINT(fmt...) printf(fmt); + +void my_printer(const char *fmt, ...) { + va_list args; + va_start(args, fmt); + printf(fmt, args); + va_end(args); +} + +#define WRAP(fmt...) wrapper(fmt); +void wrapper(const char *fmt, ...) { + va_list args; + va_start(args, fmt); + printf(fmt, args); + va_end(args); +} + +typedef int zx_status_t; + +const char *zx_status_get_string(zx_status_t status) { return "status"; } + +int main() { + zx_status_t status = 0; + printf("%d", status); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: non-human-readable log message + // CHECK-FIXES: printf("%s", zx_status_get_string(status)); + printf("%s", zx_status_get_string(status)); + my_printer("%d", status); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: non-human-readable log message + // CHECK-FIXES: my_printer("%s", zx_status_get_string(status)); + PRINT("%d", status); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: non-human-readable log message + // CHECK-FIXES: PRINT("%s", zx_status_get_string(status)); + + WRAP("%d", status); + wrapper("%d", status); + + unsigned notstatus = 0; + printf("%d", notstatus); + printf("%s", zx_status_get_string(status)); + my_printer("%d", notstatus); + PRINT("%d", notstatus); + WRAP("%d", notstatus); + wrapper("%d", notstatus); + + printf("%d, %d", status, notstatus); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: non-human-readable log message + // CHECK-FIXES: printf("%s, %d", zx_status_get_string(status), notstatus); + printf("%s, %d", zx_status_get_string(status), notstatus); + my_printer("%d, %d", status, notstatus); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: non-human-readable log message + // CHECK-FIXES: my_printer("%s, %d", zx_status_get_string(status), notstatus); + printf("%d, %d", notstatus, status); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: non-human-readable log message + // CHECK-FIXES: printf("%d, %s", notstatus, zx_status_get_string(status)); + PRINT("%d, %d", status, notstatus); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: non-human-readable log message + // CHECK-FIXES: PRINT("%s, %d", zx_status_get_string(status), notstatus); + + WRAP("%d, %d", notstatus, status); + wrapper("%d, %d", notstatus, status); + +}