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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
