Skip to content

Commit

Permalink
[tint][resolver] Error if ptr store type is not storable
Browse files Browse the repository at this point in the history
We were allowing validation of pointers-to-pointers which violates the rule:

> If a pointer type appears in the program source, it must also be valid to declare a variable, somewhere in the program, with the pointer type’s address space, store type, and access mode.

This requires that the ptr store type is storable.

Fixed: tint:2055
Change-Id: If12ce8535144429fc4e713d8ceb554617b210fee
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/155723
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
  • Loading branch information
ben-clayton authored and Dawn LUCI CQ committed Oct 11, 2023
1 parent 80b987e commit fc9b2c4
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 37 deletions.
37 changes: 0 additions & 37 deletions src/tint/lang/wgsl/resolver/address_space_validation_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -216,18 +216,6 @@ TEST_F(ResolverAddressSpaceValidationTest, GlobalVariable_Storage_Pointer) {
56:78 note: while instantiating 'var' g)");
}

TEST_F(ResolverAddressSpaceValidationTest, PointerAlias_Storage_Pointer) {
// type t = ptr<storage, ptr<private, f32>>;
Alias("t", ty.ptr<storage>(Source{{56, 78}}, ty.ptr<private_, f32>(Source{{12, 34}})));

ASSERT_FALSE(r()->Resolve());

EXPECT_EQ(
r()->error(),
R"(12:34 error: Type 'ptr<private, f32, read_write>' cannot be used in address space 'storage' as it is non-host-shareable
56:78 note: while instantiating ptr<storage, ptr<private, f32, read_write>, read>)");
}

TEST_F(ResolverAddressSpaceValidationTest, GlobalVariable_Storage_IntScalar) {
// var<storage> g : i32;
GlobalVar("g", ty.i32(), core::AddressSpace::kStorage, Binding(0_a), Group(0_a));
Expand Down Expand Up @@ -631,18 +619,6 @@ TEST_F(ResolverAddressSpaceValidationTest, GlobalVariable_UniformPointer) {
56:78 note: while instantiating 'var' g)");
}

TEST_F(ResolverAddressSpaceValidationTest, PointerAlias_UniformPointer) {
// type t = ptr<uniform, ptr<private, f32>>;
Alias("t", ty.ptr<uniform>(Source{{56, 78}}, ty.ptr<private_, f32>(Source{{12, 34}})));

ASSERT_FALSE(r()->Resolve());

EXPECT_EQ(
r()->error(),
R"(12:34 error: Type 'ptr<private, f32, read_write>' cannot be used in address space 'uniform' as it is non-host-shareable
56:78 note: while instantiating ptr<uniform, ptr<private, f32, read_write>, read>)");
}

TEST_F(ResolverAddressSpaceValidationTest, GlobalVariable_UniformBufferIntScalar) {
// var<uniform> g : i32;
GlobalVar(Source{{56, 78}}, "g", ty.i32(), core::AddressSpace::kUniform, Binding(0_a),
Expand Down Expand Up @@ -920,19 +896,6 @@ TEST_F(ResolverAddressSpaceValidationTest, GlobalVariable_PushConstantPointer) {
56:78 note: while instantiating 'var' g)");
}

TEST_F(ResolverAddressSpaceValidationTest, PointerAlias_PushConstantPointer) {
// enable chromium_experimental_push_constant;
// type t = ptr<push_constant, ptr<private, f32>>;
Enable(wgsl::Extension::kChromiumExperimentalPushConstant);
Alias(Source{{56, 78}}, "t", ty.ptr<push_constant>(ty.ptr<private_, f32>(Source{{12, 34}})));

ASSERT_FALSE(r()->Resolve());
EXPECT_EQ(
r()->error(),
R"(12:34 error: Type 'ptr<private, f32, read_write>' cannot be used in address space 'push_constant' as it is non-host-shareable
note: while instantiating ptr<push_constant, ptr<private, f32, read_write>, read_write>)");
}

TEST_F(ResolverAddressSpaceValidationTest, GlobalVariable_PushConstantIntScalar) {
// enable chromium_experimental_push_constant;
// var<push_constant> g : i32;
Expand Down
15 changes: 15 additions & 0 deletions src/tint/lang/wgsl/resolver/type_validation_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,21 @@ TEST_F(ResolverTypeValidationTest, RuntimeArrayAsParameter_Fail) {
56:78 note: while instantiating parameter a)");
}

TEST_F(ResolverTypeValidationTest, PtrToPtr_Fail) {
// fn func(a : ptr<workgroup, ptr<workgroup, u32>>) {}
auto* param = Param("a", ty.ptr(workgroup, ty.ptr<workgroup, u32>(Source{{12, 34}})));

Func("func", Vector{param}, ty.void_(),
Vector{
Return(),
});

EXPECT_FALSE(r()->Resolve()) << r()->error();
EXPECT_EQ(
r()->error(),
R"(12:34 error: ptr<workgroup, u32, read_write> cannot be used as the store type of a pointer)");
}

TEST_F(ResolverTypeValidationTest, PtrToRuntimeArrayAsPointerParameter_Fail) {
// fn func(a : ptr<workgroup, array<u32>>) {}

Expand Down
6 changes: 6 additions & 0 deletions src/tint/lang/wgsl/resolver/validator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,12 @@ bool Validator::Pointer(const ast::TemplatedIdentifier* a, const core::type::Poi
}
}

if (auto* store_ty = s->StoreType(); !IsStorable(store_ty)) {
AddError(sem_.TypeNameOf(store_ty) + " cannot be used as the store type of a pointer",
a->arguments[1]->source);
return false;
}

return CheckTypeAccessAddressSpace(s->StoreType(), s->Access(), s->AddressSpace(), tint::Empty,
a->source);
}
Expand Down

0 comments on commit fc9b2c4

Please sign in to comment.