[PATCH] D82926: [libfuzzer] [clang] Add __has_feature(fuzzing_coverage)

[Max Moroz via Phabricator via cfe-commits]

Dor1s added a comment.

If we still decide to proceed with this, would it make sense to expand it to 
`sanitizer_coverage` based on any sancov instrumentation being enabled? As you 
mentioned in the description, there might be users who manually enable certain 
sancov flags. I think it's good to be able to support those usecases too   
(e.g. other fuzzing engines).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82926/new/

https://reviews.llvm.org/D82926



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82926: [libfuzzer] [clang] Add __has_feature(fuzzing_coverage)

[Max Moroz via Phabricator via cfe-commits]

Dor1s added a comment.

What usecase(s) do you have for this in mind?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82926/new/

https://reviews.llvm.org/D82926



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57474: Update SanitizerCoverage doc regarding the issue with pc-table and gc-sections.

[Max Moroz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Dor1s marked an inline comment as done.
Closed by commit rC352890: Update SanitizerCoverage doc regarding the issue 
with pc-table and gc-sections. (authored by Dor1s, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57474?vs=184766=184770#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57474/new/

https://reviews.llvm.org/D57474

Files:
  docs/SanitizerCoverage.rst


Index: docs/SanitizerCoverage.rst
===
--- docs/SanitizerCoverage.rst
+++ docs/SanitizerCoverage.rst
@@ -144,6 +144,11 @@
 
 **Experimental, may change or disappear in future**
 
+**Note:** this instrumentation might be incompatible with dead code stripping
+(``-Wl,-gc-sections``) for linkers other than LLD, thus resulting in a
+significant binary size overhead. For more information, see
+`Bug 34636 `_.
+
 With ``-fsanitize-coverage=pc-table`` the compiler will create a table of
 instrumented PCs. Requires either ``-fsanitize-coverage=inline-8bit-counters`` 
or
 ``-fsanitize-coverage=trace-pc-guard``.


Index: docs/SanitizerCoverage.rst
===
--- docs/SanitizerCoverage.rst
+++ docs/SanitizerCoverage.rst
@@ -144,6 +144,11 @@
 
 **Experimental, may change or disappear in future**
 
+**Note:** this instrumentation might be incompatible with dead code stripping
+(``-Wl,-gc-sections``) for linkers other than LLD, thus resulting in a
+significant binary size overhead. For more information, see
+`Bug 34636 `_.
+
 With ``-fsanitize-coverage=pc-table`` the compiler will create a table of
 instrumented PCs. Requires either ``-fsanitize-coverage=inline-8bit-counters`` or
 ``-fsanitize-coverage=trace-pc-guard``.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57474: Update SanitizerCoverage doc regarding the issue with pc-table and gc-sections.

[Max Moroz via Phabricator via cfe-commits]

Dor1s marked 2 inline comments as done.
Dor1s added inline comments.



Comment at: docs/SanitizerCoverage.rst:149
+(``-Wl,-gc-sections``), thus resulting in a significant binary size overhead.
+See `Bug 34636 `_ for more info.
+

morehouse wrote:
> Maybe add "for linkers other than LLD".  Dead stripping should work 100% for 
> LLD now.
Thanks, Matt! Done.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57474/new/

https://reviews.llvm.org/D57474



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57474: Update SanitizerCoverage doc regarding the issue with pc-table and gc-sections.

[Max Moroz via Phabricator via cfe-commits]

Dor1s updated this revision to Diff 184766.
Dor1s added a comment.
Herald added a project: clang.

Clarify that only linkers other than LLD are affected.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57474/new/

https://reviews.llvm.org/D57474

Files:
  docs/SanitizerCoverage.rst


Index: docs/SanitizerCoverage.rst
===
--- docs/SanitizerCoverage.rst
+++ docs/SanitizerCoverage.rst
@@ -144,6 +144,11 @@
 
 **Experimental, may change or disappear in future**
 
+**Note:** this instrumentation might be incompatible with dead code stripping
+(``-Wl,-gc-sections``) for linkers other than LLD, thus resulting in a
+significant binary size overhead. For more information, see
+`Bug 34636 `_.
+
 With ``-fsanitize-coverage=pc-table`` the compiler will create a table of
 instrumented PCs. Requires either ``-fsanitize-coverage=inline-8bit-counters`` 
or
 ``-fsanitize-coverage=trace-pc-guard``.


Index: docs/SanitizerCoverage.rst
===
--- docs/SanitizerCoverage.rst
+++ docs/SanitizerCoverage.rst
@@ -144,6 +144,11 @@
 
 **Experimental, may change or disappear in future**
 
+**Note:** this instrumentation might be incompatible with dead code stripping
+(``-Wl,-gc-sections``) for linkers other than LLD, thus resulting in a
+significant binary size overhead. For more information, see
+`Bug 34636 `_.
+
 With ``-fsanitize-coverage=pc-table`` the compiler will create a table of
 instrumented PCs. Requires either ``-fsanitize-coverage=inline-8bit-counters`` or
 ``-fsanitize-coverage=trace-pc-guard``.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57474: Update SanitizerCoverage doc regarding the issue with pc-table and gc-sections.

[Max Moroz via Phabricator via cfe-commits]

Dor1s added a comment.

I'm not sure if I wrote down the information correctly, PTAL :)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57474/new/

https://reviews.llvm.org/D57474



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57474: Update SanitizerCoverage doc regarding the issue with pc-table and gc-sections.

[Max Moroz via Phabricator via cfe-commits]

Dor1s created this revision.
Dor1s added a reviewer: morehouse.
Herald added a subscriber: cfe-commits.

There is a bug for this: https://bugs.llvm.org/show_bug.cgi?id=34636
But it would be also helpful to leave a note in the docs to prevent users from
running into issues, e.g. https://crbug.com/926588.


Repository:
  rC Clang

https://reviews.llvm.org/D57474

Files:
  docs/SanitizerCoverage.rst


Index: docs/SanitizerCoverage.rst
===
--- docs/SanitizerCoverage.rst
+++ docs/SanitizerCoverage.rst
@@ -144,6 +144,10 @@
 
 **Experimental, may change or disappear in future**
 
+**Note:** this instrumentation might be incompatible with dead code stripping
+(``-Wl,-gc-sections``), thus resulting in a significant binary size overhead.
+See `Bug 34636 `_ for more info.
+
 With ``-fsanitize-coverage=pc-table`` the compiler will create a table of
 instrumented PCs. Requires either ``-fsanitize-coverage=inline-8bit-counters`` 
or
 ``-fsanitize-coverage=trace-pc-guard``.


Index: docs/SanitizerCoverage.rst
===
--- docs/SanitizerCoverage.rst
+++ docs/SanitizerCoverage.rst
@@ -144,6 +144,10 @@
 
 **Experimental, may change or disappear in future**
 
+**Note:** this instrumentation might be incompatible with dead code stripping
+(``-Wl,-gc-sections``), thus resulting in a significant binary size overhead.
+See `Bug 34636 `_ for more info.
+
 With ``-fsanitize-coverage=pc-table`` the compiler will create a table of
 instrumented PCs. Requires either ``-fsanitize-coverage=inline-8bit-counters`` or
 ``-fsanitize-coverage=trace-pc-guard``.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55066: [ASan] Minor documentation fix: clarify static linking limitation.

[Max Moroz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC348863: [ASan] Minor documentation fix: clarify static 
linking limitation. (authored by Dor1s, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55066?vs=176712=177724#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55066/new/

https://reviews.llvm.org/D55066

Files:
  docs/AddressSanitizer.rst


Index: docs/AddressSanitizer.rst
===
--- docs/AddressSanitizer.rst
+++ docs/AddressSanitizer.rst
@@ -265,7 +265,7 @@
 * On 64-bit platforms AddressSanitizer maps (but not reserves) 16+ Terabytes of
   virtual address space. This means that tools like ``ulimit`` may not work as
   usually expected.
-* Static linking is not supported.
+* Static linking of executables is not supported.
 
 Supported Platforms
 ===


Index: docs/AddressSanitizer.rst
===
--- docs/AddressSanitizer.rst
+++ docs/AddressSanitizer.rst
@@ -265,7 +265,7 @@
 * On 64-bit platforms AddressSanitizer maps (but not reserves) 16+ Terabytes of
   virtual address space. This means that tools like ``ulimit`` may not work as
   usually expected.
-* Static linking is not supported.
+* Static linking of executables is not supported.
 
 Supported Platforms
 ===
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55066: [ASan] Minor documentation fix: clarify static linking limitation.

[Max Moroz via Phabricator via cfe-commits]

Dor1s added a comment.

Thanks @eugenis for explaining the issue to me over chat. I've updated the CL 
and the description. I can abandon it though, if you find it useless.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55066/new/

https://reviews.llvm.org/D55066



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55066: [ASan] Minor documentation fix: remove static linking limitation.

[Max Moroz via Phabricator via cfe-commits]

Dor1s updated this revision to Diff 176712.
Dor1s added a comment.

Restore the message with a couple clarifying words.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55066/new/

https://reviews.llvm.org/D55066

Files:
  docs/AddressSanitizer.rst


Index: docs/AddressSanitizer.rst
===
--- docs/AddressSanitizer.rst
+++ docs/AddressSanitizer.rst
@@ -265,7 +265,7 @@
 * On 64-bit platforms AddressSanitizer maps (but not reserves) 16+ Terabytes of
   virtual address space. This means that tools like ``ulimit`` may not work as
   usually expected.
-* Static linking is not supported.
+* Static linking of executables is not supported.
 
 Supported Platforms
 ===


Index: docs/AddressSanitizer.rst
===
--- docs/AddressSanitizer.rst
+++ docs/AddressSanitizer.rst
@@ -265,7 +265,7 @@
 * On 64-bit platforms AddressSanitizer maps (but not reserves) 16+ Terabytes of
   virtual address space. This means that tools like ``ulimit`` may not work as
   usually expected.
-* Static linking is not supported.
+* Static linking of executables is not supported.
 
 Supported Platforms
 ===
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55066: [ASan] Minor documentation fix: remove static linking limitation.

[Max Moroz via Phabricator via cfe-commits]

Dor1s added a comment.

In D55066#1315365 , @eugenis wrote:

> Sorry for the delay.
>  This is wrong, static linking is NOT supported.
>  You could be confusing it with static linking of asan runtime library to an 
> executable - that is and has always been the default.


Thanks, @eugenis! Should I change the message to something like "ASan runtime 
cannot be built with static linking."? Does it sound correct and perhaps less 
confusing?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55066/new/

https://reviews.llvm.org/D55066



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55066: [ASan] Minor documentation fix: remove static linking limitation.

[Max Moroz via Phabricator via cfe-commits]

Dor1s added a comment.

PTAL,  it's a single line change, I just need someone to confirm that I'm not 
mistaken :)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55066/new/

https://reviews.llvm.org/D55066



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55066: [ASan] Minor documentation fix: remove static linking limitation.

[Max Moroz via Phabricator via cfe-commits]

Dor1s created this revision.
Dor1s added reviewers: eugenis, kcc.
Herald added a subscriber: cfe-commits.

Looks like the documentation is out of date, given that static linking
is not only supported these days, but is actually a default mode.


Repository:
  rC Clang

https://reviews.llvm.org/D55066

Files:
  docs/AddressSanitizer.rst


Index: docs/AddressSanitizer.rst
===
--- docs/AddressSanitizer.rst
+++ docs/AddressSanitizer.rst
@@ -265,7 +265,6 @@
 * On 64-bit platforms AddressSanitizer maps (but not reserves) 16+ Terabytes of
   virtual address space. This means that tools like ``ulimit`` may not work as
   usually expected.
-* Static linking is not supported.
 
 Supported Platforms
 ===


Index: docs/AddressSanitizer.rst
===
--- docs/AddressSanitizer.rst
+++ docs/AddressSanitizer.rst
@@ -265,7 +265,6 @@
 * On 64-bit platforms AddressSanitizer maps (but not reserves) 16+ Terabytes of
   virtual address space. This means that tools like ``ulimit`` may not work as
   usually expected.
-* Static linking is not supported.
 
 Supported Platforms
 ===
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45178: Fixes errors with FS iterators caused by https://reviews.llvm.org/D44960

[Max Moroz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC329223: Fixes errors with FS iterators caused by 
https://reviews.llvm.org/D44960 (authored by Dor1s, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45178?vs=141026=141027#toc

Repository:
  rC Clang

https://reviews.llvm.org/D45178

Files:
  lib/Basic/VirtualFileSystem.cpp
  unittests/Basic/VirtualFileSystemTest.cpp


Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -442,16 +442,17 @@
   ScopedDir _dd(TestDirectory + "/d/d");
   ScopedDir _ddd(TestDirectory + "/d/d/d");
   ScopedLink _e("no_such_file", TestDirectory + "/e");
-  std::vector Expected = {_b, _bb, _d, _dd, _ddd};
 
-  std::vector Contents;
+  std::vector ExpectedBrokenSymlinks = {_a, _ba, _bc, _c, _e};
+  std::vector ExpectedNonBrokenSymlinks = {_b, _bb, _d, _dd, _ddd};
+  std::vector VisitedBrokenSymlinks;
+  std::vector VisitedNonBrokenSymlinks;
   std::error_code EC;
   for (vfs::recursive_directory_iterator I(*FS, Twine(TestDirectory), EC), E;
I != E; I.increment(EC)) {
-// Skip broken symlinks.
 auto EC2 = std::make_error_code(std::errc::no_such_file_or_directory);
 if (EC == EC2) {
-  EC.clear();
+  VisitedBrokenSymlinks.push_back(I->getName());
   continue;
 }
 // For bot debugging.
@@ -467,13 +468,20 @@
  << "EC message: " << EC2.message() << "\n";
 }
 ASSERT_FALSE(EC);
-Contents.push_back(I->getName());
+VisitedNonBrokenSymlinks.push_back(I->getName());
   }
 
-  // Check sorted contents.
-  llvm::sort(Contents.begin(), Contents.end());
-  EXPECT_EQ(Expected.size(), Contents.size());
-  EXPECT_TRUE(std::equal(Contents.begin(), Contents.end(), Expected.begin()));
+  // Check visited file names.
+  std::sort(VisitedBrokenSymlinks.begin(), VisitedBrokenSymlinks.end());
+  std::sort(VisitedNonBrokenSymlinks.begin(), VisitedNonBrokenSymlinks.end());
+  EXPECT_EQ(ExpectedBrokenSymlinks.size(), VisitedBrokenSymlinks.size());
+  EXPECT_TRUE(std::equal(VisitedBrokenSymlinks.begin(),
+ VisitedBrokenSymlinks.end(),
+ ExpectedBrokenSymlinks.begin()));
+  EXPECT_EQ(ExpectedNonBrokenSymlinks.size(), VisitedNonBrokenSymlinks.size());
+  EXPECT_TRUE(std::equal(VisitedNonBrokenSymlinks.begin(),
+ VisitedNonBrokenSymlinks.end(),
+ ExpectedNonBrokenSymlinks.begin()));
 }
 #endif
 
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -286,24 +286,26 @@
 
 public:
   RealFSDirIter(const Twine , std::error_code ) : Iter(Path, EC) {
-if (!EC && Iter != llvm::sys::fs::directory_iterator()) {
+if (Iter != llvm::sys::fs::directory_iterator()) {
   llvm::sys::fs::file_status S;
-  EC = llvm::sys::fs::status(Iter->path(), S, true);
+  std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);
   CurrentEntry = Status::copyWithNewName(S, Iter->path());
+  if (!EC)
+EC = ErrorCode;
 }
   }
 
   std::error_code increment() override {
 std::error_code EC;
 Iter.increment(EC);
-if (EC) {
-  return EC;
-} else if (Iter == llvm::sys::fs::directory_iterator()) {
+if (Iter == llvm::sys::fs::directory_iterator()) {
   CurrentEntry = Status();
 } else {
   llvm::sys::fs::file_status S;
-  EC = llvm::sys::fs::status(Iter->path(), S, true);
+  std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);
   CurrentEntry = Status::copyWithNewName(S, Iter->path());
+  if (!EC)
+EC = ErrorCode;
 }
 return EC;
   }


Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -442,16 +442,17 @@
   ScopedDir _dd(TestDirectory + "/d/d");
   ScopedDir _ddd(TestDirectory + "/d/d/d");
   ScopedLink _e("no_such_file", TestDirectory + "/e");
-  std::vector Expected = {_b, _bb, _d, _dd, _ddd};
 
-  std::vector Contents;
+  std::vector ExpectedBrokenSymlinks = {_a, _ba, _bc, _c, _e};
+  std::vector ExpectedNonBrokenSymlinks = {_b, _bb, _d, _dd, _ddd};
+  std::vector VisitedBrokenSymlinks;
+  std::vector VisitedNonBrokenSymlinks;
   std::error_code EC;
   for (vfs::recursive_directory_iterator I(*FS, Twine(TestDirectory), EC), E;
I != E; I.increment(EC)) {
-// Skip broken symlinks.
 auto EC2 = std::make_error_code(std::errc::no_such_file_or_directory);
 if (EC == EC2) {
-  EC.clear();
+  VisitedBrokenSymlinks.push_back(I->getName());
   continue;
 }
 // For bot debugging.
@@ -467,13 +468,20 @@

[PATCH] D45178: Fixes errors with FS iterators caused by https://reviews.llvm.org/D44960

[Max Moroz via Phabricator via cfe-commits]

Dor1s updated this revision to Diff 141026.
Dor1s added a comment.

Resolve merge conflicts and fix whitespaces.


Repository:
  rC Clang

https://reviews.llvm.org/D45178

Files:
  lib/Basic/VirtualFileSystem.cpp
  unittests/Basic/VirtualFileSystemTest.cpp


Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -442,16 +442,17 @@
   ScopedDir _dd(TestDirectory + "/d/d");
   ScopedDir _ddd(TestDirectory + "/d/d/d");
   ScopedLink _e("no_such_file", TestDirectory + "/e");
-  std::vector Expected = {_b, _bb, _d, _dd, _ddd};
 
-  std::vector Contents;
+  std::vector ExpectedBrokenSymlinks = {_a, _ba, _bc, _c, _e};
+  std::vector ExpectedNonBrokenSymlinks = {_b, _bb, _d, _dd, _ddd};
+  std::vector VisitedBrokenSymlinks;
+  std::vector VisitedNonBrokenSymlinks;
   std::error_code EC;
   for (vfs::recursive_directory_iterator I(*FS, Twine(TestDirectory), EC), E;
I != E; I.increment(EC)) {
-// Skip broken symlinks.
 auto EC2 = std::make_error_code(std::errc::no_such_file_or_directory);
 if (EC == EC2) {
-  EC.clear();
+  VisitedBrokenSymlinks.push_back(I->getName());
   continue;
 }
 // For bot debugging.
@@ -467,13 +468,20 @@
  << "EC message: " << EC2.message() << "\n";
 }
 ASSERT_FALSE(EC);
-Contents.push_back(I->getName());
+VisitedNonBrokenSymlinks.push_back(I->getName());
   }
 
-  // Check sorted contents.
-  llvm::sort(Contents.begin(), Contents.end());
-  EXPECT_EQ(Expected.size(), Contents.size());
-  EXPECT_TRUE(std::equal(Contents.begin(), Contents.end(), Expected.begin()));
+  // Check visited file names.
+  std::sort(VisitedBrokenSymlinks.begin(), VisitedBrokenSymlinks.end());
+  std::sort(VisitedNonBrokenSymlinks.begin(), VisitedNonBrokenSymlinks.end());
+  EXPECT_EQ(ExpectedBrokenSymlinks.size(), VisitedBrokenSymlinks.size());
+  EXPECT_TRUE(std::equal(VisitedBrokenSymlinks.begin(),
+ VisitedBrokenSymlinks.end(),
+ ExpectedBrokenSymlinks.begin()));
+  EXPECT_EQ(ExpectedNonBrokenSymlinks.size(), VisitedNonBrokenSymlinks.size());
+  EXPECT_TRUE(std::equal(VisitedNonBrokenSymlinks.begin(),
+ VisitedNonBrokenSymlinks.end(),
+ ExpectedNonBrokenSymlinks.begin()));
 }
 #endif
 
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -286,24 +286,26 @@
 
 public:
   RealFSDirIter(const Twine , std::error_code ) : Iter(Path, EC) {
-if (!EC && Iter != llvm::sys::fs::directory_iterator()) {
+if (Iter != llvm::sys::fs::directory_iterator()) {
   llvm::sys::fs::file_status S;
-  EC = llvm::sys::fs::status(Iter->path(), S, true);
+  std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);
   CurrentEntry = Status::copyWithNewName(S, Iter->path());
+  if (!EC)
+EC = ErrorCode;
 }
   }
 
   std::error_code increment() override {
 std::error_code EC;
 Iter.increment(EC);
-if (EC) {
-  return EC;
-} else if (Iter == llvm::sys::fs::directory_iterator()) {
+if (Iter == llvm::sys::fs::directory_iterator()) {
   CurrentEntry = Status();
 } else {
   llvm::sys::fs::file_status S;
-  EC = llvm::sys::fs::status(Iter->path(), S, true);
+  std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);
   CurrentEntry = Status::copyWithNewName(S, Iter->path());
+  if (!EC)
+EC = ErrorCode;
 }
 return EC;
   }


Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -442,16 +442,17 @@
   ScopedDir _dd(TestDirectory + "/d/d");
   ScopedDir _ddd(TestDirectory + "/d/d/d");
   ScopedLink _e("no_such_file", TestDirectory + "/e");
-  std::vector Expected = {_b, _bb, _d, _dd, _ddd};
 
-  std::vector Contents;
+  std::vector ExpectedBrokenSymlinks = {_a, _ba, _bc, _c, _e};
+  std::vector ExpectedNonBrokenSymlinks = {_b, _bb, _d, _dd, _ddd};
+  std::vector VisitedBrokenSymlinks;
+  std::vector VisitedNonBrokenSymlinks;
   std::error_code EC;
   for (vfs::recursive_directory_iterator I(*FS, Twine(TestDirectory), EC), E;
I != E; I.increment(EC)) {
-// Skip broken symlinks.
 auto EC2 = std::make_error_code(std::errc::no_such_file_or_directory);
 if (EC == EC2) {
-  EC.clear();
+  VisitedBrokenSymlinks.push_back(I->getName());
   continue;
 }
 // For bot debugging.
@@ -467,13 +468,20 @@
  << "EC message: " << EC2.message() << "\n";
 }
 ASSERT_FALSE(EC);
-Contents.push_back(I->getName());
+VisitedNonBrokenSymlinks.push_back(I->getName());
   }
 

[PATCH] D45178: Fixes errors caused by https://reviews.llvm.org/D44960

[Max Moroz via Phabricator via cfe-commits]

Dor1s added a comment.

Thanks a lot, Vedant! No worries, I'll commit it on behalf of Yuke @liaoyuke


Repository:
  rC Clang

https://reviews.llvm.org/D45178



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45178: Fixes errors caused by https://reviews.llvm.org/D44960

[Max Moroz via Phabricator via cfe-commits]

Dor1s added a comment.

Friendly ping. We'll probably land it tomorrow anyway, as it blocks another fix 
needed for llvm-cov, but having another pair of eyes to look at this would be 
still helpful :)


Repository:
  rC Clang

https://reviews.llvm.org/D45178



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45178: Fixes errors caused by https://reviews.llvm.org/D44960

[Max Moroz via Phabricator via cfe-commits]

Dor1s accepted this revision.
Dor1s added a comment.
This revision is now accepted and ready to land.

Looks good from my perspective, added Peter and Zachary who might know this 
code better :)


Repository:
  rC Clang

https://reviews.llvm.org/D45178



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits