From 10442eff7db4271d53eed553795e655068488276 Mon Sep 17 00:00:00 2001 From: Sarah Date: Wed, 16 Jun 2021 18:50:13 +0000 Subject: [PATCH] validation: limit dynamic indexes to references to matrices and arrays https://github.com/gpuweb/gpuweb/pull/1801 indexes must be of type 'i32' or 'u32' Bug: tint:867 Change-Id: Ie5bfacf87af5a06d5428dc510145e96fb156c42e Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54720 Kokoro: Kokoro Auto-Submit: Sarah Mashayekhi Reviewed-by: Antonio Maiorano Reviewed-by: Ben Clayton --- src/resolver/resolver.cc | 25 ++- src/resolver/resolver_test.cc | 161 ++++++++++++++++++ src/resolver/validation_test.cc | 2 +- src/transform/bound_array_accessors_test.cc | 4 +- .../promote_initializers_to_const_var_test.cc | 52 ------ 5 files changed, 187 insertions(+), 57 deletions(-) diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc index 028f988308..ea10cac219 100644 --- a/src/resolver/resolver.cc +++ b/src/resolver/resolver.cc @@ -1640,8 +1640,9 @@ bool Resolver::ArrayAccessor(ast::ArrayAccessorExpression* expr) { if (!Expression(expr->array())) { return false; } - Mark(expr->idx_expr()); - if (!Expression(expr->idx_expr())) { + auto* idx = expr->idx_expr(); + Mark(idx); + if (!Expression(idx)) { return false; } @@ -1661,6 +1662,26 @@ bool Resolver::ArrayAccessor(ast::ArrayAccessorExpression* expr) { return false; } + if (!TypeOf(idx)->UnwrapRef()->IsAnyOf()) { + diagnostics_.add_error("index must be of type 'i32' or 'u32', found: '" + + TypeNameOf(idx) + "'", + idx->source()); + return false; + } + + if (parent_type->Is() || parent_type->Is()) { + if (!res->Is()) { + // TODO(bclayton): expand this to allow any const_expr expression + // https://github.com/gpuweb/gpuweb/issues/1272 + auto* scalar = idx->As(); + if (!scalar || !scalar->literal()->As()) { + diagnostics_.add_error( + "index must be signed or unsigned integer literal", idx->source()); + return false; + } + } + } + // If we're extracting from a reference, we return a reference. if (auto* ref = res->As()) { ret = builder_->create(ret, ref->StorageClass(), diff --git a/src/resolver/resolver_test.cc b/src/resolver/resolver_test.cc index d70d4679c1..6ce7d8a49e 100644 --- a/src/resolver/resolver_test.cc +++ b/src/resolver/resolver_test.cc @@ -476,6 +476,69 @@ TEST_F(ResolverTest, Expr_ArrayAccessor_Array_Constant) { EXPECT_TRUE(TypeOf(acc)->Is()) << TypeOf(acc)->type_name(); } +TEST_F(ResolverTest, ArrayAccessor_Matrix_Dynamic_F32) { + Global("my_var", ty.mat2x3(), ast::StorageClass::kInput); + auto* acc = IndexAccessor("my_var", Expr(Source{{12, 34}}, 1.0f)); + WrapInFunction(acc); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: index must be of type 'i32' or 'u32', found: 'f32'"); +} + +TEST_F(ResolverTest, ArrayAccessor_Matrix_Dynamic_Ref) { + Global("my_var", ty.mat2x3(), ast::StorageClass::kInput); + auto* idx = Var("idx", ty.i32(), Construct(ty.i32())); + auto* acc = IndexAccessor("my_var", idx); + WrapInFunction(Decl(idx), acc); + + EXPECT_TRUE(r()->Resolve()) << r()->error(); +} + +TEST_F(ResolverTest, ArrayAccessor_Matrix_BothDimensions_Dynamic_Ref) { + Global("my_var", ty.mat4x4(), ast::StorageClass::kOutput); + auto* idx = Var("idx", ty.u32(), Expr(3u)); + auto* idy = Var("idy", ty.u32(), Expr(2u)); + auto* acc = IndexAccessor(IndexAccessor("my_var", idx), idy); + WrapInFunction(Decl(idx), Decl(idy), acc); + + EXPECT_TRUE(r()->Resolve()) << r()->error(); +} + +TEST_F(ResolverTest, ArrayAccessor_Matrix_Dynamic) { + GlobalConst("my_const", ty.mat2x3(), Construct(ty.mat2x3())); + auto* idx = Var("idx", ty.i32(), Construct(ty.i32())); + auto* acc = IndexAccessor("my_const", Expr(Source{{12, 34}}, idx)); + WrapInFunction(Decl(idx), acc); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: index must be signed or unsigned integer literal"); +} + +TEST_F(ResolverTest, ArrayAccessor_Matrix_XDimension_Dynamic) { + GlobalConst("my_var", ty.mat4x4(), Construct(ty.mat4x4())); + auto* idx = Var("idx", ty.u32(), Expr(3u)); + auto* acc = IndexAccessor("my_var", Expr(Source{{12, 34}}, idx)); + WrapInFunction(Decl(idx), acc); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: index must be signed or unsigned integer literal"); +} + +TEST_F(ResolverTest, ArrayAccessor_Matrix_BothDimension_Dynamic) { + GlobalConst("my_var", ty.mat4x4(), Construct(ty.mat4x4())); + auto* idx = Var("idy", ty.u32(), Expr(2u)); + auto* acc = + IndexAccessor(IndexAccessor("my_var", Expr(Source{{12, 34}}, idx)), 1); + WrapInFunction(Decl(idx), acc); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: index must be signed or unsigned integer literal"); +} + TEST_F(ResolverTest, Expr_ArrayAccessor_Matrix) { Global("my_var", ty.mat2x3(), ast::StorageClass::kInput); @@ -507,6 +570,34 @@ TEST_F(ResolverTest, Expr_ArrayAccessor_Matrix_BothDimensions) { EXPECT_TRUE(ref->StoreType()->Is()); } +TEST_F(ResolverTest, Expr_ArrayAccessor_Vector_F32) { + Global("my_var", ty.vec3(), ast::StorageClass::kInput); + auto* acc = IndexAccessor("my_var", Expr(Source{{12, 34}}, 2.0f)); + WrapInFunction(acc); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: index must be of type 'i32' or 'u32', found: 'f32'"); +} + +TEST_F(ResolverTest, Expr_ArrayAccessor_Vector_Dynamic_Ref) { + Global("my_var", ty.vec3(), ast::StorageClass::kInput); + auto* idx = Var("idx", ty.i32(), Expr(2)); + auto* acc = IndexAccessor("my_var", idx); + WrapInFunction(Decl(idx), acc); + + EXPECT_TRUE(r()->Resolve()); +} + +TEST_F(ResolverTest, Expr_ArrayAccessor_Vector_Dynamic) { + GlobalConst("my_var", ty.vec3(), Construct(ty.vec3())); + auto* idx = Var("idx", ty.i32(), Expr(2)); + auto* acc = IndexAccessor("my_var", Expr(Source{{12, 34}}, idx)); + WrapInFunction(Decl(idx), acc); + + EXPECT_TRUE(r()->Resolve()); +} + TEST_F(ResolverTest, Expr_ArrayAccessor_Vector) { Global("my_var", ty.vec3(), ast::StorageClass::kInput); @@ -698,6 +789,76 @@ TEST_F(ResolverTest, Expr_Identifier_FunctionVariable_Const) { EXPECT_EQ(VarOf(my_var_a)->Declaration(), var); } +TEST_F(ResolverTest, ArrayAccessor_Dynamic_Ref_F32) { + // var a : array = 0; + // var idx : f32 = f32(); + // var f : f32 = a[idx]; + auto* a = Var("a", ty.array(), array()); + auto* idx = Var("idx", ty.f32(), Construct(ty.f32())); + auto* f = Var("f", ty.f32(), IndexAccessor("a", Expr(Source{{12, 34}}, idx))); + Func("my_func", ast::VariableList{}, ty.void_(), + { + Decl(a), + Decl(idx), + Decl(f), + }, + ast::DecorationList{}); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: index must be of type 'i32' or 'u32', found: 'f32'"); +} + +TEST_F(ResolverTest, ArrayAccessor_Dynamic_I32) { + // let a : array = 0; + // var idx : i32 = 0; + // var f : f32 = a[idx]; + auto* a = Const("a", ty.array(), array()); + auto* idx = Var("idx", ty.i32(), Construct(ty.i32())); + auto* f = Var("f", ty.f32(), IndexAccessor("a", Expr(Source{{12, 34}}, idx))); + Func("my_func", ast::VariableList{}, ty.void_(), + { + Decl(a), + Decl(idx), + Decl(f), + }, + ast::DecorationList{}); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: index must be signed or unsigned integer literal"); +} + +TEST_F(ResolverTest, ArrayAccessor_Literal_F32) { + // let a : array; + // var f : f32 = a[2.0f]; + auto* a = Const("a", ty.array(), array()); + auto* f = + Var("a_2", ty.f32(), IndexAccessor("a", Expr(Source{{12, 34}}, 2.0f))); + Func("my_func", ast::VariableList{}, ty.void_(), + { + Decl(a), + Decl(f), + }, + ast::DecorationList{}); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: index must be of type 'i32' or 'u32', found: 'f32'"); +} +TEST_F(ResolverTest, ArrayAccessor_Literal_I32) { + // let a : array; + // var f : f32 = a[2]; + auto* a = Const("a", ty.array(), array()); + auto* f = Var("a_2", ty.f32(), IndexAccessor("a", 2)); + Func("my_func", ast::VariableList{}, ty.void_(), + { + Decl(a), + Decl(f), + }, + ast::DecorationList{}); + EXPECT_TRUE(r()->Resolve()) << r()->error(); +} + TEST_F(ResolverTest, Expr_Identifier_FunctionVariable) { auto* my_var_a = Expr("my_var"); auto* my_var_b = Expr("my_var"); diff --git a/src/resolver/validation_test.cc b/src/resolver/validation_test.cc index d71aa7d517..689ef588f7 100644 --- a/src/resolver/validation_test.cc +++ b/src/resolver/validation_test.cc @@ -2063,7 +2063,7 @@ TEST_P(MatrixConstructorTest, const auto param = GetParam(); - // Skip the test if parameters would have resuled in an invalid vec1 type. + // Skip the test if parameters would have resulted in an invalid vec1 type. if (param.rows == 2) { return; } diff --git a/src/transform/bound_array_accessors_test.cc b/src/transform/bound_array_accessors_test.cc index e2e837e310..9c9a84bf35 100644 --- a/src/transform/bound_array_accessors_test.cc +++ b/src/transform/bound_array_accessors_test.cc @@ -52,7 +52,7 @@ TEST_F(BoundArrayAccessorsTest, Array_Idx_Nested_Scalar) { auto* src = R"( var a : array; -var b : array; +var b : array; var i : u32; @@ -64,7 +64,7 @@ fn f() { auto* expect = R"( var a : array; -var b : array; +var b : array; var i : u32; diff --git a/src/transform/promote_initializers_to_const_var_test.cc b/src/transform/promote_initializers_to_const_var_test.cc index 0bb0f9b14b..35ff5ca1ce 100644 --- a/src/transform/promote_initializers_to_const_var_test.cc +++ b/src/transform/promote_initializers_to_const_var_test.cc @@ -225,58 +225,6 @@ let module_str : S = S(1, 2.0, 3); EXPECT_EQ(expect, str(got)); } -TEST_F(PromoteInitializersToConstVarTest, Bug406Array) { - // See crbug.com/tint/406 - auto* src = R"( -[[block]] -struct Uniforms { - transform : mat2x2; -}; - -[[group(0), binding(0)]] var ubo : Uniforms; - -[[builtin(vertex_index)]] var vertex_index : u32; - -[[builtin(position)]] var position : vec4; - -[[stage(vertex)]] -fn main() { - let transform : mat2x2 = ubo.transform; - var coord : vec2 = array, 3>( - vec2(-1.0, 1.0), - vec2( 1.0, 1.0), - vec2(-1.0, -1.0) - )[vertex_index]; - position = vec4(transform * coord, 0.0, 1.0); -} -)"; - - auto* expect = R"( -[[block]] -struct Uniforms { - transform : mat2x2; -}; - -[[group(0), binding(0)]] var ubo : Uniforms; - -[[builtin(vertex_index)]] var vertex_index : u32; - -[[builtin(position)]] var position : vec4; - -[[stage(vertex)]] -fn main() { - let transform : mat2x2 = ubo.transform; - let tint_symbol : array, 3> = array, 3>(vec2(-1.0, 1.0), vec2(1.0, 1.0), vec2(-1.0, -1.0)); - var coord : vec2 = tint_symbol[vertex_index]; - position = vec4((transform * coord), 0.0, 1.0); -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - TEST_F(PromoteInitializersToConstVarTest, EmptyModule) { auto* src = ""; auto* expect = "";