Index: include/clang/Driver/Driver.h =================================================================== --- include/clang/Driver/Driver.h +++ include/clang/Driver/Driver.h @@ -26,6 +26,9 @@ #include namespace llvm { +namespace cl { + class ExpandedArgs; +} namespace opt { class Arg; class ArgList; @@ -239,7 +242,9 @@ /// argument vector. A null return value does not necessarily /// indicate an error condition, the diagnostics should be queried /// to determine if an error occurred. - Compilation *BuildCompilation(ArrayRef Args); + Compilation * + BuildCompilation(ArrayRef Args, + const llvm::cl::ExpandedArgs *ResponseFilesInfo = nullptr); /// @name Driver Steps /// @{ @@ -249,7 +254,9 @@ /// ParseArgStrings - Parse the given list of strings into an /// ArgList. - llvm::opt::InputArgList *ParseArgStrings(ArrayRef Args); + llvm::opt::InputArgList * + ParseArgStrings(ArrayRef Args, + const llvm::cl::ExpandedArgs *ResponseFilesInfo = nullptr); /// BuildInputs - Construct the list of inputs and their types from /// the given arguments. Index: lib/Driver/Driver.cpp =================================================================== --- lib/Driver/Driver.cpp +++ lib/Driver/Driver.cpp @@ -29,6 +29,7 @@ #include "llvm/Option/OptSpecifier.h" #include "llvm/Option/OptTable.h" #include "llvm/Option/Option.h" +#include "llvm/Support/CommandLine.h" #include "llvm/Support/Debug.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FileSystem.h" @@ -102,7 +103,9 @@ } } -InputArgList *Driver::ParseArgStrings(ArrayRef ArgList) { +InputArgList * +Driver::ParseArgStrings(ArrayRef ArgList, + const llvm::cl::ExpandedArgs *ResponseFilesInfo) { llvm::PrettyStackTraceString CrashInfo("Command line argument parsing"); unsigned IncludedFlagsBitmask; @@ -114,7 +117,8 @@ InputArgList *Args = getOpts().ParseArgs(ArgList.begin(), ArgList.end(), MissingArgIndex, MissingArgCount, IncludedFlagsBitmask, - ExcludedFlagsBitmask); + ExcludedFlagsBitmask, + ResponseFilesInfo); // Check for missing argument error. if (MissingArgCount) @@ -287,7 +291,9 @@ return DAL; } -Compilation *Driver::BuildCompilation(ArrayRef ArgList) { +Compilation * +Driver::BuildCompilation(ArrayRef ArgList, + const llvm::cl::ExpandedArgs *ResponseFilesInfo) { llvm::PrettyStackTraceString CrashInfo("Compilation construction"); // FIXME: Handle environment options which affect driver behavior, somewhere @@ -312,7 +318,7 @@ // FIXME: This stuff needs to go into the Compilation, not the driver. bool CCCPrintActions; - InputArgList *Args = ParseArgStrings(ArgList.slice(1)); + InputArgList *Args = ParseArgStrings(ArgList.slice(1), ResponseFilesInfo); // -no-canonical-prefixes is used very early in main. Args->ClaimAllArgs(options::OPT_no_canonical_prefixes); Index: test/Driver/cl-link-at-file.c =================================================================== --- test/Driver/cl-link-at-file.c +++ test/Driver/cl-link-at-file.c @@ -0,0 +1,44 @@ +// PR17239 - The /link option, when inside a response file, should only extend +// until the end of the response file (and not the entire command line) + +// Don't attempt slash switches on msys bash. +// REQUIRES: shell-preserves-root + +// Note: %s must be preceded by -- or bound to another option, otherwise it may +// be interpreted as a command-line option, e.g. on Mac where %s is commonly +// under /Users. + +// RUN: echo /link bar.lib baz.lib > %t.args +// RUN: touch %t.obj +// RUN: %clang_cl -### @%t.args -- %t.obj 2>&1 | FileCheck %s -check-prefix=ARGS +// If the "/link" option captures all remaining args beyond its response file, +// it will also capture "--" and our input argument. In this case, Clang will +// be clueless and will emit "argument unused" warnings. If PR17239 is properly +// fixed, this should not happen because the "/link" option is restricted to +// consume only remaining args in its response file. +// ARGS-NOT: warning +// ARGS-NOT: argument unused during compilation +// Identify the linker command +// ARGS: link.exe + +// RUN: echo -- %t.obj > %t.2.args +// RUN: %clang_cl -### @%t.2.args /link /base:0x10000 2>&1 | FileCheck %s \ +// RUN: -check-prefix=BASE +// Notice how the "--" arg (which consumes remaining args) should only take +// effect until the end of its container response file. +// Even though "/base:0x10000" is not a valid clang argument, it should not +// fail because we are passing it as a linker argument (via /link) +// BASE-NOT: no such file or directory: '/base:0x10000' +// BASE: link.exe +// BASE: "/base:0x10000" + +// RUN: echo /align:4096 > %t.3.args +// RUN: %clang_cl -### @%t.2.args /link @%t3.args /base:0x10000 2>&1 | \ +// RUN: FileCheck %s -check-prefix=ALIGN +// This test checks whether a "/link" command outside a response file is able +// to consume all remaining args even if followed by response files (whether +// this response file should be expanded or not is another question that is +// outside the scope of this test). +// ALIGN-NOT: no such file or directory: '/base:0x10000' +// ALIGN: link.exe +// ALIGN: "/base:0x10000" Index: tools/driver/cc1_main.cpp =================================================================== --- tools/driver/cc1_main.cpp +++ tools/driver/cc1_main.cpp @@ -63,7 +63,7 @@ } #endif -int cc1_main(const char **ArgBegin, const char **ArgEnd, +int cc1_main(const char * const *ArgBegin, const char * const *ArgEnd, const char *Argv0, void *MainAddr) { std::unique_ptr Clang(new CompilerInstance()); IntrusiveRefCntPtr DiagID(new DiagnosticIDs()); Index: tools/driver/cc1as_main.cpp =================================================================== --- tools/driver/cc1as_main.cpp +++ tools/driver/cc1as_main.cpp @@ -141,15 +141,15 @@ DwarfVersion = 3; } - static bool CreateFromArgs(AssemblerInvocation &Res, const char **ArgBegin, - const char **ArgEnd, DiagnosticsEngine &Diags); + static bool CreateFromArgs(AssemblerInvocation &Res, const char * const *ArgBegin, + const char * const *ArgEnd, DiagnosticsEngine &Diags); }; } bool AssemblerInvocation::CreateFromArgs(AssemblerInvocation &Opts, - const char **ArgBegin, - const char **ArgEnd, + const char * const *ArgBegin, + const char * const *ArgEnd, DiagnosticsEngine &Diags) { bool Success = true; @@ -420,7 +420,7 @@ exit(1); } -int cc1as_main(const char **ArgBegin, const char **ArgEnd, +int cc1as_main(const char * const *ArgBegin, const char * const *ArgEnd, const char *Argv0, void *MainAddr) { // Print a stack trace if we signal out. sys::PrintStackTraceOnErrorSignal(); Index: tools/driver/driver.cpp =================================================================== --- tools/driver/driver.cpp +++ tools/driver/driver.cpp @@ -50,6 +50,7 @@ using namespace clang; using namespace clang::driver; using namespace llvm::opt; +using llvm::cl::ExpandedArgs; std::string GetExecutablePath(const char *Argv0, bool CanonicalPrefixes) { if (!CanonicalPrefixes) @@ -94,19 +95,17 @@ /// \param Edit - The override command to perform. /// \param SavedStrings - Set to use for storing string representations. static void ApplyOneQAOverride(raw_ostream &OS, - SmallVectorImpl &Args, - StringRef Edit, - std::set &SavedStrings) { + ExpandedArgs &Args, + StringRef Edit) { // This does not need to be efficient. if (Edit[0] == '^') { - const char *Str = - SaveStringInSet(SavedStrings, Edit.substr(1)); + const char *Str = Args.SaveString(Edit.substr(1)); OS << "### Adding argument " << Str << " at beginning\n"; Args.insert(Args.begin() + 1, Str); } else if (Edit[0] == '+') { const char *Str = - SaveStringInSet(SavedStrings, Edit.substr(1)); + Args.SaveString(Edit.substr(1)); OS << "### Adding argument " << Str << " at end\n"; Args.push_back(Str); } else if (Edit[0] == 's' && Edit[1] == '/' && Edit.endswith("/") && @@ -120,7 +119,7 @@ if (Repl != Args[i]) { OS << "### Replacing '" << Args[i] << "' with '" << Repl << "'\n"; - Args[i] = SaveStringInSet(SavedStrings, Repl); + Args[i] = Args.SaveString(Repl); } } } else if (Edit[0] == 'x' || Edit[0] == 'X') { @@ -152,7 +151,7 @@ ++i; } OS << "### Adding argument " << Edit << " at end\n"; - Args.push_back(SaveStringInSet(SavedStrings, '-' + Edit.str())); + Args.push_back(Args.SaveString('-' + Edit.str())); } else { OS << "### Unrecognized edit: " << Edit << "\n"; } @@ -160,9 +159,8 @@ /// ApplyQAOverride - Apply a comma separate list of edits to the /// input argument lists. See ApplyOneQAOverride. -static void ApplyQAOverride(SmallVectorImpl &Args, - const char *OverrideStr, - std::set &SavedStrings) { +static void ApplyQAOverride(ExpandedArgs &Args, + const char *OverrideStr) { raw_ostream *OS = &llvm::errs(); if (OverrideStr[0] == '#') { @@ -180,20 +178,19 @@ if (!End) End = S + strlen(S); if (End != S) - ApplyOneQAOverride(*OS, Args, std::string(S, End), SavedStrings); + ApplyOneQAOverride(*OS, Args, std::string(S, End)); S = End; if (*S != '\0') ++S; } } -extern int cc1_main(const char **ArgBegin, const char **ArgEnd, +extern int cc1_main(const char * const *ArgBegin, const char * const *ArgEnd, const char *Argv0, void *MainAddr); -extern int cc1as_main(const char **ArgBegin, const char **ArgEnd, +extern int cc1as_main(const char * const *ArgBegin, const char * const *ArgEnd, const char *Argv0, void *MainAddr); -static void ParseProgName(SmallVectorImpl &ArgVector, - std::set &SavedStrings, +static void ParseProgName(ExpandedArgs &ArgVector, Driver &TheDriver) { // Try to infer frontend type and default target from the program name. @@ -275,12 +272,25 @@ if (it != ArgVector.end()) ++it; const char* Strings[] = - { SaveStringInSet(SavedStrings, std::string("-target")), - SaveStringInSet(SavedStrings, Prefix) }; + { ArgVector.SaveString(std::string("-target")), + ArgVector.SaveString(Prefix) }; ArgVector.insert(it, Strings, Strings + llvm::array_lengthof(Strings)); } } +static +SmallVector GetArgumentVector(ArrayRef Args, + llvm::SpecificBumpPtrAllocator &ArgAllocator) { + SmallVector argv; + std::error_code EC = llvm::sys::Process::GetArgumentVector( + argv, Args, ArgAllocator); + if (EC) { + llvm::errs() << "error: couldn't get arguments: " << EC.message() << '\n'; + exit(1); + } + return argv; +} + namespace { class StringSetSaver : public llvm::cl::StringSaver { public: @@ -297,18 +307,15 @@ llvm::sys::PrintStackTraceOnErrorSignal(); llvm::PrettyStackTraceProgram X(argc_, argv_); - SmallVector argv; llvm::SpecificBumpPtrAllocator ArgAllocator; - std::error_code EC = llvm::sys::Process::GetArgumentVector( - argv, ArrayRef(argv_, argc_), ArgAllocator); - if (EC) { - llvm::errs() << "error: couldn't get arguments: " << EC.message() << '\n'; - return 1; - } std::set SavedStrings; StringSetSaver Saver(SavedStrings); - llvm::cl::ExpandResponseFiles(Saver, llvm::cl::TokenizeGNUCommandLine, argv); + ExpandedArgs ExpArgs = ExpandedArgs::ExpandResponseFiles( + ExpandedArgs::GNU, + GetArgumentVector(ArrayRef(argv_, argc_), ArgAllocator), + &Saver); + const SmallVectorImpl &argv = ExpArgs.get(); // Handle -cc1 integrated tools. if (argv.size() > 1 && StringRef(argv[1]).startswith("-cc1")) { @@ -338,7 +345,7 @@ // scenes. if (const char *OverrideStr = ::getenv("CCC_OVERRIDE_OPTIONS")) { // FIXME: Driver shouldn't take extra initial argument. - ApplyQAOverride(argv, OverrideStr, SavedStrings); + ApplyQAOverride(ExpArgs, OverrideStr); } std::string Path = GetExecutablePath(argv[0], CanonicalPrefixes); @@ -394,7 +401,7 @@ } llvm::InitializeAllTargets(); - ParseProgName(argv, SavedStrings, TheDriver); + ParseProgName(ExpArgs, TheDriver); // Handle CC_PRINT_OPTIONS and CC_PRINT_OPTIONS_FILE. TheDriver.CCPrintOptions = !!::getenv("CC_PRINT_OPTIONS"); @@ -411,7 +418,7 @@ if (TheDriver.CCLogDiagnostics) TheDriver.CCLogDiagnosticsFilename = ::getenv("CC_LOG_DIAGNOSTICS_FILE"); - std::unique_ptr C(TheDriver.BuildCompilation(argv)); + std::unique_ptr C(TheDriver.BuildCompilation(argv, &ExpArgs)); int Res = 0; SmallVector, 4> FailingCommands; if (C.get()) @@ -448,7 +455,7 @@ // If any timers were active but haven't been destroyed yet, print their // results now. This happens in -disable-free mode. llvm::TimerGroup::printAll(llvm::errs()); - + llvm::llvm_shutdown(); #ifdef LLVM_ON_WIN32