On Wed, Jan 23, 2019 at 9:54 AM Sam McCall <sammcc...@google.com> wrote:
> (Email is better than IRC if that's OK - I don't know this code that well
> so it takes me a while).
>
> Thanks, that's definitely interesting and not what I expected. I thought
> every call sequence r347205 changed the behavior of would have resulted in
> two calls to getStatValue().
> I guess the "pch"/"main" output is printed before the corresponding lines
> in run.sh?
>
Correct.
> Weird that we don't get any output from building the PCH, but I don't know
> well how PCH builds work.
>
> > It looks like FS->getCurrentWorkingDirectory() is set
> yet FileSystemOpts.WorkingDir.empty() is also true. Is that expected?
> I think so. The FileManager and the VFS each have their own concept of
> working directory, I guess for historical reasons.
> Making use of the VFS one but not the FileManager one seems reasonable.
>
> So the weirdness is that FileSystemStatCache::get() is returning true
> (i.e. file doesn't exist), when the file does exist.
> Possibilities:
> 1) we're serving this result from the FileSystemStatCache (and maybe it's
> being poisoned in a PCH-related way somehow). Except as far as I can tell,
> FileSystemStatCache is always null (FileManager::setStateCache has no
> callers).
> 2) the FS.openFileForRead call failed (ultimately ::open, if FS is the
> real FS)
> 3) the OwnedFile->status() call failed (ultimately ::fstat)
> 4) I'm misreading the code somehow
>
::open() fails with errno == 24, EMFILE.
This log statement here:
diff --git a/clang/lib/Basic/FileSystemStatCache.cpp
b/clang/lib/Basic/FileSystemStatCache.cpp
index d29ebb750fc..63fc4780237 100644
--- a/clang/lib/Basic/FileSystemStatCache.cpp
+++ b/clang/lib/Basic/FileSystemStatCache.cpp
@@ -70,9 +70,13 @@ bool FileSystemStatCache::get(StringRef Path, FileData
&Data, bool isFile,
//
// Because of this, check to see if the file exists with 'open'. If
the
// open succeeds, use fstat to get the stat info.
- auto OwnedFile = FS.openFileForRead(Path);
+ llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>> OwnedFile =
+ FS.openFileForRead(Path);
if (!OwnedFile) {
+if (Path.endswith("scheduling_lifecycle_state.h")) {
+fprintf(stderr, "hmm failed %s\n", OwnedFile.getError().message().c_str());
+}
// If the open fails, our "stat" fails.
R = CacheMissing;
} else {
causes clang to print "hmm failed Too many open files". That path should
maybe check if `OwnedFile.getError().value() == EMFILE &&
OwnedFile.getError().category() == std::generic_category()` on mac and
abort or diag or something more visible.
`ulimit -n` on macOS is pretty small -- do you see how your patch could
cause clang to keep more file handles open?
Here's how many files clang had open when I had a breakpoint in that error
path:
$ lsof -p 91890 | wc -l
343
>
> Could you find out which of these is going on? Either running in a
> debugger or adding some similar printfs to FileSystemStatCache::get()
> should be doable.
> I'm also going to try to work out how the patch could have affected this,
> but new vs correct much easier for me to compare than new vs old...
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
