[PATCH] D159363: [clangd] SIGSEGV at clangd: DiagnosticConsumer Is Used After Free

[Ivan Murashko via Phabricator via cfe-commits]

ivanmurashko added a comment.

In D159363#4636581 <https://reviews.llvm.org/D159363#4636581>, @kadircet wrote:

> thanks, the fix LGTM as well.
>
> but i wonder how this surfaces, to make sure we're taking necessary precautions in the future. we definitely have a dangling reference, which isn't great. but it's surprising that we access diags consumer during indexing.
> I assume it's about the modules setup you're running clangd in. Do you have any stack traces that shows the execution path? my assumption is, this triggers when clangd ends up deserializing some symbols from a module. If these end up being important diagnostics, we might want to figure out how to emit diagnostics from these stages as well.

Yes, you are right. The diags consumer is triggered when it tries to read an implicit module that has some incompatibilities with the preamble headers.

The typical stack trace is below (that is LLVM-12 specific)

  clang::DiagnosticIDs::ProcessDiag(clang::DiagnosticsEngine&)
  clang::DiagnosticsEngine::EmitCurrentDiagnostic(bool)
  clang::DiagnosticBuilder::~DiagnosticBuilder()
  clang::ASTReader::diagnoseOdrViolations()
  clang::ASTReader::FinishedDeserializing()
  clang::DeclContext::LoadLexicalDeclsFromExternalStorage()
  clang::DeclContext::decls_begin()
  clang::index::IndexingContext::indexDeclContext(clang::DeclContext const*)
  clang::declvisitor::Base<llvm::make_const_ptr, (anonymous namespace)::IndexingDeclVisitor, bool>::Visit(clang::Decl const*)
  clang::index::IndexingContext::indexDecl(clang::Decl const*)
  clang::index::IndexingContext::indexDeclContext(clang::DeclContext const*)
  clang::declvisitor::Base<llvm::make_const_ptr, (anonymous namespace)::IndexingDeclVisitor, bool>::Visit(clang::Decl const*)
  clang::index::IndexingContext::indexDecl(clang::Decl const*)
  clang::index::IndexingContext::indexDeclContext(clang::DeclContext const*)
  clang::declvisitor::Base<llvm::make_const_ptr, (anonymous namespace)::IndexingDeclVisitor, bool>::Visit(clang::Decl const*)
  clang::index::IndexingContext::indexDecl(clang::Decl const*)
  clang::index::indexTopLevelDecls(clang::ASTContext&, clang::Preprocessor&, llvm::ArrayRef<clang::Decl const*>, clang::index::IndexDataConsumer&, clang::index::IndexingOptions)
  clang::clangd::(anonymous namespace)::indexSymbols(clang::ASTContext&, std::shared_ptr<clang::Preprocessor>, llvm::ArrayRef<clang::Decl*>, clang::clangd::MainFileMacros const*, clang::clangd::CanonicalIncludes const&, bool, llvm::StringRef, bool)
  clang::clangd::indexHeaderSymbols(llvm::StringRef, clang::ASTContext&, std::shared_ptr<clang::Preprocessor>, clang::clangd::CanonicalIncludes const&)
  clang::clangd::FileIndex::updatePreamble(llvm::StringRef, llvm::StringRef, clang::ASTContext&, std::shared_ptr<clang::Preprocessor>, clang::clangd::CanonicalIncludes const&)
  void
  void
  threadFuncAsync(void*)
  start_thread


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159363/new/

https://reviews.llvm.org/D159363