commit 2b683eff60274dbe5874e81ee98a78c47169283e
author Ian McKellar <ianloic@google.com> Mon Dec 14 12:19:16 2020 -0800
committer Commit Bot <commit-bot@chromium.org> Mon Dec 14 21:03:34 2020 +0000
tree 9dc64a9c415bbae6199804f7011f52e30b9100c9
parent 6966efb0f6ff0f80b1a6bc5ee6ac2f343ecf3523

Fix crash in invalid scope subscript access

When accessing a missing scope member through the string subscript
syntax (scope["member"]) the error check didn't actually return
resulting in a NULL dereference. This fixes that and adds tests.

It also fixes some of the parse tree unit tests which weren't correctly
resetting their error values.

Bug: 209
Change-Id: Ida1415b63ce1bca1d479c732126d5336eec13fc9
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/10860
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Brett Wilson <brettw@chromium.org>

src/gn/parse_tree.cc

diff --git a/src/gn/parse_tree.cc b/src/gn/parse_tree.cc
index 74bc2af..9ab28b5 100644
--- a/src/gn/parse_tree.cc
+++ b/src/gn/parse_tree.cc
@@ -393,6 +393,7 @@
     *err =
         Err(subscript_.get(), "No value named \"" + key_value.string_value() +
                                   "\" in scope \"" + base_.value() + "\"");
+    return Value();
   }
   return *result;
 }

src/gn/parse_tree_unittest.cc

diff --git a/src/gn/parse_tree_unittest.cc b/src/gn/parse_tree_unittest.cc
index e366b54..ed28b54 100644
--- a/src/gn/parse_tree_unittest.cc
+++ b/src/gn/parse_tree_unittest.cc
@@ -34,6 +34,7 @@
   Err err;
   Value result = accessor.Execute(setup.scope(), &err);
   EXPECT_TRUE(err.has_error());
+  err = Err();
   EXPECT_EQ(Value::NONE, result.type());
 
   // Define a as a Scope. It should still fail because b isn't defined.
@@ -94,11 +95,18 @@
 
   Value invalid1 = setup.ExecuteExpression("scope[second_element_idx]", &err);
   EXPECT_TRUE(err.has_error());
+  err = Err();
   EXPECT_EQ(invalid1.type(), Value::NONE);
 
   Value invalid2 = setup.ExecuteExpression("list[bar_key]", &err);
   EXPECT_TRUE(err.has_error());
+  err = Err();
   EXPECT_EQ(invalid2.type(), Value::NONE);
+
+  Value invalid3 = setup.ExecuteExpression("scope[\"baz\"]", &err);
+  EXPECT_TRUE(err.has_error());
+  err = Err();
+  EXPECT_EQ(invalid3.type(), Value::NONE);
 }
 
 TEST(ParseTree, BlockUnusedVars) {