[clang-tools-extra] 0540485 - [libtooling][clang-tidy] Fix crashing on rendering invalid SourceRanges
Balazs Benics via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 29 00:57:14 PST 2021
Author: Balazs Benics
Date: 2021-11-29T09:56:43+01:00
New Revision: 0540485436c4dd225e6a40e6db1240f096d145d3
URL: https://github.com/llvm/llvm-project/commit/0540485436c4dd225e6a40e6db1240f096d145d3
DIFF: https://github.com/llvm/llvm-project/commit/0540485436c4dd225e6a40e6db1240f096d145d3.diff
LOG: [libtooling][clang-tidy] Fix crashing on rendering invalid SourceRanges
Invalid SourceRanges can occur generally if the code does not compile,
thus we expect clang error diagnostics.
Unlike `clang`, `clang-tidy` did not swallow invalid source ranges, but
tried to highlight them, and blow various assertions.
The following two examples produce invalid source ranges, but this is
not a complete list:
void test(x); // error: unknown type name 'x'
struct Foo {
member; // error: C++ requires a type specifier for all declarations
};
Thanks @whisperity helping me fix this.
Reviewed-By: xazax.hun
Differential Revision: https://reviews.llvm.org/D114254
Added:
Modified:
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 5b410ba9d2ea4..764866d550839 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -79,16 +79,22 @@ class ClangTidyDiagnosticRenderer : public DiagnosticRenderer {
return CharSourceRange::getCharRange(SourceRange.getBegin(), End);
};
+ // We are only interested in valid ranges.
+ auto ValidRanges =
+ llvm::make_filter_range(Ranges, [](const CharSourceRange &R) {
+ return R.getAsRange().isValid();
+ });
+
if (Level == DiagnosticsEngine::Note) {
Error.Notes.push_back(TidyMessage);
- for (const CharSourceRange &SourceRange : Ranges)
+ for (const CharSourceRange &SourceRange : ValidRanges)
Error.Notes.back().Ranges.emplace_back(Loc.getManager(),
ToCharRange(SourceRange));
return;
}
assert(Error.Message.Message.empty() && "Overwriting a diagnostic message");
Error.Message = TidyMessage;
- for (const CharSourceRange &SourceRange : Ranges)
+ for (const CharSourceRange &SourceRange : ValidRanges)
Error.Message.Ranges.emplace_back(Loc.getManager(),
ToCharRange(SourceRange));
}
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
index 414c7967bc825..033798ceec46d 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
@@ -7,6 +7,11 @@ X(f)
int a[-1];
int b[0];
+void test(x);
+struct Foo {
+ member;
+};
+
// CHECK-MESSAGES: -input.cpp:2:1: warning: no previous prototype for function 'ff' [clang-diagnostic-missing-prototypes]
// CHECK-MESSAGES: -input.cpp:1:19: note: expanded from macro 'X'
// CHECK-MESSAGES: {{^}}note: expanded from here{{$}}
@@ -14,6 +19,8 @@ int b[0];
// CHECK-MESSAGES: -input.cpp:1:14: note: expanded from macro 'X'
// CHECK-MESSAGES: -input.cpp:3:7: error: 'a' declared as an array with a negative size [clang-diagnostic-error]
// CHECK-MESSAGES: -input.cpp:4:7: warning: zero size arrays are an extension [clang-diagnostic-zero-length-array]
+// CHECK-MESSAGES: -input.cpp:6:11: error: unknown type name 'x' [clang-diagnostic-error]
+// CHECK-MESSAGES: -input.cpp:8:3: error: C++ requires a type specifier for all declarations [clang-diagnostic-error]
// CHECK-YAML: ---
// CHECK-YAML-NEXT: MainSourceFile: '{{.*}}-input.cpp'
@@ -71,4 +78,20 @@ int b[0];
// CHECK-YAML-NEXT: Length: 1
// CHECK-YAML-NEXT: Level: Warning
// CHECK-YAML-NEXT: BuildDirectory: '{{.*}}'
+// CHECK-YAML-NEXT: - DiagnosticName: clang-diagnostic-error
+// CHECK-YAML-NEXT: DiagnosticMessage:
+// CHECK-YAML-NEXT: Message: 'unknown type name ''x'''
+// CHECK-YAML-NEXT: FilePath: '{{.*}}-input.cpp'
+// CHECK-YAML-NEXT: FileOffset: 67
+// CHECK-YAML-NEXT: Replacements: []
+// CHECK-YAML-NEXT: Level: Error
+// CHECK-YAML-NEXT: BuildDirectory: '{{.*}}'
+// CHECK-YAML-NEXT: - DiagnosticName: clang-diagnostic-error
+// CHECK-YAML-NEXT: DiagnosticMessage:
+// CHECK-YAML-NEXT: Message: 'C++ requires a type specifier for all declarations'
+// CHECK-YAML-NEXT: FilePath: '{{.*}}-input.cpp'
+// CHECK-YAML-NEXT: FileOffset: 86
+// CHECK-YAML-NEXT: Replacements: []
+// CHECK-YAML-NEXT: Level: Error
+// CHECK-YAML-NEXT: BuildDirectory: '{{.*}}'
// CHECK-YAML-NEXT: ...
diff --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp b/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
index f1c5bd29e9453..11c9ac9928221 100644
--- a/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -37,6 +37,33 @@ class HighlightTestCheck : public ClangTidyCheck {
}
};
+class InvalidRangeTestCheck : public ClangTidyCheck {
+public:
+ InvalidRangeTestCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override {
+ Finder->addMatcher(ast_matchers::varDecl().bind("var"), this);
+ }
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override {
+ const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var");
+ SourceLocation ValidBeginLoc = Var->getBeginLoc();
+ SourceLocation ValidEndLoc = Var->getEndLoc();
+ SourceLocation InvalidLoc;
+ ASSERT_TRUE(ValidBeginLoc.isValid());
+ ASSERT_TRUE(ValidEndLoc.isValid());
+ ASSERT_TRUE(InvalidLoc.isInvalid());
+
+ diag(ValidBeginLoc, "valid->valid")
+ << SourceRange(ValidBeginLoc, ValidEndLoc);
+ diag(ValidBeginLoc, "valid->invalid")
+ << SourceRange(ValidBeginLoc, InvalidLoc);
+ diag(ValidBeginLoc, "invalid->valid")
+ << SourceRange(InvalidLoc, ValidEndLoc);
+ diag(ValidBeginLoc, "invalid->invalid")
+ << SourceRange(InvalidLoc, InvalidLoc);
+ }
+};
+
} // namespace
TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
@@ -66,6 +93,24 @@ TEST(ClangTidyDiagnosticConsumer, HandlesSourceRangeHighlight) {
EXPECT_EQ(7ul, Errors[0].Message.Ranges[0].Length);
}
+TEST(ClangTidyDiagnosticConsumer, InvalidSourceLocationRangesIgnored) {
+ std::vector<ClangTidyError> Errors;
+ runCheckOnCode<InvalidRangeTestCheck>("int x;", &Errors);
+ EXPECT_EQ(4ul, Errors.size());
+
+ EXPECT_EQ("invalid->invalid", Errors[0].Message.Message);
+ EXPECT_TRUE(Errors[0].Message.Ranges.empty());
+
+ EXPECT_EQ("invalid->valid", Errors[1].Message.Message);
+ EXPECT_TRUE(Errors[1].Message.Ranges.empty());
+
+ EXPECT_EQ("valid->invalid", Errors[2].Message.Message);
+ EXPECT_TRUE(Errors[2].Message.Ranges.empty());
+
+ EXPECT_EQ("valid->valid", Errors[3].Message.Message);
+ EXPECT_EQ(1ul, Errors[3].Message.Ranges.size());
+}
+
} // namespace test
} // namespace tidy
} // namespace clang
More information about the cfe-commits
mailing list