ilya-biryukov wrote: > It would be useful to have a repro or a stack trace here. In particular, in > `SourceManagerForFile`, `RequiresNullTerminator` is `false`, so the assert > should _not_ fire on a non-null terminated file
The repro is attached to the commit. Here is the full stack trace: <details> <summary>Stacktrace</summary> ``` FormatTests: /usr/local/google/home/ibiryukov/code/llvm-project/llvm/lib/Support/MemoryBuffer.cpp:53: void llvm::MemoryBuffer::init(const char *, const char *, bool): Assertion `(!RequiresNullTerminator || BufEnd[0] == 0) && "Buffer is not null terminated!"' failed. #0 0x00005650ca150618 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /usr/local/google/home/ibiryukov/code/llvm-project/llvm/lib/Support/Unix/Signals.inc:804:13 #1 0x00005650ca14e5ac llvm::sys::RunSignalHandlers() /usr/local/google/home/ibiryukov/code/llvm-project/llvm/lib/Support/Signals.cpp:106:18 #2 0x00005650ca150de1 SignalHandler(int, siginfo_t*, void*) /usr/local/google/home/ibiryukov/code/llvm-project/llvm/lib/Support/Unix/Signals.inc:0:3 #3 0x00007f7861e49e20 (/lib/x86_64-linux-gnu/libc.so.6+0x3fe20) #4 0x00007f7861e9de5c __pthread_kill_implementation ./nptl/pthread_kill.c:44:76 #5 0x00007f7861e49d82 raise ./signal/../sysdeps/posix/raise.c:27:6 #6 0x00007f7861e324f0 abort ./stdlib/abort.c:81:7 #7 0x00007f7861e32418 _nl_load_domain ./intl/loadmsgcat.c:1177:9 #8 0x00007f7861e42692 (/lib/x86_64-linux-gnu/libc.so.6+0x38692) #9 0x00005650ca10b341 (tools/clang/unittests/Format/FormatTests+0x904341) #10 0x00005650ca1264d1 ErrorOr<std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer> > > /usr/local/google/home/ibiryukov/code/llvm-project/llvm/include/llvm/Support/ErrorOr.h:89:9 #11 0x00005650ca1264d1 llvm::vfs::detail::(anonymous namespace)::InMemoryFileAdaptor::getBuffer(llvm::Twine const&, long, bool, bool) /usr/local/google/home/ibiryukov/code/llvm-project/llvm/lib/Support/VirtualFileSystem.cpp:753:12 #12 0x00005650ca120166 ~ErrorOr /usr/local/google/home/ibiryukov/code/llvm-project/llvm/include/llvm/Support/ErrorOr.h:140:10 #13 0x00005650ca120166 llvm::vfs::FileSystem::getBufferForFile(llvm::Twine const&, long, bool, bool, bool) /usr/local/google/home/ibiryukov/code/llvm-project/llvm/lib/Support/VirtualFileSystem.cpp:126:1 #14 0x00005650ca18cbff clang::FileManager::getBufferForFileImpl(llvm::StringRef, long, bool, bool, bool) const /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Basic/FileManager.cpp:578:1 #15 0x00005650ca18caa8 clang::FileManager::getBufferForFile(clang::FileEntryRef, bool, bool, std::optional<long>, bool) /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Basic/FileManager.cpp:564:1 #16 0x00005650ca198fb6 operator bool /usr/local/google/home/ibiryukov/code/llvm-project/llvm/include/llvm/Support/ErrorOr.h:146:13 #17 0x00005650ca198fb6 clang::SrcMgr::ContentCache::getBufferOrNone(clang::DiagnosticsEngine&, clang::FileManager&, clang::SourceLocation) const /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Basic/SourceManager.cpp:133:8 #18 0x00005650ca19cd87 _M_is_engaged /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/optional:469:58 #19 0x00005650ca19cd87 operator bool /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/optional:983:22 #20 0x00005650ca19cd87 clang::SourceManager::getBufferDataOrNone(clang::FileID) const /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Basic/SourceManager.cpp:783:14 #21 0x00005650ca19ccb7 clang::SourceManager::getBufferData(clang::FileID, bool*) const /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Basic/SourceManager.cpp:0:0 #22 0x00005650ca204612 fatalError /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Format/TokenAnalyzer.cpp:52:36 #23 0x00005650ca204612 clang::format::Environment::make(llvm::StringRef, llvm::StringRef, llvm::ArrayRef<clang::tooling::Range>, unsigned int, unsigned int, unsigned int) /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Format/TokenAnalyzer.cpp:74:13 #24 0x00005650ca1adf03 operator bool /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/unique_ptr.h:481:22 #25 0x00005650ca1adf03 clang::format::internal::reformat(clang::format::FormatStyle const&, llvm::StringRef, llvm::ArrayRef<clang::tooling::Range>, unsigned int, unsigned int, unsigned int, llvm::StringRef, clang::format::FormattingAttemptStatus*) /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Format/Form at.cpp:3756:8 ``` </details> It is not `SourceManagerForFile` that is a problem here, rather `SourceManager::getBufferDataOrNone` always passes `RequiresNullTerminator = true` in [this call](https://github.com/llvm/llvm-project/blob/44f4e43b4fbbcc877436be6ae894d594c210fa59/clang/lib/Basic/SourceManager.cpp#L126). Changing that seems hard as I've noted in the PR comments. PS I am not sure why we need `RequiresNullTerminator` in the `FileManager` interface in the first place, but it is quite contagious and complicating other APIs with this parameter is something that I would really aim to avoid (including the SourceManager API). https://github.com/llvm/llvm-project/pull/131299 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
