Page MenuHomePhabricator

Keep into account if files were virtual.

Authored by v.g.vassilev on May 17 2017, 6:25 AM.



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.

Diff Detail

Event Timeline

v.g.vassilev created this revision.May 17 2017, 6:25 AM
bruno added a subscriber: bruno.May 17 2017, 10:24 AM

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 added a comment.EditedMay 17 2017, 2:47 PM

We have this pattern:

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 accepted this revision.Jul 6 2017, 2:56 PM

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.

This revision is now accepted and ready to land.Jul 6 2017, 2:56 PM

@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 added a comment.Jul 6 2017, 3:06 PM

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