[clang] fdab9f1 - [Clang] Check for response file existence prior to check for recursion

Serge Pavlov via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 3 00:50:33 PDT 2022


Author: Serge Pavlov
Date: 2022-11-03T14:49:58+07:00
New Revision: fdab9f1203eea48a7b8e4c55c7ceafc54653797c

URL: https://github.com/llvm/llvm-project/commit/fdab9f1203eea48a7b8e4c55c7ceafc54653797c
DIFF: https://github.com/llvm/llvm-project/commit/fdab9f1203eea48a7b8e4c55c7ceafc54653797c.diff

LOG: [Clang] Check for response file existence prior to check for recursion

As now errors in file operation are handled, check for file existence
must be done prior to check for recursion, otherwise reported errors are
misleading.

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

Added: 
    clang/test/Driver/Inputs/inc-inexistent.rsp
    clang/test/Driver/response-file-errs.c

Modified: 
    clang/tools/driver/driver.cpp
    clang/unittests/Driver/ToolChainTest.cpp
    llvm/lib/Support/CommandLine.cpp

Removed: 
    


################################################################################
diff  --git a/clang/test/Driver/Inputs/inc-inexistent.rsp b/clang/test/Driver/Inputs/inc-inexistent.rsp
new file mode 100644
index 0000000000000..c9ecfdf88ddd0
--- /dev/null
+++ b/clang/test/Driver/Inputs/inc-inexistent.rsp
@@ -0,0 +1 @@
+ at inexistent.txt

diff  --git a/clang/test/Driver/response-file-errs.c b/clang/test/Driver/response-file-errs.c
new file mode 100644
index 0000000000000..c0e02a984b9af
--- /dev/null
+++ b/clang/test/Driver/response-file-errs.c
@@ -0,0 +1,15 @@
+// If response file does not exist, '@file; directive remains unexpanded in
+// command line.
+//
+// RUN: %clang @%S/Inputs/inexistent.rsp -### 2>&1 | FileCheck --check-prefix=INEXISTENT %s
+// INEXISTENT: @{{.*}}Inputs/inexistent.rsp
+
+// As the above case but '@file' is in response file.
+//
+// RUN: %clang @%S/Inputs/inc-inexistent.rsp -### 2>&1 | FileCheck --check-prefix=INEXISTENT2 %s
+// INEXISTENT2: @{{.*}}inexistent.txt
+
+// If file in `@file` is a directory, it is an error.
+//
+// RUN: not %clang @%S/Inputs -### 2>&1 | FileCheck --check-prefix=DIRECTORY %s
+// DIRECTORY: cannot not open file '{{.*}}Inputs': {{[Ii]}}s a directory

diff  --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp
index 2cc3b48609cb3..4b1a246d99430 100644
--- a/clang/tools/driver/driver.cpp
+++ b/clang/tools/driver/driver.cpp
@@ -378,7 +378,7 @@ int clang_main(int Argc, char **Argv) {
   llvm::cl::ExpansionContext ECtx(A, Tokenizer);
   ECtx.setMarkEOLs(MarkEOLs);
   if (llvm::Error Err = ECtx.expandResponseFiles(Args)) {
-    llvm::errs() << Err << '\n';
+    llvm::errs() << toString(std::move(Err)) << '\n';
     return 1;
   }
 

diff  --git a/clang/unittests/Driver/ToolChainTest.cpp b/clang/unittests/Driver/ToolChainTest.cpp
index b143cd6329455..b45bab06d64b8 100644
--- a/clang/unittests/Driver/ToolChainTest.cpp
+++ b/clang/unittests/Driver/ToolChainTest.cpp
@@ -596,9 +596,10 @@ TEST(ToolChainTest, ConfigInexistentInclude) {
     ASSERT_TRUE(C);
     ASSERT_TRUE(C->containsError());
     EXPECT_EQ(1U, DiagConsumer->Errors.size());
-    EXPECT_STREQ("cannot read configuration file '" USERCONFIG
-                 "': cannot not open file '" UNEXISTENT "'",
-                 DiagConsumer->Errors[0].c_str());
+    EXPECT_STRCASEEQ("cannot read configuration file '" USERCONFIG
+                     "': cannot not open file '" UNEXISTENT
+                     "': no such file or directory",
+                     DiagConsumer->Errors[0].c_str());
   }
 
 #undef USERCONFIG

diff  --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp
index 136b813b1f6c8..fbaacbbbcf8a0 100644
--- a/llvm/lib/Support/CommandLine.cpp
+++ b/llvm/lib/Support/CommandLine.cpp
@@ -1158,9 +1158,11 @@ Error ExpansionContext::expandResponseFile(
   assert(sys::path::is_absolute(FName));
   llvm::ErrorOr<std::unique_ptr<MemoryBuffer>> MemBufOrErr =
       FS->getBufferForFile(FName);
-  if (!MemBufOrErr)
-    return llvm::createStringError(
-        MemBufOrErr.getError(), Twine("cannot not open file '") + FName + "'");
+  if (!MemBufOrErr) {
+    std::error_code EC = MemBufOrErr.getError();
+    return llvm::createStringError(EC, Twine("cannot not open file '") + FName +
+                                           "': " + EC.message());
+  }
   MemoryBuffer &MemBuf = *MemBufOrErr.get();
   StringRef Str(MemBuf.getBufferStart(), MemBuf.getBufferSize());
 
@@ -1262,7 +1264,7 @@ Error ExpansionContext::expandResponseFiles(
         if (auto CWD = FS->getCurrentWorkingDirectory()) {
           CurrDir = *CWD;
         } else {
-          return make_error<StringError>(
+          return createStringError(
               CWD.getError(), Twine("cannot get absolute path for: ") + FName);
         }
       } else {
@@ -1271,49 +1273,48 @@ Error ExpansionContext::expandResponseFiles(
       llvm::sys::path::append(CurrDir, FName);
       FName = CurrDir.c_str();
     }
+
+    ErrorOr<llvm::vfs::Status> Res = FS->status(FName);
+    if (!Res || !Res->exists()) {
+      std::error_code EC = Res.getError();
+      if (!InConfigFile) {
+        // If the specified file does not exist, leave '@file' unexpanded, as
+        // libiberty does.
+        if (!EC || EC == llvm::errc::no_such_file_or_directory) {
+          ++I;
+          continue;
+        }
+      }
+      if (!EC)
+        EC = llvm::errc::no_such_file_or_directory;
+      return createStringError(EC, Twine("cannot not open file '") + FName +
+                                       "': " + EC.message());
+    }
+    const llvm::vfs::Status &FileStatus = Res.get();
+
     auto IsEquivalent =
-        [FName, this](const ResponseFileRecord &RFile) -> ErrorOr<bool> {
-      ErrorOr<llvm::vfs::Status> LHS = FS->status(FName);
-      if (!LHS)
-        return LHS.getError();
+        [FileStatus, this](const ResponseFileRecord &RFile) -> ErrorOr<bool> {
       ErrorOr<llvm::vfs::Status> RHS = FS->status(RFile.File);
       if (!RHS)
         return RHS.getError();
-      return LHS->equivalent(*RHS);
+      return FileStatus.equivalent(*RHS);
     };
 
     // Check for recursive response files.
     for (const auto &F : drop_begin(FileStack)) {
       if (ErrorOr<bool> R = IsEquivalent(F)) {
         if (R.get())
-          return make_error<StringError>(
-              Twine("recursive expansion of: '") + F.File + "'", R.getError());
+          return createStringError(
+              R.getError(), Twine("recursive expansion of: '") + F.File + "'");
       } else {
-        return make_error<StringError>(Twine("cannot open file: ") + F.File,
-                                       R.getError());
+        return createStringError(R.getError(),
+                                 Twine("cannot open file: ") + F.File);
       }
     }
 
     // Replace this response file argument with the tokenization of its
     // contents.  Nested response files are expanded in subsequent iterations.
     SmallVector<const char *, 0> ExpandedArgv;
-    if (!InConfigFile) {
-      // If the specified file does not exist, leave '@file' unexpanded, as
-      // libiberty does.
-      ErrorOr<llvm::vfs::Status> Res = FS->status(FName);
-      if (!Res) {
-        std::error_code EC = Res.getError();
-        if (EC == llvm::errc::no_such_file_or_directory) {
-          ++I;
-          continue;
-        }
-      } else {
-        if (!Res->exists()) {
-          ++I;
-          continue;
-        }
-      }
-    }
     if (Error Err = expandResponseFile(FName, ExpandedArgv))
       return Err;
 


        


More information about the cfe-commits mailing list