commit d47a7ef0cf0c39caa4a4cbe14db9576f4b861f65
author James Price <jrprice@google.com> Tue Aug 17 16:09:00 2021 +0000
committer Tint LUCI CQ <tint-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Aug 17 16:09:00 2021 +0000
tree 5f95a01401e6548d3e23ee717c1884df6c779333
parent 91181b906f3bd15f365e3b7de3a988aa7d0566c2

validation: Reject decorations on local variables

These are never valid. The WGSL parser cannot produce them, but the
SPIR-V reader can since these are not always caught by spirv-val.

Fixed: chromium:1239557
Change-Id: Ie19e4534ffb73b61beaa42046b18b2b8a3f7f65b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/62020
Auto-Submit: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Sarah Mashayekhi <sarahmashay@google.com>
Reviewed-by: Sarah Mashayekhi <sarahmashay@google.com>

src/resolver/decoration_validation_test.cc

diff --git a/src/resolver/decoration_validation_test.cc b/src/resolver/decoration_validation_test.cc
index 9786ff6..2808ea0 100644
--- a/src/resolver/decoration_validation_test.cc
+++ b/src/resolver/decoration_validation_test.cc
@@ -778,6 +778,19 @@
 12:34 note: first decoration declared here)");
 }
 
+TEST_F(VariableDecorationTest, LocalVariable) {
+  auto* v = Var("a", ty.f32(),
+                ast::DecorationList{
+                    create<ast::BindingDecoration>(Source{{12, 34}}, 2),
+                });
+
+  WrapInFunction(v);
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "12:34 error: decorations are not valid on local variables");
+}
+
 using ConstantDecorationTest = TestWithParams;
 TEST_P(ConstantDecorationTest, IsValid) {
   auto& params = GetParam();

src/resolver/resolver.cc

diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index be8e45a..3eb9629 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -3366,8 +3366,11 @@
   }
 
   for (auto* deco : var->decorations()) {
-    // TODO(bclayton): Validate decorations
     Mark(deco);
+    if (!deco->Is<ast::InternalDecoration>()) {
+      AddError("decorations are not valid on local variables", deco->source());
+      return false;
+    }
   }
 
   variable_stack_.set(var->symbol(), info);

src/resolver/resolver_test.cc

diff --git a/src/resolver/resolver_test.cc b/src/resolver/resolver_test.cc
index ad14dd3..d84dba8 100644
--- a/src/resolver/resolver_test.cc
+++ b/src/resolver/resolver_test.cc
@@ -864,11 +864,7 @@
   auto* my_var_b = Expr("my_var");
   auto* assign = Assign(my_var_a, my_var_b);
 
-  auto* var = Var("my_var", ty.f32(),
-                  ast::DecorationList{
-                      create<ast::BindingDecoration>(0),
-                      create<ast::GroupDecoration>(0),
-                  });
+  auto* var = Var("my_var", ty.f32());
 
   Func("my_func", ast::VariableList{}, ty.void_(),
        {
@@ -1937,11 +1933,7 @@
                                          ast::UnaryOp::kNot));
 
 TEST_F(ResolverTest, StorageClass_SetsIfMissing) {
-  auto* var = Var("var", ty.i32(),
-                  ast::DecorationList{
-                      create<ast::BindingDecoration>(0),
-                      create<ast::GroupDecoration>(0),
-                  });
+  auto* var = Var("var", ty.i32());
 
   auto* stmt = Decl(var);
   Func("func", ast::VariableList{}, ty.void_(), {stmt}, ast::DecorationList{});