[PATCH] D65956: clang: Diag running out of file handles while looking for files
karies added a comment.
Herald added a project: All.
Similar to the concern raised at
https://github.com/llvm/llvm-project/commit/50fcf7285eeb001d751eadac5d001a0e216fd701
we have received user reports about this patch: with `-Ino-access-permissions
-Iall-good`, clang will throw an error (EACCES) even though header search goes
on and will find the header in `all-good`. That seems a misleading an
unnecessary error, especially as the header *is* found later, yet compilation
"fails" because of this diagnostic.
I would propose to collect these errors, and where originally (before this
patch), cling would complain "file not found" we could diagnose "while
searching for this header, the following errors have been seen" and reporting
the uniq-ed set of collected diags - but *only* in the case where the file
cannot be found. This would prevent spurious diags during iteration through the
SearchDirs when HeaderSearch actually finds the file. And I am fully aware that
it's pointless to propose something without providing a differential :-/
FYI here's what we do right now to handle the EACCES case:
patch
--- a/interpreter/llvm/src/tools/clang/lib/Lex/HeaderSearch.cpp
+++ b/interpreter/llvm/src/tools/clang/lib/Lex/HeaderSearch.cpp
@@ -367,7 +367,9 @@ Optional
HeaderSearch::getFileAndSuggestModule(
std::error_code EC = llvm::errorToErrorCode(File.takeError());
if (EC != llvm::errc::no_such_file_or_directory &&
EC != llvm::errc::invalid_argument &&
-EC != llvm::errc::is_a_directory && EC !=
llvm::errc::not_a_directory) {
+EC != llvm::errc::is_a_directory &&
+EC != llvm::errc::not_a_directory &&
+EC != llvm::errc::permission_denied) {
Diags.Report(IncludeLoc, diag::err_cannot_open_file)
<< FileName << EC.message();
}
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65956/new/
https://reviews.llvm.org/D65956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65956: clang: Diag running out of file handles while looking for files
This revision was automatically updated to reflect the committed changes.
Closed by commit rL368322: clang: Diag running out of file handles while
looking for files (authored by nico, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D65956?vs=214166=214192#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65956/new/
https://reviews.llvm.org/D65956
Files:
cfe/trunk/lib/Lex/HeaderSearch.cpp
Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -309,9 +309,18 @@
ModuleMap::KnownHeader *SuggestedModule) {
// If we have a module map that might map this header, load it and
// check whether we'll have a suggestion for a module.
- auto File = getFileMgr().getFile(FileName, /*OpenFile=*/true);
- if (!File)
+ llvm::ErrorOr File =
+ getFileMgr().getFile(FileName, /*OpenFile=*/true);
+ if (!File) {
+// For rare, surprising errors (e.g. "out of file handles"), diag the EC
+// message.
+std::error_code EC = File.getError();
+if (EC != std::errc::no_such_file_or_directory &&
+EC != std::errc::is_a_directory) {
+ Diags.Report(IncludeLoc, diag::err_cannot_open_file) << EC.message();
+}
return nullptr;
+ }
// If there is a module that corresponds to this header, suggest it.
if (!findUsableModuleForHeader(*File, Dir ? Dir : (*File)->getDir(),
Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -309,9 +309,18 @@
ModuleMap::KnownHeader *SuggestedModule) {
// If we have a module map that might map this header, load it and
// check whether we'll have a suggestion for a module.
- auto File = getFileMgr().getFile(FileName, /*OpenFile=*/true);
- if (!File)
+ llvm::ErrorOr File =
+ getFileMgr().getFile(FileName, /*OpenFile=*/true);
+ if (!File) {
+// For rare, surprising errors (e.g. "out of file handles"), diag the EC
+// message.
+std::error_code EC = File.getError();
+if (EC != std::errc::no_such_file_or_directory &&
+EC != std::errc::is_a_directory) {
+ Diags.Report(IncludeLoc, diag::err_cannot_open_file) << EC.message();
+}
return nullptr;
+ }
// If there is a module that corresponds to this header, suggest it.
if (!findUsableModuleForHeader(*File, Dir ? Dir : (*File)->getDir(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65956: clang: Diag running out of file handles while looking for files
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65956/new/ https://reviews.llvm.org/D65956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65956: clang: Diag running out of file handles while looking for files
thakis created this revision.
thakis added a reviewer: rnk.
clang would only print "file not found" when it's unable to find a
header file. If the reason for that is a file handle leak, that's not a
very useful error message. For errors that aren't in a small whitelist
("file not found", "file is directory"), print an error with the
strerror() output.
This changes behavior in corner cases: If clang was out of file handles
while looking in one -I dir but then suddenly wasn't when looking in the
next -I dir, and both directories contained a file with the desired
name, previously we'd silently return the file from the second
directory. For this reason, it's important to ignore "is a directory"
for this new diag: if a file foo/foo exists and -I -Ifoo are passed, an
include of "foo" should successfully open file "foo" in directory "foo/"
instead of complaining that "./foo" is a directory.
No test since we mostly hit this when there's a handle leak somewhere,
and currently there isn't one. I manually tested this with the repro
steps in comment 2 on the bug below.
Fixes PR42524.
https://reviews.llvm.org/D65956
Files:
clang/lib/Lex/HeaderSearch.cpp
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -309,9 +309,18 @@
ModuleMap::KnownHeader *SuggestedModule) {
// If we have a module map that might map this header, load it and
// check whether we'll have a suggestion for a module.
- auto File = getFileMgr().getFile(FileName, /*OpenFile=*/true);
- if (!File)
+ llvm::ErrorOr File =
+ getFileMgr().getFile(FileName, /*OpenFile=*/true);
+ if (!File) {
+// For rare, surprising errors (e.g. "out of file handles"), diag the EC
+// message.
+std::error_code EC = File.getError();
+if (EC != std::errc::no_such_file_or_directory &&
+EC != std::errc::is_a_directory) {
+ Diags.Report(IncludeLoc, diag::err_cannot_open_file) << EC.message();
+}
return nullptr;
+ }
// If there is a module that corresponds to this header, suggest it.
if (!findUsableModuleForHeader(*File, Dir ? Dir : (*File)->getDir(),
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -309,9 +309,18 @@
ModuleMap::KnownHeader *SuggestedModule) {
// If we have a module map that might map this header, load it and
// check whether we'll have a suggestion for a module.
- auto File = getFileMgr().getFile(FileName, /*OpenFile=*/true);
- if (!File)
+ llvm::ErrorOr File =
+ getFileMgr().getFile(FileName, /*OpenFile=*/true);
+ if (!File) {
+// For rare, surprising errors (e.g. "out of file handles"), diag the EC
+// message.
+std::error_code EC = File.getError();
+if (EC != std::errc::no_such_file_or_directory &&
+EC != std::errc::is_a_directory) {
+ Diags.Report(IncludeLoc, diag::err_cannot_open_file) << EC.message();
+}
return nullptr;
+ }
// If there is a module that corresponds to this header, suggest it.
if (!findUsableModuleForHeader(*File, Dir ? Dir : (*File)->getDir(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
