[tint] Substitute overrides IR report zero array size error
Substitute was simply missing a (pipeline) error when substituting
a override (expression) into an array count when this value is zero.
This is a backport to M135 with some merge conflicts addressed.
Bug: 407368915
Change-Id: I4577b301ec3cd2adacf44a2740e6df11ddd66754
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/234935
Commit-Queue: Peter McNeeley <petermcneeley@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Peter McNeeley <petermcneeley@google.com>
Commit-Queue: James Price <jrprice@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/tint/lang/core/ir/transform/substitute_overrides.cc b/src/tint/lang/core/ir/transform/substitute_overrides.cc
index c4168bd..5cd5feb 100644
--- a/src/tint/lang/core/ir/transform/substitute_overrides.cc
+++ b/src/tint/lang/core/ir/transform/substitute_overrides.cc
@@ -195,6 +195,17 @@
return new_value.Failure();
}
+ // Pipeline creation error for zero or negative sized array. This is important as we do
+ // not check constant evaluation access against zero size.
+ int64_t cnt_size_check = new_value.Get()->Value()->ValueAs<AInt>();
+ if (cnt_size_check < 1) {
+ diag::Diagnostic error{};
+ error.severity = diag::Severity::Error;
+ error.source = ir.SourceOf(var);
+ error << "array count (" << cnt_size_check << ") must be greater than 0";
+ return Failure(error);
+ }
+
uint32_t num_elements = new_value.Get()->Value()->ValueAs<uint32_t>();
auto* new_cnt = ty.Get<core::type::ConstantArrayCount>(num_elements);
auto* new_ty = ty.Get<core::type::Array>(old_ty->ElemType(), new_cnt, old_ty->Align(),
diff --git a/src/tint/lang/core/ir/transform/substitute_overrides_test.cc b/src/tint/lang/core/ir/transform/substitute_overrides_test.cc
index 74d3c8b..40999db 100644
--- a/src/tint/lang/core/ir/transform/substitute_overrides_test.cc
+++ b/src/tint/lang/core/ir/transform/substitute_overrides_test.cc
@@ -31,9 +31,11 @@
#include <utility>
#include "gtest/gtest.h"
+#include "src/tint/lang/core/fluent_types.h"
#include "src/tint/lang/core/ir/transform/helper_test.h"
#include "src/tint/lang/core/ir/type/array_count.h"
#include "src/tint/lang/core/ir/var.h"
+#include "src/tint/lang/core/type/array.h"
namespace tint::core::ir::transform {
namespace {
@@ -1662,5 +1664,92 @@
R"(error: Pipeline overridable constant 2 with value (65505.0) is not representable in type (f16))");
}
+TEST_F(IR_SubstituteOverridesTest, OverrideArraySizeZeroFailure) {
+ ir::Var* v = nullptr;
+ b.Append(mod.root_block, [&] {
+ auto* x = b.Override("x", ty.u32());
+ x->SetOverrideId({2});
+
+ auto* cnt = ty.Get<core::ir::type::ValueArrayCount>(x->Result(0));
+ auto* ary = ty.Get<core::type::Array>(ty.u32(), cnt, 4_u, 4_u, 4_u, 4_u);
+ v = b.Var("v", ty.ptr(core::AddressSpace::kWorkgroup, ary, core::Access::kReadWrite));
+ mod.SetSource(v, Source{{7, 8}});
+ });
+
+ auto* func = b.Function("foo", ty.u32());
+ b.Append(func->Block(), [&] {
+ auto* access = b.Access(ty.ptr<workgroup, u32>(), v, 0_u);
+ auto* load = b.Load(access);
+ b.Return(func, load);
+ });
+
+ auto* src = R"(
+$B1: { # root
+ %x:u32 = override undef @id(2)
+ %v:ptr<workgroup, array<u32, %x>, read_write> = var undef
+}
+
+%foo = func():u32 {
+ $B2: {
+ %4:ptr<workgroup, u32, read_write> = access %v, 0u
+ %5:u32 = load %4
+ ret %5
+ }
+}
+)";
+ EXPECT_EQ(src, str());
+
+ SubstituteOverridesConfig cfg{};
+ cfg.map[OverrideId{2}] = 0;
+ auto result = RunWithFailure(SubstituteOverrides, cfg);
+ ASSERT_NE(result, Success);
+ EXPECT_EQ(result.Failure().reason.Str(),
+ R"(7:8 error: array count (0) must be greater than 0)");
+}
+
+TEST_F(IR_SubstituteOverridesTest, OverrideArraySizeNegativeFailure) {
+ ir::Var* v = nullptr;
+ b.Append(mod.root_block, [&] {
+ auto* x = b.Override("x", ty.i32());
+ x->SetOverrideId({2});
+
+ auto* cnt = ty.Get<core::ir::type::ValueArrayCount>(x->Result(0));
+ auto* ary = ty.Get<core::type::Array>(ty.u32(), cnt, 4_u, 4_u, 4_u, 4_u);
+ v = b.Var("v", ty.ptr(core::AddressSpace::kWorkgroup, ary, core::Access::kReadWrite));
+ mod.SetSource(v, Source{{7, 8}});
+ });
+
+ auto* func = b.Function("foo", ty.u32());
+ b.Append(func->Block(), [&] {
+ auto* access = b.Access(ty.ptr<workgroup, u32>(), v, 0_u);
+ auto* load = b.Load(access);
+ b.Return(func, load);
+ });
+
+ auto* src = R"(
+$B1: { # root
+ %x:i32 = override undef @id(2)
+ %v:ptr<workgroup, array<u32, %x>, read_write> = var undef
+}
+
+%foo = func():u32 {
+ $B2: {
+ %4:ptr<workgroup, u32, read_write> = access %v, 0u
+ %5:u32 = load %4
+ ret %5
+ }
+}
+)";
+
+ EXPECT_EQ(src, str());
+
+ SubstituteOverridesConfig cfg{};
+ cfg.map[OverrideId{2}] = -1;
+ auto result = RunWithFailure(SubstituteOverrides, cfg);
+ ASSERT_NE(result, Success);
+ EXPECT_EQ(result.Failure().reason.Str(),
+ R"(7:8 error: array count (-1) must be greater than 0)");
+}
+
} // namespace
} // namespace tint::core::ir::transform
