juliehockett created this revision.
juliehockett added reviewers: aaron.ballman, hokein, ilya-biryukov.
juliehockett added a project: clang-tools-extra.
Herald added subscribers: xazax.hun, mgorny.
Adds a checker to the Fuchsia module to flag instances of attempting to log the
system's numerical representation of the status, instead of the more
human-friendly string representation. Matches formatting functions (the printf
family and a set of user-specified formatting-like ones) that have a
zx_status_t parameter and suggests a fixit to replace the zx_status_t parameter
with a call to zx_status_get_string(param).
https://reviews.llvm.org/D45468
Files:
clang-tidy/zircon/CMakeLists.txt
clang-tidy/zircon/HumanReadableStatusCheck.cpp
clang-tidy/zircon/HumanReadableStatusCheck.h
clang-tidy/zircon/ZirconTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/zircon-human-readable-status.rst
test/clang-tidy/zircon-human-readable-status.cpp
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 <stdio.h>
+
+#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);
+
+}
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: 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/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
+ <http://clang.llvm.org/extra/clang-tidy/checks/zircon-human-readable-status.html>`_ check
+
+ FIXME: add release notes.
+
+- New `zircon-temporary-objects
+ <http://clang.llvm.org/extra/clang-tidy/checks/zircon-temporary-objects.html>`_ check
+
+ Warns on construction of specific temporary objects in the Zircon kernel.
+
Improvements to include-fixer
-----------------------------
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<HumanReadableStatusCheck>(
+ "zircon-human-readable-status");
CheckFactories.registerCheck<TemporaryObjectsCheck>(
"zircon-temporary-objects");
}
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<std::string> 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<StringRef, 32>(
+ FunctionNames.begin(), FunctionNames.end()))))))
+ .bind("formatter"),
+ this);
+}
+
+void HumanReadableStatusCheck::check(const MatchFinder::MatchResult &Result) {
+ if (const auto *D = Result.Nodes.getNodeAs<CallExpr>("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<ImplicitCastExpr>(Arg)) {
+ // Parse the first argument.
+ if (ArgCount == 0) {
+ if (const auto *Str = dyn_cast<StringLiteral>(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<DeclRefExpr>(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/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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
