Keep into account if files were virtual.

[v.g.vassilev]

Summary

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.

[bruno]

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?

[v.g.vassilev]

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: ...' when running with -verify 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...

[bruno]

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.

[v.g.vassilev]

@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.

[bruno]

That sounds even better. Thanks @v.g.vassilev

[bruno]

Ping!

lib/Basic/SourceManager.cpp

 
 static LLVM_ATTRIBUTE_NOINLINE void
 ComputeLineNumbers(DiagnosticsEngine &Diag, ContentCache *FI,
                    llvm::BumpPtrAllocator &Alloc,
                    const SourceManager &SM, bool &Invalid);
 static void ComputeLineNumbers(DiagnosticsEngine &Diag, ContentCache *FI,
                                llvm::BumpPtrAllocator &Alloc,
                                const SourceManager &SM, bool &Invalid) {
+  MemoryBuffer *Buffer = nullptr;
+  if (SM.isFileOverridden(FI->ContentsEntry))
+    Buffer
+      = const_cast<SourceManager&>(SM).getMemoryBufferForFile(FI->ContentsEntry,
+                                                              &Invalid);
+  else
     // Note that calling 'getBuffer()' may lazily page in the file.
-  MemoryBuffer *Buffer = FI->getBuffer(Diag, SM, SourceLocation(), &Invalid);
+    Buffer = FI->getBuffer(Diag, SM, SourceLocation(), &Invalid);
+
   if (Invalid)
     return;
 
   // Find the file offsets of all of the *physical* source lines.  This does
   // not look at trigraphs, escaped newlines, or anything else tricky.
   SmallVector<unsigned, 256> LineOffsets;
 
   // Line #1 starts at char 0.

lib/Frontend/VerifyDiagnosticConsumer.cpp

if (!PH.Next("@")) {
         StringRef Filename(PH.C, PH.P-PH.C);
         PH.Advance();
 
         // Lookup file via Preprocessor, like a #include.
         const DirectoryLookup *CurDir;
         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;
           continue;
         }
 
         if (SM.translateFile(FE).isInvalid())
           SM.createFileID(FE, Pos, SrcMgr::C_User);