From aff2b43596d4b8429083e40debf71da62afc2861 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Fri, 24 Mar 2023 17:27:42 +0000 Subject: [PATCH] tint/resolver: Fix ICE when using template args with builtin enums Fixes an 'unreachable AST node' ICE which is otherwise completely harmless. Bug: chromium:1427389 Change-Id: Id5a75b76ca27e00b9e44336a8dd303abed950333 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/125400 Kokoro: Kokoro Commit-Queue: Ben Clayton Reviewed-by: Antonio Maiorano --- src/tint/BUILD.gn | 1 + src/tint/CMakeLists.txt | 1 + src/tint/resolver/builtin_enum_test.cc | 164 +++++++++++++++++++++++++ src/tint/resolver/resolver.cc | 36 ++++-- 4 files changed, 190 insertions(+), 12 deletions(-) create mode 100644 src/tint/resolver/builtin_enum_test.cc diff --git a/src/tint/BUILD.gn b/src/tint/BUILD.gn index b065ed0a49..c42d1d1727 100644 --- a/src/tint/BUILD.gn +++ b/src/tint/BUILD.gn @@ -1411,6 +1411,7 @@ if (tint_build_unittests) { "resolver/atomics_validation_test.cc", "resolver/attribute_validation_test.cc", "resolver/bitcast_validation_test.cc", + "resolver/builtin_enum_test.cc", "resolver/builtin_test.cc", "resolver/builtin_validation_test.cc", "resolver/builtins_validation_test.cc", diff --git a/src/tint/CMakeLists.txt b/src/tint/CMakeLists.txt index d9b94694f6..87bdbbc3ef 100644 --- a/src/tint/CMakeLists.txt +++ b/src/tint/CMakeLists.txt @@ -907,6 +907,7 @@ if(TINT_BUILD_TESTS) resolver/atomics_validation_test.cc resolver/attribute_validation_test.cc resolver/bitcast_validation_test.cc + resolver/builtin_enum_test.cc resolver/builtin_test.cc resolver/builtin_validation_test.cc resolver/builtins_validation_test.cc diff --git a/src/tint/resolver/builtin_enum_test.cc b/src/tint/resolver/builtin_enum_test.cc new file mode 100644 index 0000000000..3d688887dd --- /dev/null +++ b/src/tint/resolver/builtin_enum_test.cc @@ -0,0 +1,164 @@ +// Copyright 2023 The Tint Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "src/tint/builtin/access.h" +#include "src/tint/builtin/address_space.h" +#include "src/tint/builtin/builtin_value.h" +#include "src/tint/builtin/interpolation_sampling.h" +#include "src/tint/builtin/interpolation_type.h" +#include "src/tint/builtin/texel_format.h" +#include "src/tint/resolver/resolver.h" +#include "src/tint/resolver/resolver_test_helper.h" + +#include "gmock/gmock.h" + +using namespace tint::number_suffixes; // NOLINT + +namespace tint::resolver { +namespace { + +//////////////////////////////////////////////////////////////////////////////// +// access +//////////////////////////////////////////////////////////////////////////////// +using ResolverAccessUsedWithTemplateArgs = ResolverTestWithParam; + +TEST_P(ResolverAccessUsedWithTemplateArgs, Test) { + // @group(0) @binding(0) var t : texture_storage_2d>; + auto* tmpl = Ident(Source{{12, 34}}, GetParam(), "T"); + GlobalVar("v", ty("texture_storage_2d", "rgba8unorm", tmpl), Group(0_u), Binding(0_u)); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), "12:34 error: access '" + std::string(GetParam()) + + "' does not take template arguments"); +} + +INSTANTIATE_TEST_SUITE_P(, + ResolverAccessUsedWithTemplateArgs, + testing::ValuesIn(builtin::kAccessStrings)); + +//////////////////////////////////////////////////////////////////////////////// +// address space +//////////////////////////////////////////////////////////////////////////////// +using ResolverAddressSpaceUsedWithTemplateArgs = ResolverTestWithParam; + +TEST_P(ResolverAddressSpaceUsedWithTemplateArgs, Test) { + // fn f(p : ptr, f32) {} + + Enable(builtin::Extension::kChromiumExperimentalFullPtrParameters); + auto* tmpl = Ident(Source{{12, 34}}, GetParam(), "T"); + Func("f", utils::Vector{Param("p", ty("ptr", tmpl, ty.f32()))}, ty.void_(), utils::Empty); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), "12:34 error: address space '" + std::string(GetParam()) + + "' does not take template arguments"); +} + +INSTANTIATE_TEST_SUITE_P(, + ResolverAddressSpaceUsedWithTemplateArgs, + testing::ValuesIn(builtin::kAddressSpaceStrings)); + +//////////////////////////////////////////////////////////////////////////////// +// builtin value +//////////////////////////////////////////////////////////////////////////////// +using ResolverBuiltinValueUsedWithTemplateArgs = ResolverTestWithParam; + +TEST_P(ResolverBuiltinValueUsedWithTemplateArgs, Test) { + // fn f(@builtin(BUILTIN) p : vec4) {} + auto* tmpl = Ident(Source{{12, 34}}, GetParam(), "T"); + Func("f", utils::Vector{Param("p", ty.vec4(), utils::Vector{Builtin(tmpl)})}, ty.void_(), + utils::Empty, utils::Vector{Stage(ast::PipelineStage::kFragment)}); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), "12:34 error: builtin value '" + std::string(GetParam()) + + "' does not take template arguments"); +} + +INSTANTIATE_TEST_SUITE_P(, + ResolverBuiltinValueUsedWithTemplateArgs, + testing::ValuesIn(builtin::kBuiltinValueStrings)); + +//////////////////////////////////////////////////////////////////////////////// +// interpolation sampling +//////////////////////////////////////////////////////////////////////////////// +using ResolverInterpolationSamplingUsedWithTemplateArgs = ResolverTestWithParam; + +TEST_P(ResolverInterpolationSamplingUsedWithTemplateArgs, Test) { + // @fragment + // fn f(@location(0) @interpolate(linear, INTERPOLATION_SAMPLING) p : vec4) {} + auto* tmpl = Ident(Source{{12, 34}}, GetParam(), "T"); + Func("f", + utils::Vector{Param("p", ty.vec4(), + utils::Vector{ + Location(0_a), + Interpolate(builtin::InterpolationType::kLinear, tmpl), + })}, + ty.void_(), utils::Empty, + utils::Vector{ + Stage(ast::PipelineStage::kFragment), + }); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), "12:34 error: interpolation sampling '" + std::string(GetParam()) + + "' does not take template arguments"); +} + +INSTANTIATE_TEST_SUITE_P(, + ResolverInterpolationSamplingUsedWithTemplateArgs, + testing::ValuesIn(builtin::kInterpolationSamplingStrings)); + +//////////////////////////////////////////////////////////////////////////////// +// interpolation type +//////////////////////////////////////////////////////////////////////////////// +using ResolverInterpolationTypeUsedWithTemplateArgs = ResolverTestWithParam; + +TEST_P(ResolverInterpolationTypeUsedWithTemplateArgs, Test) { + // @fragment + // fn f(@location(0) @interpolate(INTERPOLATION_TYPE, center) p : vec4) {} + auto* tmpl = Ident(Source{{12, 34}}, GetParam(), "T"); + Func("f", + utils::Vector{Param("p", ty.vec4(), + utils::Vector{ + Location(0_a), + Interpolate(tmpl, builtin::InterpolationSampling::kCenter), + })}, + ty.void_(), utils::Empty, + utils::Vector{ + Stage(ast::PipelineStage::kFragment), + }); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), "12:34 error: interpolation type '" + std::string(GetParam()) + + "' does not take template arguments"); +} + +INSTANTIATE_TEST_SUITE_P(, + ResolverInterpolationTypeUsedWithTemplateArgs, + testing::ValuesIn(builtin::kInterpolationTypeStrings)); + +//////////////////////////////////////////////////////////////////////////////// +// texel format +//////////////////////////////////////////////////////////////////////////////// +using ResolverTexelFormatUsedWithTemplateArgs = ResolverTestWithParam; + +TEST_P(ResolverTexelFormatUsedWithTemplateArgs, Test) { + // @group(0) @binding(0) var t : texture_storage_2d, write> + auto* tmpl = Ident(Source{{12, 34}}, GetParam(), "T"); + GlobalVar("t", ty("texture_storage_2d", ty(tmpl), "write"), Group(0_u), Binding(0_u)); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), "12:34 error: texel format '" + std::string(GetParam()) + + "' does not take template arguments"); +} + +INSTANTIATE_TEST_SUITE_P(, + ResolverTexelFormatUsedWithTemplateArgs, + testing::ValuesIn(builtin::kTexelFormatStrings)); + +} // namespace +} // namespace tint::resolver diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc index fb8af76410..d509b5c0b5 100644 --- a/src/tint/resolver/resolver.cc +++ b/src/tint/resolver/resolver.cc @@ -3048,35 +3048,47 @@ sem::Expression* Resolver::Identifier(const ast::IdentifierExpression* expr) { } if (auto access = resolved->Access(); access != builtin::Access::kUndefined) { - return builder_->create>( - expr, current_statement_, access); + return CheckNotTemplated("access", ident) + ? builder_->create>( + expr, current_statement_, access) + : nullptr; } if (auto addr = resolved->AddressSpace(); addr != builtin::AddressSpace::kUndefined) { - return builder_->create>( - expr, current_statement_, addr); + return CheckNotTemplated("address space", ident) + ? builder_->create>( + expr, current_statement_, addr) + : nullptr; } if (auto builtin = resolved->BuiltinValue(); builtin != builtin::BuiltinValue::kUndefined) { - return builder_->create>( - expr, current_statement_, builtin); + return CheckNotTemplated("builtin value", ident) + ? builder_->create>( + expr, current_statement_, builtin) + : nullptr; } if (auto i_smpl = resolved->InterpolationSampling(); i_smpl != builtin::InterpolationSampling::kUndefined) { - return builder_->create>( - expr, current_statement_, i_smpl); + return CheckNotTemplated("interpolation sampling", ident) + ? builder_->create>( + expr, current_statement_, i_smpl) + : nullptr; } if (auto i_type = resolved->InterpolationType(); i_type != builtin::InterpolationType::kUndefined) { - return builder_->create>( - expr, current_statement_, i_type); + return CheckNotTemplated("interpolation type", ident) + ? builder_->create>( + expr, current_statement_, i_type) + : nullptr; } if (auto fmt = resolved->TexelFormat(); fmt != builtin::TexelFormat::kUndefined) { - return builder_->create>( - expr, current_statement_, fmt); + return CheckNotTemplated("texel format", ident) + ? builder_->create>( + expr, current_statement_, fmt) + : nullptr; } if (auto* unresolved = resolved->Unresolved()) {