commit c6841d3124c89a4d75f04ce04ad819a615324aa8
author Nico Weber <thakis@chromium.org> Sat Jan 05 00:01:29 2019 -0500
committer Commit Bot <commit-bot@chromium.org> Sat Jan 05 05:31:40 2019 +0000
tree 07a354068bfee3787413ebb2e853cfff17281e83
parent 54be6f11b8a4f9e4f491fcd84f3a5880c4623098

Fix a crash on invalid on `not_needed(scope)` without a 2nd argument.

While here, also fix an accepts-invalid where GN would accept
and silently ignore a third argument if the first argument was
a string or a list.

Bug: gn:38
Change-Id: I552585a9467284095ea583ee0fce01e4dad09c0c
Reviewed-on: https://gn-review.googlesource.com/c/3600
Reviewed-by: Petr Hosek <phosek@google.com>
Commit-Queue: Petr Hosek <phosek@google.com>

tools/gn/functions.cc

diff --git a/tools/gn/functions.cc b/tools/gn/functions.cc
index 4c423cb..6abe758 100644
--- a/tools/gn/functions.cc
+++ b/tools/gn/functions.cc
@@ -700,6 +700,12 @@
   // Extract the source scope if different from current one.
   Scope* source = scope;
   if (value->type() == Value::SCOPE) {
+    if (args_cur == args_vector.end()) {
+      *err = Err(
+          function, "Wrong number of arguments.",
+          "The first argument is a scope, expecting two or three arguments.");
+      return Value();
+    }
     // Copy the scope value if it will be overridden.
     if (value == &result_value) {
       scope_value = Value(nullptr, value->scope_value()->MakeClosure());
@@ -712,6 +718,11 @@
       return Value();
     value = &result_value;
     args_cur++;
+  } else if (args_vector.size() > 2) {
+    *err = Err(
+        function, "Wrong number of arguments.",
+        "The first argument is not a scope, expecting one or two arguments.");
+    return Value();
   }
 
   // Extract the exclusion list if defined.

tools/gn/functions_target_unittest.cc

diff --git a/tools/gn/functions_target_unittest.cc b/tools/gn/functions_target_unittest.cc
index 3edd77d..642b8ce 100644
--- a/tools/gn/functions_target_unittest.cc
+++ b/tools/gn/functions_target_unittest.cc
@@ -89,6 +89,37 @@
   ASSERT_TRUE(err.has_error());
   EXPECT_EQ("Not supported with a variable list.", err.message());
 
+  TestParseInput argcount_error_input(
+      "source_set(\"foo\") {\n"
+      "  not_needed()\n"
+      "}\n");
+  ASSERT_FALSE(argcount_error_input.has_error());
+  err = Err();
+  argcount_error_input.parsed()->Execute(setup.scope(), &err);
+  ASSERT_TRUE(err.has_error());
+  EXPECT_EQ("Wrong number of arguments.", err.message());
+
+  TestParseInput scope_error_input(
+      "source_set(\"foo\") {\n"
+      "  a = {x = 1 y = 2}\n"
+      "  not_needed(a)\n"
+      "}\n");
+  ASSERT_FALSE(scope_error_input.has_error());
+  err = Err();
+  scope_error_input.parsed()->Execute(setup.scope(), &err);
+  ASSERT_TRUE(err.has_error());
+  EXPECT_EQ("Wrong number of arguments.", err.message());
+
+  TestParseInput string_error_input(
+      "source_set(\"foo\") {\n"
+      "  not_needed(\"*\", {}, \"*\")\n"
+      "}\n");
+  ASSERT_FALSE(string_error_input.has_error());
+  err = Err();
+  string_error_input.parsed()->Execute(setup.scope(), &err);
+  ASSERT_TRUE(err.has_error());
+  EXPECT_EQ("Wrong number of arguments.", err.message());
+
   TestParseInput template_input(
       R"(# Test that not_needed() propagates through templates correctly;
       # no error should arise from not using "a".