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 @@ +@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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits