[clang-tools-extra] 2524000 - [clangd] Fix a nullptr-dereference crash in computeIncludeCleanerFindings.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2023-04-13T12:17:39+02:00
New Revision: 2524000187fc56a3f818a6562199037a90108eda

URL: https://github.com/llvm/llvm-project/commit/2524000187fc56a3f818a6562199037a90108eda
DIFF: https://github.com/llvm/llvm-project/commit/2524000187fc56a3f818a6562199037a90108eda.diff

LOG: [clangd] Fix a nullptr-dereference crash in computeIncludeCleanerFindings.

Be more robust, we shuold not crash  when we cannot find the corresponding token from the
tokenbuffer.

Differential Revision: https://reviews.llvm.org/D147686

Added: 
    

Modified: 
    clang-tools-extra/clangd/IncludeCleaner.cpp
    clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index e645b1cce6a09..b0e4d8dee5568 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -405,9 +405,14 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {
         // through an #include.
         while (SM.getFileID(Loc) != SM.getMainFileID())
           Loc = SM.getIncludeLoc(SM.getFileID(Loc));
-        const auto *Token = AST.getTokens().spelledTokenAt(Loc);
-        MissingIncludeDiagInfo DiagInfo{Ref.Target, Token->range(SM),
-                                        Providers};
+        auto TouchingTokens =
+            syntax::spelledTokensTouching(Loc, AST.getTokens());
+        assert(!TouchingTokens.empty());
+        // Loc points to the start offset of the ref token, here we use the last
+        // element of the TouchingTokens, e.g. avoid getting the "::" for
+        // "ns::^abc".
+        MissingIncludeDiagInfo DiagInfo{
+            Ref.Target, TouchingTokens.back().range(SM), Providers};
         MissingIncludes.push_back(std::move(DiagInfo));
       });
   std::vector<const Inclusion *> UnusedIncludes =

diff  --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index c0c15cd0db8e5..d0b99b7150779 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -413,6 +413,28 @@ TEST(IncludeCleaner, MacroExpandedThroughIncludes) {
   EXPECT_EQ(RefRange, Findings[1].SymRefRange);
 }
 
+TEST(IncludeCleaner, NoCrash) {
+  TestTU TU;
+  Annotations MainCode(R"cpp(
+    #include "all.h"
+    void test() {
+      [[1s]];
+    }
+    )cpp");
+  TU.Code = MainCode.code();
+  TU.AdditionalFiles["foo.h"] =
+      guard("int operator\"\"s(unsigned long long) { return 0; }");
+  TU.AdditionalFiles["all.h"] = guard("#include \"foo.h\"");
+  ParsedAST AST = TU.build();
+  const auto &MissingIncludes =
+      computeIncludeCleanerFindings(AST).MissingIncludes;
+  EXPECT_THAT(MissingIncludes, testing::SizeIs(1));
+  auto &SM = AST.getSourceManager();
+  EXPECT_EQ(
+      halfOpenToRange(SM, MissingIncludes.front().SymRefRange.toCharRange(SM)),
+      MainCode.range());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang