[PATCH] D33275: Keep into account if files were virtual.
bruno added a comment. Ping! https://reviews.llvm.org/D33275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33275: Keep into account if files were virtual.
bruno added a comment. That sounds even better. Thanks @v.g.vassilev https://reviews.llvm.org/D33275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33275: Keep into account if files were virtual.
v.g.vassilev added a comment. @rsmith suggested to move this one interface layer down, that is `ContentCache::getBuffer`. Moving it there might fix more issues. I will try to improve it and land the new version if that's not urgent for you. https://reviews.llvm.org/D33275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33275: Keep into account if files were virtual.
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. We're hitting a very similar problem to this one, which I think this patch will likely fix. I can't come up with a reproducible testcase either. If you can write one better yet, but I'm ok with this as is. LGTM. https://reviews.llvm.org/D33275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33275: Keep into account if files were virtual.
v.g.vassilev added a comment. We have this pattern: FileID FID; const clang::FileEntry* FE = SM.getFileManager().getVirtualFile(source_name.str(), InputSize, 0 /* mod time*/); SM.overrideFileContents(FE, std::move(MB)); // MB is a llvm::MemoryBuffer FID = SM.createFileID(FE, NewLoc, SrcMgr::C_User); ... PP.EnterSourceFile(FID, /*DirLookup*/0, NewLoc); // PP is a clang::Preprocessor If the memory buffer contains eg. '//expected-error: ...' this forces the ContentCache to compute some source location information IIRC. This leads in hitting the disk, ignoring the fact that this is a virtual file with an overridden content. I will try to come up with a test but I may need to land some interpreter infrastructure first... https://reviews.llvm.org/D33275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33275: Keep into account if files were virtual.
bruno added a comment. Hi Vassil, In which case specifically this trigger for you? I'm curious because we're sometimes hitting non-deterministic crashes in ComputeLineNumbers, wonder if it's related. Any chance you can come up with a testcase? https://reviews.llvm.org/D33275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33275: Keep into account if files were virtual.
v.g.vassilev created this revision. In some cases we do not have files on disk. They are only available in the FileManager as virtual files and the SourceManager overrides their content. https://reviews.llvm.org/D33275 Files: lib/Basic/SourceManager.cpp lib/Frontend/VerifyDiagnosticConsumer.cpp Index: lib/Frontend/VerifyDiagnosticConsumer.cpp === --- lib/Frontend/VerifyDiagnosticConsumer.cpp +++ lib/Frontend/VerifyDiagnosticConsumer.cpp @@ -401,6 +401,9 @@ const FileEntry *FE = PP->LookupFile(Pos, Filename, false, nullptr, nullptr, CurDir, nullptr, nullptr, nullptr, nullptr); +// Check if the file was virtual +if (!FE) + FE = SM.getFileManager().getFile(Filename); if (!FE) { Diags.Report(Pos.getLocWithOffset(PH.C-PH.Begin), diag::err_verify_missing_file) << Filename << KindStr; Index: lib/Basic/SourceManager.cpp === --- lib/Basic/SourceManager.cpp +++ lib/Basic/SourceManager.cpp @@ -1225,8 +1225,15 @@ static void ComputeLineNumbers(DiagnosticsEngine , ContentCache *FI, llvm::BumpPtrAllocator , const SourceManager , bool ) { - // Note that calling 'getBuffer()' may lazily page in the file. - MemoryBuffer *Buffer = FI->getBuffer(Diag, SM, SourceLocation(), ); + MemoryBuffer *Buffer = nullptr; + if (SM.isFileOverridden(FI->ContentsEntry)) +Buffer + = const_cast(SM).getMemoryBufferForFile(FI->ContentsEntry, + ); + else +// Note that calling 'getBuffer()' may lazily page in the file. +Buffer = FI->getBuffer(Diag, SM, SourceLocation(), ); + if (Invalid) return; Index: lib/Frontend/VerifyDiagnosticConsumer.cpp === --- lib/Frontend/VerifyDiagnosticConsumer.cpp +++ lib/Frontend/VerifyDiagnosticConsumer.cpp @@ -401,6 +401,9 @@ const FileEntry *FE = PP->LookupFile(Pos, Filename, false, nullptr, nullptr, CurDir, nullptr, nullptr, nullptr, nullptr); +// Check if the file was virtual +if (!FE) + FE = SM.getFileManager().getFile(Filename); if (!FE) { Diags.Report(Pos.getLocWithOffset(PH.C-PH.Begin), diag::err_verify_missing_file) << Filename << KindStr; Index: lib/Basic/SourceManager.cpp === --- lib/Basic/SourceManager.cpp +++ lib/Basic/SourceManager.cpp @@ -1225,8 +1225,15 @@ static void ComputeLineNumbers(DiagnosticsEngine , ContentCache *FI, llvm::BumpPtrAllocator , const SourceManager , bool ) { - // Note that calling 'getBuffer()' may lazily page in the file. - MemoryBuffer *Buffer = FI->getBuffer(Diag, SM, SourceLocation(), ); + MemoryBuffer *Buffer = nullptr; + if (SM.isFileOverridden(FI->ContentsEntry)) +Buffer + = const_cast (SM).getMemoryBufferForFile(FI->ContentsEntry, + ); + else +// Note that calling 'getBuffer()' may lazily page in the file. +Buffer = FI->getBuffer(Diag, SM, SourceLocation(), ); + if (Invalid) return; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits