From 2e1bf78ad68962924ad2c5c98b9e80714a221dd2 Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Fri, 22 Aug 2014 19:29:17 +0000 Subject: [PATCH] Fix PR17239 by changing the semantics of the RemainingArgsClass Option kind This patch contains the LLVM side of the fix of PR17239. This bug that 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. My 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. Reviewed By: rnk Differential Revision: http://reviews.llvm.org/D4899 Patch by Rafael Auler! git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@216280 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Support/CommandLine.h | 24 +++++++++++++++---- lib/Option/OptTable.cpp | 5 ++++ lib/Option/Option.cpp | 12 ++++++---- lib/Support/CommandLine.cpp | 43 ++++++++++++++++++++++++++++------- unittests/Support/CommandLineTest.cpp | 6 ++--- 5 files changed, 70 insertions(+), 20 deletions(-) diff --git a/include/llvm/Support/CommandLine.h b/include/llvm/Support/CommandLine.h index fdd901200fe..751a3c8bce9 100644 --- a/include/llvm/Support/CommandLine.h +++ b/include/llvm/Support/CommandLine.h @@ -1790,9 +1790,12 @@ public: /// /// \param [in] Source The string to be split on whitespace with quotes. /// \param [in] Saver Delegates back to the caller for saving parsed strings. +/// \param [in] MarkEOLs true if tokenizing a response file and you want end of +/// lines and end of the response file to be marked with a nullptr string. /// \param [out] NewArgv All parsed strings are appended to NewArgv. void TokenizeGNUCommandLine(StringRef Source, StringSaver &Saver, - SmallVectorImpl &NewArgv); + SmallVectorImpl &NewArgv, + bool MarkEOLs = false); /// \brief Tokenizes a Windows command line which may contain quotes and escaped /// quotes. @@ -1802,25 +1805,36 @@ void TokenizeGNUCommandLine(StringRef Source, StringSaver &Saver, /// /// \param [in] Source The string to be split on whitespace with quotes. /// \param [in] Saver Delegates back to the caller for saving parsed strings. +/// \param [in] MarkEOLs true if tokenizing a response file and you want end of +/// lines and end of the response file to be marked with a nullptr string. /// \param [out] NewArgv All parsed strings are appended to NewArgv. void TokenizeWindowsCommandLine(StringRef Source, StringSaver &Saver, - SmallVectorImpl &NewArgv); + SmallVectorImpl &NewArgv, + bool MarkEOLs = false); /// \brief String tokenization function type. Should be compatible with either /// Windows or Unix command line tokenizers. typedef void (*TokenizerCallback)(StringRef Source, StringSaver &Saver, - SmallVectorImpl &NewArgv); + SmallVectorImpl &NewArgv, + bool MarkEOLs); /// \brief Expand response files on a command line recursively using the given /// StringSaver and tokenization strategy. Argv should contain the command line -/// before expansion and will be modified in place. +/// before expansion and will be modified in place. If requested, Argv will +/// also be populated with nullptrs indicating where each response file line +/// ends, which is useful for the "/link" argument that needs to consume all +/// remaining arguments only until the next end of line, when in a response +/// file. /// /// \param [in] Saver Delegates back to the caller for saving parsed strings. /// \param [in] Tokenizer Tokenization strategy. Typically Unix or Windows. /// \param [in,out] Argv Command line into which to expand response files. +/// \param [in] MarkEOLs Mark end of lines and the end of the response file +/// with nullptrs in the Argv vector. /// \return true if all @files were expanded successfully or there were none. bool ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer, - SmallVectorImpl &Argv); + SmallVectorImpl &Argv, + bool MarkEOLs = false); } // End namespace cl diff --git a/lib/Option/OptTable.cpp b/lib/Option/OptTable.cpp index ef9a3e75a15..dca02c17e53 100644 --- a/lib/Option/OptTable.cpp +++ b/lib/Option/OptTable.cpp @@ -264,6 +264,11 @@ InputArgList *OptTable::ParseArgs(const char *const *ArgBegin, MissingArgIndex = MissingArgCount = 0; unsigned Index = 0, End = ArgEnd - ArgBegin; while (Index < End) { + // Ingore nullptrs, they are response file's EOL markers + if (Args->getArgString(Index) == nullptr) { + ++Index; + continue; + } // Ignore empty arguments (other things may still take them as arguments). StringRef Str = Args->getArgString(Index); if (Str == "") { diff --git a/lib/Option/Option.cpp b/lib/Option/Option.cpp index 10662a33c27..cdc63c353f0 100644 --- a/lib/Option/Option.cpp +++ b/lib/Option/Option.cpp @@ -169,7 +169,8 @@ Arg *Option::accept(const ArgList &Args, return nullptr; Index += 2; - if (Index > Args.getNumInputArgStrings()) + if (Index > Args.getNumInputArgStrings() || + Args.getArgString(Index - 1) == nullptr) return nullptr; return new Arg(UnaliasedOption, Spelling, @@ -200,7 +201,8 @@ Arg *Option::accept(const ArgList &Args, // Otherwise it must be separate. Index += 2; - if (Index > Args.getNumInputArgStrings()) + if (Index > Args.getNumInputArgStrings() || + Args.getArgString(Index - 1) == nullptr) return nullptr; return new Arg(UnaliasedOption, Spelling, @@ -209,7 +211,8 @@ Arg *Option::accept(const ArgList &Args, case JoinedAndSeparateClass: // Always matches. Index += 2; - if (Index > Args.getNumInputArgStrings()) + if (Index > Args.getNumInputArgStrings() || + Args.getArgString(Index - 1) == nullptr) return nullptr; return new Arg(UnaliasedOption, Spelling, Index - 2, @@ -221,7 +224,8 @@ Arg *Option::accept(const ArgList &Args, if (ArgSize != strlen(Args.getArgString(Index))) return nullptr; Arg *A = new Arg(UnaliasedOption, Spelling, Index++); - while (Index < Args.getNumInputArgStrings()) + while (Index < Args.getNumInputArgStrings() && + Args.getArgString(Index) != nullptr) A->getValues().push_back(Args.getArgString(Index++)); return A; } diff --git a/lib/Support/CommandLine.cpp b/lib/Support/CommandLine.cpp index 4c1df5c47dd..2c5174dc7d3 100644 --- a/lib/Support/CommandLine.cpp +++ b/lib/Support/CommandLine.cpp @@ -474,13 +474,18 @@ static bool isGNUSpecial(char C) { } void cl::TokenizeGNUCommandLine(StringRef Src, StringSaver &Saver, - SmallVectorImpl &NewArgv) { + SmallVectorImpl &NewArgv, + bool MarkEOLs) { SmallString<128> Token; for (size_t I = 0, E = Src.size(); I != E; ++I) { // Consume runs of whitespace. if (Token.empty()) { - while (I != E && isWhitespace(Src[I])) + while (I != E && isWhitespace(Src[I])) { + // Mark the end of lines in response files + if (MarkEOLs && Src[I] == '\n') + NewArgv.push_back(nullptr); ++I; + } if (I == E) break; } @@ -521,6 +526,9 @@ void cl::TokenizeGNUCommandLine(StringRef Src, StringSaver &Saver, // Append the last token after hitting EOF with no whitespace. if (!Token.empty()) NewArgv.push_back(Saver.SaveString(Token.c_str())); + // Mark the end of response files + if (MarkEOLs) + NewArgv.push_back(nullptr); } /// Backslashes are interpreted in a rather complicated way in the Windows-style @@ -562,7 +570,8 @@ static size_t parseBackslash(StringRef Src, size_t I, SmallString<128> &Token) { } void cl::TokenizeWindowsCommandLine(StringRef Src, StringSaver &Saver, - SmallVectorImpl &NewArgv) { + SmallVectorImpl &NewArgv, + bool MarkEOLs) { SmallString<128> Token; // This is a small state machine to consume characters until it reaches the @@ -572,8 +581,12 @@ void cl::TokenizeWindowsCommandLine(StringRef Src, StringSaver &Saver, // INIT state indicates that the current input index is at the start of // the string or between tokens. if (State == INIT) { - if (isWhitespace(Src[I])) + if (isWhitespace(Src[I])) { + // Mark the end of lines in response files + if (MarkEOLs && Src[I] == '\n') + NewArgv.push_back(nullptr); continue; + } if (Src[I] == '"') { State = QUOTED; continue; @@ -596,6 +609,9 @@ void cl::TokenizeWindowsCommandLine(StringRef Src, StringSaver &Saver, NewArgv.push_back(Saver.SaveString(Token.c_str())); Token.clear(); State = INIT; + // Mark the end of lines in response files + if (MarkEOLs && Src[I] == '\n') + NewArgv.push_back(nullptr); continue; } if (Src[I] == '"') { @@ -626,11 +642,15 @@ void cl::TokenizeWindowsCommandLine(StringRef Src, StringSaver &Saver, // Append the last token after hitting EOF with no whitespace. if (!Token.empty()) NewArgv.push_back(Saver.SaveString(Token.c_str())); + // Mark the end of response files + if (MarkEOLs) + NewArgv.push_back(nullptr); } static bool ExpandResponseFile(const char *FName, StringSaver &Saver, TokenizerCallback Tokenizer, - SmallVectorImpl &NewArgv) { + SmallVectorImpl &NewArgv, + bool MarkEOLs = false) { ErrorOr> MemBufOrErr = MemoryBuffer::getFile(FName); if (!MemBufOrErr) @@ -648,7 +668,7 @@ static bool ExpandResponseFile(const char *FName, StringSaver &Saver, } // Tokenize the contents into NewArgv. - Tokenizer(Str, Saver, NewArgv); + Tokenizer(Str, Saver, NewArgv, MarkEOLs); return true; } @@ -656,13 +676,19 @@ static bool ExpandResponseFile(const char *FName, StringSaver &Saver, /// \brief Expand response files on a command line recursively using the given /// StringSaver and tokenization strategy. bool cl::ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer, - SmallVectorImpl &Argv) { + SmallVectorImpl &Argv, + bool MarkEOLs) { unsigned RspFiles = 0; bool AllExpanded = true; // Don't cache Argv.size() because it can change. for (unsigned I = 0; I != Argv.size(); ) { const char *Arg = Argv[I]; + // Check if it is an EOL marker + if (Arg == nullptr) { + ++I; + continue; + } if (Arg[0] != '@') { ++I; continue; @@ -678,7 +704,8 @@ bool cl::ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer, // FIXME: If a nested response file uses a relative path, is it relative to // the cwd of the process or the response file? SmallVector ExpandedArgv; - if (!ExpandResponseFile(Arg + 1, Saver, Tokenizer, ExpandedArgv)) { + if (!ExpandResponseFile(Arg + 1, Saver, Tokenizer, ExpandedArgv, + MarkEOLs)) { // We couldn't read this file, so we leave it in the argument stream and // move on. AllExpanded = false; diff --git a/unittests/Support/CommandLineTest.cpp b/unittests/Support/CommandLineTest.cpp index e4a1b67c47e..ac8d3d8c9d6 100644 --- a/unittests/Support/CommandLineTest.cpp +++ b/unittests/Support/CommandLineTest.cpp @@ -153,14 +153,14 @@ class StrDupSaver : public cl::StringSaver { }; typedef void ParserFunction(StringRef Source, llvm::cl::StringSaver &Saver, - SmallVectorImpl &NewArgv); - + SmallVectorImpl &NewArgv, + bool MarkEOLs); void testCommandLineTokenizer(ParserFunction *parse, const char *Input, const char *const Output[], size_t OutputSize) { SmallVector Actual; StrDupSaver Saver; - parse(Input, Saver, Actual); + parse(Input, Saver, Actual, /*MarkEOLs=*/false); EXPECT_EQ(OutputSize, Actual.size()); for (unsigned I = 0, E = Actual.size(); I != E; ++I) { if (I < OutputSize) -- 2.11.0