Handle errors in expansion of response files

[sepavloff]

Summary

Previously an error raised during an expansion of response files (including
configuration files) was ignored and only the fact of its presence was
reported to the user with generic error messages. This made it difficult to
analyze problems. For example, if a configuration file tried to read an
inexistent file, the error message said that 'configuration file cannot
be found', which is wrong and misleading.

This change enhances handling errors in the expansion so that users
could get more informative error messages.

[sepavloff]

Make UTF-16 string properly aligned

[rovka]

clang/test/Driver/config-file-errs.c

16

I think you might need to use the same trick as this test to get the test to pass on both Windows and Linux (since they use different capitalization).

[rovka]

Nice tests! :) I have one microscopic nit and one question (because I don't know an awful lot about config files). Thanks for working on this.

llvm/lib/Support/CommandLine.cpp

1188

Why do you need to add !InConfigFile here and below?

1208

Nit: Why reverse this branch? The code around it continues early, so it's easier to read if this continues early too.

[mgorny]

llvm/lib/Support/CommandLine.cpp

1209

Also, I think you can use consume_front() here.

[sepavloff]

Small changes proposed by reviewers

[sepavloff]

llvm/lib/Support/CommandLine.cpp

1188

It is not necessary, because now configuration file is expanded with the flag RelativeNames set. It however seems more correct to check for InConfigFile also, hopefully it make code a bit clearly.

1208

You are right. Initially this patch was combined with D136354, where additional checks was added. I changed this code to be more natural.

1209

Yes, changed code accordingly.

[mgorny]

clang/lib/Driver/Driver.cpp

1042–1044

Either I'm missing something or you're not using EC.

1054

Not necessarily a goal for this patch but wouldn't this check fit better in readConfigFile()?

clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp

67–69

The braces seem unnecessary.

[sepavloff]

Updated patch

[sepavloff]

clang/lib/Driver/Driver.cpp

1042–1044

You are right, this is remnants of previous implementation. Thanks for the catch.

1054

Yes, it makes sense. Moved.

clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp

67–69

Fixed.

[sepavloff]

Ping.

[mgorny]

clang/unittests/Driver/ToolChainTest.cpp

513–515

Wouldn't it be possible to use std::string, or maybe even std::array<char, ...> here? I think it'd be less error-prone.

[sepavloff]

clang/unittests/Driver/ToolChainTest.cpp

513–515

Nor std::string neither std::array<char,N> provide custom alignment.

UTF-16 string is constructed here from array of chars to avoid problems with endianness. It consists of 2-byte elements and is expected to be aligned on 2-byte boundary. Array of chars is aligned on byte and sometimes the test failed due to invalid alignment.

So we have to use bare pointer to have guaranteed alignment.

[mgorny]

LGTM

clang/unittests/Driver/ToolChainTest.cpp

513–515

Ah. Could you leave a comment to make the alignment requirements clear? I'm a bit surprised that we have to meet alignment requirements when "reading" a file.

[sepavloff]

Thanks!

[uabelho]

Hello,

I'm wondering if this is really working as expected/intended.

With this patch, if I do

clang empty.c @atfileesc.txt

on an empty file empty.c and atfileesc.txt containing just

-I @name-with-at/

then I get the following error

cannot open file: /repo/uabelho/main-github/llvm/atfileesc.txt
Program aborted due to an unhandled Error:
cannot open file: /repo/uabelho/main-github/llvm/atfileesc.txt
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: build-all/bin/clang empty.c @atfileesc.txt -dry-run
 #0 0x0000000002fc9873 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (build-all/bin/clang+0x2fc9873)
 #1 0x0000000002fc758e llvm::sys::RunSignalHandlers() (build-all/bin/clang+0x2fc758e)
 #2 0x0000000002fc9bf6 SignalHandler(int) (build-all/bin/clang+0x2fc9bf6)
 #3 0x00007f678e7ae630 __restore_rt (/lib64/libpthread.so.0+0xf630)
 #4 0x00007f678bef5387 raise (/lib64/libc.so.6+0x36387)
 #5 0x00007f678bef6a78 abort (/lib64/libc.so.6+0x37a78)
 #6 0x0000000002f39d00 llvm::Error::fatalUncheckedError() const (build-all/bin/clang+0x2f39d00)
 #7 0x00000000009d5e3a clang_main(int, char**) (build-all/bin/clang+0x9d5e3a)
 #8 0x00007f678bee1555 __libc_start_main (/lib64/libc.so.6+0x22555)
 #9 0x00000000009d233b _start (build-all/bin/clang+0x9d233b)
Abort (core dumped)

Note that the file it is complaining about indeed exists:

-> ls -l /repo/uabelho/main-github/llvm/atfileesc.txt
-rw-r--r-- 1 uabelho eusers 18 Nov  1 11:15 /repo/uabelho/main-github/llvm/atfileesc.txt

and as I wrote above it contains

-> cat /repo/uabelho/main-github/llvm/atfileesc.txt
-I @name-with-at/

[sepavloff]

Thank you for reporting the issue. It was fixed in fdab9f1203ee.

[uabelho]

In D136090#3905845, @sepavloff wrote:

Thank you for reporting the issue. It was fixed in fdab9f1203ee.

Thanks!

clang/include/clang/Basic/DiagnosticDriverKinds.td

 def err_drv_malformed_sanitizer_ignorelist : Error<
   "malformed sanitizer ignorelist: '%0'">;
 def err_drv_malformed_sanitizer_coverage_allowlist : Error<
   "malformed sanitizer coverage allowlist: '%0'">;
 def err_drv_malformed_sanitizer_coverage_ignorelist : Error<
   "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<
   "option '%0' requires input to be LLVM bitcode">;
 
 def err_target_unsupported_arch
   : Error<"the target architecture '%0' is not supported by the target '%1'">;
 def err_cpu_unsupported_isa

clang/lib/Driver/Driver.cpp

if (Opt->isClaimed())
     Copy->claim();
   Copy->setOwnsValues(Opt->getOwnsValues());
   Opt->setOwnsValues(false);
   Args.append(Copy);
 }
 
 bool Driver::readConfigFile(StringRef FileName,
                             llvm::cl::ExpansionContext &ExpCtx) {
+  // Try opening the given file.
+  auto Status = getVFS().status(FileName);
+  if (!Status) {
+    Diag(diag::err_drv_cannot_open_config_file)
+        << FileName << Status.getError().message();
+    return true;
+  }
+  if (Status->getType() != llvm::sys::fs::file_type::regular_file) {
+    Diag(diag::err_drv_cannot_open_config_file)
+        << FileName << "not a regular file";
+    return true;
+  }
+
   // 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;
   }
 
   // Read options from config file.
   llvm::SmallString<128> CfgFileName(FileName);
   llvm::sys::path::native(CfgFileName);
   bool ContainErrors;
   std::unique_ptr<InputArgList> NewOptions = std::make_unique<InputArgList>(
bool Driver::loadConfigFiles() {
   SmallString<128> CfgFilePath;
   if (CLOptions) {
     for (auto CfgFileName : CLOptions->getAllArgValues(options::OPT_config)) {
       // If argument contains directory separator, treat it as a path to
       // configuration file.
       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 (getVFS().makeAbsolute(CfgFilePath)) {
+            Diag(diag::err_drv_cannot_open_config_file)
+                << CfgFilePath << "cannot get absolute path";
             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;
         for (const StringRef &SearchDir : CfgFileSearchDirs)
           if (!SearchDir.empty())
             Diag(diag::note_drv_config_file_searched_in) << SearchDir;
         return true;
       }
 
       // Try to read the config file, return on error.
       if (readConfigFile(CfgFilePath, ExpCtx))
         return true;
     }
   }
 

clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp

for (auto &Cmd : Cmds) {
         Argv.push_back(Arg.c_str());
         if (!Arg.empty())
           SeenRSPFile |= Arg.front() == '@';
       }
       if (!SeenRSPFile)
         continue;
       llvm::BumpPtrAllocator Alloc;
       llvm::cl::ExpansionContext ECtx(Alloc, Tokenizer);
-      ECtx.setVFS(FS.get())
+      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);
     }
     return Cmds;
   }
 
 private:
   std::unique_ptr<CompilationDatabase> Base;

clang/test/Driver/config-file-errs.c

 //--- '--config' must be followed by config file name.
 //
 // RUN: not %clang --config 2>&1 | FileCheck %s -check-prefix CHECK-MISSING-FILE
 // CHECK-MISSING-FILE: argument to '--config' is missing (expected 1 value)
 
 
 //--- '--config' must not be found in config files.
 //
 // RUN: not %clang --config %S/Inputs/config-6.cfg 2>&1 | FileCheck %s -check-prefix CHECK-NESTED
 // CHECK-NESTED: option '--config' is not allowed inside configuration file
 
 
 //--- 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: {{[Nn]}}o such file or directory
 
 
 //--- All '--config' arguments must be existing files.
 //
 // RUN: not %clang --config %S/Inputs/config-4.cfg --config somewhere/nonexistent-config-file 2>&1 | FileCheck %s -check-prefix CHECK-NONEXISTENT
 
 
 //--- Argument of '--config' must exist somewhere in well-known directories, if it is specified by bare name.

clang/tools/driver/driver.cpp

static int ExecuteCC1Tool(SmallVectorImpl<const char *> &ArgV) {
   // If we call the cc1 tool from the clangDriver library (through
   // Driver::CC1Main), we need to clean up the options usage count. The options
   // are currently global, and they might have been used previously by the
   // driver.
   llvm::cl::ResetAllOptionOccurrences();
 
   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")
     return cc1_main(makeArrayRef(ArgV).slice(1), ArgV[0], GetExecutablePathVP);
   if (Tool == "-cc1as")
     return cc1as_main(makeArrayRef(ArgV).slice(2), ArgV[0],
                       GetExecutablePathVP);
   if (Tool == "-cc1gen-reproducer")
int clang_main(int Argc, char **Argv) {
   if (RSPQuoting == Windows || (RSPQuoting == Default && ClangCLMode))
     Tokenizer = &llvm::cl::TokenizeWindowsCommandLine;
   else
     Tokenizer = &llvm::cl::TokenizeGNUCommandLine;
 
   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.
   auto FirstArg = llvm::find_if(llvm::drop_begin(Args),
                                 [](const char *A) { return A != nullptr; });
   if (FirstArg != Args.end() && StringRef(*FirstArg).startswith("-cc1")) {
     // If -cc1 came from a response file, remove the EOL sentinels.
     if (MarkEOLs) {

clang/unittests/Driver/ToolChainTest.cpp

std::unique_ptr<Compilation> C(TheDriver.BuildCompilation(
         {"/home/test/bin/clang", "--config", "root.cfg",
          "--config-system-dir=", "--config-user-dir="}));
     ASSERT_TRUE(C);
     ASSERT_FALSE(C->containsError());
     EXPECT_EQ("/opt/sdk/platform2", TheDriver.SysRoot);
   }
 }
 
+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
+  // UTF-16 string must be aligned on 2-byte boundary. Strings and char arrays
+  // do not provide necessary alignment, so copy constant string into properly
+  // allocated memory in heap.
+  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.

flang/tools/flang-driver/driver.cpp

static int executeFC1Tool(llvm::SmallVectorImpl<const char *> &argV) {
   return 1;
 }
 
 static void ExpandResponseFiles(
     llvm::StringSaver &saver, llvm::SmallVectorImpl<const char *> &args) {
   // 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) {
 
   // Initialize variables to call the driver
   llvm::InitLLVM x(argc, argv);
   llvm::SmallVector<const char *, 256> args(argv, argv + argc);
 

llvm/include/llvm/Support/CommandLine.h

public:
   /// \param [in] CfgFile Path to configuration file.
   /// \param [out] Argv Array to which the read options are added.
   /// \return true if the file was successfully read.
   ///
   /// It reads content of the specified file, tokenizes it and expands "@file"
   /// 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
 /// environment variable EnvVar and command line options, then expands
 /// response files recursively.
 /// \return true if all @files were expanded successfully or there were none.
 bool expandResponseFiles(int Argc, const char *const *Argv, const char *EnvVar,
                          SmallVectorImpl<const char *> &NewArgv);
 
 /// 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
 /// files recursively. The tokenizer is a predefined GNU or Windows one.
 /// \return true if all @files were expanded successfully or there were none.
 bool expandResponseFiles(int Argc, const char *const *Argv, const char *EnvVar,
                          StringSaver &Saver,
                          SmallVectorImpl<const char *> &NewArgv);

llvm/lib/Support/CommandLine.cpp

if (!ResponseFile.empty()) {
     const StringRef Remaining = ArgString.substr(StartPos);
     if (!Remaining.empty())
       llvm::sys::path::append(ResponseFile, Remaining);
     Arg = Saver.save(ResponseFile.str()).data();
   }
 }
 
 // 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());
 
   // If we have a UTF-16 byte order mark, convert to UTF-8 for parsing.
   ArrayRef<char> BufRef(MemBuf.getBufferStart(), MemBuf.getBufferEnd());
   std::string UTF8Buf;
   if (hasUTF16ByteOrderMark(BufRef)) {
     if (!convertUTF16ToUTF8String(BufRef, UTF8Buf))
       return llvm::createStringError(std::errc::illegal_byte_sequence,
                                      "Could not convert UTF16 to UTF8");
     Str = StringRef(UTF8Buf);
   }
   // If we see UTF-8 BOM sequence at the beginning of a file, we shall remove
   // these bytes before parsing.
   // Reference: http://en.wikipedia.org/wiki/UTF-8#Byte_order_mark
   else if (hasUTF8ByteOrderMark(BufRef))
     Str = StringRef(BufRef.data() + 3, BufRef.size() - 3);
 
   // 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);
     llvm::sys::path::append(ResponseFile, FileName);
     Arg = Saver.save(ResponseFile.str()).data();
   }
   return Error::success();
 }
 
 /// 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;
   };
 
   // To detect recursive response files, we maintain a stack of files and the
   // position of the last argument in the file. This position is updated
   // dynamically as we recursively expand files.
for (unsigned I = 0; I != Argv.size();) {
     // Note that CurrentDir is only used for top-level rsp files, the rest will
     // always have an absolute path deduced from the containing file.
     SmallString<128> CurrDir;
     if (llvm::sys::path::is_relative(FName)) {
       if (CurrentDir.empty()) {
         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;
       }
       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;
+    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
       // arguments, minus the response file itself.
       Record.End += ExpandedArgv.size() - 1;
     }
 
     FileStack.push_back({FName, I + ExpandedArgv.size()});
     Argv.erase(Argv.begin() + I);
     Argv.insert(Argv.begin() + I, ExpandedArgv.begin(), ExpandedArgv.end());
   }
 
   // If successful, the top of the file stack will mark the end of the Argv
   // stream. A failure here indicates a bug in the stack popping logic above.
   // Note that FileStack may have more than one element at this point because we
   // 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,
                              const char *EnvVar, StringSaver &Saver,
                              SmallVectorImpl<const char *> &NewArgv) {
   auto Tokenize = Triple(sys::getProcessTriple()).isOSWindows()
                       ? cl::TokenizeWindowsCommandLine
                       : cl::TokenizeGNUCommandLine;
   // The environment variable specifies initial options.
   if (EnvVar)
     if (llvm::Optional<std::string> EnvValue = sys::Process::GetEnv(EnvVar))
       Tokenize(*EnvValue, Saver, NewArgv, /*MarkEOLs=*/false);
 
   // 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)
     : Saver(A), Tokenizer(T), FS(vfs::getRealFileSystem().get()) {}
 
 bool ExpansionContext::findConfigFile(StringRef FileName,
                                       SmallVectorImpl<char> &FilePath) {
   SmallString<128> CfgFilePath;
if (FileExists(CfgFilePath)) {
       FilePath.assign(CfgFilePath.begin(), CfgFilePath.end());
       return true;
     }
   }
 
   return false;
 }
 
-bool ExpansionContext::readConfigFile(StringRef CfgFile,
+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);
 }
 
 static void initCommonOptions();
 bool cl::ParseCommandLineOptions(int argc, const char *const *argv,
                                  StringRef Overview, raw_ostream *Errs,
                                  const char *EnvVar,
                                  bool LongOptionsUseDoubleDash) {

 
 bool CommandLineParser::ParseCommandLineOptions(int argc,
                                                 const char *const *argv,
                                                 StringRef Overview,
                                                 raw_ostream *Errs,
                                                 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;
 
   // Determine whether or not there are an unlimited number of positionals
   bool HasUnlimitedPositionals = false;
 
   int FirstArg = 1;
   SubCommand *ChosenSubCommand = &SubCommand::getTopLevel();

llvm/unittests/Support/CommandLineTest.cpp

#endif
   IncRef.append(IncludedFileName);
   llvm::SmallVector<const char *, 4> Argv = {"test/test", "-flag_1",
                                              IncRef.c_str(), "-flag_2"};
 
   // Expand response files.
   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",
                          "-option_21", "-option_22", "-option_23=abcd",
                          "-option_3=abcd", "-option_31", "-option_32",
                          "-option_33=abcd", "-option_4=efjk", "-flag_2"}));
 }
 
#endif
   BumpPtrAllocator A;
 #ifdef _WIN32
   cl::TokenizerCallback Tokenizer = cl::TokenizeWindowsCommandLine;
 #else
   cl::TokenizerCallback Tokenizer = cl::TokenizeGNUCommandLine;
 #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(),
                                  {"test/test", "-option_1", "-option_x",
                                   "-option_2", "-option_x", SelfFileRef.c_str(),
                                   NestedFileRef.c_str(), SelfFileRef.c_str(),
                                   "-option_3"}));
 }
#endif
   SmallVector<const char *, 4> Argv = {"test/test"};
   Argv.append(NON_RSP_AT_ARGS, "@non_rsp_at_arg");
   std::string ResponseFileRef = ("@" + ResponseFilePath).str();
   Argv.push_back(ResponseFileRef.c_str());
 
   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);
   size_t i = 0;
   EXPECT_STREQ(Argv[i++], "test/test");
   for (; i < 1 + NON_RSP_AT_ARGS; ++i)
     EXPECT_STREQ(Argv[i], "@non_rsp_at_arg");
   EXPECT_STREQ(Argv[i++], "-foo");
#endif
   StringRef InnerFileContents = "-flag";
   FS.addFile(InnerFile, 0, MemoryBuffer::getMemBuffer(InnerFileContents));
 
   SmallVector<const char *, 2> Argv = {"test/test", "@dir/outer.rsp"};
 
   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"}));
 }
 
 TEST(CommandLineTest, ResponseFileEOLs) {
   vfs::InMemoryFileSystem FS;
 #ifdef _WIN32
   const char *TestRoot = "C:\\";
 #else
   const char *TestRoot = "//net";
 #endif
   FS.setCurrentWorkingDirectory(TestRoot);
   FS.addFile("eols.rsp", 0,
              MemoryBuffer::getMemBuffer("-Xclang -Wno-whatever\n input.cpp"));
   SmallVector<const char *, 2> Argv = {"clang", "@eols.rsp"};
   BumpPtrAllocator A;
   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());
   for (size_t I = 0, E = std::size(Expected); I < E; ++I) {
     if (Expected[I] == nullptr) {
       ASSERT_EQ(Argv[I], nullptr);
     } else {
       ASSERT_STREQ(Expected[I], Argv[I]);
     }
   }
 }
 
+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();
 
   StackOption<std::string> Opt1("opt1", cl::init("true"));
   StackOption<bool> Opt2("opt2", cl::init(true));
   cl::alias Alias("alias", llvm::cl::aliasopt(Opt2));
   StackOption<int> Opt3("opt3", cl::init(3));
 
TEST(CommandLineTest, ReadConfigFile) {
   // file.
   llvm::SmallString<128> CurrDir;
   std::error_code EC = llvm::sys::fs::current_path(CurrDir);
   EXPECT_TRUE(!EC);
   EXPECT_NE(CurrDir.str(), TestDir.path());
 
   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],
                ("-option_2=" + TestDir.path() + "/dir1").str().c_str());
   EXPECT_STREQ(Argv[2], ("-option_3=" + TestDir.path()).str().c_str());
   EXPECT_STREQ(Argv[3], "-option_4");
   EXPECT_STREQ(Argv[4], TestDir.path().str().c_str());
   EXPECT_STREQ(Argv[5], ("-option_5=" + TestDir.path()).str().c_str());