Index: llvm/trunk/lib/Support/CommandLine.cpp =================================================================== --- llvm/trunk/lib/Support/CommandLine.cpp +++ llvm/trunk/lib/Support/CommandLine.cpp @@ -1097,43 +1097,84 @@ bool cl::ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer, SmallVectorImpl &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 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 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; } Index: llvm/trunk/unittests/Support/CommandLineTest.cpp =================================================================== --- llvm/trunk/unittests/Support/CommandLineTest.cpp +++ llvm/trunk/unittests/Support/CommandLineTest.cpp @@ -790,7 +790,9 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 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 Argv = {"test/test", SelfFileRef.c_str(), + "-option_3"}; BumpPtrAllocator A; StringSaver Saver(A); #ifdef _WIN32 @@ -865,11 +908,16 @@ 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) {