sepavloff updated this revision to Diff 469957.
sepavloff added a comment.
Small changes proposed by reviewers
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136090/new/
https://reviews.llvm.org/D136090
Files:
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Driver/Driver.cpp
clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
clang/test/Driver/config-file-errs.c
clang/tools/driver/driver.cpp
clang/unittests/Driver/ToolChainTest.cpp
flang/tools/flang-driver/driver.cpp
llvm/include/llvm/Support/CommandLine.h
llvm/lib/Support/CommandLine.cpp
llvm/unittests/Support/CommandLineTest.cpp
Index: llvm/unittests/Support/CommandLineTest.cpp
===================================================================
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -871,7 +871,7 @@
llvm::BumpPtrAllocator A;
llvm::cl::ExpansionContext ECtx(A, llvm::cl::TokenizeGNUCommandLine);
ECtx.setVFS(&FS).setCurrentDir(TestRoot).setRelativeNames(true);
- ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+ ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
EXPECT_THAT(Argv, testing::Pointwise(
StringEquality(),
{"test/test", "-flag_1", "-option_1", "-option_2",
@@ -933,7 +933,14 @@
#endif
llvm::cl::ExpansionContext ECtx(A, Tokenizer);
ECtx.setVFS(&FS).setCurrentDir(TestRoot);
- ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
+ llvm::Error Err = ECtx.expandResponseFiles(Argv);
+ ASSERT_TRUE((bool)Err);
+ SmallString<128> FilePath = SelfFilePath;
+ std::error_code EC = FS.makeAbsolute(FilePath);
+ ASSERT_FALSE((bool)EC);
+ std::string ExpectedMessage =
+ std::string("recursive expansion of: '") + std::string(FilePath) + "'";
+ ASSERT_TRUE(toString(std::move(Err)) == ExpectedMessage);
EXPECT_THAT(Argv,
testing::Pointwise(StringEquality(),
@@ -971,7 +978,7 @@
BumpPtrAllocator A;
llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine);
ECtx.setVFS(&FS).setCurrentDir(TestRoot);
- ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
+ ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
// ASSERT instead of EXPECT to prevent potential out-of-bounds access.
ASSERT_EQ(Argv.size(), 1 + NON_RSP_AT_ARGS + 2);
@@ -1005,7 +1012,7 @@
BumpPtrAllocator A;
llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine);
ECtx.setVFS(&FS).setCurrentDir(TestRoot).setRelativeNames(true);
- ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+ ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
EXPECT_THAT(Argv,
testing::Pointwise(StringEquality(), {"test/test", "-flag"}));
}
@@ -1025,7 +1032,7 @@
llvm::cl::ExpansionContext ECtx(A, cl::TokenizeWindowsCommandLine);
ECtx.setVFS(&FS).setCurrentDir(TestRoot).setMarkEOLs(true).setRelativeNames(
true);
- ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+ ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
const char *Expected[] = {"clang", "-Xclang", "-Wno-whatever", nullptr,
"input.cpp"};
ASSERT_EQ(std::size(Expected), Argv.size());
@@ -1038,6 +1045,30 @@
}
}
+TEST(CommandLineTest, BadResponseFile) {
+ BumpPtrAllocator A;
+ StringSaver Saver(A);
+ TempDir ADir("dir", /*Unique*/ true);
+ SmallString<128> AFilePath = ADir.path();
+ llvm::sys::path::append(AFilePath, "file.rsp");
+ std::string AFileExp = std::string("@") + std::string(AFilePath.str());
+ SmallVector<const char *, 2> Argv = {"clang", AFileExp.c_str()};
+
+ bool Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv);
+ ASSERT_TRUE(Res);
+ ASSERT_EQ(2U, Argv.size());
+ ASSERT_STREQ(Argv[0], "clang");
+ ASSERT_STREQ(Argv[1], AFileExp.c_str());
+
+ std::string ADirExp = std::string("@") + std::string(ADir.path());
+ Argv = {"clang", ADirExp.c_str()};
+ Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv);
+ ASSERT_FALSE(Res);
+ ASSERT_EQ(2U, Argv.size());
+ ASSERT_STREQ(Argv[0], "clang");
+ ASSERT_STREQ(Argv[1], ADirExp.c_str());
+}
+
TEST(CommandLineTest, SetDefaultValue) {
cl::ResetCommandLineParser();
@@ -1145,9 +1176,9 @@
llvm::BumpPtrAllocator A;
llvm::cl::ExpansionContext ECtx(A, cl::tokenizeConfigFile);
- bool Result = ECtx.readConfigFile(ConfigFile.path(), Argv);
+ llvm::Error Result = ECtx.readConfigFile(ConfigFile.path(), Argv);
- EXPECT_TRUE(Result);
+ EXPECT_FALSE((bool)Result);
EXPECT_EQ(Argv.size(), 13U);
EXPECT_STREQ(Argv[0], "-option_1");
EXPECT_STREQ(Argv[1],
Index: llvm/lib/Support/CommandLine.cpp
===================================================================
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -1153,14 +1153,14 @@
}
// FName must be an absolute path.
-llvm::Error
-ExpansionContext::expandResponseFile(StringRef FName,
- SmallVectorImpl<const char *> &NewArgv) {
+Error ExpansionContext::expandResponseFile(
+ StringRef FName, SmallVectorImpl<const char *> &NewArgv) {
assert(sys::path::is_absolute(FName));
llvm::ErrorOr<std::unique_ptr<MemoryBuffer>> MemBufOrErr =
FS->getBufferForFile(FName);
if (!MemBufOrErr)
- return llvm::errorCodeToError(MemBufOrErr.getError());
+ return llvm::createStringError(
+ MemBufOrErr.getError(), Twine("cannot not open file '") + FName + "'");
MemoryBuffer &MemBuf = *MemBufOrErr.get();
StringRef Str(MemBuf.getBufferStart(), MemBuf.getBufferSize());
@@ -1182,29 +1182,30 @@
// Tokenize the contents into NewArgv.
Tokenizer(Str, Saver, NewArgv, MarkEOLs);
- if (!RelativeNames)
+ // Expanded file content may require additional transformations, like using
+ // absolute paths instead of relative in '@file' constructs or expanding
+ // macros.
+ if (!RelativeNames && !InConfigFile)
return Error::success();
- llvm::StringRef BasePath = llvm::sys::path::parent_path(FName);
- // If names of nested response files should be resolved relative to including
- // file, replace the included response file names with their full paths
- // obtained by required resolution.
- for (auto &Arg : NewArgv) {
- if (!Arg)
+
+ StringRef BasePath = llvm::sys::path::parent_path(FName);
+ for (auto I = NewArgv.begin(), E = NewArgv.end(); I != E; ++I) {
+ const char *&Arg = *I;
+ if (Arg == nullptr)
continue;
// Substitute <CFGDIR> with the file's base path.
if (InConfigFile)
ExpandBasePaths(BasePath, Saver, Arg);
- // Skip non-rsp file arguments.
- if (Arg[0] != '@')
+ // Get expanded file name.
+ StringRef FileName(Arg);
+ if (!FileName.consume_front("@"))
continue;
-
- StringRef FileName(Arg + 1);
- // Skip if non-relative.
if (!llvm::sys::path::is_relative(FileName))
continue;
+ // Update expansion construct.
SmallString<128> ResponseFile;
ResponseFile.push_back('@');
ResponseFile.append(BasePath);
@@ -1216,9 +1217,8 @@
/// Expand response files on a command line recursively using the given
/// StringSaver and tokenization strategy.
-bool ExpansionContext::expandResponseFiles(
+Error ExpansionContext::expandResponseFiles(
SmallVectorImpl<const char *> &Argv) {
- bool AllExpanded = true;
struct ResponseFileRecord {
std::string File;
size_t End;
@@ -1262,9 +1262,8 @@
if (auto CWD = FS->getCurrentWorkingDirectory()) {
CurrDir = *CWD;
} else {
- // TODO: The error should be propagated up the stack.
- llvm::consumeError(llvm::errorCodeToError(CWD.getError()));
- return false;
+ return make_error<StringError>(
+ CWD.getError(), Twine("cannot get absolute path for: ") + FName);
}
} else {
CurrDir = CurrentDir;
@@ -1272,43 +1271,51 @@
llvm::sys::path::append(CurrDir, FName);
FName = CurrDir.c_str();
}
- auto IsEquivalent = [FName, this](const ResponseFileRecord &RFile) {
- llvm::ErrorOr<llvm::vfs::Status> LHS = FS->status(FName);
- if (!LHS) {
- // TODO: The error should be propagated up the stack.
- llvm::consumeError(llvm::errorCodeToError(LHS.getError()));
- return false;
- }
- llvm::ErrorOr<llvm::vfs::Status> RHS = FS->status(RFile.File);
- if (!RHS) {
- // TODO: The error should be propagated up the stack.
- llvm::consumeError(llvm::errorCodeToError(RHS.getError()));
- return false;
- }
+ auto IsEquivalent =
+ [FName, this](const ResponseFileRecord &RFile) -> ErrorOr<bool> {
+ ErrorOr<llvm::vfs::Status> LHS = FS->status(FName);
+ if (!LHS)
+ return LHS.getError();
+ ErrorOr<llvm::vfs::Status> RHS = FS->status(RFile.File);
+ if (!RHS)
+ return RHS.getError();
return LHS->equivalent(*RHS);
};
// Check for recursive response files.
- if (any_of(drop_begin(FileStack), IsEquivalent)) {
- // This file is recursive, so we leave it in the argument stream and
- // move on.
- AllExpanded = false;
- ++I;
- continue;
+ 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());
+ } else {
+ return make_error<StringError>(Twine("cannot open file: ") + F.File,
+ R.getError());
+ }
}
// 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 (llvm::Error Err = expandResponseFile(FName, ExpandedArgv)) {
- // We couldn't read this file, so we leave it in the argument stream and
- // move on.
- // TODO: The error should be propagated up the stack.
- llvm::consumeError(std::move(Err));
- AllExpanded = false;
- ++I;
- continue;
+ 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;
for (ResponseFileRecord &Record : FileStack) {
// Increase the end of all active records by the number of newly expanded
@@ -1327,7 +1334,7 @@
// don't have a chance to pop the stack when encountering recursive files at
// the end of the stream, so seeing that doesn't indicate a bug.
assert(FileStack.size() > 0 && Argv.size() == FileStack.back().End);
- return AllExpanded;
+ return Error::success();
}
bool cl::expandResponseFiles(int Argc, const char *const *Argv,
@@ -1344,7 +1351,21 @@
// Command line options can override the environment variable.
NewArgv.append(Argv + 1, Argv + Argc);
ExpansionContext ECtx(Saver.getAllocator(), Tokenize);
- return ECtx.expandResponseFiles(NewArgv);
+ if (Error Err = ECtx.expandResponseFiles(NewArgv)) {
+ errs() << toString(std::move(Err)) << '\n';
+ return false;
+ }
+ return true;
+}
+
+bool cl::ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer,
+ SmallVectorImpl<const char *> &Argv) {
+ ExpansionContext ECtx(Saver.getAllocator(), Tokenizer);
+ if (Error Err = ECtx.expandResponseFiles(Argv)) {
+ errs() << toString(std::move(Err)) << '\n';
+ return false;
+ }
+ return true;
}
ExpansionContext::ExpansionContext(BumpPtrAllocator &A, TokenizerCallback T)
@@ -1387,22 +1408,20 @@
return false;
}
-bool ExpansionContext::readConfigFile(StringRef CfgFile,
- SmallVectorImpl<const char *> &Argv) {
+Error ExpansionContext::readConfigFile(StringRef CfgFile,
+ SmallVectorImpl<const char *> &Argv) {
SmallString<128> AbsPath;
if (sys::path::is_relative(CfgFile)) {
AbsPath.assign(CfgFile);
if (std::error_code EC = FS->makeAbsolute(AbsPath))
- return false;
+ return make_error<StringError>(
+ EC, Twine("cannot get absolute path for " + CfgFile));
CfgFile = AbsPath.str();
}
InConfigFile = true;
RelativeNames = true;
- if (llvm::Error Err = expandResponseFile(CfgFile, Argv)) {
- // TODO: The error should be propagated up the stack.
- llvm::consumeError(std::move(Err));
- return false;
- }
+ if (Error Err = expandResponseFile(CfgFile, Argv))
+ return Err;
return expandResponseFiles(Argv);
}
@@ -1458,25 +1477,28 @@
bool LongOptionsUseDoubleDash) {
assert(hasOptions() && "No options specified!");
+ ProgramOverview = Overview;
+ bool IgnoreErrors = Errs;
+ if (!Errs)
+ Errs = &errs();
+ bool ErrorParsing = false;
+
// Expand response files.
SmallVector<const char *, 20> newArgv(argv, argv + argc);
BumpPtrAllocator A;
ExpansionContext ECtx(A, Triple(sys::getProcessTriple()).isOSWindows()
? cl::TokenizeWindowsCommandLine
: cl::TokenizeGNUCommandLine);
- ECtx.expandResponseFiles(newArgv);
+ if (Error Err = ECtx.expandResponseFiles(newArgv)) {
+ *Errs << toString(std::move(Err)) << '\n';
+ return false;
+ }
argv = &newArgv[0];
argc = static_cast<int>(newArgv.size());
// Copy the program name into ProgName, making sure not to overflow it.
ProgramName = std::string(sys::path::filename(StringRef(argv[0])));
- ProgramOverview = Overview;
- bool IgnoreErrors = Errs;
- if (!Errs)
- Errs = &errs();
- bool ErrorParsing = false;
-
// Check out the positional arguments to collect information about them.
unsigned NumPositionalRequired = 0;
Index: llvm/include/llvm/Support/CommandLine.h
===================================================================
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -2206,10 +2206,10 @@
/// commands resolving file names in them relative to the directory where
/// CfgFilename resides. It also expands "<CFGDIR>" to the base path of the
/// current config file.
- bool readConfigFile(StringRef CfgFile, SmallVectorImpl<const char *> &Argv);
+ Error readConfigFile(StringRef CfgFile, SmallVectorImpl<const char *> &Argv);
/// Expands constructs "@file" in the provided array of arguments recursively.
- bool expandResponseFiles(SmallVectorImpl<const char *> &Argv);
+ Error expandResponseFiles(SmallVectorImpl<const char *> &Argv);
};
/// A convenience helper which concatenates the options specified by the
@@ -2221,11 +2221,8 @@
/// A convenience helper which supports the typical use case of expansion
/// function call.
-inline bool ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer,
- SmallVectorImpl<const char *> &Argv) {
- ExpansionContext ECtx(Saver.getAllocator(), Tokenizer);
- return ECtx.expandResponseFiles(Argv);
-}
+bool ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer,
+ SmallVectorImpl<const char *> &Argv);
/// A convenience helper which concatenates the options specified by the
/// environment variable EnvVar and command line options, then expands response
Index: flang/tools/flang-driver/driver.cpp
===================================================================
--- flang/tools/flang-driver/driver.cpp
+++ flang/tools/flang-driver/driver.cpp
@@ -77,7 +77,9 @@
// We're defaulting to the GNU syntax, since we don't have a CL mode.
llvm::cl::TokenizerCallback tokenizer = &llvm::cl::TokenizeGNUCommandLine;
llvm::cl::ExpansionContext ExpCtx(saver.getAllocator(), tokenizer);
- ExpCtx.expandResponseFiles(args);
+ if (llvm::Error Err = ExpCtx.expandResponseFiles(args)) {
+ llvm::errs() << toString(std::move(Err)) << '\n';
+ }
}
int main(int argc, const char **argv) {
Index: clang/unittests/Driver/ToolChainTest.cpp
===================================================================
--- clang/unittests/Driver/ToolChainTest.cpp
+++ clang/unittests/Driver/ToolChainTest.cpp
@@ -541,4 +541,202 @@
}
}
+struct FileSystemWithError : public llvm::vfs::FileSystem {
+ llvm::ErrorOr<llvm::vfs::Status> status(const Twine &Path) override {
+ return std::make_error_code(std::errc::no_such_file_or_directory);
+ }
+ llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
+ openFileForRead(const Twine &Path) override {
+ return std::make_error_code(std::errc::permission_denied);
+ }
+ llvm::vfs::directory_iterator dir_begin(const Twine &Dir,
+ std::error_code &EC) override {
+ return llvm::vfs::directory_iterator();
+ }
+ std::error_code setCurrentWorkingDirectory(const Twine &Path) override {
+ return std::make_error_code(std::errc::permission_denied);
+ }
+ llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override {
+ return std::make_error_code(std::errc::permission_denied);
+ }
+};
+
+TEST(ToolChainTest, ConfigFileError) {
+ IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
+ IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
+ std::unique_ptr<SimpleDiagnosticConsumer> DiagConsumer(
+ new SimpleDiagnosticConsumer());
+ DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagConsumer.get(), false);
+ IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS(new FileSystemWithError);
+
+ Driver TheDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags,
+ "clang LLVM compiler", FS);
+ std::unique_ptr<Compilation> C(
+ TheDriver.BuildCompilation({"/home/test/bin/clang", "--no-default-config",
+ "--config", "./root.cfg", "--version"}));
+ ASSERT_TRUE(C);
+ ASSERT_TRUE(C->containsError());
+ EXPECT_EQ(1U, Diags.getNumErrors());
+ EXPECT_STREQ("configuration file './root.cfg' cannot be opened: cannot get "
+ "absolute path",
+ DiagConsumer->Errors[0].c_str());
+}
+
+TEST(ToolChainTest, BadConfigFile) {
+ IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
+ IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
+ std::unique_ptr<SimpleDiagnosticConsumer> DiagConsumer(
+ new SimpleDiagnosticConsumer());
+ DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagConsumer.get(), false);
+ IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FS(
+ new llvm::vfs::InMemoryFileSystem);
+
+#ifdef _WIN32
+ const char *TestRoot = "C:\\";
+#define FILENAME "C:/opt/root.cfg"
+#define DIRNAME "C:/opt"
+#else
+ const char *TestRoot = "/";
+#define FILENAME "/opt/root.cfg"
+#define DIRNAME "/opt"
+#endif
+ llvm::BumpPtrAllocator Alloc;
+ char *StrBuff = (char *)Alloc.Allocate(6, 2);
+ std::memcpy(StrBuff, "\xFF\xFE\x00\xD8\x00\x00", 6);
+ StringRef BadUTF(StrBuff, 6);
+ FS->setCurrentWorkingDirectory(TestRoot);
+ FS->addFile("/opt/root.cfg", 0,
+ llvm::MemoryBuffer::getMemBuffer(BadUTF));
+ FS->addFile("/home/user/test.cfg", 0,
+ llvm::MemoryBuffer::getMemBuffer("@file.rsp"));
+
+ {
+ Driver TheDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags,
+ "clang LLVM compiler", FS);
+ std::unique_ptr<Compilation> C(TheDriver.BuildCompilation(
+ {"/home/test/bin/clang", "--config", "/opt/root.cfg", "--version"}));
+ ASSERT_TRUE(C);
+ ASSERT_TRUE(C->containsError());
+ EXPECT_EQ(1U, DiagConsumer->Errors.size());
+ EXPECT_STREQ("cannot read configuration file '" FILENAME
+ "': Could not convert UTF16 to UTF8",
+ DiagConsumer->Errors[0].c_str());
+ }
+ DiagConsumer->clear();
+ {
+ Driver TheDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags,
+ "clang LLVM compiler", FS);
+ std::unique_ptr<Compilation> C(TheDriver.BuildCompilation(
+ {"/home/test/bin/clang", "--config", "/opt", "--version"}));
+ ASSERT_TRUE(C);
+ ASSERT_TRUE(C->containsError());
+ EXPECT_EQ(1U, DiagConsumer->Errors.size());
+ EXPECT_STREQ("configuration file '" DIRNAME
+ "' cannot be opened: not a regular file",
+ DiagConsumer->Errors[0].c_str());
+ }
+ DiagConsumer->clear();
+ {
+ Driver TheDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags,
+ "clang LLVM compiler", FS);
+ std::unique_ptr<Compilation> C(TheDriver.BuildCompilation(
+ {"/home/test/bin/clang", "--config", "root",
+ "--config-system-dir=", "--config-user-dir=", "--version"}));
+ ASSERT_TRUE(C);
+ ASSERT_TRUE(C->containsError());
+ EXPECT_EQ(1U, DiagConsumer->Errors.size());
+ EXPECT_STREQ("configuration file 'root' cannot be found",
+ DiagConsumer->Errors[0].c_str());
+ }
+
+#undef FILENAME
+#undef DIRNAME
+}
+
+TEST(ToolChainTest, ConfigInexistentInclude) {
+ IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
+ IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
+ std::unique_ptr<SimpleDiagnosticConsumer> DiagConsumer(
+ new SimpleDiagnosticConsumer());
+ DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagConsumer.get(), false);
+ IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FS(
+ new llvm::vfs::InMemoryFileSystem);
+
+#ifdef _WIN32
+ const char *TestRoot = "C:\\";
+#define USERCONFIG "C:\\home\\user\\test.cfg"
+#define UNEXISTENT "C:\\home\\user\\file.rsp"
+#else
+ const char *TestRoot = "/";
+#define USERCONFIG "/home/user/test.cfg"
+#define UNEXISTENT "/home/user/file.rsp"
+#endif
+ FS->setCurrentWorkingDirectory(TestRoot);
+ FS->addFile("/home/user/test.cfg", 0,
+ llvm::MemoryBuffer::getMemBuffer("@file.rsp"));
+
+ {
+ Driver TheDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags,
+ "clang LLVM compiler", FS);
+ std::unique_ptr<Compilation> C(TheDriver.BuildCompilation(
+ {"/home/test/bin/clang", "--config", "test.cfg",
+ "--config-system-dir=", "--config-user-dir=/home/user", "--version"}));
+ 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());
+ }
+
+#undef USERCONFIG
+#undef UNEXISTENT
+}
+
+TEST(ToolChainTest, ConfigRecursiveInclude) {
+ IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
+ IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
+ std::unique_ptr<SimpleDiagnosticConsumer> DiagConsumer(
+ new SimpleDiagnosticConsumer());
+ DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagConsumer.get(), false);
+ IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FS(
+ new llvm::vfs::InMemoryFileSystem);
+
+#ifdef _WIN32
+ const char *TestRoot = "C:\\";
+#define USERCONFIG "C:\\home\\user\\test.cfg"
+#define INCLUDED1 "C:\\home\\user\\file1.cfg"
+#else
+ const char *TestRoot = "/";
+#define USERCONFIG "/home/user/test.cfg"
+#define INCLUDED1 "/home/user/file1.cfg"
+#endif
+ FS->setCurrentWorkingDirectory(TestRoot);
+ FS->addFile("/home/user/test.cfg", 0,
+ llvm::MemoryBuffer::getMemBuffer("@file1.cfg"));
+ FS->addFile("/home/user/file1.cfg", 0,
+ llvm::MemoryBuffer::getMemBuffer("@file2.cfg"));
+ FS->addFile("/home/user/file2.cfg", 0,
+ llvm::MemoryBuffer::getMemBuffer("@file3.cfg"));
+ FS->addFile("/home/user/file3.cfg", 0,
+ llvm::MemoryBuffer::getMemBuffer("@file1.cfg"));
+
+ {
+ Driver TheDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags,
+ "clang LLVM compiler", FS);
+ std::unique_ptr<Compilation> C(TheDriver.BuildCompilation(
+ {"/home/test/bin/clang", "--config", "test.cfg",
+ "--config-system-dir=", "--config-user-dir=/home/user", "--version"}));
+ ASSERT_TRUE(C);
+ ASSERT_TRUE(C->containsError());
+ EXPECT_EQ(1U, DiagConsumer->Errors.size());
+ EXPECT_STREQ("cannot read configuration file '" USERCONFIG
+ "': recursive expansion of: '" INCLUDED1 "'",
+ DiagConsumer->Errors[0].c_str());
+ }
+
+#undef USERCONFIG
+#undef INCLUDED1
+}
+
} // end anonymous namespace.
Index: clang/tools/driver/driver.cpp
===================================================================
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -309,7 +309,10 @@
llvm::BumpPtrAllocator A;
llvm::cl::ExpansionContext ECtx(A, llvm::cl::TokenizeGNUCommandLine);
- ECtx.expandResponseFiles(ArgV);
+ if (llvm::Error Err = ECtx.expandResponseFiles(ArgV)) {
+ llvm::errs() << toString(std::move(Err)) << '\n';
+ return 1;
+ }
StringRef Tool = ArgV[1];
void *GetExecutablePathVP = (void *)(intptr_t)GetExecutablePath;
if (Tool == "-cc1")
@@ -373,7 +376,11 @@
if (MarkEOLs && Args.size() > 1 && StringRef(Args[1]).startswith("-cc1"))
MarkEOLs = false;
llvm::cl::ExpansionContext ECtx(A, Tokenizer);
- ECtx.setMarkEOLs(MarkEOLs).expandResponseFiles(Args);
+ ECtx.setMarkEOLs(MarkEOLs);
+ if (llvm::Error Err = ECtx.expandResponseFiles(Args)) {
+ llvm::errs() << Err << '\n';
+ return 1;
+ }
// Handle -cc1 integrated tools, even if -cc1 was expanded from a response
// file.
Index: clang/test/Driver/config-file-errs.c
===================================================================
--- clang/test/Driver/config-file-errs.c
+++ clang/test/Driver/config-file-errs.c
@@ -13,7 +13,7 @@
//--- Argument of '--config' must be existing file, if it is specified by path.
//
// RUN: not %clang --config somewhere/nonexistent-config-file 2>&1 | FileCheck %s -check-prefix CHECK-NONEXISTENT
-// CHECK-NONEXISTENT: configuration file '{{.*}}somewhere/nonexistent-config-file' does not exist
+// CHECK-NONEXISTENT: configuration file '{{.*}}somewhere{{.}}nonexistent-config-file' cannot be opened: No such file or directory
//--- All '--config' arguments must be existing files.
Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===================================================================
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -61,9 +61,12 @@
continue;
llvm::BumpPtrAllocator Alloc;
llvm::cl::ExpansionContext ECtx(Alloc, Tokenizer);
- ECtx.setVFS(FS.get())
- .setCurrentDir(Cmd.Directory)
- .expandResponseFiles(Argv);
+ llvm::Error Err = ECtx.setVFS(FS.get())
+ .setCurrentDir(Cmd.Directory)
+ .expandResponseFiles(Argv);
+ if (Err) {
+ llvm::errs() << Err;
+ }
// Don't assign directly, Argv aliases CommandLine.
std::vector<std::string> ExpandedArgv(Argv.begin(), Argv.end());
Cmd.CommandLine = std::move(ExpandedArgv);
Index: clang/lib/Driver/Driver.cpp
===================================================================
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -942,8 +942,9 @@
llvm::cl::ExpansionContext &ExpCtx) {
// Try reading the given file.
SmallVector<const char *, 32> NewCfgArgs;
- if (!ExpCtx.readConfigFile(FileName, NewCfgArgs)) {
- Diag(diag::err_drv_cannot_read_config_file) << FileName;
+ if (llvm::Error Err = ExpCtx.readConfigFile(FileName, NewCfgArgs)) {
+ Diag(diag::err_drv_cannot_read_config_file)
+ << FileName << toString(std::move(Err));
return true;
}
@@ -1025,15 +1026,23 @@
if (llvm::sys::path::has_parent_path(CfgFileName)) {
CfgFilePath.assign(CfgFileName);
if (llvm::sys::path::is_relative(CfgFilePath)) {
- if (getVFS().makeAbsolute(CfgFilePath))
- return true;
- auto Status = getVFS().status(CfgFilePath);
- if (!Status ||
- Status->getType() != llvm::sys::fs::file_type::regular_file) {
- Diag(diag::err_drv_config_file_not_exist) << CfgFilePath;
+ if (std::error_code EC = getVFS().makeAbsolute(CfgFilePath)) {
+ Diag(diag::err_drv_cannot_open_config_file)
+ << CfgFilePath << "cannot get absolute path";
return true;
}
}
+ auto Status = getVFS().status(CfgFilePath);
+ if (!Status) {
+ Diag(diag::err_drv_cannot_open_config_file)
+ << CfgFilePath << Status.getError().message();
+ return true;
+ }
+ if (Status->getType() != llvm::sys::fs::file_type::regular_file) {
+ Diag(diag::err_drv_cannot_open_config_file)
+ << CfgFilePath << "not a regular file";
+ return true;
+ }
} else if (!ExpCtx.findConfigFile(CfgFileName, CfgFilePath)) {
// Report an error that the config file could not be found.
Diag(diag::err_drv_config_file_not_found) << CfgFileName;
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -214,14 +214,14 @@
"malformed sanitizer coverage ignorelist: '%0'">;
def err_drv_duplicate_config : Error<
"no more than one option '--config' is allowed">;
-def err_drv_config_file_not_exist : Error<
- "configuration file '%0' does not exist">;
+def err_drv_cannot_open_config_file : Error<
+ "configuration file '%0' cannot be opened: %1">;
def err_drv_config_file_not_found : Error<
"configuration file '%0' cannot be found">;
def note_drv_config_file_searched_in : Note<
"was searched for in the directory: %0">;
def err_drv_cannot_read_config_file : Error<
- "cannot read configuration file '%0'">;
+ "cannot read configuration file '%0': %1">;
def err_drv_nested_config_file: Error<
"option '--config' is not allowed inside configuration file">;
def err_drv_arg_requires_bitcode_input: Error<
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
