From 78c5ce71333b502a58f9050f083e3f9476bb9fef Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Fri, 22 Aug 2014 19:29:30 +0000 Subject: [PATCH] Fix PR17239 by changing the semantics of the RemainingArgsClass Option kind This patch aims at fixing PR17239. This bug happens because the /link (clang-cl.exe argument) is marked as "consume all remaining arguments". However, when inside a response file, /link should only consume all remaining arguments inside the response file where it is located, not the entire command line after expansion. The LLVM side of the patch will change the semantics of the RemainingArgsClass kind to always consume only until the end of the response file when the option originally came from a response file. There are only two options in this class: dash dash (--) and /link. This is the Clang side of the patch in http://reviews.llvm.org/D4899 Reviewered By: rafael, rnk Differential Revision: http://reviews.llvm.org/D4900 Patch by Rafael Auler! git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@216281 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Driver/Driver.cpp | 7 +++++-- test/Driver/Inputs/cc1-response.txt | 4 ++++ test/Driver/cc1-response-files.c | 2 ++ test/Driver/cl-link-at-file.c | 22 ++++++++++++++++++++++ tools/driver/driver.cpp | 29 +++++++++++++++++++++++++++-- 5 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 test/Driver/Inputs/cc1-response.txt create mode 100644 test/Driver/cc1-response-files.c create mode 100644 test/Driver/cl-link-at-file.c diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp index f6a39a9ab7..482f9cf173 100644 --- a/lib/Driver/Driver.cpp +++ b/lib/Driver/Driver.cpp @@ -83,6 +83,9 @@ void Driver::ParseDriverMode(ArrayRef Args) { getOpts().getOption(options::OPT_driver_mode).getPrefixedName(); for (size_t I = 0, E = Args.size(); I != E; ++I) { + // Ingore nullptrs, they are response file's EOL markers + if (Args[I] == nullptr) + continue; const StringRef Arg = Args[I]; if (!Arg.startswith(OptName)) continue; @@ -102,7 +105,7 @@ void Driver::ParseDriverMode(ArrayRef Args) { } } -InputArgList *Driver::ParseArgStrings(ArrayRef ArgList) { +InputArgList *Driver::ParseArgStrings(ArrayRef ArgStrings) { llvm::PrettyStackTraceString CrashInfo("Command line argument parsing"); unsigned IncludedFlagsBitmask; @@ -111,7 +114,7 @@ InputArgList *Driver::ParseArgStrings(ArrayRef ArgList) { getIncludeExcludeOptionFlagMasks(); unsigned MissingArgIndex, MissingArgCount; - InputArgList *Args = getOpts().ParseArgs(ArgList.begin(), ArgList.end(), + InputArgList *Args = getOpts().ParseArgs(ArgStrings.begin(), ArgStrings.end(), MissingArgIndex, MissingArgCount, IncludedFlagsBitmask, ExcludedFlagsBitmask); diff --git a/test/Driver/Inputs/cc1-response.txt b/test/Driver/Inputs/cc1-response.txt new file mode 100644 index 0000000000..0236fdc6aa --- /dev/null +++ b/test/Driver/Inputs/cc1-response.txt @@ -0,0 +1,4 @@ + + +-cc1 +-triple i686-pc-windows-msvc diff --git a/test/Driver/cc1-response-files.c b/test/Driver/cc1-response-files.c new file mode 100644 index 0000000000..f47e6448a6 --- /dev/null +++ b/test/Driver/cc1-response-files.c @@ -0,0 +1,2 @@ +// RUN: %clang @%S/Inputs/cc1-response.txt -fsyntax-only -disable-llvm-optzns +int main() {} diff --git a/test/Driver/cl-link-at-file.c b/test/Driver/cl-link-at-file.c new file mode 100644 index 0000000000..f817ce523a --- /dev/null +++ b/test/Driver/cl-link-at-file.c @@ -0,0 +1,22 @@ +// 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 diff --git a/tools/driver/driver.cpp b/tools/driver/driver.cpp index 2533401082..524ead29a7 100644 --- a/tools/driver/driver.cpp +++ b/tools/driver/driver.cpp @@ -116,6 +116,9 @@ static void ApplyOneQAOverride(raw_ostream &OS, ReplPattern = ReplPattern.slice(0, ReplPattern.size()-1); for (unsigned i = 1, e = Args.size(); i != e; ++i) { + // Ignore end-of-line response file markers + if (Args[i] == nullptr) + continue; std::string Repl = llvm::Regex(MatchPattern).sub(ReplPattern, Args[i]); if (Repl != Args[i]) { @@ -142,6 +145,9 @@ static void ApplyOneQAOverride(raw_ostream &OS, } else if (Edit[0] == 'O') { for (unsigned i = 1; i < Args.size();) { const char *A = Args[i]; + // Ignore end-of-line response file markers + if (A == nullptr) + continue; if (A[0] == '-' && A[1] == 'O' && (A[2] == '\0' || (A[3] == '\0' && (A[2] == 's' || A[2] == 'z' || @@ -384,14 +390,33 @@ int main(int argc_, const char **argv_) { std::set SavedStrings; StringSetSaver Saver(SavedStrings); - llvm::cl::ExpandResponseFiles(Saver, llvm::cl::TokenizeGNUCommandLine, argv); - // Handle -cc1 integrated tools. + // Determines whether we want nullptr markers in argv to indicate response + // files end-of-lines. We only use this for the /LINK driver argument. + bool MarkEOLs = true; if (argv.size() > 1 && StringRef(argv[1]).startswith("-cc1")) + MarkEOLs = false; + llvm::cl::ExpandResponseFiles(Saver, llvm::cl::TokenizeGNUCommandLine, argv, + MarkEOLs); + + // Handle -cc1 integrated tools, even if -cc1 was expanded from a response + // file. + auto FirstArg = std::find_if(argv.begin() + 1, argv.end(), + [](const char *A) { return A != nullptr; }); + if (FirstArg != argv.end() && StringRef(*FirstArg).startswith("-cc1")) { + // If -cc1 came from a response file, remove the EOL sentinels. + if (MarkEOLs) { + auto newEnd = std::remove(argv.begin(), argv.end(), nullptr); + argv.resize(newEnd - argv.begin()); + } return ExecuteCC1Tool(argv, argv[1] + 4); + } bool CanonicalPrefixes = true; for (int i = 1, size = argv.size(); i < size; ++i) { + // Skip end-of-line response file markers + if (argv[i] == nullptr) + continue; if (StringRef(argv[i]) == "-no-canonical-prefixes") { CanonicalPrefixes = false; break; -- 2.40.0