[cfe-dev] [Unit Tests] BasicTests filesystem test failures

[David Greene via cfe-dev]

That may be a commit that caused a problem but I don't have either of those commits
in my history.  A git bisect clearly pointed to r342232.  Anyway, I'm glad the issue was
visible to others and was fixed.

I won't be able to test the newer history for a couple of days but if there are issues with
r343488 and beyond, I'll let you know.

Thanks!

                                              -David

________________________________________
From: Sam McCall <sam.mccall at gmail.com>
Sent: Tuesday, October 2, 2018 2:28:16 AM
To: David Greene
Cc: cfe-dev at lists.llvm.org; Fedor Sergeev
Subject: Re: [Unit Tests] BasicTests filesystem test failures

Hi David,

I believe this was rather caused by r343460 and then fixed in r343488 (sorry about the delay between the two!)
The change in symlink behavior was deliberate but I didn't expect the clang tests to depend on it.

Please let me know if this is still broken after r343488.

Cheers, Sam

On Tue, Oct 2, 2018 at 4:32 AM David Greene <dag at cray.com<mailto:dag at cray.com>> wrote:
These tests are broken on X86-64 and AArch64 SLES 12:

[  FAILED  ] 2 tests, listed below:
[  FAILED  ] VirtualFileSystemTest.BrokenSymlinkRealFSIteration
[  FAILED  ] VirtualFileSystemTest.BrokenSymlinkRealFSRecursiveIteration

/clang/unittests/Basic/VirtualFileSystemTest.cpp:443: Failure
Value of: I->path() == _b
  Actual: false
Expected: true
/clang/unittests/Basic/VirtualFileSystemTest.cpp:443: Failure
Value of: I->path() == _b
  Actual: false
Expected: true
[  FAILED  ] VirtualFileSystemTest.BrokenSymlinkRealFSIteration (1 ms)
[ RUN      ] VirtualFileSystemTest.BasicRealFSRecursiveIteration
[       OK ] VirtualFileSystemTest.BasicRealFSRecursiveIteration (0 ms)
[ RUN      ] VirtualFileSystemTest.BrokenSymlinkRealFSRecursiveIteration
/clang/unittests/Basic/VirtualFileSystemTest.cpp:534: Failure
      Expected: ExpectedBrokenSymlinks.size()
      Which is: 5
To be equal to: VisitedBrokenSymlinks.size()
      Which is: 0
/clang/unittests/Basic/VirtualFileSystemTest.cpp:538: Failure
      Expected: ExpectedNonBrokenSymlinks.size()
      Which is: 5
To be equal to: VisitedNonBrokenSymlinks.size()
      Which is: 10
/clang/unittests/Basic/VirtualFileSystemTest.cpp:541: Failure
Value of: std::equal(VisitedNonBrokenSymlinks.begin(), VisitedNonBrokenSymlinks.end(), ExpectedNonBrokenSymlinks.begin())
  Actual: false
Expected: true
[  FAILED  ] VirtualFileSystemTest.BrokenSymlinkRealFSRecursiveIteration (1 ms)

The first failures (path inequality) are caused by the directory iterators not
skipping broken symlinks.  I did not dive deeply into the other failures as I
assume they show the same issue.

This commit appears to have caused the regression:

commit 446fa15e649709fc8bde40ed422d1e4794ac9559
Author: Sam McCall <sam.mccall at gmail.com<mailto:sam.mccall at gmail.com>>
Date:   Fri Sep 14 12:47:38 2018 +0000

    [VFS] vfs::directory_iterator yields path and file type instead of full Status

    Summary:
    Most callers I can find are using only `getName()`. Type is used by the
    recursive iterator.

    Now we don't have to call stat() on every listed file (on most platforms).
    Exceptions are e.g. Solaris where readdir() doesn't include type information.
    On those platforms we'll still stat() - see D51918.

    The result is significantly faster (stat() can be slow).
    My motivation: this may allow us to improve clang IO on large TUs with long
    include search paths. Caching readdir() results may allow us to skip many stat()
    and open() operations on nonexistent files.

    Reviewers: bkramer

    Subscribers: fedor.sergeev, cfe-commits

    Differential Revision: https://reviews.llvm.org/D51921

    git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@342232 91177308-0d34-0410-b5e6-96231b3b80d8

I am not well versed enough in the VFS layer to propose a fix, but I am
happy to help test patches.

                                    -David