Skip to content

Commit 23b233c

Browse files
schenka0zyn0217
andauthored
[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 <[email protected]>
1 parent 5abbb7b commit 23b233c

File tree

2 files changed

+24
-1
lines changed

2 files changed

+24
-1
lines changed

clang-tools-extra/clangd/SourceCode.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,11 @@ bool isSpelledInSource(SourceLocation Loc, const SourceManager &SM) {
232232
if (Loc.isFileID())
233233
return true;
234234
auto Spelling = SM.getDecomposedSpellingLoc(Loc);
235-
StringRef SpellingFile = SM.getSLocEntry(Spelling.first).getFile().getName();
235+
bool InvalidSLocEntry = false;
236+
const auto SLocEntry = SM.getSLocEntry(Spelling.first, &InvalidSLocEntry);
237+
if (InvalidSLocEntry)
238+
return false;
239+
StringRef SpellingFile = SLocEntry.getFile().getName();
236240
if (SpellingFile == "<scratch space>")
237241
return false;
238242
if (SpellingFile == "<built-in>")

clang-tools-extra/clangd/unittests/SourceCodeTests.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -813,6 +813,25 @@ TEST(SourceCodeTests, isKeywords) {
813813
EXPECT_FALSE(isKeyword("override", LangOpts));
814814
}
815815

816+
TEST(SourceCodeTests, isSpelledInSource) {
817+
Annotations Test("");
818+
ParsedAST AST = TestTU::withCode(Test.code()).build();
819+
const SourceManager &SM = AST.getSourceManager();
820+
821+
EXPECT_TRUE(
822+
isSpelledInSource(SM.getLocForStartOfFile(SM.getMainFileID()), SM));
823+
824+
// Check that isSpelledInSource() handles various invalid source locations
825+
// gracefully.
826+
//
827+
// Returning true for SourceLocation() is a behavior that falls out of the
828+
// current implementation, which has an early exit for isFileID().
829+
// FIXME: Should it return false on SourceLocation()? Does it matter?
830+
EXPECT_TRUE(isSpelledInSource(SourceLocation(), SM));
831+
EXPECT_FALSE(isSpelledInSource(
832+
SourceLocation::getFromRawEncoding(SourceLocation::UIntTy(1 << 31)), SM));
833+
}
834+
816835
struct IncrementalTestStep {
817836
llvm::StringRef Src;
818837
llvm::StringRef Contents;

0 commit comments

Comments
 (0)