Crash from SourceCode.cpp:235

[aschenk-mw]

I was getting a crash in clangd on a large source file with many includes. I tried to bisect the file to create a small standalone repo, but was not able to do so and am otherwise unable to share the source.

I've included the stack below. I did build llvm with -DCMAKE_BUILD_TYPE=RelWithDebInfo and attach gdb

What I did notice is SourceCode.cpp:235
StringRef SpellingFile = SM.getSLocEntry(Spelling.first).getFile().getName();

the FileId to getSLocEntry is invalid. If I change SourceCode.cpp to something like:

  bool InvalidSLocEntry = false;
  auto SLocEntry = SM.getSLocEntry(Spelling.first, &InvalidSLocEntry);
  if(InvalidSLocEntry) {
    return false;
  }
  StringRef SpellingFile = SLocEntry.getFile().getName();

then at least I don't get a crash any longer. When running clangd --check on the problematic file I do still get:
I[15:50:02.353] Building AST...
E[15:50:03.089] [fe_pch_malformed] Line 1: malformed or corrupted AST file: 'incorrectly-formatted source location entry in AST file'
I[15:50:03.089] Indexing AST...

So there is still a secondary underlying bug I guess, but at least now I'm not crashing clangd from vscode whenever I open this file.

Logs
See backtrace below

System information
Output of clangd --version:
clangd version 17.0.4
Features: linux
Platform: x86_64-unknown-linux-gnu

Editor/LSP plugin:
VSCode, but reproducible with clangd --check

Operating system:
Debian GNU/Linux 11

Backtrace from gdb

#0 clang::SrcMgr::FileInfo::getName (this=) at /ssd/llvm/llvm-project-17.0.4.src/clang/include/clang/Basic/SourceManager.h:349
#1 clang::clangd::isSpelledInSource (Loc=Loc@entry=..., SM=...) at /ssd/llvm/llvm-project-17.0.4.src/clang-tools-extra/clangd/SourceCode.cpp:240
#2 0x00005555566f102e in clang::clangd::nameLocation (D=..., SM=...) at /ssd/llvm/llvm-project-17.0.4.src/clang-tools-extra/clangd/AST.cpp:177
#3 0x0000555556a392e8 in clang::clangd::(anonymous namespace)::isPrivateProtoDecl (ND=...) at /ssd/llvm/llvm-project-17.0.4.src/clang-tools-extra/clangd/index/SymbolCollector.cpp:71
#4 clang::clangd::SymbolCollector::shouldCollectSymbol (ND=..., ASTCtx=..., Opts=..., IsMainFileOnly=) at /ssd/llvm/llvm-project-17.0.4.src/clang-tools-extra/clangd/index/SymbolCollector.cpp:517
#5 0x0000555556a399df in clang::clangd::SymbolCollector::handleDeclOccurrence (this=0x7fffffffa618, D=0x5555637776e0, Roles=131084, Relations=..., Loc=..., ASTNode=...)
at /ssd/llvm/llvm-project-17.0.4.src/clang-tools-extra/clangd/index/SymbolCollector.cpp:609
#6 0x000055555716d654 in clang::index::IndexingContext::handleDeclOccurrence (this=this@entry=0x7fffffffa1f8, D=D@entry=0x5555637776e0, Loc=Loc@entry=..., IsRef=, Parent=Parent@entry=0x5555637775f0, Roles=131084, Relations=...,
OrigE=0x555563777958, OrigD=0x5555637776e0, ContainerDC=0x555563777638) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexingContext.cpp:450
#7 0x000055555716d706 in clang::index::IndexingContext::handleReference (this=0x7fffffffa1f8, D=0x5555637776e0, Loc=..., Parent=0x5555637775f0, DC=0x555563777638, Roles=8, Relations=..., RefE=0x555563777958, RefD=0x0)
at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexingContext.cpp:89
#8 0x000055555719c7c7 in (anonymous namespace)::BodyIndexer::VisitMemberExpr (E=0x555563777958, this=) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexBody.cpp:153
#9 clang::RecursiveASTVisitor<(anonymous namespace)::BodyIndexer>::WalkUpFromMemberExpr (S=0x555563777958, this=) at /ssd/llvm/llvm-project-17.0.4.src/build/tools/clang/include/clang/AST/StmtNodes.inc:1255
#10 clang::RecursiveASTVisitor<(anonymous namespace)::BodyIndexer>::TraverseMemberExpr (this=this@entry=0x7fffffff9e88, S=S@entry=0x555563777958, Queue=Queue@entry=0x7fffffff9de8)
at /ssd/llvm/llvm-project-17.0.4.src/clang/include/clang/AST/RecursiveASTVisitor.h:2454
#11 0x0000555557190c5c in clang::RecursiveASTVisitor<(anonymous namespace)::BodyIndexer>::dataTraverseNode (this=0x7fffffff9e88, S=0x555563777958, Queue=0x7fffffff9de8)
at /ssd/llvm/llvm-project-17.0.4.src/build/tools/clang/include/clang/AST/StmtNodes.inc:1255
#12 clang::RecursiveASTVisitor<(anonymous namespace)::BodyIndexer>::TraverseStmt (this=this@entry=0x7fffffff9e88, S=, Queue=Queue@entry=0x0)
at /ssd/llvm/llvm-project-17.0.4.src/clang/include/clang/AST/RecursiveASTVisitor.h:686
#13 0x000055555718f659 in clang::index::IndexingContext::indexBody (this=, S=0x0, Parent=, DC=0x4f) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexBody.cpp:508
#14 0x00005555571812ab in (anonymous namespace)::IndexingDeclVisitor::VisitFunctionDecl (this=0x7fffffffa050, D=0x5555637775f0) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexDecl.cpp:299
#15 0x000055555717f8d9 in clang::index::IndexingContext::indexDecl (this=this@entry=0x7fffffffa1f8, D=D@entry=0x5555637775f0) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexDecl.cpp:783
#16 0x000055555717fe1b in clang::index::IndexingContext::indexDeclContext (this=0x7fffffffa1f8, DC=) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexDecl.cpp:795
#17 (anonymous namespace)::IndexingDeclVisitor::VisitNamespaceDecl (this=, D=) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexDecl.cpp:594
#18 0x000055555717f8d9 in clang::index::IndexingContext::indexDecl (this=this@entry=0x7fffffffa1f8, D=D@entry=0x555563777060) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexDecl.cpp:783
#19 0x000055555717fe1b in clang::index::IndexingContext::indexDeclContext (this=0x7fffffffa1f8, DC=) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexDecl.cpp:795
#20 (anonymous namespace)::IndexingDeclVisitor::VisitNamespaceDecl (this=, D=) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexDecl.cpp:594
#21 0x000055555717f8d9 in clang::index::IndexingContext::indexDecl (this=this@entry=0x7fffffffa1f8, D=D@entry=0x55557dea5500) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexDecl.cpp:783
#22 0x000055555717fe1b in clang::index::IndexingContext::indexDeclContext (this=0x7fffffffa1f8, DC=) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexDecl.cpp:795
#23 (anonymous namespace)::IndexingDeclVisitor::VisitNamespaceDecl (this=, D=) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexDecl.cpp:594
#24 0x000055555717f8d9 in clang::index::IndexingContext::indexDecl (this=this@entry=0x7fffffffa1f8, D=D@entry=0x55557dea5478) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexDecl.cpp:783
#25 0x000055555717fe1b in clang::index::IndexingContext::indexDeclContext (this=0x7fffffffa1f8, DC=) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexDecl.cpp:795
#26 (anonymous namespace)::IndexingDeclVisitor::VisitNamespaceDecl (this=, D=) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexDecl.cpp:594
#27 0x000055555717f8d9 in clang::index::IndexingContext::indexDecl (this=this@entry=0x7fffffffa1f8, D=D@entry=0x55557dea5408) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexDecl.cpp:783
#28 0x000055555717fae0 in clang::index::IndexingContext::indexTopLevelDecl (this=0x7fffffffa1f8, D=0x55557dea5408) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexDecl.cpp:810
#29 0x000055555716c27c in clang::index::indexTopLevelDecls (Ctx=..., PP=..., Decls=..., DataConsumer=..., Opts=...) at /ssd/llvm/llvm-project-17.0.4.src/clang/lib/Index/IndexingAction.cpp:286
#30 0x0000555556a15a16 in clang::clangd::(anonymous namespace)::indexSymbols (AST=..., PP=..., DeclsToIndex=..., MacroRefsToIndex=MacroRefsToIndex@entry=0x7fffffffbb90, PI=..., IsIndexMainAST=, Version=...,
CollectMainFileRefs=) at /ssd/llvm/llvm-project-17.0.4.src/clang-tools-extra/clangd/index/FileIndex.cpp:83
#31 0x0000555556a1575b in clang::clangd::indexMainDecls (AST=...) at /ssd/llvm/llvm-project-17.0.4.src/clang-tools-extra/clangd/index/FileIndex.cpp:224
#32 0x0000555556a1a597 in clang::clangd::FileIndex::updateMain (this=0x7fffffffbce0, Path=..., AST=...) at /ssd/llvm/llvm-project-17.0.4.src/clang-tools-extra/clangd/index/FileIndex.cpp:470
#33 0x00005555566e5d08 in clang::clangd::(anonymous namespace)::Checker::buildAST (this=0x7fffffffb378) at /ssd/llvm/llvm-project-17.0.4.src/clang-tools-extra/clangd/tool/Check.cpp:259
#34 clang::clangd::check (File=..., TFS=..., Opts=...) at /ssd/llvm/llvm-project-17.0.4.src/clang-tools-extra/clangd/tool/Check.cpp:487
#35 0x00005555566db4dc in clang::clangd::clangdMain (argc=, argv=) at /ssd/llvm/llvm-project-17.0.4.src/clang-tools-extra/clangd/tool/ClangdMain.cpp:975

[HighCommander4]

I think the described change is a reasonable hardening of the implementation of isSpelledInSource() to handle an invalid source location as input. Would you like to submit a PR for it?

It would of course be nice to further investigate why the calling code is passing in an invalid source location, but without a reproducer we can't really do that.

[aschenk-mw]

Duplicate of llvm/llvm-project#76667