[PATCH] D83536: [clangd] Index refs to main-file symbols as well

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:727
+  // Run the collector again with CollectMainFileRefs = true.
+  InMemoryFileSystem = new llvm::vfs::InMemoryFileSystem();
+  CollectorOpts.CollectMainFileRefs = true;
----------------
nridge wrote:
> kadircet wrote:
> > i don't think this is needed, am I missing something? IIRC, inmemoryfs should overwrite any existing files and even if it doesn't we are not really changing file contents in here.
> I initially didn't have this, and got the following assertion in the second `runSymbolCollector()` call:
> 
> ```ClangdTests: llvm/src/llvm/lib/Support/MemoryBuffer.cpp:48: void llvm::MemoryBuffer::init(const char *, const char *, bool): Assertion `(!RequiresNullTerminator || BufEnd[0] == 0) && "Buffer is not null terminated!"' failed.
>  #0 0x00007f4242501c67 llvm::sys::PrintStackTrace(llvm::raw_ostream&) llvm/src/llvm/lib/Support/Unix/Signals.inc:563:11
>  #1 0x00007f4242501e09 PrintStackTraceSignalHandler(void*) llvm/src/llvm/lib/Support/Unix/Signals.inc:624:1
>  #2 0x00007f424250060b llvm::sys::RunSignalHandlers() llvm/src/llvm/lib/Support/Signals.cpp:67:5
>  #3 0x00007f424250251d SignalHandler(int) llvm/src/llvm/lib/Support/Unix/Signals.inc:405:1
>  #4 0x00007f42482a10e0 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x110e0)
>  #5 0x00007f424157dfff raise /build/glibc-77giwP/glibc-2.24/signal/../sysdeps/unix/sysv/linux/raise.c:51:0
>  #6 0x00007f424157f42a abort /build/glibc-77giwP/glibc-2.24/stdlib/abort.c:91:0
>  #7 0x00007f4241576e67 __assert_fail_base /build/glibc-77giwP/glibc-2.24/assert/assert.c:92:0
>  #8 0x00007f4241576f12 (/lib/x86_64-linux-gnu/libc.so.6+0x2bf12)
>  #9 0x00007f4242427350 llvm::MemoryBuffer::init(char const*, char const*, bool) llvm/src/llvm/lib/Support/MemoryBuffer.cpp:49:17
> #10 0x00007f42424275b4 (anonymous namespace)::MemoryBufferMem<llvm::MemoryBuffer>::MemoryBufferMem(llvm::StringRef, bool) llvm/src/llvm/lib/Support/MemoryBuffer.cpp:89:3
> #11 0x00007f4242427400 llvm::MemoryBuffer::getMemBuffer(llvm::StringRef, llvm::StringRef, bool) llvm/src/llvm/lib/Support/MemoryBuffer.cpp:115:7
> #12 0x00007f424249ab20 llvm::vfs::detail::(anonymous namespace)::InMemoryFileAdaptor::getBuffer(llvm::Twine const&, long, bool, bool) llvm/src/llvm/lib/Support/VirtualFileSystem.cpp:615:12
> #13 0x00007f4242b0ecf1 clang::FileManager::getBufferForFile(clang::FileEntry const*, bool, bool) llvm/src/clang/lib/Basic/FileManager.cpp:474:5
> #14 0x00007f4242b52013 clang::SrcMgr::ContentCache::getBuffer(clang::DiagnosticsEngine&, clang::FileManager&, clang::SourceLocation, bool*) const llvm/src/clang/lib/Basic/SourceManager.cpp:172:8
> #15 0x00007f4242fe64c0 clang::SourceManager::getBuffer(clang::FileID, clang::SourceLocation, bool*) const llvm/src/clang/include/clang/Basic/SourceManager.h:976:5
> #16 0x00007f4242fe2425 clang::Preprocessor::EnterSourceFile(clang::FileID, clang::DirectoryLookup const*, clang::SourceLocation) llvm/src/clang/lib/Lex/PPLexerChange.cpp:77:29
> #17 0x00007f424301c104 clang::Preprocessor::EnterMainSourceFile() llvm/src/clang/lib/Lex/Preprocessor.cpp:547:5
> #18 0x00007f423d4a23fe clang::ParseAST(clang::Sema&, bool, bool) llvm/src/clang/lib/Parse/ParseAST.cpp:144:33
> #19 0x00007f42466df752 clang::ASTFrontendAction::ExecuteAction() llvm/src/clang/lib/Frontend/FrontendAction.cpp:1059:1
> #20 0x00007f42466df118 clang::FrontendAction::Execute() llvm/src/clang/lib/Frontend/FrontendAction.cpp:954:7
> #21 0x00007f4246655ae4 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) llvm/src/clang/lib/Frontend/CompilerInstance.cpp:984:23
> #22 0x00007f42472a622a clang::tooling::FrontendActionFactory::runInvocation(std::shared_ptr<clang::CompilerInvocation>, clang::FileManager*, std::shared_ptr<clang::PCHContainerOperations>, clang::DiagnosticConsumer*) llvm/src/clang/lib/Tooling/Tooling.cpp:410:14
> #23 0x00007f42472a60d4 clang::tooling::ToolInvocation::runInvocation(char const*, clang::driver::Compilation*, std::shared_ptr<clang::CompilerInvocation>, std::shared_ptr<clang::PCHContainerOperations>) llvm/src/clang/lib/Tooling/Tooling.cpp:385:3
> #24 0x00007f42472a5015 clang::tooling::ToolInvocation::run() llvm/src/clang/lib/Tooling/Tooling.cpp:370:3
> #25 0x0000000000bc916b clang::clangd::(anonymous namespace)::SymbolCollectorTest::runSymbolCollector(llvm::StringRef, llvm::StringRef, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) llvm/src/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:283:15
> #26 0x0000000000bda339 clang::clangd::(anonymous namespace)::SymbolCollectorTest_Refs_Test::TestBody() llvm/src/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:729:3
> ```
ah makes sense. looks like we are calling `MemoryBuffer:getMemBuffer`, which would make the buffers unusable after `runSymbolCollector` exits. I must've been lucky enough to not overwrite the stack.

could you put a comment mentioning the situation? then feel free to land.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83536