From: Yitzhak Mandelbaum Date: Wed, 4 May 2022 18:47:08 +0000 (+0000) Subject: [clang-tidy] Escape diagnostic messages before passing to `diag` in Transformer. X-Git-Tag: upstream/15.0.7~8626 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9a8d33dbd8a851ccb9821d5d1346aa225398cadc;p=platform%2Fupstream%2Fllvm.git [clang-tidy] Escape diagnostic messages before passing to `diag` in Transformer. Messages generated by Transformer rules may have `%` in them, which needs to be escaped before being passed to `diag`, which interprets them specially (and crashes if they are misused). Differential Revision: https://reviews.llvm.org/D124952 --- diff --git a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp index b8b2c25..bd76e67 100644 --- a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp @@ -27,6 +27,33 @@ static void verifyRule(const RewriteRuleWith &Rule) { " explicitly provide an empty explanation if none is desired"); } +// If a string unintentionally containing '%' is passed as a diagnostic, Clang +// will claim the string is ill-formed and assert-fail. This function escapes +// such strings so they can be safely used in diagnostics. +std::string escapeForDiagnostic(std::string ToEscape) { + // Optimize for the common case that the string does not contain `%` at the + // cost of an extra scan over the string in the slow case. + auto Pos = ToEscape.find('%'); + if (Pos == ToEscape.npos) + return ToEscape; + + std::string Result; + Result.reserve(ToEscape.size()); + // Convert position to a count. + ++Pos; + Result.append(ToEscape, 0, Pos); + Result += '%'; + + for (auto N = ToEscape.size(); Pos < N; ++Pos) { + const char C = ToEscape.at(Pos); + Result += C; + if (C == '%') + Result += '%'; + } + + return Result; +} + TransformerClangTidyCheck::TransformerClangTidyCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), @@ -99,7 +126,8 @@ void TransformerClangTidyCheck::check( } // Associate the diagnostic with the location of the first change. - DiagnosticBuilder Diag = diag((*Edits)[0].Range.getBegin(), *Explanation); + DiagnosticBuilder Diag = + diag((*Edits)[0].Range.getBegin(), escapeForDiagnostic(*Explanation)); for (const auto &T : *Edits) switch (T.Kind) { case transformer::EditKind::Range: diff --git a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp index 832b8b8..a3600ab 100644 --- a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp @@ -90,6 +90,27 @@ TEST(TransformerClangTidyCheckTest, DiagnosticsCorrectlyGenerated) { EXPECT_EQ(Errors[0].Message.FileOffset, 10U); } +TEST(TransformerClangTidyCheckTest, DiagnosticMessageEscaped) { + class GiveDiagWithPercentSymbol : public TransformerClangTidyCheck { + public: + GiveDiagWithPercentSymbol(StringRef Name, ClangTidyContext *Context) + : TransformerClangTidyCheck(makeRule(returnStmt(), + noopEdit(node(RootID)), + cat("bad code: x % y % z")), + Name, Context) {} + }; + std::string Input = "int somecode() { return 0; }"; + std::vector Errors; + EXPECT_EQ(Input, + test::runCheckOnCode(Input, &Errors)); + ASSERT_EQ(Errors.size(), 1U); + // The message stored in this field shouldn't include escaped percent signs, + // because the diagnostic printer should have _unescaped_ them when processing + // the diagnostic. The only behavior observable/verifiable by the test is that + // the presence of the '%' doesn't crash Clang. + EXPECT_EQ(Errors[0].Message.Message, "bad code: x % y % z"); +} + class IntLitCheck : public TransformerClangTidyCheck { public: IntLitCheck(StringRef Name, ClangTidyContext *Context)