[PATCH] D124952: [clang-tidy] Escape diagnostic messages before passing to `diag` in Transformer.
Yitzhak Mandelbaum via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed May 4 11:49:28 PDT 2022
ymandel created this revision.
ymandel added a reviewer: gribozavr2.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang-tools-extra.
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).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D124952
Files:
clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
Index: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -90,6 +90,25 @@
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<ClangTidyError> Errors;
+ EXPECT_EQ(Input,
+ test::runCheckOnCode<GiveDiagWithPercentSymbol>(Input, &Errors));
+ ASSERT_EQ(Errors.size(), 1U);
+ // This should match the input; this test only verifies that the presence of
+ // '%' 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)
Index: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
@@ -27,6 +27,33 @@
" 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 @@
}
// 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:
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124952.427101.patch
Type: text/x-patch
Size: 3175 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220504/27ca2fa8/attachment.bin>
More information about the cfe-commits
mailing list