ChuanqiXu9 wrote: > > I didn't update that patch. I observed a similar crash and I didn't meet it > > after I use this patch. That crash starts with TUScheduler's UpdatePreamble > > and then calls into ParsedAST::build. > > For me the trace is the following and it looks like that this patch can't > prevent this crash (but I applied this patch just to be sure and behavior is > the same). > > ``` > #0 0x000055ef32d0d0e0 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) > (.../llvm-project/build_clang/bin/clangd+0x52a0e0) > #1 0x000055ef32d09f6d SignalHandler(int, siginfo_t*, void*) Signals.cpp:0:0 > #2 0x00007fee7d0fb520 (/lib/x86_64-linux-gnu/libc.so.6+0x42520) > #3 0x000055ef3552c821 > clang::ASTReader::FindExternalLexicalDecls(clang::DeclContext const*, > llvm::function_ref<bool (clang::Decl::Kind)>, > llvm::SmallVectorImpl<clang::Decl*>&) > (.../llvm-project/build_clang/bin/clangd+0x2d49821) > #4 0x000055ef32e69ac9 > clang::DeclContext::LoadLexicalDeclsFromExternalStorage() const > (.../llvm-project/build_clang/bin/clangd+0x686ac9) > #5 0x000055ef32e69c4d clang::DeclContext::decls_begin() const > (.../llvm-project/build_clang/bin/clangd+0x686c4d) > #6 0x000055ef345dcf49 clang::clangd::indexHeaderSymbols(llvm::StringRef, > clang::ASTContext&, clang::Preprocessor&, > clang::include_cleaner::PragmaIncludes const&, clang::clangd::SymbolOrigin) > (.../llvm-project/build_clang/bin/clangd+0x1df9f49) > #7 0x000055ef345dd2ad > clang::clangd::FileIndex::updatePreamble(llvm::StringRef, llvm::StringRef, > clang::ASTContext&, clang::Preprocessor&, > clang::include_cleaner::PragmaIncludes const&) > (.../llvm-project/build_clang/bin/clangd+0x1dfa2ad) > #8 0x000055ef3426b18c void > llvm::detail::UniqueFunctionBase<void>::CallImpl<clang::clangd::(anonymous > namespace)::UpdateIndexCallbacks::onPreambleAST(llvm::StringRef, > llvm::StringRef, clang::clangd::CapturedASTCtx, > std::shared_ptr<clang::include_cleaner::PragmaIncludes > const>)::'lambda'()>(void*) ClangdServer.cpp:0:0 > #9 0x000055ef34675a06 void* > llvm::thread::ThreadProxy<std::tuple<clang::clangd::AsyncTaskRunner::runAsync(llvm::Twine > const&, llvm::unique_function<void ()>)::'lambda0'()>>(void*) > Threading.cpp:0:0 > #10 0x00007fee7d14dac3 start_thread ./nptl/pthread_create.c:442:8 > #11 0x00007fee7d1df8d0 ./misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:83:0 > ```
It is a different crash trace for me. The key point for me is in my workloads I didn't import in headers. So the current patch fixes all the crash for me. But this is indeed an evidance that this is better. As preamble actually is a lex level thing. > > > I think if you really want that, you can make a patch and let's review it. > > I plan to check how difficult it will be to implement this at the end of this > week if no one does this first The key point is, you need to look into PrecompiledPreamble (the name is worse then) in clang and you need to manage all the overriden files to make sure the `canReuse()` interface works as expected. So the amount of changes are larger. And I didn't like a lot of if-else ... I hate long workarounds. But after all, I encourage you to look on it. I will feel much better if there were someone else to contribute to this. https://github.com/llvm/llvm-project/pull/187432 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
