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 <zyn7...@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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
