[llvm] r363005 - [Support] Explicitly detect recursive response files
Shoaib Meenai via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 10 16:24:02 PDT 2019
Author: smeenai
Date: Mon Jun 10 16:24:02 2019
New Revision: 363005
URL: http://llvm.org/viewvc/llvm-project?rev=363005&view=rev
Log:
[Support] Explicitly detect recursive response files
Previous detection relied upon an arbitrary hard coded limit of 21
response files, which some code bases were running up against.
The new detection maintains a stack of processing response files and
explicitly checks if a newly encountered file is in the current stack.
Some bookkeeping data is necessary in order to detect when to pop the
stack.
Patch by Chris Glover.
Differential Revision: https://reviews.llvm.org/D62798
Modified:
llvm/trunk/lib/Support/CommandLine.cpp
llvm/trunk/unittests/Support/CommandLineTest.cpp
Modified: llvm/trunk/lib/Support/CommandLine.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/CommandLine.cpp?rev=363005&r1=363004&r2=363005&view=diff
==============================================================================
--- llvm/trunk/lib/Support/CommandLine.cpp (original)
+++ llvm/trunk/lib/Support/CommandLine.cpp Mon Jun 10 16:24:02 2019
@@ -1097,43 +1097,84 @@ static bool ExpandResponseFile(StringRef
bool cl::ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer,
SmallVectorImpl<const char *> &Argv,
bool MarkEOLs, bool RelativeNames) {
- unsigned ExpandedRspFiles = 0;
bool AllExpanded = true;
+ struct ResponseFileRecord {
+ const char *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.
+ SmallVector<ResponseFileRecord, 3> FileStack;
+
+ // Push a dummy entry that represents the initial command line, removing
+ // the need to check for an empty list.
+ FileStack.push_back({"", Argv.size()});
// Don't cache Argv.size() because it can change.
for (unsigned I = 0; I != Argv.size();) {
+ while (I == FileStack.back().End) {
+ // Passing the end of a file's argument list, so we can remove it from the
+ // stack.
+ FileStack.pop_back();
+ }
+
const char *Arg = Argv[I];
// Check if it is an EOL marker
if (Arg == nullptr) {
++I;
continue;
}
+
if (Arg[0] != '@') {
++I;
continue;
}
- // If we have too many response files, leave some unexpanded. This avoids
- // crashing on self-referential response files.
- if (ExpandedRspFiles > 20)
- return false;
+ const char *FName = Arg + 1;
+ auto IsEquivalent = [FName](const ResponseFileRecord &RFile) {
+ return sys::fs::equivalent(RFile.File, FName);
+ };
+
+ // Check for recursive response files.
+ if (std::any_of(FileStack.begin() + 1, FileStack.end(), IsEquivalent)) {
+ // This file is recursive, so we leave it in the argument stream and
+ // move on.
+ AllExpanded = false;
+ ++I;
+ continue;
+ }
// 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 (ExpandResponseFile(Arg + 1, Saver, Tokenizer, ExpandedArgv, MarkEOLs,
- RelativeNames)) {
- ++ExpandedRspFiles;
- } else {
+ if (!ExpandResponseFile(FName, Saver, Tokenizer, ExpandedArgv, MarkEOLs,
+ RelativeNames)) {
// We couldn't read this file, so we leave it in the argument stream and
// move on.
AllExpanded = false;
++I;
continue;
}
+
+ 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;
}
Modified: llvm/trunk/unittests/Support/CommandLineTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CommandLineTest.cpp?rev=363005&r1=363004&r2=363005&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/CommandLineTest.cpp (original)
+++ llvm/trunk/unittests/Support/CommandLineTest.cpp Mon Jun 10 16:24:02 2019
@@ -790,7 +790,9 @@ TEST(CommandLineTest, ResponseFiles) {
EXPECT_TRUE(IncludedFile.is_open());
IncludedFile << "-option_1 -option_2\n"
"@incdir/resp2\n"
- "-option_3=abcd\n";
+ "-option_3=abcd\n"
+ "@incdir/resp3\n"
+ "-option_4=efjk\n";
IncludedFile.close();
// Directory for included file.
@@ -808,6 +810,15 @@ TEST(CommandLineTest, ResponseFiles) {
IncludedFile2 << "-option_23=abcd\n";
IncludedFile2.close();
+ // Create second included response file of second level.
+ llvm::SmallString<128> IncludedFileName3;
+ llvm::sys::path::append(IncludedFileName3, IncDir, "resp3");
+ std::ofstream IncludedFile3(IncludedFileName3.c_str());
+ EXPECT_TRUE(IncludedFile3.is_open());
+ IncludedFile3 << "-option_31 -option_32\n";
+ IncludedFile3 << "-option_33=abcd\n";
+ IncludedFile3.close();
+
// Prepare 'file' with reference to response file.
SmallString<128> IncRef;
IncRef.append(1, '@');
@@ -821,7 +832,7 @@ TEST(CommandLineTest, ResponseFiles) {
bool Res = llvm::cl::ExpandResponseFiles(
Saver, llvm::cl::TokenizeGNUCommandLine, Argv, false, true);
EXPECT_TRUE(Res);
- EXPECT_EQ(Argv.size(), 9U);
+ EXPECT_EQ(Argv.size(), 13U);
EXPECT_STREQ(Argv[0], "test/test");
EXPECT_STREQ(Argv[1], "-flag_1");
EXPECT_STREQ(Argv[2], "-option_1");
@@ -830,8 +841,13 @@ TEST(CommandLineTest, ResponseFiles) {
EXPECT_STREQ(Argv[5], "-option_22");
EXPECT_STREQ(Argv[6], "-option_23=abcd");
EXPECT_STREQ(Argv[7], "-option_3=abcd");
- EXPECT_STREQ(Argv[8], "-flag_2");
+ EXPECT_STREQ(Argv[8], "-option_31");
+ EXPECT_STREQ(Argv[9], "-option_32");
+ EXPECT_STREQ(Argv[10], "-option_33=abcd");
+ EXPECT_STREQ(Argv[11], "-option_4=efjk");
+ EXPECT_STREQ(Argv[12], "-flag_2");
+ llvm::sys::fs::remove(IncludedFileName3);
llvm::sys::fs::remove(IncludedFileName2);
llvm::sys::fs::remove(IncDir);
llvm::sys::fs::remove(IncludedFileName);
@@ -843,18 +859,45 @@ TEST(CommandLineTest, RecursiveResponseF
std::error_code EC = sys::fs::createUniqueDirectory("unittest", TestDir);
EXPECT_TRUE(!EC);
- SmallString<128> ResponseFilePath;
- sys::path::append(ResponseFilePath, TestDir, "recursive.rsp");
- std::string ResponseFileRef = std::string("@") + ResponseFilePath.c_str();
-
- std::ofstream ResponseFile(ResponseFilePath.str());
- EXPECT_TRUE(ResponseFile.is_open());
- ResponseFile << ResponseFileRef << "\n";
- ResponseFile << ResponseFileRef << "\n";
- ResponseFile.close();
-
- // Ensure the recursive expansion terminates.
- SmallVector<const char *, 4> Argv = {"test/test", ResponseFileRef.c_str()};
+ SmallString<128> SelfFilePath;
+ sys::path::append(SelfFilePath, TestDir, "self.rsp");
+ std::string SelfFileRef = std::string("@") + SelfFilePath.c_str();
+
+ SmallString<128> NestedFilePath;
+ sys::path::append(NestedFilePath, TestDir, "nested.rsp");
+ std::string NestedFileRef = std::string("@") + NestedFilePath.c_str();
+
+ SmallString<128> FlagFilePath;
+ sys::path::append(FlagFilePath, TestDir, "flag.rsp");
+ std::string FlagFileRef = std::string("@") + FlagFilePath.c_str();
+
+ std::ofstream SelfFile(SelfFilePath.str());
+ EXPECT_TRUE(SelfFile.is_open());
+ SelfFile << "-option_1\n";
+ SelfFile << FlagFileRef << "\n";
+ SelfFile << NestedFileRef << "\n";
+ SelfFile << SelfFileRef << "\n";
+ SelfFile.close();
+
+ std::ofstream NestedFile(NestedFilePath.str());
+ EXPECT_TRUE(NestedFile.is_open());
+ NestedFile << "-option_2\n";
+ NestedFile << FlagFileRef << "\n";
+ NestedFile << SelfFileRef << "\n";
+ NestedFile << NestedFileRef << "\n";
+ NestedFile.close();
+
+ std::ofstream FlagFile(FlagFilePath.str());
+ EXPECT_TRUE(FlagFile.is_open());
+ FlagFile << "-option_x\n";
+ FlagFile.close();
+
+ // Ensure:
+ // Recursive expansion terminates
+ // Recursive files never expand
+ // Non-recursive repeats are allowed
+ SmallVector<const char *, 4> Argv = {"test/test", SelfFileRef.c_str(),
+ "-option_3"};
BumpPtrAllocator A;
StringSaver Saver(A);
#ifdef _WIN32
@@ -865,11 +908,16 @@ TEST(CommandLineTest, RecursiveResponseF
bool Res = cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false);
EXPECT_FALSE(Res);
- // Ensure some expansion took place.
- EXPECT_GT(Argv.size(), 2U);
+ EXPECT_EQ(Argv.size(), 9U);
EXPECT_STREQ(Argv[0], "test/test");
- for (size_t i = 1; i < Argv.size(); ++i)
- EXPECT_STREQ(Argv[i], ResponseFileRef.c_str());
+ EXPECT_STREQ(Argv[1], "-option_1");
+ EXPECT_STREQ(Argv[2], "-option_x");
+ EXPECT_STREQ(Argv[3], "-option_2");
+ EXPECT_STREQ(Argv[4], "-option_x");
+ EXPECT_STREQ(Argv[5], SelfFileRef.c_str());
+ EXPECT_STREQ(Argv[6], NestedFileRef.c_str());
+ EXPECT_STREQ(Argv[7], SelfFileRef.c_str());
+ EXPECT_STREQ(Argv[8], "-option_3");
}
TEST(CommandLineTest, ResponseFilesAtArguments) {
More information about the llvm-commits
mailing list