Skip to content

Commit c8df4fb

Browse files
committed
[Support] Fix recursive response file expansion guard
Response file expansion limits the amount of expansion to prevent potential infinite recursion. However, the current logic assumes that any argument beginning with @ is a response file, which is not true for e.g. `-Xlinker -rpath -Xlinker @executable_path/../lib` on Darwin. Having too many of these non-response file arguments beginning with @ prevents actual response files from being expanded. Instead, limit based on the number of successful response file expansions, which should still prevent infinite recursion but also avoid false positives. Differential Revision: https://reviews.llvm.org/D60631 llvm-svn: 358452
1 parent c849746 commit c8df4fb

File tree

2 files changed

+43
-4
lines changed

2 files changed

+43
-4
lines changed

llvm/lib/Support/CommandLine.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,7 +1040,7 @@ static bool ExpandResponseFile(StringRef FName, StringSaver &Saver,
10401040
bool cl::ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer,
10411041
SmallVectorImpl<const char *> &Argv,
10421042
bool MarkEOLs, bool RelativeNames) {
1043-
unsigned RspFiles = 0;
1043+
unsigned ExpandedRspFiles = 0;
10441044
bool AllExpanded = true;
10451045

10461046
// Don't cache Argv.size() because it can change.
@@ -1058,14 +1058,16 @@ bool cl::ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer,
10581058

10591059
// If we have too many response files, leave some unexpanded. This avoids
10601060
// crashing on self-referential response files.
1061-
if (RspFiles++ > 20)
1061+
if (ExpandedRspFiles > 20)
10621062
return false;
10631063

10641064
// Replace this response file argument with the tokenization of its
10651065
// contents. Nested response files are expanded in subsequent iterations.
10661066
SmallVector<const char *, 0> ExpandedArgv;
1067-
if (!ExpandResponseFile(Arg + 1, Saver, Tokenizer, ExpandedArgv,
1068-
MarkEOLs, RelativeNames)) {
1067+
if (ExpandResponseFile(Arg + 1, Saver, Tokenizer, ExpandedArgv, MarkEOLs,
1068+
RelativeNames)) {
1069+
++ExpandedRspFiles;
1070+
} else {
10691071
// We couldn't read this file, so we leave it in the argument stream and
10701072
// move on.
10711073
AllExpanded = false;

llvm/unittests/Support/CommandLineTest.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -813,6 +813,43 @@ TEST(CommandLineTest, RecursiveResponseFiles) {
813813
EXPECT_STREQ(Argv[i], ResponseFileRef.c_str());
814814
}
815815

816+
TEST(CommandLineTest, ResponseFilesAtArguments) {
817+
SmallString<128> TestDir;
818+
std::error_code EC = sys::fs::createUniqueDirectory("unittest", TestDir);
819+
EXPECT_TRUE(!EC);
820+
821+
SmallString<128> ResponseFilePath;
822+
sys::path::append(ResponseFilePath, TestDir, "test.rsp");
823+
824+
std::ofstream ResponseFile(ResponseFilePath.c_str());
825+
EXPECT_TRUE(ResponseFile.is_open());
826+
ResponseFile << "-foo" << "\n";
827+
ResponseFile << "-bar" << "\n";
828+
ResponseFile.close();
829+
830+
// Ensure we expand rsp files after lots of non-rsp arguments starting with @.
831+
constexpr size_t NON_RSP_AT_ARGS = 64;
832+
llvm::SmallVector<const char *, 4> Argv = {"test/test"};
833+
Argv.append(NON_RSP_AT_ARGS, "@non_rsp_at_arg");
834+
std::string ResponseFileRef = std::string("@") + ResponseFilePath.c_str();
835+
Argv.push_back(ResponseFileRef.c_str());
836+
837+
llvm::BumpPtrAllocator A;
838+
llvm::StringSaver Saver(A);
839+
bool Res = llvm::cl::ExpandResponseFiles(
840+
Saver, llvm::cl::TokenizeGNUCommandLine, Argv, false, false);
841+
EXPECT_FALSE(Res);
842+
843+
// ASSERT instead of EXPECT to prevent potential out-of-bounds access.
844+
ASSERT_EQ(Argv.size(), 1 + NON_RSP_AT_ARGS + 2);
845+
size_t i = 0;
846+
EXPECT_STREQ(Argv[i++], "test/test");
847+
for (; i < 1 + NON_RSP_AT_ARGS; ++i)
848+
EXPECT_STREQ(Argv[i], "@non_rsp_at_arg");
849+
EXPECT_STREQ(Argv[i++], "-foo");
850+
EXPECT_STREQ(Argv[i++], "-bar");
851+
}
852+
816853
TEST(CommandLineTest, SetDefautValue) {
817854
cl::ResetCommandLineParser();
818855

0 commit comments

Comments
 (0)