[clang-tools-extra] 23b233c - [clangd] Fix isSpelledInSource crash on invalid FileID (#76668)

[via cfe-commits]

Author: Andrew Schenk
Date: 2024-01-28T02:34:44-05:00
New Revision: 23b233c8adad5b81e185e50d04356fab64c2f870

URL: https://github.com/llvm/llvm-project/commit/23b233c8adad5b81e185e50d04356fab64c2f870
DIFF: https://github.com/llvm/llvm-project/commit/23b233c8adad5b81e185e50d04356fab64c2f870.diff

LOG: [clangd] Fix isSpelledInSource crash on invalid FileID (#76668)

This fixes the crash reported in #76667 and adds an initial unit test
for isSpelledInSource().

Note that in that issue there was still some underlying corrupted AST,
but this at least makes isSpelledInSource() robust to it.

---------

Co-authored-by: Younan Zhang <zyn7109 at gmail.com>

Added: 
    

Modified: 
    clang-tools-extra/clangd/SourceCode.cpp
    clang-tools-extra/clangd/unittests/SourceCodeTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
index 835038423fdf37..3e741f6e0b536b 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -232,7 +232,11 @@ bool isSpelledInSource(SourceLocation Loc, const SourceManager &SM) {
   if (Loc.isFileID())
     return true;
   auto Spelling = SM.getDecomposedSpellingLoc(Loc);
-  StringRef SpellingFile = SM.getSLocEntry(Spelling.first).getFile().getName();
+  bool InvalidSLocEntry = false;
+  const auto SLocEntry = SM.getSLocEntry(Spelling.first, &InvalidSLocEntry);
+  if (InvalidSLocEntry)
+    return false;
+  StringRef SpellingFile = SLocEntry.getFile().getName();
   if (SpellingFile == "<scratch space>")
     return false;
   if (SpellingFile == "<built-in>")

diff  --git a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
index 08abde87df6d4d..1be5b7f6a8dbba 100644
--- a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -813,6 +813,25 @@ TEST(SourceCodeTests, isKeywords) {
   EXPECT_FALSE(isKeyword("override", LangOpts));
 }
 
+TEST(SourceCodeTests, isSpelledInSource) {
+  Annotations Test("");
+  ParsedAST AST = TestTU::withCode(Test.code()).build();
+  const SourceManager &SM = AST.getSourceManager();
+
+  EXPECT_TRUE(
+      isSpelledInSource(SM.getLocForStartOfFile(SM.getMainFileID()), SM));
+
+  // Check that isSpelledInSource() handles various invalid source locations
+  // gracefully.
+  //
+  // Returning true for SourceLocation() is a behavior that falls out of the
+  // current implementation, which has an early exit for isFileID().
+  // FIXME: Should it return false on SourceLocation()? Does it matter?
+  EXPECT_TRUE(isSpelledInSource(SourceLocation(), SM));
+  EXPECT_FALSE(isSpelledInSource(
+      SourceLocation::getFromRawEncoding(SourceLocation::UIntTy(1 << 31)), SM));
+}
+
 struct IncrementalTestStep {
   llvm::StringRef Src;
   llvm::StringRef Contents;