[Unit Tests] BasicTests filesystem test failures

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[Unit Tests] BasicTests filesystem test failures

Yvan Roux via cfe-dev
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 <[hidden email]>
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
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Unit Tests] BasicTests filesystem test failures

Yvan Roux via cfe-dev
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 <[hidden email]> 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 <[hidden email]>
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

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Unit Tests] BasicTests filesystem test failures

Yvan Roux 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 <[hidden email]>
Sent: Tuesday, October 2, 2018 2:28:16 AM
To: David Greene
Cc: [hidden email]; 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 <[hidden email]<mailto:[hidden email]>> 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 <[hidden email]<mailto:[hidden email]>>
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
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev