depfile_parser_test: test buggy -MP behavior

This ensures the current behavior of rejecting this case due to `x` being reused as an input.
ninja-build · Nov 19, 2019 · cbd644d · cbd644d
1 parent 8684cdb
commit cbd644d
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 2 deletions.
diff --git a/src/depfile_parser.cc b/src/depfile_parser.cc
@@ -51,6 +51,7 @@ bool DepfileParser::Parse(string* content, string* err) {
   char* end = in + content->size();
   bool have_target = false;
   bool parsing_targets = true;
+  bool poisoned_input = false;
   while (in < end) {
     bool have_newline = false;
     // out: current output point (typically same as in, but can fall behind
@@ -295,21 +296,30 @@ bool DepfileParser::Parse(string* content, string* err) {
     if (len > 0) {
       StringPiece piece = StringPiece(filename, len);
       // If we've seen this as an input before, skip it.
-      if (find(ins_.begin(), ins_.end(), piece) == ins_.end()) {
+      vector<StringPiece>::iterator pos = find(ins_.begin(), ins_.end(), piece);
+      if (pos == ins_.end()) {
         if (is_dependency) {
+          if (poisoned_input) {
+            *err = "inputs may not also have inputs";
+            return false;
+          }
           // New input.
           ins_.push_back(piece);
         } else {
           // Check for a new output.
           if (find(outs_.begin(), outs_.end(), piece) == outs_.end())
             outs_.push_back(piece);
         }
+      } else if (!is_dependency) {
+        // We've passed an input on the left side; reject new inputs.
+        poisoned_input = true;
       }
     }
 
     if (have_newline) {
       // A newline ends a rule so the next filename will be a new target.
       parsing_targets = true;
+      poisoned_input = false;
     }
   }
   if (!have_target) {

diff --git a/src/depfile_parser.in.cc b/src/depfile_parser.in.cc
@@ -50,6 +50,7 @@ bool DepfileParser::Parse(string* content, string* err) {
   char* end = in + content->size();
   bool have_target = false;
   bool parsing_targets = true;
+  bool poisoned_input = false;
   while (in < end) {
     bool have_newline = false;
     // out: current output point (typically same as in, but can fall behind
@@ -147,21 +148,30 @@ bool DepfileParser::Parse(string* content, string* err) {
     if (len > 0) {
       StringPiece piece = StringPiece(filename, len);
       // If we've seen this as an input before, skip it.
-      if (find(ins_.begin(), ins_.end(), piece) == ins_.end()) {
+      vector<StringPiece>::iterator pos = find(ins_.begin(), ins_.end(), piece);
+      if (pos == ins_.end()) {
         if (is_dependency) {
+          if (poisoned_input) {
+            *err = "inputs may not also have inputs";
+            return false;
+          }
           // New input.
           ins_.push_back(piece);
         } else {
           // Check for a new output.
           if (find(outs_.begin(), outs_.end(), piece) == outs_.end())
             outs_.push_back(piece);
         }
+      } else if (!is_dependency) {
+        // We've passed an input on the left side; reject new inputs.
+        poisoned_input = true;
       }
     }
 
     if (have_newline) {
       // A newline ends a rule so the next filename will be a new target.
       parsing_targets = true;
+      poisoned_input = false;
     }
   }
   if (!have_target) {

diff --git a/src/depfile_parser_test.cc b/src/depfile_parser_test.cc
@@ -332,3 +332,12 @@ TEST_F(DepfileParserTest, MultipleRulesDifferentOutputs) {
   EXPECT_EQ("y", parser_.ins_[1].AsString());
   EXPECT_EQ("z", parser_.ins_[2].AsString());
 }
+
+TEST_F(DepfileParserTest, BuggyMP) {
+  string err;
+  EXPECT_FALSE(Parse("foo: x y z\n"
+                     "x: alsoin\n"
+                     "y:\n"
+                     "z:\n", &err));
+  ASSERT_EQ("inputs may not also have inputs", err);
+}